-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[ty] Make tuple instantiations sound #18987
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 |
|---|---|---|
|
|
@@ -946,6 +946,13 @@ impl<'db> Type<'db> { | |
| matches!(self, Type::ClassLiteral(..)) | ||
| } | ||
|
|
||
| pub(crate) const fn into_tuple(self) -> Option<TupleType<'db>> { | ||
| match self { | ||
| Type::Tuple(tuple_type) => Some(tuple_type), | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| /// Turn a class literal (`Type::ClassLiteral` or `Type::GenericAlias`) into a `ClassType`. | ||
| /// Since a `ClassType` must be specialized, apply the default specialization to any | ||
| /// unspecialized generic class literal. | ||
|
|
@@ -4237,6 +4244,27 @@ impl<'db> Type<'db> { | |
| .into() | ||
| } | ||
|
|
||
| Some(KnownClass::Tuple) => { | ||
| let object = Type::object(db); | ||
|
|
||
| CallableBinding::from_overloads( | ||
| self, | ||
| [ | ||
| Signature::new(Parameters::empty(), Some(TupleType::empty(db))), | ||
| Signature::new( | ||
| Parameters::new([Parameter::positional_only(Some( | ||
| Name::new_static("iterable"), | ||
| )) | ||
| .with_annotated_type( | ||
| KnownClass::Iterable.to_specialized_instance(db, [object]), | ||
| )]), | ||
| Some(TupleType::homogeneous(db, object)), | ||
| ), | ||
| ], | ||
| ) | ||
| .into() | ||
| } | ||
|
|
||
| // Most class literal constructor calls are handled by `try_call_constructor` and | ||
| // not via getting the signature here. This signature can still be used in some | ||
| // cases (e.g. evaluating callable subtyping). TODO improve this definition | ||
|
|
@@ -4276,14 +4304,24 @@ impl<'db> Type<'db> { | |
| .into() | ||
| } | ||
|
|
||
| Type::GenericAlias(_) => { | ||
| Type::GenericAlias(alias) => { | ||
| let instantiated = Type::instance(db, ClassType::from(alias)); | ||
|
|
||
| let parameters = if alias.origin(db).is_known(db, KnownClass::Tuple) { | ||
| let spec = alias.specialization(db).tuple(db); | ||
| let mut parameter = | ||
| Parameter::positional_only(Some(Name::new_static("iterable"))) | ||
| .with_annotated_type(instantiated); | ||
|
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. With the
Member
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.
Hmm, not sure I totally follow this. Can you give an example of something that should be covered in this PR but isn't? :-)
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. Sorry, wasn't requesting any changes here with this comment, just leaving me/others a breadcrumb for a future improvement |
||
| if matches!(spec.size_hint().1, Some(0)) { | ||
| parameter = parameter.with_default_type(TupleType::empty(db)); | ||
| } | ||
| Parameters::new([parameter]) | ||
| } else { | ||
| Parameters::gradual_form() | ||
| }; | ||
| // TODO annotated return type on `__new__` or metaclass `__call__` | ||
| // TODO check call vs signatures of `__new__` and/or `__init__` | ||
| Binding::single( | ||
| self, | ||
| Signature::new(Parameters::gradual_form(), self.to_instance(db)), | ||
| ) | ||
| .into() | ||
| Binding::single(self, Signature::new(parameters, Some(instantiated))).into() | ||
| } | ||
|
|
||
| Type::SubclassOf(subclass_of_type) => match subclass_of_type.subclass_of() { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -972,6 +972,28 @@ impl<'db> Bindings<'db> { | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| Some(KnownClass::Tuple) if overload_index == 1 => { | ||||||||||
| if let [Some(argument)] = overload.parameter_types() { | ||||||||||
| let overridden_return = | ||||||||||
| argument.into_tuple().map(Type::Tuple).unwrap_or_else(|| { | ||||||||||
|
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 think this would avoid unwrapping the argument type just to then rewrap it:
Suggested change
(though it depends on an
Member
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.
Suggested change
Adding a |
||||||||||
| // Some awkward special handling is required here because of the fact | ||||||||||
| // that calling `try_iterate()` on `Never` returns `Never`, | ||||||||||
| // but `tuple[Never, ...]` eagerly simplifies to `tuple[()]`, | ||||||||||
| // which will cause us to emit false positives if we index into the tuple | ||||||||||
| let specialization = if argument.is_never() { | ||||||||||
| Type::unknown() | ||||||||||
| } else { | ||||||||||
| argument.try_iterate(db).expect( | ||||||||||
| "try_iterate() should not fail on a type \ | ||||||||||
| assignable to `Iterable`", | ||||||||||
| ) | ||||||||||
| }; | ||||||||||
| TupleType::homogeneous(db, specialization) | ||||||||||
| }); | ||||||||||
| overload.set_return_type(overridden_return); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| _ => {} | ||||||||||
| }, | ||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2334,6 +2334,7 @@ pub enum KnownClass { | |
| NamedTuple, | ||
| NewType, | ||
| SupportsIndex, | ||
| Iterable, | ||
| // Collections | ||
| ChainMap, | ||
| Counter, | ||
|
|
@@ -2426,6 +2427,7 @@ impl KnownClass { | |
| | Self::Float | ||
| | Self::Enum | ||
| | Self::ABCMeta | ||
| | KnownClass::Iterable | ||
|
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. nit: |
||
| // Empty tuples are AlwaysFalse; non-empty tuples are AlwaysTrue | ||
| | Self::NamedTuple | ||
| // Evaluating `NotImplementedType` in a boolean context was deprecated in Python 3.9 | ||
|
|
@@ -2513,6 +2515,7 @@ impl KnownClass { | |
| | Self::DefaultDict | ||
| | Self::OrderedDict | ||
| | Self::NewType | ||
| | Self::Iterable | ||
| | Self::BaseExceptionGroup => false, | ||
| } | ||
| } | ||
|
|
@@ -2531,7 +2534,7 @@ impl KnownClass { | |
| /// 2. It's probably more performant. | ||
| const fn is_protocol(self) -> bool { | ||
| match self { | ||
| Self::SupportsIndex => true, | ||
| Self::SupportsIndex | Self::Iterable => true, | ||
|
|
||
| Self::Any | ||
| | Self::Bool | ||
|
|
@@ -2648,6 +2651,7 @@ impl KnownClass { | |
| Self::Enum => "Enum", | ||
| Self::ABCMeta => "ABCMeta", | ||
| Self::Super => "super", | ||
| Self::Iterable => "Iterable", | ||
| // For example, `typing.List` is defined as `List = _Alias()` in typeshed | ||
| Self::StdlibAlias => "_Alias", | ||
| // This is the name the type of `sys.version_info` has in typeshed, | ||
|
|
@@ -2882,6 +2886,7 @@ impl KnownClass { | |
| | Self::TypeVar | ||
| | Self::NamedTuple | ||
| | Self::StdlibAlias | ||
| | Self::Iterable | ||
| | Self::SupportsIndex => KnownModule::Typing, | ||
| Self::TypeAliasType | ||
| | Self::TypeVarTuple | ||
|
|
@@ -2984,6 +2989,7 @@ impl KnownClass { | |
| | Self::NewType | ||
| | Self::Field | ||
| | Self::KwOnly | ||
| | Self::Iterable | ||
| | Self::NamedTupleFallback => false, | ||
| } | ||
| } | ||
|
|
@@ -3052,6 +3058,7 @@ impl KnownClass { | |
| | Self::NewType | ||
| | Self::Field | ||
| | Self::KwOnly | ||
| | Self::Iterable | ||
| | Self::NamedTupleFallback => false, | ||
| } | ||
| } | ||
|
|
@@ -3101,6 +3108,7 @@ impl KnownClass { | |
| "NewType" => Self::NewType, | ||
| "TypeAliasType" => Self::TypeAliasType, | ||
| "TypeVar" => Self::TypeVar, | ||
| "Iterable" => Self::Iterable, | ||
| "ParamSpec" => Self::ParamSpec, | ||
| "ParamSpecArgs" => Self::ParamSpecArgs, | ||
| "ParamSpecKwargs" => Self::ParamSpecKwargs, | ||
|
|
@@ -3197,6 +3205,7 @@ impl KnownClass { | |
| | Self::ParamSpecKwargs | ||
| | Self::TypeVarTuple | ||
| | Self::NamedTuple | ||
| | Self::Iterable | ||
| | Self::NewType => matches!(module, KnownModule::Typing | KnownModule::TypingExtensions), | ||
| } | ||
| } | ||
|
|
||
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.
I think I follow the constructors here, but can you put a comment with the Python syntax of the signature you're trying to construct here?