Skip to content

Commit

Permalink
[red-knot] Consolidate all gradual types into single Type variant (#1…
Browse files Browse the repository at this point in the history
…5386)

Prompted by

> One nit: I think we need to consider `Any` and `Unknown` and `Todo` as
all (gradually) equivalent to each other, and thus `type & Any` and
`type & Unknown` and `type & Todo` as also equivalent. The distinction
between `Any` vs `Unknown` vs `Todo` is entirely about
provenance/debugging, there is no type level distinction. (And I've been
wondering if the `Any` vs `Unknown` distinction is really worth it.)

The thought here is that _most_ places want to treat `Any`, `Unknown`,
and `Todo` identically. So this PR simplifies things by having a single
`Type::Any` variant, and moves the provenance part into a new `AnyType`
type. If you need to treat e.g. `Todo` differently, you still can by
pattern-matching into the `AnyType`. But if you don't, you can just use
`Type::Any(_)`.

(This would also allow us to (more easily) distinguish "unknown via an
unannotated value" from "unknown because of a typing error" should we
want to do that in the future)

---------

Co-authored-by: Alex Waygood <[email protected]>
Co-authored-by: Carl Meyer <[email protected]>
  • Loading branch information
3 people authored Jan 10, 2025
1 parent b33cf5b commit baf0683
Show file tree
Hide file tree
Showing 10 changed files with 249 additions and 246 deletions.
173 changes: 95 additions & 78 deletions crates/red_knot_python_semantic/src/types.rs

Large diffs are not rendered by default.

