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

ANN201 false positive when functions are defined within functions #69

Closed
sumnerevans opened this issue Feb 22, 2020 · 5 comments · Fixed by #72
Closed

ANN201 false positive when functions are defined within functions #69

sumnerevans opened this issue Feb 22, 2020 · 5 comments · Fixed by #72
Assignees
Labels
bug Something isn't working
Milestone

Comments

@sumnerevans
Copy link

sumnerevans commented Feb 22, 2020

Describe the bug
When an outer function returns None, but a function which is defined within it returns something that isn't None, the outer function is falsly marked with an ANN2** error.

Edit: Note this happens even when suppress-none-returning = True.

To Reproduce
Minimal code example to reproduce the behavior:

def foo():
    def bar() -> int:
        return 1

    print(bar())


foo()

When running with --supress-none-returning:

$ flake8 --suppress-none-returning test.py
test.py:1:11: ANN201 Missing return type annotation for public function

Version Information

$ flake8 --version
3.7.9 (flake8-annotations: 2.0.0, flake8-comprehensions: 3.2.2, flake8-print: 3.1.4, flake8_pep3101: 1.2.1, import-order: 0.18.1, mccabe: 0.6.1, pycodestyle: 2.5.0, pyflakes: 2.1.1) CPython 3.8.1 on Linux

As well as your Python version:

$ python -V
Python 3.8.1
@sumnerevans sumnerevans added the bug Something isn't working label Feb 22, 2020
@sco1
Copy link
Owner

sco1 commented Feb 22, 2020

This isn’t a false positive, foo should be annotated here as it implicitly returns None.

@sumnerevans
Copy link
Author

sumnerevans commented Feb 22, 2020

Sorry, meant to mention that this happens even when suppress-none-returning = True. I've edited the original description to reflect this.

@sco1 sco1 added this to the 2.0.1 milestone Feb 22, 2020
@sco1 sco1 self-assigned this Feb 22, 2020
@sco1
Copy link
Owner

sco1 commented Feb 23, 2020

Well I know why this is happening but it's looking like the fix is going to be challenging.

While the return node visitor approach is an elegant one for most cases, it breaks for nested functions because it is visiting the return nodes without any knowledge of their containing function. In the code snippet above, for example, the return node ends up being visited twice, once for bar, as expected, and the other for foo, since the return node is in the body of the foo function node.

Conceptually, this is simply fixed by skipping nested function nodes, but the actual implementation of the fix doesn't seem to be a simple one.

@sco1
Copy link
Owner

sco1 commented Mar 1, 2020

Hi @sumnerevans, these issues should be fixed now by v2.0.1. Please let us know if we've missed anything!

@sumnerevans
Copy link
Author

Thanks for getting to this so quickly! Just upgraded, and this issue appears to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants