fix[ux]: add missing filename to syntax exceptions#4343
fix[ux]: add missing filename to syntax exceptions#4343charles-cooper merged 37 commits intovyperlang:masterfrom
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4343 +/- ##
===========================================
- Coverage 91.31% 49.36% -41.95%
===========================================
Files 113 113
Lines 16065 16083 +18
Branches 2705 2706 +1
===========================================
- Hits 14670 7940 -6730
- Misses 964 7529 +6565
- Partials 431 614 +183 ☔ View full report in Codecov by Sentry. |
| 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]>
| 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.
| 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}
| 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.resolved_path, node.lineno |
There was a problem hiding this comment.
we check .get("path") but we access .resolved_path - is that correct?
|
when raising the exception: except SyntaxError as e:
# TODO: Ensure 1-to-1 match of source_code:reformatted_code SyntaxErrors
raise SyntaxException(str(e), vyper_source, e.lineno, e.offset) from Nonewe use str(e) which will get propagated as the message. during the construction, the path is not provided, thus the main.vy
def foo():
uint256 a = passyielding: see |
|
I believe all the changes have been addressed so it's ready for another review |
| self.lineno = None | ||
| self.col_offset = None | ||
| self.annotations = None | ||
| self.path = None |
There was a problem hiding this comment.
for clarity, let's rename this to self.resolved_path
| except SyntaxError as e: | ||
| # TODO: Ensure 1-to-1 match of source_code:reformatted_code SyntaxErrors | ||
| raise SyntaxException(str(e), vyper_source, e.lineno, e.offset) from None | ||
| # SyntaxError offset is 1-based, not 0-based |
There was a problem hiding this comment.
please add docs to comment:
| # SyntaxError offset is 1-based, not 0-based | |
| # SyntaxError offset is 1-based, not 0-based | |
| # https://docs.python.org/3/library/exceptions.html#SyntaxError.offset |
charles-cooper
left a comment
charles-cooper
left a comment
There was a problem hiding this comment.
couple tiny nits, i think otherwise this is ready
charles-cooper
changed the title
5 participants
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
Description for the changelog
Cute Animal Picture