Skip to content

Commit

Permalink
[red-knot] Reduce usage of From<Type> implementations when working …
Browse files Browse the repository at this point in the history 
…with `Symbol`s (#16076)
  • Loading branch information
@AlexWaygood
AlexWaygood authored Feb 11, 2025
1 parent 69d86d1 commit df1d430
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 77 deletions.
13 changes: 12 additions & 1 deletion crates/red_knot_python_semantic/src/symbol.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
types::{Type, UnionType},
types::{todo_type, Type, UnionType},
Db,
};

Expand Down Expand Up @@ -33,6 +33,17 @@ pub(crate) enum Symbol<'db> {
}

impl<'db> Symbol<'db> {
/// Constructor that creates a `Symbol` with boundness [`Boundness::Bound`].
pub(crate) fn bound(ty: impl Into<Type<'db>>) -> Self {
Symbol::Type(ty.into(), Boundness::Bound)
}

/// Constructor that creates a [`Symbol`] with a [`crate::types::TodoType`] type
/// and boundness [`Boundness::Bound`].
pub(crate) fn todo(message: &'static str) -> Self {
Symbol::Type(todo_type!(message), Boundness::Bound)
}

pub(crate) fn is_unbound(&self) -> bool {
matches!(self, Symbol::Unbound)
}
Expand Down
118 changes: 62 additions & 56 deletions crates/red_knot_python_semantic/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db>
}
// Symbol is possibly undeclared and (possibly) bound
Symbol::Type(inferred_ty, boundness) => Symbol::Type(
UnionType::from_elements(db, [inferred_ty, declared_ty].iter().copied()),
UnionType::from_elements(db, [inferred_ty, declared_ty]),
boundness,
),
}
Expand All @@ -159,7 +159,7 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db>
Err((declared_ty, _)) => {
// Intentionally ignore conflicting declared types; that's not our problem,
// it's the problem of the module we are importing from.
declared_ty.inner_type().into()
Symbol::bound(declared_ty.inner_type())
}
}

Expand Down Expand Up @@ -187,18 +187,15 @@ fn symbol<'db>(db: &'db dyn Db, scope: ScopeId<'db>, name: &str) -> Symbol<'db>
&& file_to_module(db, scope.file(db))
.is_some_and(|module| module.is_known(KnownModule::Typing))
{
return Symbol::Type(Type::BooleanLiteral(true), Boundness::Bound);
return Symbol::bound(Type::BooleanLiteral(true));
}
if name == "platform"
&& file_to_module(db, scope.file(db))
.is_some_and(|module| module.is_known(KnownModule::Sys))
{
match Program::get(db).python_platform(db) {
crate::PythonPlatform::Identifier(platform) => {
return Symbol::Type(
Type::StringLiteral(StringLiteralType::new(db, platform.as_str())),
Boundness::Bound,
);
return Symbol::bound(Type::string_literal(db, platform.as_str()));
}
crate::PythonPlatform::All => {
// Fall through to the looked up type
Expand Down Expand Up @@ -401,9 +398,16 @@ fn symbol_from_bindings<'db>(
/// If we look up the declared type of `variable` in the scope of class `C`, we will get
/// the type `int`, a "declaredness" of [`Boundness::PossiblyUnbound`], and the information
/// that this comes with a [`TypeQualifiers::CLASS_VAR`] type qualifier.
#[derive(Debug)]
pub(crate) struct SymbolAndQualifiers<'db>(Symbol<'db>, TypeQualifiers);

impl SymbolAndQualifiers<'_> {
/// Constructor that creates a [`SymbolAndQualifiers`] instance with a [`TodoType`] type
/// and no qualifiers.
fn todo(message: &'static str) -> Self {
Self(Symbol::todo(message), TypeQualifiers::empty())
}

fn is_class_var(&self) -> bool {
self.1.contains(TypeQualifiers::CLASS_VAR)
}
Expand All @@ -419,12 +423,6 @@ impl<'db> From<Symbol<'db>> for SymbolAndQualifiers<'db> {
}
}

impl<'db> From<Type<'db>> for SymbolAndQualifiers<'db> {
fn from(ty: Type<'db>) -> Self {
SymbolAndQualifiers(ty.into(), TypeQualifiers::empty())
}
}

/// The result of looking up a declared type from declarations; see [`symbol_from_declarations`].
type SymbolFromDeclarationsResult<'db> =
Result<SymbolAndQualifiers<'db>, (TypeAndQualifiers<'db>, Box<[Type<'db>]>)>;
Expand Down Expand Up @@ -560,6 +558,11 @@ macro_rules! todo_type {
$crate::types::TodoType::Message($message),
))
};
($message:ident) => {
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo(
$crate::types::TodoType::Message($message),
))
};
}

#[cfg(not(debug_assertions))]
Expand All @@ -570,6 +573,9 @@ macro_rules! todo_type {
($message:literal) => {
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo(crate::types::TodoType))
};
($message:ident) => {
$crate::types::Type::Dynamic($crate::types::DynamicType::Todo(crate::types::TodoType))
};
}

