Improve errors for invalid Get objects
#10593
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| return f"{expr.func.value.id}.{expr.func.attr}()" # type: ignore[attr-defined] | ||
|
|
||
| # Fall back to the name of the ast node's class. | ||
| return str(type(expr)) |
There was a problem hiding this comment.
The original code used type(expr).__name__, but I think this made the error confusing because we would output Str rather than <class '_ast.Str'>.
| def test_parse_get_types_wrong_number_args() -> None: | ||
| assert_parse_get_types_fails( | ||
| "Get()", | ||
| expected_explanation="Expected either two or three arguments, but got 0 arguments.", |
There was a problem hiding this comment.
I know this is brittle to be doing exact string matches. But because we're trying to improve the quality of our error messages, I think this fragility is justified so that we are confident the error message is rendering exactly how we'd like.
| @@ -153,19 +155,19 @@ def test_valid_get_unresolvable_subject_declared_type(self) -> None: | |||
| self._parse_rule_gets("Get(int, DNE, 'bob')") | |||
|
|
|||
| def test_invalid_get_no_subject_args(self) -> None: | |||
There was a problem hiding this comment.
These tests duplicate selectors_test.py now. I didn't delete them though because these are small and I'm thinking they act like ~integration tests, i.e. that everything is wired up correctly.
|
Ran the tests with Py36, 37, and 38 to ensure that these new tests pass. (The AST changes between versions) |
There are two times an invalid
Getcan trigger errors: a) when parsing the AST inrules.py, or b) when Python creates the actualGetobject, i.e. callingGet.__init__(). Typically, the former happens first, which was resulting in very confusing error messages, like this for the invalid lineawait Get(str, AddressInput.parse("")):After
Invalid # of args:
Invalid product:
Invalid subject:
[ci skip-rust]
[ci skip-build-wheels]