Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Track root cause of why a type is inferred as Unknown #12986

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 65 additions & 23 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,15 @@ pub(crate) fn definitions_ty<'db>(
}

/// Unique ID for a type.
#[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum Type<'db> {
/// the dynamic type: a statically-unknown set of values
Any,
/// 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,
Unknown(UnknownTypeKind),
/// 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,
Expand Down Expand Up @@ -189,10 +189,6 @@ impl<'db> Type<'db> {
matches!(self, Type::Unbound)
}

pub const fn is_unknown(&self) -> bool {
matches!(self, Type::Unknown)
}

pub const fn is_never(&self) -> bool {
matches!(self, Type::Never)
}
Expand All @@ -207,18 +203,24 @@ impl<'db> Type<'db> {
}
}

/// Apply a function to the type (if it is a non-union); or,
/// if it is a union, apply the function to each element of the union
/// and build a new union from the result
#[must_use]
pub fn replace_unbound_with(&self, db: &'db dyn Db, replacement: Type<'db>) -> Type<'db> {
pub fn recursive_transform(
self,
db: &'db dyn Db,
mut transform_fn: impl FnMut(Type<'db>) -> Type<'db>,
) -> Self {
match self {
Type::Unbound => replacement,
Type::Union(union) => union
.elements(db)
.into_iter()
.fold(UnionBuilder::new(db), |builder, ty| {
builder.add(ty.replace_unbound_with(db, replacement))
builder.add(transform_fn(ty))
})
.build(),
ty => *ty,
ty => transform_fn(ty),
}
}

Expand All @@ -241,23 +243,23 @@ impl<'db> Type<'db> {
Type::Any => Type::Any,
Type::Never => {
// TODO: attribute lookup on Never type
Type::Unknown
Type::Unknown(UnknownTypeKind::RedKnotLimitation)
}
Type::Unknown => Type::Unknown,
Type::Unknown(kind) => Type::Unknown(*kind),
Type::Unbound => Type::Unbound,
Type::None => {
// TODO: attribute lookup on None type
Type::Unknown
Type::Unknown(UnknownTypeKind::RedKnotLimitation)
}
Type::Function(_) => {
// TODO: attribute lookup on function type
Type::Unknown
Type::Unknown(UnknownTypeKind::RedKnotLimitation)
}
Type::Module(file) => global_symbol_ty_by_name(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::Unknown(UnknownTypeKind::RedKnotLimitation)
}
Type::Union(union) => union
.elements(db)
Expand All @@ -269,23 +271,66 @@ impl<'db> Type<'db> {
Type::Intersection(_) => {
// TODO perform the get_member on each type in the intersection
// TODO return the intersection of those results
Type::Unknown
Type::Unknown(UnknownTypeKind::RedKnotLimitation)
}
Type::IntLiteral(_) => {
// TODO raise error
Type::Unknown
Type::Unknown(UnknownTypeKind::RedKnotLimitation)
}
Type::BooleanLiteral(_) => Type::Unknown,
Type::BooleanLiteral(_) => Type::Unknown(UnknownTypeKind::RedKnotLimitation),
}
}

#[must_use]
pub fn instance(&self) -> Type<'db> {
match self {
Type::Any => Type::Any,
Type::Unknown => Type::Unknown,
Type::Unknown(kind) => Type::Unknown(*kind),
Type::Class(class) => Type::Instance(*class),
_ => Type::Unknown, // TODO type errors
_ => Type::Unknown(UnknownTypeKind::RedKnotLimitation), // TODO type errors
}
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum UnknownTypeKind {
/// Temporary variant that indicates that we *should*
/// be able to infer a type here in due course, but currently can't
RedKnotLimitation,

/// Invalid syntax in the node means we can't infer a type here
InvalidSyntax,

/// An `Unknown` type stemming from some kind of type error.
///
/// Examples:
/// - A function was called with argument(s) of incorrect type(s), leading
/// to a diagnostic being emitted and the expression being evaluated as `Unknown`.
/// - An expression was deemed to take place between two invalid types, e.g. `1 + "foo"`.
/// (This is really the same as the first example, since expressions ultimately desguar
/// to function calls, e.g. `1 + "foo"` desugars approximately to `type(1).__add__("foo")`.)
/// - An attribute or method was looked up on an instance of a type that doesn't have that
/// attribute or method.
TypeError,
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved

/// The symbol was unannotated and the true type can't be reasonably inferred
UnannotatedSymbol,

/// The type of symbols imported by imports that could not be resolved
UnresolvedImport,

/// A "second-order" Unknown type, that results from an interaction with
/// a "first-order" Unknown type. For example, if the type of `x` is `Unknown`,
/// the type of `x + 1` will also be `Unknown`
SecondOrder,
}

impl UnknownTypeKind {
pub(crate) fn union(self, other: Self) -> Self {
if self == other {
self
} else {
UnknownTypeKind::SecondOrder
}
}
}
Expand Down Expand Up @@ -456,9 +501,6 @@ mod tests {
);
}

#[ignore = "\
A spurious second 'Unresolved import' diagnostic message is emitted on `b.py`, \
despite the symbol existing in the symbol table for `a.py`"]
Comment on lines -459 to -461
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following ecd9e6a, the key benefit of this PR is now that this test can be unskipped. This improvement is also reflected in the fact that six spurious "unresolved import" diagnostics go away in the benchmarks.

#[test]
fn resolved_import_of_symbol_from_unresolved_import() {
let mut db = setup_db();
Expand Down
14 changes: 12 additions & 2 deletions crates/red_knot_python_semantic/src/types/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@
//! * No type in an intersection can be a supertype of any other type in the intersection (just
//! eliminate the supertype from the intersection).
//! * An intersection containing two non-overlapping types should simplify to [`Type::Never`].
use crate::types::{IntersectionType, Type, UnionType};
use crate::types::{IntersectionType, Type, UnionType, UnknownTypeKind};
use crate::{Db, FxOrderSet};

pub(crate) struct UnionBuilder<'db> {
elements: FxOrderSet<Type<'db>>,
unknown_elements: Option<UnknownTypeKind>,
db: &'db dyn Db,
}

Expand All @@ -38,6 +39,7 @@ impl<'db> UnionBuilder<'db> {
Self {
db,
elements: FxOrderSet::default(),
unknown_elements: None,
}
}

Expand All @@ -48,6 +50,12 @@ impl<'db> UnionBuilder<'db> {
self.elements.extend(&union.elements(self.db));
}
Type::Never => {}
Type::Unknown(kind) => {
self.unknown_elements = Some(
self.unknown_elements
.map_or(kind, |existing| existing.union(kind)),
);
}
_ => {
self.elements.insert(ty);
}
Expand All @@ -56,7 +64,9 @@ impl<'db> UnionBuilder<'db> {
self
}

pub(crate) fn build(self) -> Type<'db> {
pub(crate) fn build(mut self) -> Type<'db> {
self.elements
.extend(self.unknown_elements.map(Type::Unknown));
match self.elements.len() {
0 => Type::Never,
1 => self.elements[0],
Expand Down
2 changes: 1 addition & 1 deletion crates/red_knot_python_semantic/src/types/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl Display for DisplayType<'_> {
match self.ty {
Type::Any => f.write_str("Any"),
Type::Never => f.write_str("Never"),
Type::Unknown => f.write_str("Unknown"),
Type::Unknown(_) => f.write_str("Unknown"),
Type::Unbound => f.write_str("Unbound"),
Type::None => f.write_str("None"),
Type::Module(file) => {
Expand Down
Loading
Loading