pub(crate) use todo_type;
Expand Down Expand Up @@ -1688,17 +1694,17 @@ impl<'db> Type<'db> {
#[must_use]
pub(crate) fn member(&self, db: &'db dyn Db, name: &str) -> Symbol<'db> {
if name == "__class__" {
return self.to_meta_type(db).into();
return Symbol::bound(self.to_meta_type(db));
}

match self {
Type::Dynamic(_) => self.into(),
Type::Dynamic(_) => Symbol::bound(self),

Type::Never => todo_type!("attribute lookup on Never").into(),
Type::Never => Symbol::todo("attribute lookup on Never"),

Type::FunctionLiteral(_) => match name {
"__get__" => todo_type!("`__get__` method on functions").into(),
"__call__" => todo_type!("`__call__` method on functions").into(),
"__get__" => Symbol::todo("`__get__` method on functions"),
"__call__" => Symbol::todo("`__call__` method on functions"),
_ => KnownClass::FunctionType.to_instance(db).member(db, name),
},

Expand All @@ -1711,12 +1717,12 @@ impl<'db> Type<'db> {
Type::KnownInstance(known_instance) => known_instance.member(db, name),

Type::Instance(InstanceType { class }) => match (class.known(db), name) {
(Some(KnownClass::VersionInfo), "major") => {
Type::IntLiteral(Program::get(db).python_version(db).major.into()).into()
}
(Some(KnownClass::VersionInfo), "minor") => {
Type::IntLiteral(Program::get(db).python_version(db).minor.into()).into()
}
(Some(KnownClass::VersionInfo), "major") => Symbol::bound(Type::IntLiteral(
Program::get(db).python_version(db).major.into(),
)),
(Some(KnownClass::VersionInfo), "minor") => Symbol::bound(Type::IntLiteral(
Program::get(db).python_version(db).minor.into(),
)),
_ => {
let SymbolAndQualifiers(symbol, _) = class.instance_member(db, name);
symbol
Expand Down Expand Up @@ -1762,30 +1768,30 @@ impl<'db> Type<'db> {
Type::Intersection(_) => {
// TODO perform the get_member on each type in the intersection
// TODO return the intersection of those results
todo_type!("Attribute access on `Intersection` types").into()
Symbol::todo("Attribute access on `Intersection` types")
}

Type::IntLiteral(_) => match name {
"real" | "numerator" => self.into(),
"real" | "numerator" => Symbol::bound(self),
// TODO more attributes could probably be usefully special-cased
_ => KnownClass::Int.to_instance(db).member(db, name),
},

Type::BooleanLiteral(bool_value) => match name {
"real" | "numerator" => Type::IntLiteral(i64::from(*bool_value)).into(),
"real" | "numerator" => Symbol::bound(Type::IntLiteral(i64::from(*bool_value))),
_ => KnownClass::Bool.to_instance(db).member(db, name),
},

Type::StringLiteral(_) => {
// TODO defer to `typing.LiteralString`/`builtins.str` methods
// from typeshed's stubs
todo_type!("Attribute access on `StringLiteral` types").into()
Symbol::todo("Attribute access on `StringLiteral` types")
}

Type::LiteralString => {
// TODO defer to `typing.LiteralString`/`builtins.str` methods
// from typeshed's stubs
todo_type!("Attribute access on `LiteralString` types").into()
Symbol::todo("Attribute access on `LiteralString` types")
}

Type::BytesLiteral(_) => KnownClass::Bytes.to_instance(db).member(db, name),
Expand All @@ -1797,13 +1803,13 @@ impl<'db> Type<'db> {

Type::Tuple(_) => {
// TODO: implement tuple methods
todo_type!("Attribute access on heterogeneous tuple types").into()
Symbol::todo("Attribute access on heterogeneous tuple types")
}

Type::AlwaysTruthy | Type::AlwaysFalsy => match name {
"__bool__" => {
// TODO should be `Callable[[], Literal[True/False]]`
todo_type!("`__bool__` for `AlwaysTruthy`/`AlwaysFalsy` Type variants").into()
Symbol::todo("`__bool__` for `AlwaysTruthy`/`AlwaysFalsy` Type variants")
}
_ => Type::object(db).member(db, name),
},
Expand Down Expand Up @@ -2528,18 +2534,6 @@ impl<'db> From<&Type<'db>> for Type<'db> {
}
}

impl<'db> From<Type<'db>> for Symbol<'db> {
fn from(value: Type<'db>) -> Self {
Symbol::Type(value, Boundness::Bound)
}
}

impl<'db> From<&Type<'db>> for Symbol<'db> {
fn from(value: &Type<'db>) -> Self {
Self::from(*value)
}
}

#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
pub enum DynamicType {
// An explicitly annotated `typing.Any`
Expand Down Expand Up @@ -2572,7 +2566,7 @@ impl std::fmt::Display for DynamicType {

bitflags! {
/// Type qualifiers that appear in an annotation expression.
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
#[derive(Copy, Clone, Debug, Eq, PartialEq, Default)]
pub(crate) struct TypeQualifiers: u8 {
/// `typing.ClassVar`
const CLASS_VAR = 1 << 0;
Expand All @@ -2599,6 +2593,14 @@ impl<'db> TypeAndQualifiers<'db> {
Self { inner, qualifiers }
}

/// Constructor that creates a [`TypeAndQualifiers`] instance with type `Unknown` and no qualifiers.
pub(crate) fn unknown() -> Self {
Self {
inner: Type::unknown(),
qualifiers: TypeQualifiers::empty(),
}
}

/// Forget about type qualifiers and only return the inner type.
pub(crate) fn inner_type(&self) -> Type<'db> {
self.inner
Expand Down Expand Up @@ -3330,7 +3332,7 @@ impl<'db> KnownInstanceType<'db> {
(Self::TypeAliasType(alias), "__name__") => Type::string_literal(db, alias.name(db)),
_ => return self.instance_fallback(db).member(db, name),
};
ty.into()
Symbol::bound(ty)
}
}

Expand Down Expand Up @@ -3788,8 +3790,7 @@ impl<'db> ModuleLiteralType<'db> {
full_submodule_name.extend(&submodule_name);
if imported_submodules.contains(&full_submodule_name) {
if let Some(submodule) = resolve_module(db, &full_submodule_name) {
let submodule_ty = Type::module_literal(db, importing_file, submodule);
return Symbol::Type(submodule_ty, Boundness::Bound);
return Symbol::bound(Type::module_literal(db, importing_file, submodule));
}
}
}
Expand Down Expand Up @@ -4123,7 +4124,7 @@ impl<'db> Class<'db> {
pub(crate) fn class_member(self, db: &'db dyn Db, name: &str) -> Symbol<'db> {
if name == "__mro__" {
let tuple_elements = self.iter_mro(db).map(Type::from);
return TupleType::from_elements(db, tuple_elements).into();
return Symbol::bound(TupleType::from_elements(db, tuple_elements));
}

for superclass in self.iter_mro(db) {
Expand Down Expand Up @@ -4163,7 +4164,9 @@ impl<'db> Class<'db> {
for superclass in self.iter_mro(db) {
match superclass {
ClassBase::Dynamic(_) => {
return todo_type!("instance attribute on class with dynamic base").into();
return SymbolAndQualifiers::todo(
"instance attribute on class with dynamic base",
);
}
ClassBase::Class(class) => {
if let member @ SymbolAndQualifiers(Symbol::Type(_, _), _) =
Expand Down Expand Up @@ -4213,7 +4216,7 @@ impl<'db> Class<'db> {
.and_then(|assignments| assignments.get(name))
else {
if inferred_type_from_class_body.is_some() {
return union_of_inferred_types.build().into();
return Symbol::bound(union_of_inferred_types.build());
}
return Symbol::Unbound;
};
Expand All @@ -4230,7 +4233,7 @@ impl<'db> Class<'db> {
let annotation_ty = infer_expression_type(db, *annotation);

// TODO: check if there are conflicting declarations
return annotation_ty.into();
return Symbol::bound(annotation_ty);
}
AttributeAssignment::Unannotated { value } => {
// We found an un-annotated attribute assignment of the form:
Expand Down Expand Up @@ -4270,7 +4273,7 @@ impl<'db> Class<'db> {
}
}

union_of_inferred_types.build().into()
Symbol::bound(union_of_inferred_types.build())
}

/// A helper function for `instance_member` that looks up the `name` attribute only on
Expand Down Expand Up @@ -4299,12 +4302,12 @@ impl<'db> Class<'db> {
// just a temporary heuristic to provide a broad categorization into properties
// and non-property methods.
if function.has_decorator(db, KnownClass::Property.to_class_literal(db)) {
todo_type!("@property").into()
SymbolAndQualifiers::todo("@property")
} else {
todo_type!("bound method").into()
SymbolAndQualifiers::todo("bound method")
}
} else {
SymbolAndQualifiers(Symbol::Type(declared_ty, Boundness::Bound), qualifiers)
SymbolAndQualifiers(Symbol::bound(declared_ty), qualifiers)
}
}
Ok(SymbolAndQualifiers(Symbol::Unbound, _)) => {
Expand All @@ -4319,7 +4322,10 @@ impl<'db> Class<'db> {
}
Err((declared_ty, _conflicting_declarations)) => {
// There are conflicting declarations for this attribute in the class body.
SymbolAndQualifiers(declared_ty.inner_type().into(), declared_ty.qualifiers())
SymbolAndQualifiers(
Symbol::bound(declared_ty.inner_type()),
declared_ty.qualifiers(),
)
}
}
} else {
Expand Down
Loading

0 comments on commit df1d430

Please sign in to comment.