Repository navigation
New semantic analyzer: conditional function definitions #6429
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
527f48c
fde9f36
6766a77
8f2bd31
5c1e2f1
f900d64
aa4bc61
5be8cc2
971afc4
baa9c74
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 |
|---|---|---|
|
|
@@ -1417,3 +1417,68 @@ reveal_type(x) # E: Revealed type is 'Tuple[builtins.int, builtins.int]' | |
| [file b.py] | ||
| import a | ||
| x = (1, 2) | ||
|
|
||
| [case testNewAnalyzerConditionalFunc] | ||
| if int(): | ||
| def f(x: int) -> None: | ||
| pass | ||
| def g(x: int) -> None: | ||
| pass | ||
| elif bool(): | ||
| def f(x: int) -> None: | ||
| 1() # E: "int" not callable | ||
| def g(x: str) -> None: # E: All conditional function variants must have identical signatures | ||
| pass | ||
| else: | ||
| def f(x: int) -> None: | ||
| ''() # E: "str" not callable | ||
|
|
||
| reveal_type(g) # E: Revealed type is 'def (x: builtins.int)' | ||
|
|
||
| [case testNewAnalyzerConditionalFuncDefer] | ||
| if int(): | ||
| def f(x: A) -> None: | ||
| pass | ||
| def g(x: A) -> None: | ||
| pass | ||
| else: | ||
| def f(x: A) -> None: | ||
| 1() # E: "int" not callable | ||
| def g(x: str) -> None: # E: All conditional function variants must have identical signatures | ||
| pass | ||
|
|
||
| reveal_type(g) # E: Revealed type is 'def (x: __main__.A)' | ||
|
|
||
| class A: pass | ||
|
|
||
| [case testNewAnalyzerConditionalDecoratedFunc] | ||
| from typing import Callable | ||
|
|
||
| def dec(f: Callable[[int], None]) -> Callable[[str], None]: | ||
| pass | ||
|
|
||
| if int(): | ||
| from m import f | ||
| else: | ||
| @dec | ||
| def f(x: int) -> None: | ||
|
Member
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. Do we have a test case for when this causes a type error? (Decorated function signature differs from original one.)
Collaborator
Author
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 doesn't work correctly with the old semantic analyzer either. I'll create an issue about this. |
||
| 1() # E: "int" not callable | ||
| reveal_type(f) # E: Revealed type is 'def (x: builtins.str)' | ||
| [file m.py] | ||
| def f(x: str) -> None: pass | ||
|
Member
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 would add a test that a function with same name as a class is still semantically analyzed. In general, do we have enough tests for something like: x = 1
class C:
pass
class C:
x + 'no way'etc? |
||
|
|
||
| [case testNewAnalyzerConditionallyDefineFuncOverVar] | ||
| from typing import Callable | ||
|
|
||
| if int(): | ||
| f: Callable[[str], None] | ||
| else: | ||
| def f(x: str) -> None: | ||
| ... | ||
| reveal_type(f) # E: Revealed type is 'def (builtins.str)' | ||
|
Member
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. Does old semantic analyzer support the opposite order? I mean: if int():
def f(x: str) -> None:
...
else:
f: Callable[[str], None]If yes, then the new one should probably support this too. Otherwise, this is probably not important.
Collaborator
Author
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 generates an error with the old semantic analyzer as well. |
||
|
|
||
| [case testNewAnalyzerConditionallyDefineFuncOverClass] | ||
| class C: | ||
| 1() # E: "int" not callable | ||
| def C() -> None: # E: Name 'C' already defined on line 1 | ||
| ''() # E: "str" not callable | ||
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.
This explanation is not very clear. I would rather say that we don't add function top-level functions when we just enter each of it during second phase of analysis -- processing of functions.
Also I would rather explain why this is needed (I agree that this looks reasonable).