Make expression checking always produce Any once a node has been deferred #6860
Conversation
…rred This fixes an issue where the Any that is inferred temporarily when a node is deferred could cause a bad overload to be picked and produce a spurious error even though it checked properly the *second* time.
There was a problem hiding this comment.
Thanks!
In general the solution looks right, but there is one thing that disturbs me a bit. While the overload overhaul last year we specifically took care about the case where an argument to an overloaded function is Any. If there are more that one matching overload, we should return Any to avoid further false positives. Does this mean that there is a breach in our logic? cc @Michael0x2a
| _S = TypeVar('_S') | ||
| _T = TypeVar('_T') | ||
| _R = TypeVar('_R') | ||
| _F = TypeVar('_F', bound=Callable[..., Any]) |
There was a problem hiding this comment.
This type variable is not used in the test.
| def partial(*args: Any) -> Any: | ||
| pass | ||
|
|
||
|
|
| @@ -3276,7 +3276,7 @@ def accept(self, | |||
| has_any_type(typ)): | |||
| self.msg.disallowed_any_type(typ, node) | |||
|
|
|||
| if not self.chk.in_checked_function(): | |||
| if not self.chk.in_checked_function() or self.chk.current_node_deferred: | |||
There was a problem hiding this comment.
I would return TypeOfAny.special_form for the latter case.
|
@ilevkivskyi -- I did a bit of digging, and it seems the breach you were talking about is rooted in how we're computing the When you try running the test case in this PR, the args are of type It turned out the reason for this is that Not sure if/how this should be fixed -- I'm not very familiar with what the node-deferring semantics are supposed to be. Maybe we could add a new TypeOfAny for deferred nodes, or maybe we could add a check to the overload logic to return early if a node is deferred? Or maybe Sully's fix is still the best approach overall. |
|
I'm going to merge this as is, since we need this at Dropbox urgently. The improvements discussed above may make sense as separate PRs. |
|
@Michael0x2a Coming back to this I think we can live with this fix as is. I will only submit a PR with minor tests cleanups I suggested. |
…rred (python#6860) This fixes an issue where the Any that is inferred temporarily when a node is deferred could cause a bad overload to be picked and produce a spurious error even though it checked properly the *second* time.
This fixes an issue where the Any that is inferred temporarily when a
node is deferred could cause a bad overload to be picked and produce
a spurious error even though it checked properly the second time.