Unexpected conditional expression type inference change #7986
Comments
|
I would say this is rather a bug (I am however not sure whether it previously worked on purpose or by coincidence). I think the inferred type of |
|
Just to be clear: the new behavior is a bug, or the old behavior is a bug? |
|
The only thing I can think of is the changes to I'm not 100% sure, but I feel the new behavior is probably a bug? I tried playing around with the example function, and I could never get it to return a list, so a inferred return type of But maybe the deeper bug is that there's a discrepancy between how we handle |
IMO the new behaviour is a false positive. |
I think there is a chance a pretty simple fix exists. Will you have time to look into preserving |
|
Sure, I can take a look. |
This pull request modifies all of our `copy_modified` type methods so they always copy over the previously inferred values of the `can_be_true` and `can_be_false` fields. I'm not entirely convinced copying over these fields is fully correct: we can sometimes end up accidentally inferring the wrong result when we try reassigning variables or calling mutating functions. For example, mypy incorrectly infers 'bool' in the following two variants of the example in python#7986 instead of `Union[List[str], bool]`: def bad1(a: List[str], b: bool) -> bool: x = a and b a = ["foo"] y: bool return x or y # Should be an error def bad2(a: List[str], b: bool) -> bool: x = a and b a.append("foo") y: bool return x or y # Should be an error But we always copy over these two fields for non-generic Instances, I think it would be consistent to just always do so, for all types. Fixes python#7986, probably.
This pull request modifies the `last_known_value` eraser so it
always copies over the previously inferred value of the
`can_be_true` and `can_be_false` fields.
This isn't entirely typesafe -- for example, we can sometimes
accidentally infer the wrong result if we try reassigning variables
or call a mutating function :
def bad1(a: List[str], b: bool) -> bool:
x = a and b
a = ["foo"]
y: bool
return x or y # Should be an error
def bad2(a: List[str], b: bool) -> bool:
x = a and b
a.append("foo")
y: bool
return x or y # Should be an error
But this was apparently the old behavior/something mypy wasn't
previously able to detect, so I guess this is fine for now.
I also decided against modifying every `copy_modified` method in
all the type classes because of this reason: I wanted to limit
the spread of this potentially misleading additional inferred info.
Fixes python#7986, probably.
This pull request modifies the `last_known_value` eraser so it
always copies over the previously inferred value of the
`can_be_true` and `can_be_false` fields.
This isn't entirely typesafe -- for example, we can sometimes
accidentally infer the wrong result if we try reassigning variables
or call a mutating function :
```
def bad1(a: List[str], b: bool) -> bool:
x = a and b
a = ["foo"]
y: bool
return x or y # Should be an error
def bad2(a: List[str], b: bool) -> bool:
x = a and b
a.append("foo")
y: bool
return x or y # Should be an error
```
But this was apparently the old behavior/something mypy wasn't
previously able to detect, so I guess this is fine for now.
I also decided against modifying every `copy_modified` method in
all the type classes because of this reason: I wanted to limit
the spread of this potentially misleading additional inferred info.
Fixes #7986, probably.
#7955 caused this example to start generating type errors:
Previously, this generated no errors. It's unclear whether this fixed a false negative or whether this is a regression.
@Michael0x2a Do you have an idea of what might be going on?
The text was updated successfully, but these errors were encountered: