[used before def] rework builtin handling #14483
Conversation
This comment has been minimized.
This comment has been minimized.
mypy/partially_defined.py
Outdated
| builtins_mod = names.get("__builtins__", None) | ||
| if builtins_mod: | ||
| assert isinstance(builtins_mod.node, MypyFile) | ||
| self.builtins = set(builtins_mod.node.names.keys()) |
There was a problem hiding this comment.
As a minor optimization, we could store builtins_mod.node.names in self.builtins (i.e. a SymbolTable instance). This would avoid having to create a set with all the items in builtins.
| # flags: --enable-error-code used-before-def | ||
|
|
||
| # When doing multiple passes, mypy resolves references slightly differently. | ||
| # In this case, it would refer the earlier `range` call to the range class defined below. |
There was a problem hiding this comment.
Change range in the comment to type.
There was a problem hiding this comment.
Oops! I realized that range wasn't included in fixtures by default, so switched to type but didn't update the comment. Thanks!
|
Diff from mypy_primer, showing the effect of this PR on open source code: spark (https://github.com/apache/spark)
- python/pyspark/pandas/namespace.py:159: error: Name "range" is used before definition [used-before-def]
|
| @@ -597,7 +603,7 @@ def visit_starred_pattern(self, o: StarredPattern) -> None: | |||
| super().visit_starred_pattern(o) | |||
|
|
|||
| def visit_name_expr(self, o: NameExpr) -> None: | |||
| if refers_to_builtin(o): | |||
| if o.name in self.builtins: | |||
There was a problem hiding this comment.
Hmm this should only affect references to global definitions. Can you add a test case to ensure that a local variable named type is still processed? A potential fix would be to check if fullname starts with builtins..
There was a problem hiding this comment.
It think that you can also check that the kind of SymbolTableNode is GDEF.
…pes (#14517) While working on #14483, we discovered that variable inheritance didn't work quite right. In particular, functions would inherit variables from outer scope. On the surface, this is what you want but actually, they only inherit the scope if there isn't a colliding definition within that scope. Here's an example: ```python class c: pass def f0() -> None: s = c() # UnboundLocalError is raised when this code is executed. class c: pass def f1() -> None: s = c() # No error. ``` This PR also fixes issues with builtins (exactly the same example as above but instead of `c` we have a builtin). Fixes #14213 (as much as is reasonable to do)
When doing multiple passes, in the example below,
rangewill refer to current's module range. When doing a single pass,rangewill refer tobuiltins.range:Instead of looking at the output of semanal to check if a variable is resolving to a
builtinspackage, we can just check if it's part of builtins module.Fixes #14476.