Allow adjacent conditionally-defined overloads #19042
Conversation
|
cc @cdce8p as original author - could you have a look, please? |
This comment has been minimized.
This comment has been minimized.
|
Okay, we've even had one such false positive in primer! https://github.com/enthought/comtypes/blob/430bbdca8c0a146139acb990146ecbe06bdbf803/comtypes/_post_coinit/misc.py#L143 |
|
This looks like it should fix #17521 as well, but I'm not able to check this right now. |
|
It does, thanks! I expected this to be too niche to run a mypy-issues scan, but that was a wrong estimate:( |
This comment has been minimized.
This comment has been minimized.
| def spam(v: int) -> list[int]: ... | ||
|
|
||
| def spam(v: "int | str") -> "list[str] | list[int]": | ||
| return [] |
There was a problem hiding this comment.
Similar to above. Actually this looks identical to ham. Was this intentional?
There was a problem hiding this comment.
Yes, it was (or rather didn't matter) as the only thing I was testing here was that two adjacent conditional overloads are interpreted correctly. But your other comment raises a good point, modified accordingly.
| [typing fixtures/typing-medium.pyi] | ||
|
|
||
| [case testAdjacentConditionalOverloads] | ||
| # flags: --always-true True |
There was a problem hiding this comment.
Maybe use some other name than True, since this overlaps with the normal True, which is confusing.
There was a problem hiding this comment.
Done, but I'm not sure it's actually better - all surrounding tests use --always-true True, so now this one is inconsistent which might be surprising or misleading for future readers.
There was a problem hiding this comment.
Ah I didn't notice the others. This may be still useful in case future readers use the new approach as an example.
…9015-adjacent-conditional-overloads
|
Diff from mypy_primer, showing the effect of this PR on open source code: comtypes (https://github.com/enthought/comtypes)
- comtypes/_post_coinit/misc.py:146: error: An overloaded function outside a stub file must have an implementation [no-overload-impl]
- comtypes/_post_coinit/misc.py:162: error: Name "CoGetClassObject" already defined on line 146 [no-redef]
|
3 participants
Fixes #19015. Fixes #17521. Side question: why do we do that extra work to show "is already defined" error instead of "implementation must come last" in the following setting?
We have to track it specially and it doesn't work when
f2is decorated, and I struggle to understand how "already defined" is better than an overload-specific message "implementation must come last".