-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[ty] Literal promotion refactor #20646
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1166,21 +1166,26 @@ impl<'db> Type<'db> { | |
| } | ||
| } | ||
|
|
||
| /// If this type is a literal, promote it to a type that this literal is an instance of. | ||
| /// Promote (possibly nested) literals to types that these literals are instances of. | ||
| /// | ||
| /// Note that this function tries to promote literals to a more user-friendly form than their | ||
| /// fallback instance type. For example, `def _() -> int` is promoted to `Callable[[], int]`, | ||
| /// as opposed to `FunctionType`. | ||
| pub(crate) fn literal_promotion_type(self, db: &'db dyn Db) -> Option<Type<'db>> { | ||
| pub(crate) fn promote_literals(self, db: &'db dyn Db) -> Type<'db> { | ||
| self.apply_type_mapping(db, &TypeMapping::PromoteLiterals) | ||
| } | ||
|
|
||
| /// Like [`Type::promote_literals`], but does not recurse into nested types. | ||
| fn promote_literals_impl(self, db: &'db dyn Db) -> Type<'db> { | ||
| match self { | ||
| Type::StringLiteral(_) | Type::LiteralString => Some(KnownClass::Str.to_instance(db)), | ||
| Type::BooleanLiteral(_) => Some(KnownClass::Bool.to_instance(db)), | ||
| Type::IntLiteral(_) => Some(KnownClass::Int.to_instance(db)), | ||
| Type::BytesLiteral(_) => Some(KnownClass::Bytes.to_instance(db)), | ||
| Type::ModuleLiteral(_) => Some(KnownClass::ModuleType.to_instance(db)), | ||
| Type::EnumLiteral(literal) => Some(literal.enum_class_instance(db)), | ||
| Type::FunctionLiteral(literal) => Some(Type::Callable(literal.into_callable_type(db))), | ||
| _ => None, | ||
| Type::StringLiteral(_) | Type::LiteralString => KnownClass::Str.to_instance(db), | ||
| Type::BooleanLiteral(_) => KnownClass::Bool.to_instance(db), | ||
| Type::IntLiteral(_) => KnownClass::Int.to_instance(db), | ||
| Type::BytesLiteral(_) => KnownClass::Bytes.to_instance(db), | ||
| Type::ModuleLiteral(_) => KnownClass::ModuleType.to_instance(db), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not actually sure when promotion of module-literal types is desirable -- we might want to remove it from this method. As you discovered on your "remove
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was thinking the exact same thing when I changed this code. In fact, I did arrive here in the first place because promotion of module literals was causing the exact problem you are describing. I'll propose that as a separate change. Edit: I should have read the rest of your comment before writing mine. |
||
| Type::EnumLiteral(literal) => literal.enum_class_instance(db), | ||
| Type::FunctionLiteral(literal) => Type::Callable(literal.into_callable_type(db)), | ||
| _ => self, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -6080,8 +6085,7 @@ impl<'db> Type<'db> { | |
| let function = Type::FunctionLiteral(function.apply_type_mapping_impl(db, type_mapping, visitor)); | ||
|
|
||
| match type_mapping { | ||
| TypeMapping::PromoteLiterals => function.literal_promotion_type(db) | ||
| .expect("function literal should have a promotion type"), | ||
| TypeMapping::PromoteLiterals => function.promote_literals_impl(db), | ||
| _ => function | ||
| } | ||
| } | ||
|
|
@@ -6189,8 +6193,7 @@ impl<'db> Type<'db> { | |
| TypeMapping::ReplaceSelf { .. } | | ||
| TypeMapping::MarkTypeVarsInferable(_) | | ||
| TypeMapping::Materialize(_) => self, | ||
| TypeMapping::PromoteLiterals => self.literal_promotion_type(db) | ||
| .expect("literal type should have a promotion type"), | ||
| TypeMapping::PromoteLiterals => self.promote_literals_impl(db) | ||
| } | ||
|
|
||
| Type::Dynamic(_) => match type_mapping { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2625,6 +2625,12 @@ pub(crate) fn report_undeclared_protocol_member( | |
| SubclassOfInner::Dynamic(_) => return false, | ||
| }, | ||
| Type::NominalInstance(instance) => instance.class(db), | ||
| Type::Union(union) => { | ||
| return union | ||
| .elements(db) | ||
| .iter() | ||
| .all(|elem| should_give_hint(db, *elem)); | ||
| } | ||
|
Comment on lines
+2628
to
+2633
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Admittedly more of a showcase for the refactor here. I'm also happy to remove this again (and the test above). |
||
| _ => return false, | ||
| }; | ||
|
|
||
|
|
@@ -2656,9 +2662,7 @@ pub(crate) fn report_undeclared_protocol_member( | |
| if definition.kind(db).is_unannotated_assignment() { | ||
| let binding_type = binding_type(db, definition); | ||
|
|
||
| let suggestion = binding_type | ||
| .literal_promotion_type(db) | ||
| .unwrap_or(binding_type); | ||
| let suggestion = binding_type.promote_literals(db); | ||
|
|
||
| if should_give_hint(db, suggestion) { | ||
| diagnostic.set_primary_message(format_args!( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unlikely to ever appear in practice. But a nice way to demonstrate how the previous
Type::literal_promotion_typewas different fromType::promote_literals(this only works with the latter).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you mean, i write protocols like this all the time