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
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions mypy/partially_defined.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
AssignmentExpr,
AssignmentStmt,
BreakStmt,
ClassDef,
Context,
ContinueStmt,
DictionaryComprehension,
Expand Down Expand Up @@ -258,13 +259,16 @@ def variable_may_be_undefined(self, name: str, context: Context) -> None:
if self.msg.errors.is_error_code_enabled(errorcodes.PARTIALLY_DEFINED):
self.msg.variable_may_be_undefined(name, context)

def process_definition(self, name: str) -> None:
# Was this name previously used? If yes, it's a use-before-definition error.
refs = self.tracker.pop_undefined_ref(name)
for ref in refs:
self.var_used_before_def(name, ref)
self.tracker.record_definition(name)

def process_lvalue(self, lvalue: Lvalue | None) -> None:
if isinstance(lvalue, NameExpr):
# Was this name previously used? If yes, it's a use-before-definition error.
refs = self.tracker.pop_undefined_ref(lvalue.name)
for ref in refs:
self.var_used_before_def(lvalue.name, ref)
self.tracker.record_definition(lvalue.name)
self.process_definition(lvalue.name)
elif isinstance(lvalue, StarExpr):
self.process_lvalue(lvalue.expr)
elif isinstance(lvalue, (ListExpr, TupleExpr)):
Expand Down Expand Up @@ -314,7 +318,7 @@ def visit_match_stmt(self, o: MatchStmt) -> None:
self.tracker.end_branch_statement()

def visit_func_def(self, o: FuncDef) -> None:
self.tracker.record_definition(o.name)
self.process_definition(o.name)
self.tracker.enter_scope()
super().visit_func_def(o)
self.tracker.exit_scope()
Expand Down Expand Up @@ -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)

super().visit_class_def(o)

def visit_import(self, o: Import) -> None:
for mod, alias in o.ids:
if alias is not None:
Expand Down
11 changes: 11 additions & 0 deletions test-data/unit/check-partially-defined.test
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,17 @@ def f0(b: bool) -> None:
fn = lambda: 2
y = fn # E: Name "fn" may be undefined

[case testUseBeforeDefClass]
# flags: --enable-error-code partially-defined --enable-error-code use-before-def
def f(x: A): # No error here.
pass
y = A() # E: Name "A" is used before definition
class A: pass

[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.

[case testGenerator]
# flags: --enable-error-code partially-defined
if int():
Expand Down