Better error message for unsupported kwargs in function calls #2798
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2798 +/- ##
==========================================
+ Coverage 87.46% 87.51% +0.04%
==========================================
Files 93 94 +1
Lines 9960 10007 +47
Branches 2471 2480 +9
==========================================
+ Hits 8712 8758 +46
- Misses 782 783 +1
Partials 466 466
Continue to review full report at Codecov.
|
| validate_expected_type(kwarg.arg, kwarg.value) | ||
| raise ArgumentException( | ||
| ( | ||
| "Usage of kwarg in Vyper is restricted to gas=, " |
There was a problem hiding this comment.
this error message is a big improvement. without complicating the code too much, can we make it even better with a suggestion? would be good to see something like
vyper.exceptions.ArgumentException: Usage of kwarg in Vyper is restricted to gas=, value= and skip_contract_check=.
(hint: Try removing the kwarg: `Foo(msg.sender).foo(1, 2)`)
contract "tmp/foo.vy", function "foo", line 9:28
8 def foo():
---> 9 Foo(msg.sender).foo(1, y=2)
-----------------------------------^
10
|
Using regex to generate the suggestion is not perfect in this case: The suggestion should be |
| @@ -469,7 +470,23 @@ def fetch_call_return(self, node: vy_ast.Call) -> Optional[BaseTypeDefinition]: | |||
| if not isinstance(kwarg.value, vy_ast.NameConstant): | |||
| raise InvalidType("skip_contract_check must be literal bool", kwarg.value) | |||
| else: | |||
| validate_expected_type(kwarg.arg, kwarg.value) | |||
| kwarg_pattern = fr"{kwarg.arg}\s*=\s*{re.escape(kwarg.value.node_source_code)}" | |||
There was a problem hiding this comment.
add a comment explaining what we are trying to do here
| @@ -469,7 +470,23 @@ def fetch_call_return(self, node: vy_ast.Call) -> Optional[BaseTypeDefinition]: | |||
| if not isinstance(kwarg.value, vy_ast.NameConstant): | |||
| raise InvalidType("skip_contract_check must be literal bool", kwarg.value) | |||
| else: | |||
| validate_expected_type(kwarg.arg, kwarg.value) | |||
| kwarg_pattern = fr"{kwarg.arg}\s*=\s*{re.escape(kwarg.value.node_source_code)}" | |||
| modified_line = re.sub( | |||
There was a problem hiding this comment.
i don't generally like regexes for this kind of thing, they are like hand grenades in that their behavior is hard to predict. but as this seems to work even in edge cases - i'll allow it.
There was a problem hiding this comment.
Got it. I think an alternative is to create a copy of the AST node, modify it (moving the kwarg node from keywords to args), and then generate the source code from the modified AST node. However, this would require some sort of pretty print function for each node type.
What I did
Fix for #2525
How I did it
Throw an
ArgumentExceptionerror during validation of kwargs other thangas,valueandskip_contract_check.How to verify it
See tests.
Commit message
Better error message for unsupported kwargs in function calls.
Description for the changelog
Better error message for unsupported kwargs in function calls.
Cute Animal Picture