fix[ux]: add missing filename to syntax exceptions #4343

sandbubbles · 2024-11-01T19:36:23Z

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

this commit adds a filename to `SyntaxException`s. previously, they did
not include filename information because that is typically added from
the `Module` AST node fields. but, at the time a `SyntaxException`
is thrown, the AST is not yet available, so the normal handler does
not have the filename info. this commit adds the filename info in two
places where the path is known, one in natspec.py and one in parse.py.

Description for the changelog

Cute Animal Picture

codecov · 2024-11-02T10:21:29Z

Codecov Report

Attention: Patch coverage is 38.09524% with 13 lines in your changes missing coverage. Please review.

Project coverage is 49.36%. Comparing base (c8691ac) to head (fd8d78d).

Files with missing lines	Patch %	Lines
vyper/exceptions.py	25.00%	6 Missing ⚠️
vyper/ast/parse.py	42.85%	4 Missing ⚠️
vyper/ast/natspec.py	50.00%	3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (c8691ac) and HEAD (fd8d78d). Click for more details.

HEAD has 116 uploads less than BASE

Flag BASE (c8691ac) HEAD (fd8d78d)

140 24

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.
📢 Have feedback on the report? Share it here.

charles-cooper · 2024-11-07T08:47:46Z

vyper/exceptions.py

@@ -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}", '


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

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]>

charles-cooper · 2024-11-19T17:04:53Z

vyper/exceptions.py

@@ -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):


_append_contract() sounds like it modifies the Exception, but this is a helper function.

charles-cooper · 2024-11-19T17:07:37Z

vyper/compiler/phases.py

@@ -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)


i just checked natspec.py and don't see that it throws SyntaxException anywhere?

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)

charles-cooper

left some comments

vyper/ast/natspec.py

cyberthirst · 2024-12-03T16:33:18Z

vyper/exceptions.py

@@ -128,12 +129,17 @@ 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.resolved_path, node.lineno


we check .get("path") but we access .resolved_path - is that correct?

cyberthirst · 2024-12-03T17:53:22Z

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 None

we use str(e) which will get propagated as the message. during the construction, the path is not provided, thus the unknown is still displayed:

main.vy
def foo():
    uint256 a = pass

yielding:

vyper.exceptions.SyntaxException: invalid syntax (<unknown>, line 2)

  contract "tests/custom/test.vy:2", line 2:13 
       1 def foo():
  ---> 2     uint256 a = pass
  --------------------^

see vyper.exceptions.SyntaxException: invalid syntax (<unknown>, line 2)

vyper/exceptions.py

sandbubbles · 2024-12-11T09:41:05Z

I believe all the changes have been addressed so it's ready for another review

charles-cooper · 2024-12-12T15:19:35Z

vyper/exceptions.py

