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

[use before def] handle class and function definitions #14203

Merged
merged 5 commits into from
Nov 29, 2022

Conversation

ilinum
Copy link
Collaborator

@ilinum ilinum commented Nov 27, 2022

Previously, we would ignore any class definitions and would fail to detect undefined classes and functions. This updates the logic to handle them.

Closes #686

@ilinum ilinum changed the title [partially defined] handle class definitions [use before def] handle class and function definitions Nov 27, 2022
@github-actions

This comment has been minimized.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

LG, but I have few questions/suggestions.

@@ -435,6 +439,10 @@ def visit_with_stmt(self, o: WithStmt) -> None:
self.process_lvalue(idx)
o.body.accept(self)

def visit_class_def(self, o: ClassDef) -> None:
self.process_definition(o.name)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we have enter_scope() for functions but not for classes? A variable defined in a class body will not be visible outside (as a variable, will still be available as an attribute of course)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, good point. Added enter and exit scope calls (with tests that were failing before)

[case testUseBeforeDefFunc]
# flags: --enable-error-code partially-defined --enable-error-code use-before-def
foo() # E: Name "foo" is used before definition
def foo(): pass
Copy link
Member

Choose a reason for hiding this comment

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

Just in case, could you please add test for classes within functions and vice versa? Also maybe add a test where a variable is defined in a class and then is being red outside of class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@ilinum ilinum merged commit d094c38 into python:master Nov 29, 2022
@ilinum ilinum deleted the partial/classdefs branch November 29, 2022 01:00
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.

References to names before they're bound should generate an error (UnboundLocalError)
2 participants