Skip to content

Commit

Permalink
[red-knot] feat: introduce a new [Type::Todo] variant (#13548)
Browse files Browse the repository at this point in the history
This variant shows inference that is not yet implemented..

## Summary

PR #13500 reopened the idea of adding a new type variant to keep track
of not-implemented features in Red Knot.

It was based off of #12986 with a more generic approach of keeping track
of different kind of unknowns. Discussion in #13500 agreed that keeping
track of different `Unknown` is complicated for now, and this feature is
better achieved through a new variant of `Type`.

### Requirements

Requirements for this implementation can be summed up with some extracts
of comment from @carljm on the previous PR

> So at the moment we are leaning towards simplifying this PR to just
use a new top-level variant, which behaves like Any and Unknown but
represents inference that is not yet implemented in red-knot.

> I think the general rule should be that Todo should propagate only
when the presence of the input Todo caused the output to be unknown.
>
> To take a specific example, the inferred result of addition must be
Unknown if either operand is Unknown. That is, Unknown + X will always
be Unknown regardless of what X is. (Same for X + Unknown.) In this
case, I believe that Unknown + Todo (or Todo + Unknown) should result in
Unknown, not result in Todo. If we fix the upstream source of the Todo,
the result would still be Unknown, so it's not useful to propagate the
Todo in this case: it wrongly suggests that the output is unknown
because of a todo item.

## Test Plan

This PR does not introduce new tests, but it did required to edit some
tests with the display of `[Type::Todo]` (currently `@Todo`), which
suggests that those test are placeholders requirements for features we
don't support yet.
  • Loading branch information
Slyces authored Sep 30, 2024
1 parent 9d8a4c0 commit 6cdf996
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 79 deletions.
96 changes: 59 additions & 37 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,31 +233,40 @@ fn declarations_ty<'db>(
/// Unique ID for a type.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum Type<'db> {
/// the dynamic type: a statically-unknown set of values
/// The dynamic type: a statically-unknown set of values
Any,
/// the empty set of values
/// The empty set of values
Never,
/// unknown type (either no annotation, or some kind of type error)
/// equivalent to Any, or possibly to object in strict mode
/// Unknown type (either no annotation, or some kind of type error).
/// Equivalent to Any, or possibly to object in strict mode
Unknown,
/// name does not exist or is not bound to any value (this represents an error, but with some
/// Name does not exist or is not bound to any value (this represents an error, but with some
/// leniency options it could be silently resolved to Unknown in some cases)
Unbound,
/// the None object -- TODO remove this in favor of Instance(types.NoneType)
/// The None object -- TODO remove this in favor of Instance(types.NoneType)
None,
/// a specific function object
/// Temporary type for symbols that can't be inferred yet because of missing implementations.
/// Behaves equivalently to `Any`.
///
/// This variant should eventually be removed once red-knot is spec-compliant.
///
/// General rule: `Todo` should only propagate when the presence of the input `Todo` caused the
/// output to be unknown. An output should only be `Todo` if fixing all `Todo` inputs to be not
/// `Todo` would change the output type.
Todo,
/// A specific function object
Function(FunctionType<'db>),
/// The `typing.reveal_type` function, which has special `__call__` behavior.
RevealTypeFunction(FunctionType<'db>),
/// a specific module object
/// A specific module object
Module(File),
/// a specific class object
/// A specific class object
Class(ClassType<'db>),
/// the set of Python objects with the given class in their __class__'s method resolution order
/// The set of Python objects with the given class in their __class__'s method resolution order
Instance(ClassType<'db>),
/// the set of objects in any of the types in the union
/// The set of objects in any of the types in the union
Union(UnionType<'db>),
/// the set of objects in all of the types in the intersection
/// The set of objects in all of the types in the intersection
Intersection(IntersectionType<'db>),
/// An integer literal
IntLiteral(i64),
Expand Down Expand Up @@ -402,8 +411,8 @@ impl<'db> Type<'db> {
return true;
}
match (self, target) {
(Type::Unknown | Type::Any, _) => false,
(_, Type::Unknown | Type::Any) => false,
(Type::Unknown | Type::Any | Type::Todo, _) => false,
(_, Type::Unknown | Type::Any | Type::Todo) => false,
(Type::Never, _) => true,
(_, Type::Never) => false,
(Type::IntLiteral(_), Type::Instance(class))
Expand Down Expand Up @@ -438,8 +447,8 @@ impl<'db> Type<'db> {
/// [assignable to]: https://typing.readthedocs.io/en/latest/spec/concepts.html#the-assignable-to-or-consistent-subtyping-relation
pub(crate) fn is_assignable_to(self, db: &'db dyn Db, target: Type<'db>) -> bool {
match (self, target) {
(Type::Unknown | Type::Any, _) => true,
(_, Type::Unknown | Type::Any) => true,
(Type::Unknown | Type::Any | Type::Todo, _) => true,
(_, Type::Unknown | Type::Any | Type::Todo) => true,
(ty, Type::Union(union)) => union
.elements(db)
.iter()
Expand Down Expand Up @@ -475,53 +484,54 @@ impl<'db> Type<'db> {
Type::Any => Type::Any,
Type::Never => {
// TODO: attribute lookup on Never type
Type::Unknown
Type::Todo
}
Type::Unknown => Type::Unknown,
Type::Unbound => Type::Unbound,
Type::None => {
// TODO: attribute lookup on None type
Type::Unknown
Type::Todo
}
Type::Function(_) | Type::RevealTypeFunction(_) => {
// TODO: attribute lookup on function type
Type::Unknown
Type::Todo
}
Type::Module(file) => global_symbol_ty(db, *file, name),
Type::Class(class) => class.class_member(db, name),
Type::Instance(_) => {
// TODO MRO? get_own_instance_member, get_instance_member
Type::Unknown
Type::Todo
}
Type::Union(union) => union.map(db, |element| element.member(db, name)),
Type::Intersection(_) => {
// TODO perform the get_member on each type in the intersection
// TODO return the intersection of those results
Type::Unknown
Type::Todo
}
Type::IntLiteral(_) => {
// TODO raise error
Type::Unknown
Type::Todo
}
Type::BooleanLiteral(_) => Type::Unknown,
Type::BooleanLiteral(_) => Type::Todo,
Type::StringLiteral(_) => {
// TODO defer to `typing.LiteralString`/`builtins.str` methods
// from typeshed's stubs
Type::Unknown
Type::Todo
}
Type::LiteralString => {
// TODO defer to `typing.LiteralString`/`builtins.str` methods
// from typeshed's stubs
Type::Unknown
Type::Todo
}
Type::BytesLiteral(_) => {
// TODO defer to Type::Instance(<bytes from typeshed>).member
Type::Unknown
Type::Todo
}
Type::Tuple(_) => {
// TODO: implement tuple methods
Type::Unknown
Type::Todo
}
Type::Todo => Type::Todo,
}
}

Expand All @@ -531,7 +541,9 @@ impl<'db> Type<'db> {
/// when `bool(x)` is called on an object `x`.
fn bool(&self, db: &'db dyn Db) -> Truthiness {
match self {
Type::Any | Type::Never | Type::Unknown | Type::Unbound => Truthiness::Ambiguous,
Type::Any | Type::Todo | Type::Never | Type::Unknown | Type::Unbound => {
Truthiness::Ambiguous
}
Type::None => Truthiness::AlwaysFalse,
Type::Function(_) | Type::RevealTypeFunction(_) => Truthiness::AlwaysTrue,
Type::Module(_) => Truthiness::AlwaysTrue,
Expand Down Expand Up @@ -602,11 +614,13 @@ impl<'db> Type<'db> {
}

// TODO: handle classes which implement the `__call__` protocol
Type::Instance(_instance_ty) => CallOutcome::callable(Type::Unknown),
Type::Instance(_instance_ty) => CallOutcome::callable(Type::Todo),

// `Any` is callable, and its return type is also `Any`.
Type::Any => CallOutcome::callable(Type::Any),

Type::Todo => CallOutcome::callable(Type::Todo),

Type::Unknown => CallOutcome::callable(Type::Unknown),

Type::Union(union) => CallOutcome::union(
Expand All @@ -619,7 +633,7 @@ impl<'db> Type<'db> {
),

// TODO: intersection types
Type::Intersection(_) => CallOutcome::callable(Type::Unknown),
Type::Intersection(_) => CallOutcome::callable(Type::Todo),

_ => CallOutcome::not_callable(self),
}
Expand All @@ -640,6 +654,12 @@ impl<'db> Type<'db> {
};
}

if let Type::Unknown | Type::Any = self {
// Explicit handling of `Unknown` and `Any` necessary until `type[Unknown]` and
// `type[Any]` are not defined as `Todo` anymore.
return IterationOutcome::Iterable { element_ty: self };
}

// `self` represents the type of the iterable;
// `__iter__` and `__next__` are both looked up on the class of the iterable:
let iterable_meta_type = self.to_meta_type(db);
Expand Down Expand Up @@ -686,13 +706,14 @@ impl<'db> Type<'db> {
pub fn to_instance(&self, db: &'db dyn Db) -> Type<'db> {
match self {
Type::Any => Type::Any,
Type::Todo => Type::Todo,
Type::Unknown => Type::Unknown,
Type::Unbound => Type::Unknown,
Type::Never => Type::Never,
Type::Class(class) => Type::Instance(*class),
Type::Union(union) => union.map(db, |element| element.to_instance(db)),
// TODO: we can probably do better here: --Alex
Type::Intersection(_) => Type::Unknown,
Type::Intersection(_) => Type::Todo,
// TODO: calling `.to_instance()` on any of these should result in a diagnostic,
// since they already indicate that the object is an instance of some kind:
Type::BooleanLiteral(_)
Expand Down Expand Up @@ -723,18 +744,19 @@ impl<'db> Type<'db> {
Type::IntLiteral(_) => builtins_symbol_ty(db, "int"),
Type::Function(_) | Type::RevealTypeFunction(_) => types_symbol_ty(db, "FunctionType"),
Type::Module(_) => types_symbol_ty(db, "ModuleType"),
Type::Tuple(_) => builtins_symbol_ty(db, "tuple"),
Type::None => typeshed_symbol_ty(db, "NoneType"),
// TODO not accurate if there's a custom metaclass...
Type::Class(_) => builtins_symbol_ty(db, "type"),
// TODO can we do better here? `type[LiteralString]`?
Type::StringLiteral(_) | Type::LiteralString => builtins_symbol_ty(db, "str"),
// TODO: `type[Any]`?
Type::Any => Type::Any,
Type::Any => Type::Todo,
// TODO: `type[Unknown]`?
Type::Unknown => Type::Unknown,
Type::Unknown => Type::Todo,
// TODO intersections
Type::Intersection(_) => Type::Unknown,
Type::Tuple(_) => builtins_symbol_ty(db, "tuple"),
Type::Intersection(_) => Type::Todo,
Type::Todo => Type::Todo,
}
}

Expand Down Expand Up @@ -1064,7 +1086,7 @@ impl<'db> FunctionType<'db> {
// rather than from `bar`'s return annotation
// in order to determine the type that `bar` returns
if !function_stmt_node.decorator_list.is_empty() {
return Type::Unknown;
return Type::Todo;
}

function_stmt_node
Expand All @@ -1073,7 +1095,7 @@ impl<'db> FunctionType<'db> {
.map(|returns| {
if function_stmt_node.is_async {
// TODO: generic `types.CoroutineType`!
Type::Unknown
Type::Todo
} else {
definition_expression_ty(db, definition, returns.as_ref())
}
Expand Down
3 changes: 2 additions & 1 deletion crates/red_knot_python_semantic/src/types/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ impl<'db> InnerIntersectionBuilder<'db> {

/// Adds a positive type to this intersection.
fn add_positive(&mut self, db: &'db dyn Db, ty: Type<'db>) {
// TODO `Any`/`Unknown`/`Todo` actually should not self-cancel
match ty {
Type::Intersection(inter) => {
let pos = inter.positive(db);
Expand All @@ -234,7 +235,7 @@ impl<'db> InnerIntersectionBuilder<'db> {

/// Adds a negative type to this intersection.
fn add_negative(&mut self, db: &'db dyn Db, ty: Type<'db>) {
// TODO Any/Unknown actually should not self-cancel
// TODO `Any`/`Unknown`/`Todo` actually should not self-cancel
match ty {
Type::Intersection(intersection) => {
let pos = intersection.negative(db);
Expand Down
3 changes: 3 additions & 0 deletions crates/red_knot_python_semantic/src/types/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ impl Display for DisplayRepresentation<'_> {
Type::Unknown => f.write_str("Unknown"),
Type::Unbound => f.write_str("Unbound"),
Type::None => f.write_str("None"),
// `[Type::Todo]`'s display should be explicit that is not a valid display of
// any other type
Type::Todo => f.write_str("@Todo"),
Type::Module(file) => {
write!(f, "<module '{:?}'>", file.path(self.db))
}
Expand Down
Loading

0 comments on commit 6cdf996

Please sign in to comment.