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.

Sign up for GitHub

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] rename {Class,Module,Function} => {Class,Module,Function}Literal #13873

Merged
merged 6 commits into from
Oct 22, 2024
Merged

[red-knot] rename {Class,Module,Function} => {Class,Module,Function}Literal #13873

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d613375
[red-knot] rename {Class,Module,Function} => {Class,Module,Function}L…
sharkdp Oct 22, 2024
c4c447d
Rename LiteralTypeKind => CondensedDisplayTypeKind
sharkdp Oct 22, 2024
9fe1c90
Rename corresponding methods
sharkdp Oct 22, 2024
97c28f5
Update crates/red_knot_python_semantic/src/types/display.rs
sharkdp Oct 22, 2024
70a1ca0
Remove the need for is_class_literal
sharkdp Oct 22, 2024
3505e7a
Introduce more Type::is_xyz variants
sharkdp Oct 22, 2024
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
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/semantic_model.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ mod tests {
let model = SemanticModel::new(&db, foo);
let ty = function.ty(&model);

assert!(matches!(ty, Type::Function(_)));
assert!(matches!(ty, Type::FunctionLiteral(_)));
Copy link
Member

Choose a reason for hiding this comment

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

nit: we could add is_function_literal() and is_class_literal methods to Type, and use them here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method does not exist 😄. I could do assert!(ty.into_function_literal_type().is_some()), but I don't think that's much better than matching on the variant?

Copy link
Member

Choose a reason for hiding this comment

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

This method does not exist 😄

I know! Sorry for being unclear. I'm suggesting we could add Type::is_function_literal() and Type::is_class_literal() methods to the Type enum, for consistency with our existing Type::is_unbound and Type::is_never methods. It doesn't have to be done in this PR, but it also might as well if we're touching these lines anyway ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, sorry. I misread.

is_function_literal does not exist, but is_class_literal exists. It does something different from just matching against Type::ClassLiteral though 🙈

pub fn is_class_literal(&self, db: &'db dyn Db) -> bool {
    match self {
        Type::Union(union) => union.elements(db).iter().all(|ty| ty.is_class_literal(db)),
        Type::ClassLiteral(_) => true,
        // / TODO include type[X], once we add that type
        _ => false,
    }
}

So if we want to do this properly, I guess we would have to rename the existing is_class_literal function...

Copy link
Member

Choose a reason for hiding this comment

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

So if we want to do this properly, I guess we would have to rename the existing is_class_literal function...

We're well down the renaming rabbit hole now. May as well 😆

It seems like the question the existing is_class_literal method is actually trying to answer is "Is the type a subtype of Instance(builtins.type)?". Maybe we could try to just get rid of it and make is_subtype_of work correctly for Instance(builtins.type), so that it recognises all Type::ClassLiterals as subtypes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did that now.


Ok(())
}
Expand All @@ -222,7 +222,7 @@ mod tests {
let model = SemanticModel::new(&db, foo);
let ty = class.ty(&model);

assert!(matches!(ty, Type::Class(_)));
assert!(matches!(ty, Type::ClassLiteral(_)));

Ok(())
}
Expand All @@ -243,7 +243,7 @@ mod tests {
let model = SemanticModel::new(&db, bar);
let ty = alias.ty(&model);

assert!(matches!(ty, Type::Class(_)));
assert!(matches!(ty, Type::ClassLiteral(_)));

Ok(())
}
Expand Down
104 changes: 52 additions & 52 deletions crates/red_knot_python_semantic/src/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,11 @@ pub enum Type<'db> {
/// `Todo` would change the output type.
Todo,
/// A specific function object
Function(FunctionType<'db>),
FunctionLiteral(FunctionType<'db>),
/// A specific module object
Module(File),
ModuleLiteral(File),
/// A specific class object
Class(ClassType<'db>),
ClassLiteral(ClassType<'db>),
/// 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
Expand Down Expand Up @@ -292,28 +292,28 @@ impl<'db> Type<'db> {
matches!(self, Type::Never)
}

pub const fn into_class_type(self) -> Option<ClassType<'db>> {
pub const fn into_class_literal_type(self) -> Option<ClassType<'db>> {
match self {
Type::Class(class_type) => Some(class_type),
Type::ClassLiteral(class_type) => Some(class_type),
_ => None,
}
}

pub fn expect_class(self) -> ClassType<'db> {
self.into_class_type()
.expect("Expected a Type::Class variant")
pub fn expect_class_literal(self) -> ClassType<'db> {
self.into_class_literal_type()
.expect("Expected a Type::ClassLiteral variant")
}

pub const fn into_module_type(self) -> Option<File> {
pub const fn into_module_literal_type(self) -> Option<File> {
match self {
Type::Module(file) => Some(file),
Type::ModuleLiteral(file) => Some(file),
_ => None,
}
}

pub fn expect_module(self) -> File {
self.into_module_type()
.expect("Expected a Type::Module variant")
pub fn expect_module_literal(self) -> File {
self.into_module_literal_type()
.expect("Expected a Type::ModuleLiteral variant")
}

pub const fn into_union_type(self) -> Option<UnionType<'db>> {
Expand All @@ -340,16 +340,16 @@ impl<'db> Type<'db> {
.expect("Expected a Type::Intersection variant")
}

pub const fn into_function_type(self) -> Option<FunctionType<'db>> {
pub const fn into_function_literal_type(self) -> Option<FunctionType<'db>> {
match self {
Type::Function(function_type) => Some(function_type),
Type::FunctionLiteral(function_type) => Some(function_type),
_ => None,
}
}

pub fn expect_function(self) -> FunctionType<'db> {
self.into_function_type()
.expect("Expected a Type::Function variant")
pub fn expect_function_literal(self) -> FunctionType<'db> {
self.into_function_literal_type()
.expect("Expected a Type::FunctionLiteral variant")
}

pub const fn into_int_literal_type(self) -> Option<i64> {
Expand Down Expand Up @@ -387,17 +387,17 @@ impl<'db> Type<'db> {

pub fn is_stdlib_symbol(&self, db: &'db dyn Db, module_name: &str, name: &str) -> bool {
match self {
Type::Class(class) => class.is_stdlib_symbol(db, module_name, name),
Type::Function(function) => function.is_stdlib_symbol(db, module_name, name),
Type::ClassLiteral(class) => class.is_stdlib_symbol(db, module_name, name),
Type::FunctionLiteral(function) => function.is_stdlib_symbol(db, module_name, name),
_ => false,
}
}

/// Return true if the type is a class or a union of classes.
pub fn is_class(&self, db: &'db dyn Db) -> bool {
pub fn is_class_literal(&self, db: &'db dyn Db) -> bool {
carljm marked this conversation as resolved.
Show resolved Hide resolved
match self {
Type::Union(union) => union.elements(db).iter().all(|ty| ty.is_class(db)),
Type::Class(_) => true,
Type::Union(union) => union.elements(db).iter().all(|ty| ty.is_class_literal(db)),
Type::ClassLiteral(_) => true,
// / TODO include type[X], once we add that type
_ => false,
}
Expand Down Expand Up @@ -517,17 +517,17 @@ impl<'db> Type<'db> {
| Type::IntLiteral(..)
| Type::StringLiteral(..)
| Type::BytesLiteral(..)
| Type::Function(..)
| Type::Module(..)
| Type::Class(..)),
| Type::FunctionLiteral(..)
| Type::ModuleLiteral(..)
| Type::ClassLiteral(..)),
right @ (Type::None
| Type::BooleanLiteral(..)
| Type::IntLiteral(..)
| Type::StringLiteral(..)
| Type::BytesLiteral(..)
| Type::Function(..)
| Type::Module(..)
| Type::Class(..)),
| Type::FunctionLiteral(..)
| Type::ModuleLiteral(..)
| Type::ClassLiteral(..)),
) => left != right,

(Type::None, Type::Instance(class_type)) | (Type::Instance(class_type), Type::None) => {
Expand Down Expand Up @@ -577,12 +577,12 @@ impl<'db> Type<'db> {
(Type::BytesLiteral(..), _) | (_, Type::BytesLiteral(..)) => true,

(
Type::Function(..) | Type::Module(..) | Type::Class(..),
Type::FunctionLiteral(..) | Type::ModuleLiteral(..) | Type::ClassLiteral(..),
Type::Instance(class_type),
)
| (
Type::Instance(class_type),
Type::Function(..) | Type::Module(..) | Type::Class(..),
Type::FunctionLiteral(..) | Type::ModuleLiteral(..) | Type::ClassLiteral(..),
) => !class_type.is_known(db, KnownClass::Object),

(Type::Instance(..), Type::Instance(..)) => {
Expand Down Expand Up @@ -643,7 +643,7 @@ impl<'db> Type<'db> {
// are both of type Literal[345], for example.
false
}
Type::None | Type::BooleanLiteral(_) | Type::Function(..) | Type::Class(..) | Type::Module(..) => true,
Type::None | Type::BooleanLiteral(_) | Type::FunctionLiteral(..) | Type::ClassLiteral(..) | Type::ModuleLiteral(..) => true,
Type::Tuple(..) => {
// The empty tuple is a singleton on CPython and PyPy, but not on other Python
// implementations such as GraalPy. Its *use* as a singleton is discouraged and
Expand Down Expand Up @@ -675,9 +675,9 @@ impl<'db> Type<'db> {
pub(crate) fn is_single_valued(self, db: &'db dyn Db) -> bool {
match self {
Type::None
| Type::Function(..)
| Type::Module(..)
| Type::Class(..)
| Type::FunctionLiteral(..)
| Type::ModuleLiteral(..)
| Type::ClassLiteral(..)
| Type::IntLiteral(..)
| Type::BooleanLiteral(..)
| Type::StringLiteral(..)
Expand Down Expand Up @@ -747,12 +747,12 @@ impl<'db> Type<'db> {
// TODO: attribute lookup on None type
Type::Todo
}
Type::Function(_) => {
Type::FunctionLiteral(_) => {
// TODO: attribute lookup on function type
Type::Todo
}
Type::Module(file) => global_symbol_ty(db, *file, name),
Type::Class(class) => class.class_member(db, name),
Type::ModuleLiteral(file) => global_symbol_ty(db, *file, name),
Type::ClassLiteral(class) => class.class_member(db, name),
Type::Instance(_) => {
// TODO MRO? get_own_instance_member, get_instance_member
Type::Todo
Expand Down Expand Up @@ -800,9 +800,9 @@ impl<'db> Type<'db> {
Truthiness::Ambiguous
}
Type::None => Truthiness::AlwaysFalse,
Type::Function(_) => Truthiness::AlwaysTrue,
Type::Module(_) => Truthiness::AlwaysTrue,
Type::Class(_) => {
Type::FunctionLiteral(_) => Truthiness::AlwaysTrue,
Type::ModuleLiteral(_) => Truthiness::AlwaysTrue,
Type::ClassLiteral(_) => {
// TODO: lookup `__bool__` and `__len__` methods on the class's metaclass
// More info in https://docs.python.org/3/library/stdtypes.html#truth-value-testing
Truthiness::Ambiguous
Expand Down Expand Up @@ -847,7 +847,7 @@ impl<'db> Type<'db> {
fn call(self, db: &'db dyn Db, arg_types: &[Type<'db>]) -> CallOutcome<'db> {
match self {
// TODO validate typed call arguments vs callable signature
Type::Function(function_type) => match function_type.known(db) {
Type::FunctionLiteral(function_type) => match function_type.known(db) {
None => CallOutcome::callable(function_type.return_type(db)),
Some(KnownFunction::RevealType) => CallOutcome::revealed(
function_type.return_type(db),
Expand All @@ -856,7 +856,7 @@ impl<'db> Type<'db> {
},

// TODO annotated return type on `__new__` or metaclass `__call__`
Type::Class(class) => {
Type::ClassLiteral(class) => {
CallOutcome::callable(match class.known(db) {
// If the class is the builtin-bool class (for example `bool(1)`), we try to
// return the specific truthiness value of the input arg, `Literal[True]` for
Expand Down Expand Up @@ -976,17 +976,17 @@ impl<'db> Type<'db> {
Type::Unknown => Type::Unknown,
Type::Unbound => Type::Unknown,
Type::Never => Type::Never,
Type::Class(class) => Type::Instance(*class),
Type::ClassLiteral(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::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(_)
| Type::BytesLiteral(_)
| Type::Function(_)
| Type::FunctionLiteral(_)
| Type::Instance(_)
| Type::Module(_)
| Type::ModuleLiteral(_)
| Type::IntLiteral(_)
| Type::StringLiteral(_)
| Type::Tuple(_)
Expand All @@ -1002,17 +1002,17 @@ impl<'db> Type<'db> {
match self {
Type::Unbound => Type::Unbound,
Type::Never => Type::Never,
Type::Instance(class) => Type::Class(*class),
Type::Instance(class) => Type::ClassLiteral(*class),
Type::Union(union) => union.map(db, |ty| ty.to_meta_type(db)),
Type::BooleanLiteral(_) => KnownClass::Bool.to_class(db),
Type::BytesLiteral(_) => KnownClass::Bytes.to_class(db),
Type::IntLiteral(_) => KnownClass::Int.to_class(db),
Type::Function(_) => KnownClass::FunctionType.to_class(db),
Type::Module(_) => KnownClass::ModuleType.to_class(db),
Type::FunctionLiteral(_) => KnownClass::FunctionType.to_class(db),
Type::ModuleLiteral(_) => KnownClass::ModuleType.to_class(db),
Type::Tuple(_) => KnownClass::Tuple.to_class(db),
Type::None => KnownClass::NoneType.to_class(db),
// TODO not accurate if there's a custom metaclass...
Type::Class(_) => KnownClass::Type.to_class(db),
Type::ClassLiteral(_) => KnownClass::Type.to_class(db),
// TODO can we do better here? `type[LiteralString]`?
Type::StringLiteral(_) | Type::LiteralString => KnownClass::Str.to_class(db),
// TODO: `type[Any]`?
Expand Down Expand Up @@ -1642,7 +1642,7 @@ impl<'db> ClassType<'db> {
// TODO: we need to iterate over the *MRO* here, not the bases
(other == self)
|| self.bases(db).any(|base| match base {
Type::Class(base_class) => base_class == other,
Type::ClassLiteral(base_class) => base_class == other,
// `is_subclass_of` is checking the subtype relation, in which gradual types do not
// participate, so we should not return `True` if we find `Any/Unknown` in the
// bases.
Expand Down Expand Up @@ -2007,7 +2007,7 @@ mod tests {
let type_a = super::global_symbol_ty(&db, module, "A");
let type_x = super::global_symbol_ty(&db, module, "x");

assert!(matches!(type_a, Type::Class(_)));
assert!(matches!(type_a, Type::ClassLiteral(_)));
assert!(matches!(type_x, Type::Union(_)));

assert!(!type_a.is_disjoint_from(&db, type_x));
Expand Down
Loading
Loading