@@ -54,6 +54,7 @@ def __init__(self, message="Error Message not found.", *items, hint=None, prev_d
        self.lineno = None
        self.col_offset = None
        self.annotations = None
+        self.path = None


for clarity, let's rename this to self.resolved_path

charles-cooper · 2024-12-12T15:24:13Z

vyper/ast/parse.py

@@ -60,7 +76,8 @@ def parse_to_ast_with_settings(
        py_ast = python_ast.parse(pre_parser.reformatted_code)
    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


please add docs to comment:

Suggested change

# 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

couple tiny nits, i think otherwise this is ready

vyper/exceptions.py

sandbubbles added 10 commits November 1, 2024 11:15

add path info to syntax exception

4425feb

set path for natspec exceptions

3316865

lint

90723e1

rewrite the path attribute

bdf5a42

test natspec exception contains file

363cf1d

test version exception contains filename

241511b

test imported invalid compiler version is attributed correctly

b3c8ef5

check syntax exception in json contains filename

02c5765

check the exception is attributed to the correct file

548d583

test syntax exceptions report file correctly

67dcfb8

fix isort errors

2dded1a

sandbubbles assigned charles-cooper Nov 7, 2024

charles-cooper reviewed Nov 7, 2024

View reviewed changes

sandbubbles and others added 5 commits November 7, 2024 11:43

refactor[ux]: add venom as experimental-codegen alias (vyperlang#…

2dfa033

…4337) Make "venom" an alternative alias to the "experimental_codegen" flag. Add the alias to cli, pre-parser, and json parser.

feat[docs]: add Telegram badge to README.md (vyperlang#4342)

b6591b4

--------- Co-authored-by: Charles Cooper <[email protected]>

extract adding contract info into a function

9450428

update error message verification to include line number

97b3abe

sandbubbles marked this pull request as ready for review November 9, 2024 10:21

Merge branch 'master' into fix/syntax-exception-missing-filename

1b0e02a

charles-cooper reviewed Nov 19, 2024

View reviewed changes

charles-cooper reviewed Nov 20, 2024

View reviewed changes

sandbubbles added 4 commits November 21, 2024 11:23

wrap parse_natspec in a try catch to add path

05e24d6

remove changes to _annotate functions

5101a27

rename method to "_format_contract_details"

7f44b10

lint

88174e7

github-advanced-security bot found potential problems Nov 21, 2024

View reviewed changes

vyper/ast/natspec.py Fixed Show fixed Hide fixed

vyper/ast/natspec.py Fixed Show fixed Hide fixed

remove redundant "pass"

e66da98

cyberthirst reviewed Dec 3, 2024

View reviewed changes

sandbubbles added 3 commits December 7, 2024 16:54

check resolved_path is not unknown

af391a6

use only the msg of SyntaxError

c09dc65

Merge branch 'master' into fix/syntax-exception-missing-filename

6ca0cbb

sandbubbles requested a review from charles-cooper December 7, 2024 16:22

charles-cooper reviewed Dec 7, 2024

View reviewed changes

vyper/exceptions.py Show resolved Hide resolved

charles-cooper approved these changes Dec 7, 2024

View reviewed changes

charles-cooper requested a review from cyberthirst December 7, 2024 17:47

sandbubbles added 3 commits December 7, 2024 19:10

adjust the offset by 1

fd8d78d

test offset points to correct place

78362e6

add comment

5ea4e2c

cyberthirst added the release - must release blocker label Dec 11, 2024

sandbubbles requested a review from charles-cooper December 12, 2024 15:19

charles-cooper reviewed Dec 12, 2024

View reviewed changes

vyper/exceptions.py Outdated Show resolved Hide resolved

charles-cooper added 2 commits December 13, 2024 11:27

nits

5022cfd

fix lint

7f0b3f4

charles-cooper approved these changes Dec 13, 2024

View reviewed changes

move comment

531cdca

cyberthirst approved these changes Dec 13, 2024

View reviewed changes

Merge branch 'master' into fix/syntax-exception-missing-filename

8e87405

charles-cooper enabled auto-merge (squash) December 13, 2024 16:38

charles-cooper changed the title ~~fix[ux]: syntax exception missing filename~~ fix[ux]: add missing filename to syntax exceptions Dec 13, 2024

charles-cooper merged commit c951dea into vyperlang:master Dec 13, 2024
157 checks passed

charles-cooper mentioned this pull request Dec 18, 2024

SyntaxException does not contain filename #4285

Closed

charles-cooper mentioned this pull request Dec 30, 2024

fix[ux]: improve error message for syntax exceptions #4226

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix[ux]: add missing filename to syntax exceptions #4343

fix[ux]: add missing filename to syntax exceptions #4343

sandbubbles commented Nov 1, 2024 •

edited by charles-cooper

Loading

codecov bot commented Nov 2, 2024 •

edited

Loading

charles-cooper Nov 7, 2024

sandbubbles Nov 7, 2024

charles-cooper Nov 19, 2024

charles-cooper Nov 19, 2024

charles-cooper Nov 20, 2024

charles-cooper left a comment

cyberthirst Dec 3, 2024

cyberthirst commented Dec 3, 2024

sandbubbles commented Dec 11, 2024

charles-cooper Dec 12, 2024

charles-cooper Dec 12, 2024 •

edited

Loading

charles-cooper left a 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

fix[ux]: add missing filename to syntax exceptions #4343

fix[ux]: add missing filename to syntax exceptions #4343

Conversation

sandbubbles commented Nov 1, 2024 • edited by charles-cooper Loading

What I did

How I did it

How to verify it

Commit message

Description for the changelog

Cute Animal Picture

codecov bot commented Nov 2, 2024 • edited Loading

Codecov Report

charles-cooper Nov 7, 2024

Choose a reason for hiding this comment

sandbubbles Nov 7, 2024

Choose a reason for hiding this comment

charles-cooper Nov 19, 2024

Choose a reason for hiding this comment

charles-cooper Nov 19, 2024

Choose a reason for hiding this comment

charles-cooper Nov 20, 2024

Choose a reason for hiding this comment

charles-cooper left a comment

Choose a reason for hiding this comment

cyberthirst Dec 3, 2024

Choose a reason for hiding this comment

cyberthirst commented Dec 3, 2024

sandbubbles commented Dec 11, 2024

charles-cooper Dec 12, 2024

Choose a reason for hiding this comment

charles-cooper Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

charles-cooper left a comment

Choose a reason for hiding this comment

sandbubbles commented Nov 1, 2024 •

edited by charles-cooper

Loading

codecov bot commented Nov 2, 2024 •

edited

Loading

charles-cooper Dec 12, 2024 •

edited

Loading