44 changes: 22 additions & 22 deletions crates/red_knot_python_semantic/src/types/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ impl<'db> InnerIntersectionBuilder<'db> {
self.add_positive(db, *neg);
}
}
ty @ (Type::Any | Type::Unknown | Type::Todo(_)) => {
ty @ Type::Dynamic(_) => {
// Adding any of these types to the negative side of an intersection
// is equivalent to adding it to the positive side. We do this to
// simplify the representation.
Expand Down Expand Up @@ -434,7 +434,7 @@ mod tests {
fn build_intersection() {
let db = setup_db();
let t0 = Type::IntLiteral(0);
let ta = Type::Any;
let ta = Type::any();
let intersection = IntersectionBuilder::new(&db)
.add_positive(ta)
.add_negative(t0)
Expand All @@ -457,7 +457,7 @@ mod tests {
#[test]
fn build_intersection_flatten_positive() {
let db = setup_db();
let ta = Type::Any;
let ta = Type::any();
let t1 = Type::IntLiteral(1);
let t2 = Type::IntLiteral(2);
let i0 = IntersectionBuilder::new(&db)
Expand All @@ -477,7 +477,7 @@ mod tests {
#[test]
fn build_intersection_flatten_negative() {
let db = setup_db();
let ta = Type::Any;
let ta = Type::any();
let t1 = Type::IntLiteral(1);
let t2 = KnownClass::Int.to_instance(&db);
// i0 = Any & ~Literal[1]
Expand All @@ -503,13 +503,13 @@ mod tests {
let db = setup_db();

let ty = IntersectionBuilder::new(&db)
.add_negative(Type::Any)
.add_negative(Type::any())
.build();
assert_eq!(ty, Type::Any);
assert_eq!(ty, Type::any());

let ty = IntersectionBuilder::new(&db)
.add_positive(Type::Never)
.add_negative(Type::Any)
.add_negative(Type::any())
.build();
assert_eq!(ty, Type::Never);
}
Expand All @@ -519,32 +519,32 @@ mod tests {
let db = setup_db();

let ty = IntersectionBuilder::new(&db)
.add_positive(Type::Unknown)
.add_positive(Type::Unknown)
.add_positive(Type::unknown())
.add_positive(Type::unknown())
.build();
assert_eq!(ty, Type::Unknown);
assert_eq!(ty, Type::unknown());

let ty = IntersectionBuilder::new(&db)
.add_positive(Type::Unknown)
.add_negative(Type::Unknown)
.add_positive(Type::unknown())
.add_negative(Type::unknown())
.build();
assert_eq!(ty, Type::Unknown);
assert_eq!(ty, Type::unknown());

let ty = IntersectionBuilder::new(&db)
.add_negative(Type::Unknown)
.add_negative(Type::Unknown)
.add_negative(Type::unknown())
.add_negative(Type::unknown())
.build();
assert_eq!(ty, Type::Unknown);
assert_eq!(ty, Type::unknown());

let ty = IntersectionBuilder::new(&db)
.add_positive(Type::Unknown)
.add_positive(Type::unknown())
.add_positive(Type::IntLiteral(0))
.add_negative(Type::Unknown)
.add_negative(Type::unknown())
.build();
assert_eq!(
ty,
IntersectionBuilder::new(&db)
.add_positive(Type::Unknown)
.add_positive(Type::unknown())
.add_positive(Type::IntLiteral(0))
.build()
);
Expand All @@ -555,7 +555,7 @@ mod tests {
let db = setup_db();
let t0 = Type::IntLiteral(0);
let t1 = Type::IntLiteral(1);
let ta = Type::Any;
let ta = Type::any();
let u0 = UnionType::from_elements(&db, [t0, t1]);

let union = IntersectionBuilder::new(&db)
Expand Down Expand Up @@ -958,8 +958,8 @@ mod tests {
assert_eq!(ty, Type::BooleanLiteral(!bool_value));
}

#[test_case(Type::Any)]
#[test_case(Type::Unknown)]
#[test_case(Type::any())]
#[test_case(Type::unknown())]
#[test_case(todo_type!())]
fn build_intersection_t_and_negative_t_does_not_simplify(ty: Type) {
let db = setup_db();
Expand Down
10 changes: 5 additions & 5 deletions crates/red_knot_python_semantic/src/types/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl<'db> CallOutcome<'db> {
match (acc, ty) {
(None, None) => None,
(None, Some(ty)) => Some(UnionBuilder::new(db).add(ty)),
(Some(builder), ty) => Some(builder.add(ty.unwrap_or(Type::Unknown))),
(Some(builder), ty) => Some(builder.add(ty.unwrap_or(Type::unknown()))),
}
})
.map(UnionBuilder::build),
Expand Down Expand Up @@ -206,7 +206,7 @@ impl<'db> CallOutcome<'db> {
}
Self::NotCallable { not_callable_ty } => Err(NotCallableError::Type {
not_callable_ty: *not_callable_ty,
return_ty: Type::Unknown,
return_ty: Type::unknown(),
}),
Self::PossiblyUnboundDunderCall {
called_ty,
Expand All @@ -215,7 +215,7 @@ impl<'db> CallOutcome<'db> {
callable_ty: *called_ty,
return_ty: call_outcome
.return_ty(context.db())
.unwrap_or(Type::Unknown),
.unwrap_or(Type::unknown()),
}),
Self::Union {
outcomes,
Expand All @@ -228,7 +228,7 @@ impl<'db> CallOutcome<'db> {
let return_ty = match outcome {
Self::NotCallable { not_callable_ty } => {
not_callable.push(*not_callable_ty);
Type::Unknown
Type::unknown()
}
Self::RevealType {
binding,
Expand Down Expand Up @@ -307,7 +307,7 @@ impl<'db> CallOutcome<'db> {
}
}

Ok(Type::Unknown)
Ok(Type::unknown())
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/src/types/call/bind.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ pub(crate) fn bind_call<'db>(

CallBinding {
callable_ty,
return_ty: signature.return_ty.unwrap_or(Type::Unknown),
return_ty: signature.return_ty.unwrap_or(Type::unknown()),
parameter_tys: parameter_tys
.into_iter()
.map(|opt_ty| opt_ty.unwrap_or(Type::Unknown))
.map(|opt_ty| opt_ty.unwrap_or(Type::unknown()))
.collect(),
errors,
}
Expand Down
46 changes: 20 additions & 26 deletions crates/red_knot_python_semantic/src/types/class_base.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::types::{
todo_type, Class, ClassLiteralType, KnownClass, KnownInstanceType, TodoType, Type,
todo_type, Class, ClassLiteralType, DynamicType, KnownClass, KnownInstanceType, Type,
};
use crate::Db;
use itertools::Either;
Expand All @@ -8,19 +8,25 @@ use itertools::Either;
///
/// This is much more limited than the [`Type`] enum:
/// all types that would be invalid to have as a class base are
/// transformed into [`ClassBase::Unknown`]
/// transformed into [`ClassBase::unknown`]
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, salsa::Update)]
pub enum ClassBase<'db> {
Any,
Unknown,
Todo(TodoType),
Dynamic(DynamicType),
Class(Class<'db>),
}

impl<'db> ClassBase<'db> {
pub const fn any() -> Self {
Self::Dynamic(DynamicType::Any)
}

pub const fn unknown() -> Self {
Self::Dynamic(DynamicType::Unknown)
}

pub const fn is_dynamic(self) -> bool {
match self {
ClassBase::Any | ClassBase::Unknown | ClassBase::Todo(_) => true,
ClassBase::Dynamic(_) => true,
ClassBase::Class(_) => false,
}
}
Expand All @@ -34,9 +40,7 @@ impl<'db> ClassBase<'db> {
impl std::fmt::Display for Display<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.base {
ClassBase::Any => f.write_str("Any"),
ClassBase::Todo(todo) => todo.fmt(f),
ClassBase::Unknown => f.write_str("Unknown"),
ClassBase::Dynamic(dynamic) => dynamic.fmt(f),
ClassBase::Class(class) => write!(f, "<class '{}'>", class.name(self.db)),
}
}
Expand All @@ -50,7 +54,7 @@ impl<'db> ClassBase<'db> {
KnownClass::Object
.to_class_literal(db)
.into_class_literal()
.map_or(Self::Unknown, |ClassLiteralType { class }| {
.map_or(Self::unknown(), |ClassLiteralType { class }| {
Self::Class(class)
})
}
Expand All @@ -60,9 +64,7 @@ impl<'db> ClassBase<'db> {
/// Return `None` if `ty` is not an acceptable type for a class base.
pub(super) fn try_from_ty(db: &'db dyn Db, ty: Type<'db>) -> Option<Self> {
match ty {
Type::Any => Some(Self::Any),
Type::Unknown => Some(Self::Unknown),
Type::Todo(todo) => Some(Self::Todo(todo)),
Type::Dynamic(dynamic) => Some(Self::Dynamic(dynamic)),
Type::ClassLiteral(ClassLiteralType { class }) => Some(Self::Class(class)),
Type::Union(_) => None, // TODO -- forces consideration of multiple possible MROs?
Type::Intersection(_) => None, // TODO -- probably incorrect?
Expand Down Expand Up @@ -104,8 +106,8 @@ impl<'db> ClassBase<'db> {
| KnownInstanceType::Not
| KnownInstanceType::Intersection
| KnownInstanceType::TypeOf => None,
KnownInstanceType::Unknown => Some(Self::Unknown),
KnownInstanceType::Any => Some(Self::Any),
KnownInstanceType::Unknown => Some(Self::unknown()),
KnownInstanceType::Any => Some(Self::any()),
// TODO: Classes inheriting from `typing.Type` et al. also have `Generic` in their MRO
KnownInstanceType::Dict => {
Self::try_from_ty(db, KnownClass::Dict.to_class_literal(db))
Expand Down Expand Up @@ -150,7 +152,7 @@ impl<'db> ClassBase<'db> {
pub(super) fn into_class(self) -> Option<Class<'db>> {
match self {
Self::Class(class) => Some(class),
_ => None,
Self::Dynamic(_) => None,
}
}

Expand All @@ -160,13 +162,7 @@ impl<'db> ClassBase<'db> {
db: &'db dyn Db,
) -> Either<impl Iterator<Item = ClassBase<'db>>, impl Iterator<Item = ClassBase<'db>>> {
match self {
ClassBase::Any => Either::Left([ClassBase::Any, ClassBase::object(db)].into_iter()),
ClassBase::Unknown => {
Either::Left([ClassBase::Unknown, ClassBase::object(db)].into_iter())
}
ClassBase::Todo(todo) => {
Either::Left([ClassBase::Todo(todo), ClassBase::object(db)].into_iter())
}
ClassBase::Dynamic(_) => Either::Left([self, ClassBase::object(db)].into_iter()),
ClassBase::Class(class) => Either::Right(class.iter_mro(db)),
}
}
Expand All @@ -181,9 +177,7 @@ impl<'db> From<Class<'db>> for ClassBase<'db> {
impl<'db> From<ClassBase<'db>> for Type<'db> {
fn from(value: ClassBase<'db>) -> Self {
match value {
ClassBase::Any => Type::Any,
ClassBase::Todo(todo) => Type::Todo(todo),
ClassBase::Unknown => Type::Unknown,
ClassBase::Dynamic(dynamic) => Type::Dynamic(dynamic),
ClassBase::Class(class) => Type::class_literal(class),
}
}
Expand Down
10 changes: 2 additions & 8 deletions crates/red_knot_python_semantic/src/types/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,8 @@ struct DisplayRepresentation<'db> {
impl Display for DisplayRepresentation<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self.ty {
Type::Any => f.write_str("Any"),
Type::Dynamic(dynamic) => dynamic.fmt(f),
Type::Never => f.write_str("Never"),
Type::Unknown => f.write_str("Unknown"),
Type::Instance(InstanceType { class }) => {
let representation = match class.known(self.db) {
Some(KnownClass::NoneType) => "None",
Expand All @@ -76,9 +75,6 @@ impl Display for DisplayRepresentation<'_> {
};
f.write_str(representation)
}
// `[Type::Todo]`'s display should be explicit that is not a valid display of
// any other type
Type::Todo(todo) => write!(f, "@Todo{todo}"),
Type::ModuleLiteral(module) => {
write!(f, "<module '{}'>", module.module(self.db).name())
}
Expand All @@ -88,9 +84,7 @@ impl Display for DisplayRepresentation<'_> {
// Only show the bare class name here; ClassBase::display would render this as
// type[<class 'Foo'>] instead of type[Foo].
ClassBase::Class(class) => write!(f, "type[{}]", class.name(self.db)),
base @ (ClassBase::Any | ClassBase::Todo(_) | ClassBase::Unknown) => {
write!(f, "type[{}]", base.display(self.db))
}
ClassBase::Dynamic(dynamic) => write!(f, "type[{dynamic}]"),
},
Type::KnownInstance(known_instance) => f.write_str(known_instance.repr(self.db)),
Type::FunctionLiteral(function) => f.write_str(function.name(self.db)),
Expand Down
Loading

0 comments on commit baf0683

Please sign in to comment.