Skip to content

Commit

Permalink
[red-knot] Simplify tuples containing Never (#14744)
Browse files Browse the repository at this point in the history
## Summary

Simplify tuples containing `Never` to `Never`:

```py
from typing import Never

def never() -> Never: ...

reveal_type((1, never(), "foo"))  # revealed: Never
```

I should note that mypy and pyright do *not* perform this
simplification. I don't know why.


There is [only one
place](https://github.com/astral-sh/ruff/blob/5137fcc9c81610f687b6cb45413ef83c2c5eea73/crates/red_knot_python_semantic/src/types/infer.rs#L1477-L1484)
where we use `TupleType::new` directly (instead of `Type::tuple`, which
changes behavior here). This appears when creating `TypeVar`
constraints, and it looks to me like it should stay this way, because
we're using `TupleType` to store a list of constraints there, instead of
an actual type. We also store `tuple[constraint1, constraint2, …]` as
the type for the `constraint1, constraint2, …` tuple expression. This
would mean that we infer a type of `tuple[str, Never]` for the following
type variable constraints, without simplifying it to `Never`. This seems
like a weird edge case that's maybe not worth looking further into?!
```py
from typing import Never

#         vvvvvvvvvv
def f[T: (str, Never)](x: T):
    pass
```

## Test Plan

- Added a new unit test. Did not add additional Markdown tests as that
seems superfluous.
- Tested the example above using red knot, mypy, pyright.
- Verified that this allows us to remove `contains_never` from the
property tests
(#14178 (comment))
  • Loading branch information
sharkdp authored Dec 3, 2024
1 parent c2e17d0 commit a69dfd4
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 6 deletions.
36 changes: 32 additions & 4 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,11 @@ impl<'db> Type<'db> {
Self::BytesLiteral(BytesLiteralType::new(db, bytes))
}

pub fn tuple(db: &'db dyn Db, elements: &[Type<'db>]) -> Self {
Self::Tuple(TupleType::new(db, elements))
pub fn tuple<T: Into<Type<'db>>>(
db: &'db dyn Db,
elements: impl IntoIterator<Item = T>,
) -> Self {
TupleType::from_elements(db, elements)
}

#[must_use]
Expand Down Expand Up @@ -3015,6 +3018,23 @@ pub struct TupleType<'db> {
}

impl<'db> TupleType<'db> {
pub fn from_elements<T: Into<Type<'db>>>(
db: &'db dyn Db,
types: impl IntoIterator<Item = T>,
) -> Type<'db> {
let mut elements = vec![];

for ty in types {
let ty = ty.into();
if ty.is_never() {
return Type::Never;
}
elements.push(ty);
}

Type::Tuple(Self::new(db, elements.into_boxed_slice()))
}

pub fn get(&self, db: &'db dyn Db, index: usize) -> Option<Type<'db>> {
self.elements(db).get(index).copied()
}
Expand Down Expand Up @@ -3122,13 +3142,21 @@ pub(crate) mod tests {
builder.build()
}
Ty::Tuple(tys) => {
let elements: Vec<Type> = tys.into_iter().map(|ty| ty.into_type(db)).collect();
Type::tuple(db, &elements)
let elements = tys.into_iter().map(|ty| ty.into_type(db));
Type::tuple(db, elements)
}
}
}
}

#[test_case(Ty::Tuple(vec![Ty::Never]))]
#[test_case(Ty::Tuple(vec![Ty::BuiltinInstance("str"), Ty::Never, Ty::BuiltinInstance("int")]))]
#[test_case(Ty::Tuple(vec![Ty::Tuple(vec![Ty::Never])]))]
fn tuple_containing_never_simplifies_to_never(ty: Ty) {
let db = setup_db();
assert_eq!(ty.into_type(&db), Type::Never);
}

#[test_case(Ty::BuiltinInstance("str"), Ty::BuiltinInstance("object"))]
#[test_case(Ty::BuiltinInstance("int"), Ty::BuiltinInstance("object"))]
#[test_case(Ty::Unknown, Ty::IntLiteral(1))]
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4537,7 +4537,7 @@ impl<'db> TypeInferenceBuilder<'db> {
if element_could_alter_type_of_whole_tuple(single_element, single_element_ty) {
todo_type!()
} else {
Type::tuple(self.db, &[single_element_ty])
Type::tuple(self.db, [single_element_ty])
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/types/unpacker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<'db> Unpacker<'db> {
// there would be a cost and it's not clear that it's worth it.
let value_ty = Type::tuple(
self.db,
&vec![Type::LiteralString; string_literal_ty.len(self.db)],
std::iter::repeat(Type::LiteralString).take(string_literal_ty.len(self.db)),
);
self.unpack(target, value_ty, scope);
}
Expand Down

0 comments on commit a69dfd4

Please sign in to comment.