Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: adjust FuncionDef offset to not include decorators #2102

Conversation

iamdefinitelyahuman
Copy link
Contributor

What I did

Adjust FunctionDef offset to not include decorators. This ensures a more readable annotation when an exception is raised.

Before:

vyper.exceptions.FunctionDeclarationException: Missing or unmatched return statements in function 'foo'
line 12:0 
     11
---> 12 @external
--------^
     13 @payable

After:

vyper.exceptions.FunctionDeclarationException: Missing or unmatched return statements in function 'foo'
line 14:0 
     13 @payable
---> 14 def foo() -> uint256:
--------^
     15     pass

How I did it

When converting from Python to Vyper AST, modify FunctionDef.lineno to be +1 from the end_lineno of the final decorator.

How to verify it

Run tests.

Cute Animal Picture

image

@codecov-commenter
Copy link

codecov-commenter commented Jul 6, 2020

Codecov Report

Merging #2102 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2102   +/-   ##
=======================================
  Coverage   85.00%   85.00%           
=======================================
  Files          83       83           
  Lines        8247     8251    +4     
  Branches     1990     1991    +1     
=======================================
+ Hits         7010     7014    +4     
  Misses        740      740           
  Partials      497      497           
Impacted Files Coverage Δ
vyper/ast/nodes.py 93.70% <100.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e6141a...81cc458. Read the comment docs.

Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about if someone puts a comment between the last decorator and the function def?

@iamdefinitelyahuman
Copy link
Contributor Author

The only way to be more accurate is to look for the specific def token. Doable, but also a fair bit of work and this fix should solve it in 99% of cases.

@iamdefinitelyahuman iamdefinitelyahuman merged commit 159fefe into vyperlang:master Jul 6, 2020
@iamdefinitelyahuman iamdefinitelyahuman deleted the fix-function-offset branch July 6, 2020 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants