-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix crashes in class scoped imports #12023
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
Changes from all commits
8c61bb3
43dfe0d
c92c172
3ebfda3
63f8634
aa2457e
80b6109
6eedcc4
7aa37cd
cc55023
037ce8c
2629040
5615fc1
a14004e
2119c3d
e09bc02
b93491a
7bd60db
c467399
1dc469d
4f32a01
6d02762
02dd54e
4523607
e71c191
d5d80bd
13f9e5f
08ea1a9
862b4d8
6f04070
010c936
18286eb
c43756b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7134,3 +7134,112 @@ class B(A): # E: Final class __main__.B has abstract attributes "foo" | |
[case testUndefinedBaseclassInNestedClass] | ||
class C: | ||
class C1(XX): pass # E: Name "XX" is not defined | ||
|
||
[case testClassScopeImportFunction] | ||
class Foo: | ||
from mod import foo | ||
|
||
reveal_type(Foo.foo) # N: Revealed type is "def (x: builtins.int, y: builtins.int) -> builtins.int" | ||
reveal_type(Foo().foo) # E: Invalid self argument "Foo" to attribute function "foo" with type "Callable[[int, int], int]" \ | ||
# N: Revealed type is "def (y: builtins.int) -> builtins.int" | ||
[file mod.py] | ||
def foo(x: int, y: int) -> int: ... | ||
|
||
[case testClassScopeImportVariable] | ||
class Foo: | ||
from mod import foo | ||
|
||
reveal_type(Foo.foo) # N: Revealed type is "builtins.int" | ||
reveal_type(Foo().foo) # N: Revealed type is "builtins.int" | ||
[file mod.py] | ||
foo: int | ||
|
||
[case testClassScopeImportModule] | ||
class Foo: | ||
import mod | ||
|
||
reveal_type(Foo.mod) # N: Revealed type is "builtins.object" | ||
reveal_type(Foo.mod.foo) # N: Revealed type is "builtins.int" | ||
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. I think 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. Okay, can add! 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. It's revealed as builtins.object, not types.ModuleType, but pretty sure that's just because of the stub fixtures |
||
[file mod.py] | ||
foo: int | ||
|
||
[case testClassScopeImportFunctionAlias] | ||
class Foo: | ||
from mod import foo | ||
bar = foo | ||
|
||
from mod import const_foo | ||
const_bar = const_foo | ||
|
||
reveal_type(Foo.foo) # N: Revealed type is "def (x: builtins.int, y: builtins.int) -> builtins.int" | ||
reveal_type(Foo.bar) # N: Revealed type is "def (x: builtins.int, y: builtins.int) -> builtins.int" | ||
reveal_type(Foo.const_foo) # N: Revealed type is "builtins.int" | ||
reveal_type(Foo.const_bar) # N: Revealed type is "builtins.int" | ||
[file mod.py] | ||
def foo(x: int, y: int) -> int: ... | ||
const_foo: int | ||
|
||
[case testClassScopeImportModuleStar] | ||
class Foo: | ||
from mod import * | ||
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. This is actually a syntax error, you can only do 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. Huh, TIL. Tried on Python 2 and it works but gives me a SyntaxWarning. Thanks Python 3! 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. I think tests should tell us about syntax errors. Why do they pass? 🤔 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. Most syntax errors happen at parse time, but some, like this one, do not. The ones that happen later often cause problems for mypy, e.g. this: #11499 or 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. Another example is |
||
|
||
reveal_type(Foo.foo) # N: Revealed type is "builtins.int" | ||
reveal_type(Foo.bar) # N: Revealed type is "def (x: builtins.int) -> builtins.int" | ||
reveal_type(Foo.baz) # E: "Type[Foo]" has no attribute "baz" \ | ||
# N: Revealed type is "Any" | ||
[file mod.py] | ||
foo: int | ||
def bar(x: int) -> int: ... | ||
|
||
[case testClassScopeImportFunctionNested] | ||
class Foo: | ||
class Bar: | ||
from mod import baz | ||
|
||
reveal_type(Foo.Bar.baz) # N: Revealed type is "def (x: builtins.int) -> builtins.int" | ||
reveal_type(Foo.Bar().baz) # E: Invalid self argument "Bar" to attribute function "baz" with type "Callable[[int], int]" \ | ||
# N: Revealed type is "def () -> builtins.int" | ||
[file mod.py] | ||
def baz(x: int) -> int: ... | ||
|
||
[case testClassScopeImportUndefined] | ||
class Foo: | ||
from unknown import foo # E: Cannot find implementation or library stub for module named "unknown" \ | ||
# N: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports | ||
|
||
reveal_type(Foo.foo) # N: Revealed type is "Any" | ||
reveal_type(Foo().foo) # N: Revealed type is "Any" | ||
|
||
[case testClassScopeImportWithFollowImports] | ||
# flags: --follow-imports=skip | ||
class Foo: | ||
from mod import foo | ||
|
||
reveal_type(Foo().foo) # N: Revealed type is "Any" | ||
[file mod.py] | ||
def foo(x: int, y: int) -> int: ... | ||
|
||
[case testClassScopeImportVarious] | ||
class Foo: | ||
from mod1 import foo | ||
from mod2 import foo # E: Name "foo" already defined on line 2 | ||
|
||
from mod1 import meth1 | ||
def meth1(self, a: str) -> str: ... # E: Name "meth1" already defined on line 5 | ||
|
||
def meth2(self, a: str) -> str: ... | ||
from mod1 import meth2 # E: Name "meth2" already defined on line 8 | ||
|
||
class Bar: | ||
from mod1 import foo | ||
|
||
import mod1 | ||
reveal_type(Foo.foo) # N: Revealed type is "def (x: builtins.int, y: builtins.int) -> builtins.int" | ||
reveal_type(Bar.foo) # N: Revealed type is "def (x: builtins.int, y: builtins.int) -> builtins.int" | ||
reveal_type(mod1.foo) # N: Revealed type is "def (x: builtins.int, y: builtins.int) -> builtins.int" | ||
[file mod1.py] | ||
def foo(x: int, y: int) -> int: ... | ||
def meth1(x: int) -> int: ... | ||
def meth2(x: int) -> int: ... | ||
[file mod2.py] | ||
def foo(z: str) -> int: ... |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,9 +131,9 @@ def f() -> None: pass | |
[case testImportWithinClassBody2] | ||
import typing | ||
class C: | ||
from m import f | ||
from m import f # E: Method must have at least one argument | ||
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. This seems to be placed on the wrong line. 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. This is actually on the correct line. In this kind of situation, I agree that for the code in this test, where there is no attempt to use 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. Oh, I see! 👍 |
||
f() | ||
f(C) # E: Too many arguments for "f" | ||
f(C) # E: Too many arguments for "f" of "C" | ||
[file m.py] | ||
def f() -> None: pass | ||
[out] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2722,7 +2722,7 @@ import m | |
|
||
[file m.py] | ||
class C: | ||
from mm import f | ||
from mm import f # E: Method must have at least one argument | ||
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. The same here 🤔 |
||
@dec(f) | ||
def m(self): pass | ||
|
||
|
@@ -2742,7 +2742,7 @@ import m | |
|
||
[file m/__init__.py] | ||
class C: | ||
from m.m import f | ||
from m.m import f # E: Method must have at least one argument | ||
@dec(f) | ||
def m(self): pass | ||
|
||
|
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.
Maybe we can somehow create a new function here to make the inference better? Right now - this is sooo complex, but I understand the problem you have with mypyc 🙂
Uh oh!
There was an error while loading. Please reload this page.
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.
mypy is actually able to narrow the types just fine. If you remove
f
, everything type checks perfectly. It's only mypyc that's the issue :'( I've tried a lot of variants, if you have a suggestion that you think is better and that mypyc will understand, I'm happy to try!