-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[used before def] improve handling of global definitions in local scopes #14517
Merged
Merged
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3a14986
[used before def] rework builtin handling
ilinum 5415d4e
store SymbolTable instance + update comment
ilinum 66a7802
Merge branch 'master' into used-before-def-multipass
ilinum 342c05d
rework scope inheritance in defs
ilinum ddc4418
extend fixes to all non-generator scopes
ilinum 8e69cd4
minor refactor
ilinum 286840f
Merge branch 'master' into used-before-def-scope
ilinum 33561c2
test whitespace
ilinum cff35c6
Merge branch 'master' into used-before-def-scope
ilinum 43ddea4
fix mypyc test
ilinum b5a02a4
fix default args
ilinum 78b0f55
fix tests
ilinum 9adbe70
Merge branch 'master' into used-before-def-multipass
ilinum 39c22d5
more efficient builtin handling
ilinum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
rework scope inheritance in defs
- Loading branch information
commit 342c05dc7c34a3496d4901379cd474092ef31715
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -210,7 +210,6 @@ def f0() -> None: | |
y = x | ||
x = 1 # No error. | ||
|
||
|
||
[case testGlobalDeclarationAfterUsage] | ||
# flags: --enable-error-code possibly-undefined --enable-error-code used-before-def | ||
def f0() -> None: | ||
|
@@ -219,6 +218,7 @@ def f0() -> None: | |
x = 1 # No error. | ||
|
||
x = 2 | ||
|
||
[case testVarDefinedInOuterScope] | ||
# flags: --enable-error-code possibly-undefined --enable-error-code used-before-def | ||
def f0() -> None: | ||
|
@@ -227,13 +227,57 @@ def f0() -> None: | |
|
||
f0() | ||
x = 1 | ||
|
||
[case testDefinedInOuterScopeNoError] | ||
# flags: --enable-error-code possibly-undefined --enable-error-code used-before-def | ||
def foo() -> None: | ||
bar() | ||
|
||
def bar() -> None: | ||
foo() | ||
|
||
[case testClassFromOuterScopeRedefined] | ||
# flags: --enable-error-code possibly-undefined --enable-error-code used-before-def | ||
class c: pass | ||
|
||
def f0() -> None: | ||
s = c() # E: Name "c" is used before definition | ||
class c: pass | ||
|
||
|
||
def f1() -> None: | ||
s = c() # No error. | ||
|
||
|
||
def f2() -> None: | ||
s = c() # E: Name "c" is used before definition | ||
if int(): | ||
class c: pass | ||
|
||
[case testVarFromOuterScopeRedefined] | ||
# flags: --enable-error-code possibly-undefined --enable-error-code used-before-def | ||
x = 0 | ||
|
||
def f0() -> None: | ||
y = x # E: Name "x" is used before definition | ||
x = 0 | ||
|
||
def f1() -> None: | ||
y = x # No error. | ||
|
||
def f2() -> None: | ||
y = x # E: Name "x" is used before definition | ||
global x | ||
|
||
def f3() -> None: | ||
global x | ||
y = x # No error. | ||
|
||
def f4() -> None: | ||
if int(): | ||
x = 0 | ||
y = x # E: Name "x" may be undefined | ||
|
||
[case testFuncParams] | ||
# flags: --enable-error-code possibly-undefined | ||
def foo(a: int) -> None: | ||
|
@@ -829,67 +873,56 @@ def f4() -> None: | |
x = z # E: Name "z" is used before definition | ||
z: int = 2 | ||
|
||
[case testUsedBeforeDefImportsBasic] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when I wrote these import tests, I misunderstood how they would actually behave at runtime. |
||
# flags: --enable-error-code used-before-def | ||
[case testUsedBeforeDefImportsBasicImportNoError] | ||
# flags: --enable-error-code used-before-def --enable-error-code possibly-undefined --disable-error-code no-redef | ||
import foo # type: ignore | ||
import x.y # type: ignore | ||
|
||
def f0() -> None: | ||
a = foo # No error. | ||
foo: int = 1 | ||
a = foo # No error. | ||
foo: int = 1 | ||
|
||
def f1() -> None: | ||
a = y # E: Name "y" is used before definition | ||
y: int = 1 | ||
[case testUsedBeforeDefImportsDotImport] | ||
# flags: --enable-error-code used-before-def --enable-error-code possibly-undefined --disable-error-code no-redef | ||
import x.y # type: ignore | ||
|
||
def f2() -> None: | ||
a = x # No error. | ||
x: int = 1 | ||
a = y # E: Name "y" is used before definition | ||
y: int = 1 | ||
|
||
def f3() -> None: | ||
a = x.y # No error. | ||
x: int = 1 | ||
b = x # No error. | ||
x: int = 1 | ||
|
||
c = x.y # No error. | ||
x: int = 1 | ||
|
||
[case testUsedBeforeDefImportBasicRename] | ||
# flags: --enable-error-code used-before-def | ||
# flags: --enable-error-code used-before-def --disable-error-code=no-redef | ||
import x.y as z # type: ignore | ||
from typing import Any | ||
|
||
def f0() -> None: | ||
a = z # No error. | ||
z: int = 1 | ||
a = z # No error. | ||
z: int = 1 | ||
|
||
def f1() -> None: | ||
a = x # E: Name "x" is used before definition | ||
x: int = 1 | ||
a = x # E: Name "x" is used before definition | ||
x: int = 1 | ||
|
||
def f2() -> None: | ||
a = x.y # E: Name "x" is used before definition | ||
x: Any = 1 | ||
|
||
def f3() -> None: | ||
a = y # E: Name "y" is used before definition | ||
y: int = 1 | ||
a = y # E: Name "y" is used before definition | ||
y: int = 1 | ||
|
||
[case testUsedBeforeDefImportFrom] | ||
# flags: --enable-error-code used-before-def | ||
# flags: --enable-error-code used-before-def --disable-error-code no-redef | ||
from foo import x # type: ignore | ||
|
||
def f0() -> None: | ||
a = x # No error. | ||
x: int = 1 | ||
a = x # No error. | ||
x: int = 1 | ||
|
||
[case testUsedBeforeDefImportFromRename] | ||
# flags: --enable-error-code used-before-def | ||
# flags: --enable-error-code used-before-def --disable-error-code no-redef | ||
from foo import x as y # type: ignore | ||
|
||
def f0() -> None: | ||
a = y # No error. | ||
y: int = 1 | ||
a = y # No error. | ||
y: int = 1 | ||
|
||
def f1() -> None: | ||
a = x # E: Name "x" is used before definition | ||
x: int = 1 | ||
a = x # E: Name "x" is used before definition | ||
x: int = 1 | ||
|
||
[case testUsedBeforeDefFunctionDeclarations] | ||
# flags: --enable-error-code used-before-def | ||
|
@@ -905,7 +938,7 @@ def f0() -> None: | |
# flags: --enable-error-code used-before-def | ||
|
||
def f0() -> None: | ||
s = type(123) | ||
s = type(123) # E: Name "type" is used before definition | ||
type = "abc" | ||
a = type | ||
|
||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I wonder if this might degrade performance, since builtins have over 200 definitions, and it looks like we are iterating over all of them for every module. Can you measure the time spent in this loop when performing a self check, for example?
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.
I have found a way to deal with this that doesn't involve calling
record_definition
every time! So performance should be unchanged.