[red-knot] Simplify tuples containing Never#14744
Conversation
sharkdp
requested review from
AlexWaygood,
MichaReiser and
carljm
as code owners
|
sharkdp
force-pushed
the
david/simplify-tuple-containing-never
branch
from
9ccef1a to
9b22492
Compare
sharkdp
force-pushed
the
david/simplify-tuple-containing-never
branch
from
9b22492 to
310a4a9
Compare
| pub fn tuple<T: Into<Type<'db>>>( | ||
| db: &'db dyn Db, | ||
| elements: impl IntoIterator<Item = T>, | ||
| ) -> Self { | ||
| TupleType::from_elements(db, elements) | ||
| } |
There was a problem hiding this comment.
nit: this method now feels a bit redundant. I'd probably have got rid of it and changed the callsites to use the new TupleType::from_elements() method
Mypy in general appears not to do much eager simplification at all, e.g. it doesn't even simplify this union: def f(x: int | int):
reveal_type(x) # mypy: revealed type is "Union[builtins.int, builtins.int]"Pyright does a lot more eager simplification than mypy. But it's important to note, I think, that it's only relatively recently that we formalized the idea that |
* main: [`ruff`] Extend unnecessary-regular-expression to non-literal strings (`RUF055`) (#14679) Minor followups to RUF052 (#14755) [red-knot] Property tests (#14178) [red-knot] `is_subtype_of` fix for `KnownInstance` types (#14750) Improve docs for flake8-use-pathlib rules (#14741) [`ruff`] Implemented `used-dummy-variable` (`RUF052`) (#14611) [red-knot] Simplify tuples containing `Never` (#14744) Possible fix for flaky file watching test (#14543) [`flake8-import-conventions`] Improve syntax check for aliases supplied in configuration for `unconventional-import-alias (ICN001)` (#14745) [red-knot] Deeper understanding of `LiteralString` (#14649) red-knot: support narrowing for bool(E) (#14668) [`refurb`] Handle non-finite decimals in `verbose-decimal-constructor (FURB157)` (#14596) [red-knot] Re-enable linter corpus tests (#14736)
## Summary Remove `Type::tuple` in favor of `TupleType::from_elements`, avoid a few intermediate `Vec`tors. Resolves an old [review comment](#14744 (comment)). ## Test Plan New regression test for something I ran into while implementing this.
3 participants
Summary
Simplify tuples containing
NevertoNever:I should note that mypy and pyright do not perform this simplification. I don't know why.
There is only one place where we use
TupleType::newdirectly (instead ofType::tuple, which changes behavior here). This appears when creatingTypeVarconstraints, and it looks to me like it should stay this way, because we're usingTupleTypeto store a list of constraints there, instead of an actual type. We also storetuple[constraint1, constraint2, …]as the type for theconstraint1, constraint2, …tuple expression. This would mean that we infer a type oftuple[str, Never]for the following type variable constraints, without simplifying it toNever. This seems like a weird edge case that's maybe not worth looking further into?!Test Plan
contains_neverfrom the property tests ([red-knot] Property tests #14178 (comment))