-
- Notifications
You must be signed in to change notification settings - Fork 307
Handle FunctionDef blockstart_tolineno edge cases #2880
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
Conversation
The function definition can be quite tricky. Take below example: ``` # Case A def foo(bar: str) -> None: pass # Case B def foo( bar:str ): pass # Case C def foo( bar:str ) -> None: pass # Case D def foo( bar:str ): pass ``` Currently we only handled Case A, B. With this commit we can cover case C, but not Case D.
Pierre-Sassoulas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for opening a PR, do you mind adding automated test for this, please ?
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## main #2880 +/- ## ========================================== + Coverage 93.30% 93.31% +0.01% ========================================== Files 92 92 Lines 11194 11196 +2 ========================================== + Hits 10445 10448 +3 + Misses 749 748 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
π New features to boost your workflow:
|
Added 4 test cases to test different argument and return statement lineno, also with or without annotations.
Fix placement of `#@`
Add another test case for FunctionDef's blockstart_tolineno, which without annotations will still work.
Remove comment that is not accurate.
Sure! I have added some test cases. Sorry for making some amends, should be good now :) |
Pierre-Sassoulas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just waiting for the coverage job to show up :)
Getting the lineno of the start of the block for function definition(`FunctionDef.blockstart_tolineno`) can be quite tricky. Take below example: ```python # Case A def foo(bar: str) -> None: pass # should returns line=1 # Case B def foo( bar:str): pass # should returns line=2 # Case C def foo( bar:str ) -> None: pass # should returns line=3 # Case D def foo( bar:str ): # should returns line=3 pass ``` Currently we only handled Case A, B. With this commit we can cover case C. But for Case D, we will need a better solution (cherry picked from commit 8fa18c7)
| Great first contribution to astroid ! |
β¦dge cases (#2881) Handle FunctionDef blockstart_tolineno edge cases (#2880) Getting the lineno of the start of the block for function definition(`FunctionDef.blockstart_tolineno`) can be quite tricky. Take below example: ```python # Case A def foo(bar: str) -> None: pass # should returns line=1 # Case B def foo( bar:str): pass # should returns line=2 # Case C def foo( bar:str ) -> None: pass # should returns line=3 # Case D def foo( bar:str ): # should returns line=3 pass ``` Currently we only handled Case A, B. With this commit we can cover case C. But for Case D, we will need a better solution (cherry picked from commit 8fa18c7) Co-authored-by: Low, Zhi Hao <lowzhao@gmail.com>
Type of Changes
Description
Getting the lineno of the start of the block for function definition(
FunctionDef.blockstart_tolineno) can be quite tricky. Take below example:Currently we only handled Case A, B. With this commit we can cover case C.
But for Case D, we will need a better solution