fix[ux]: syntax exception missing filename #4343
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4343 +/- ##
==========================================
- Coverage 91.23% 88.48% -2.76%
==========================================
Files 112 112
Lines 16010 16026 +16
Branches 2697 2698 +1
==========================================
- Hits 14607 14180 -427
- Misses 968 1322 +354
- Partials 435 524 +89 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
vyper/exceptions.py
Outdated
| @@ -134,6 +135,9 @@ def format_annotation(self, value): | |||
| if fn_node: | |||
| node_msg = f'{node_msg}function "{fn_node.name}", ' | |||
|
|
|||
| if self.path is not None: | |||
| node_msg = f'{node_msg}contract "{self.path}", ' | |||
There was a problem hiding this comment.
code smell that this code is similar with line 132 above -- and in fact, line 132 appends lineno but this does not. also the path could be appended twice in case there is both a node and .path set on this exception
There was a problem hiding this comment.
is it better now?
…4337) Make "venom" an alternative alias to the "experimental_codegen" flag. Add the alias to cli, pre-parser, and json parser.
the call to `setup()` would fail on windows when there are unicode characters in `README.md`, because files are apparently opened with encoding `cp1252` by default on windows. this commit ensures the file is opened with `utf-8` encoding.
--------- Co-authored-by: Charles Cooper <[email protected]>
vyper/exceptions.py
Outdated
| @@ -151,6 +155,9 @@ def _add_hint(self, msg): | |||
| return msg | |||
| return msg + f"\n (hint: {self.hint})" | |||
|
|
|||
| def _append_contract(self, msg, path, lineno): | |||
There was a problem hiding this comment.
_append_contract() sounds like it modifies the Exception, but this is a helper function.
vyper/compiler/phases.py
Outdated
| @@ -174,7 +175,11 @@ def resolved_imports(self): | |||
| def _annotate(self) -> tuple[natspec.NatspecOutput, vy_ast.Module]: | |||
| module = self._resolve_imports[0] | |||
| analyze_module(module) | |||
| nspec = natspec.parse_natspec(module) | |||
| try: | |||
| nspec = natspec.parse_natspec(module) | |||
There was a problem hiding this comment.
i just checked natspec.py and don't see that it throws SyntaxException anywhere?
There was a problem hiding this comment.
nvm -- NatspecSyntaxException inherits from SyntaxException. however it will be better to have the try/catch in parse_natspec itself (in case it's ever used from somewhere else)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
vyper/exceptions.py
Outdated
| @@ -128,12 +129,15 @@ def format_annotation(self, value): | |||
|
|
|||
| # TODO: handle cases where module is None or vy_ast.Module | |||
| if module_node.get("path") not in (None, "<unknown>"): | |||
| node_msg = f'{node_msg}contract "{module_node.path}:{node.lineno}", ' | |||
| node_msg = self._format_contract_details(node_msg, module_node.path, node.lineno) | |||
There was a problem hiding this comment.
here we use module_node.path, but in the exception handlers added in this commit, resolved_path is used
There was a problem hiding this comment.
thank you! @cyberthirst see if there is anything else you would like to add
What I did
Print filename for syntax exceptions as in #4285.
How I did it
Rethrow syntax exceptions from a point where path is known (two places - one for natspec, one for the rest).
How to verify it
Commit message
Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)
Description for the changelog
Cute Animal Picture