[ty] implement typing.NewType
#20126
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Diagnostic diff on typing conformance testsChanges were detected when running ty on typing conformance tests--- old-output.txt 2025-08-28 07:13:12.704519896 +0000
+++ new-output.txt 2025-08-28 07:13:15.228529548 +0000
@@ -46,10 +46,15 @@
aliases_implicit.py:119:10: error[invalid-type-form] Variable of type `Literal["int | str"]` is not allowed in a type expression
aliases_implicit.py:128:1: error[type-assertion-failure] Argument does not have asserted type `list[str]`
aliases_implicit.py:133:6: error[call-non-callable] Object of type `UnionType` is not callable
-aliases_newtype.py:15:1: error[type-assertion-failure] Argument does not have asserted type `int`
-aliases_newtype.py:18:1: error[invalid-assignment] Object of type `NewType` is not assignable to `type`
-aliases_newtype.py:26:21: error[invalid-base] Invalid class base with type `NewType`
-aliases_newtype.py:63:43: error[too-many-positional-arguments] Too many positional arguments to bound method `__init__`: expected 3, got 4
+aliases_newtype.py:11:8: error[invalid-argument-type] Argument is incorrect: Expected `int`, found `Literal["user"]`
+aliases_newtype.py:12:1: error[invalid-assignment] Object of type `Literal[42]` is not assignable to `UserId`
+aliases_newtype.py:18:1: error[invalid-assignment] Object of type `UserId` is not assignable to `type`
+aliases_newtype.py:26:21: error[invalid-base] Invalid class base with type `UserId`
+aliases_newtype.py:41:6: error[invalid-type-form] `GoodNewType1` is a `NewType` and cannot be specialized
+aliases_newtype.py:47:15: error[invalid-newtype] The second argument to `typing.NewType` must be a class or another `NewType`
+aliases_newtype.py:54:15: error[invalid-newtype] The second argument to `typing.NewType` must be a class or another `NewType`
+aliases_newtype.py:63:43: error[too-many-positional-arguments] Too many positional arguments to class `NewType`: expected 2, got 3
+aliases_newtype.py:65:15: error[invalid-newtype] The second argument to `typing.NewType` must be a class or another `NewType`
aliases_type_statement.py:17:1: error[unresolved-attribute] Type `typing.TypeAliasType` has no attribute `bit_count`
aliases_type_statement.py:19:1: error[call-non-callable] Object of type `TypeAliasType` is not callable
aliases_type_statement.py:23:7: error[unresolved-attribute] Type `typing.TypeAliasType` has no attribute `other_attrib`
@@ -515,7 +520,7 @@
generics_typevartuple_basic.py:12:14: error[invalid-argument-type] `@Todo(starred expression)` is not a valid argument to `Generic`
generics_typevartuple_basic.py:16:26: error[invalid-return-type] Function always implicitly returns `None`, which is not assignable to return type `tuple[@Todo(PEP 646), ...]`
generics_typevartuple_basic.py:23:13: error[invalid-argument-type] `@Todo(starred expression)` is not a valid argument to `Generic`
-generics_typevartuple_basic.py:42:34: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `tuple[@Todo(PEP 646), ...]`, found `Literal[1]`
+generics_typevartuple_basic.py:42:34: error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `tuple[@Todo(PEP 646), ...]`, found `Height`
generics_typevartuple_basic.py:65:27: error[unknown-argument] Argument `covariant` does not match any known parameter of function `__new__`
generics_typevartuple_basic.py:66:27: error[too-many-positional-arguments] Too many positional arguments to function `__new__`: expected 2, got 4
generics_typevartuple_basic.py:67:27: error[unknown-argument] Argument `bound` does not match any known parameter of function `__new__`
@@ -859,5 +864,5 @@
typeddicts_usage.py:28:1: error[missing-typed-dict-key] Missing required key 'name' in TypedDict `Movie` constructor
typeddicts_usage.py:28:18: error[invalid-key] Invalid key access on TypedDict `Movie`: Unknown key "title"
typeddicts_usage.py:40:24: error[invalid-type-form] The special form `typing.TypedDict` is not allowed in type expressions. Did you mean to use a concrete TypedDict or `collections.abc.Mapping[str, object]` instead?
-Found 860 diagnostics
+Found 865 diagnostics
WARN A fatal error occurred while checking some files. Not all project files were analyzed. See the diagnostics list above for details. |
|
AlexWaygood
reviewed
Aug 28, 2025
| f.write_str("]") | ||
| } | ||
| KnownInstanceType::NewType(declaration) => { | ||
| f.write_str(declaration.name(self.db)) |
There was a problem hiding this comment.
I think this should be something like
Suggested change
| f.write_str(declaration.name(self.db)) | |
| write!(f, "<NewType pseudo-class '{}'>, declaration.name(self.db)) |
So that the display of a NewType pseudo-class clearly differentiates it from an "instance of that pseudo-class". Otherwise the output of reveal_type for something like this is quite confusing:
from typing import NewType, reveal_type
X = NewType("X", int)
reveal_type(X) # We reveal `X` here
reveal_type(X(42)) # W also reveal `X` here currently, but the object returned by `X(42)`
# inhabits a very different type in our model to the object `X` itself!
|
the typing conformance diff all looks good! New diagnostics being emitted on lines that have |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
472 | 0 | 3 |
unused-ignore-comment |
0 | 15 | 0 |
invalid-return-type |
2 | 1 | 8 |
unsupported-operator |
2 | 0 | 4 |
type-assertion-failure |
2 | 0 | 2 |
unresolved-attribute |
0 | 0 | 2 |
no-matching-overload |
1 | 0 | 0 |
| Total | 479 | 16 | 19 |
|
It's a big ecosystem diff but all the ones I've looked at so far are true positives, which is sort-of shocking -- I guess lots of people out there are just misusing |
AlexWaygood
reviewed
Aug 28, 2025
| /// Checks for the creation of invalid `NewType`s | ||
| /// | ||
| /// ## Why is this bad? | ||
| /// There are several requirements that you must follow when creating a `NewType`. |
There was a problem hiding this comment.
I think if I were a user I would immediately have the question here "...but why must I follow these requirements?" 😆 I think we should either explain a bit more clearly here why the rules around NewType are the way they are, or link to some docs elsewhere that give that explanation
AlexWaygood
reviewed
Aug 28, 2025
| /// - `x`: `Type::NominalInstance(...(NominalInstanceInner::NewType(NewTypeInstance { .. }))` | ||
| #[salsa::interned(debug, heap_size=ruff_memory_usage::heap_size)] | ||
| #[derive(PartialOrd, Ord)] | ||
| pub struct NewTypeInstance<'db> { |
There was a problem hiding this comment.
I think it would be better to simply name this struct NewType, since it can both appear in the type Type::KnownInstance(KnownInstanceType::NewType(..)) and in the type Type::NominalInstance(NominalInstanceType { inner: NominalInstanceInner::NewType(..) }).
The first of those is a type which means "a set of Python objects with exactly length one, where the sole inhabitant of the set is an object returned by a specific all to NewType()". The second of those is a type which means "a set of Python objects with unknown size, where all inhabitants of the set are said to be 'instances' of a specific NewType pseudo-class". The first is therefore similar conceptually to a Type::ClassLiteral type, and the second is similar conceptually to a "normal" nominal-instance type, but using the word "Instance" in the name of this struct implies that it can only ever be used as the wrapped data in instance types, which isn't true.
I think there's also enough code here that it's worth moving the struct into a newtypes.rs submodule -- types.rs is already huge, and I think we'd like to make it smaller.
Comment on lines
+7308
to
+7314
| let mut base = self.base(db); | ||
| loop { | ||
| match base { | ||
| NewTypeBase::ClassType(class) => return class, | ||
| NewTypeBase::NewType(newtype) => base = newtype.base(db), | ||
| } | ||
| } |
There was a problem hiding this comment.
could shave off a few lines here by making it recursive?
Suggested change
| let mut base = self.base(db); | |
| loop { | |
| match base { | |
| NewTypeBase::ClassType(class) => return class, | |
| NewTypeBase::NewType(newtype) => base = newtype.base(db), | |
| } | |
| } | |
| match self.base(db) { | |
| NewTypeBase::ClassType(class) => return class, | |
| NewTypeBase::NewType(newtype) => newtype.base_class_type(db), | |
| } |
Comment on lines
+7369
to
+7381
| let mut candidate_newtype = self; | ||
| loop { | ||
| if candidate_newtype == other { | ||
| // Either `self` is `other`, or it's a (transitive) NewType wrapper of it. | ||
| return true; | ||
| } | ||
| match candidate_newtype.base(db) { | ||
| NewTypeBase::NewType(base_newtype) => candidate_newtype = base_newtype, | ||
| // Classes can't inherit from NewTypes, so if we reach the base `ClassType` without | ||
| // seeing `other`, then we can't be a subtype of it. | ||
| NewTypeBase::ClassType(_) => return false, | ||
| } | ||
| } |
There was a problem hiding this comment.
similarly here, I'd be tempted to use recursion, unless you think this will be faster? We generally use recursion for this kind of thing elsewhere
Suggested change
| let mut candidate_newtype = self; | |
| loop { | |
| if candidate_newtype == other { | |
| // Either `self` is `other`, or it's a (transitive) NewType wrapper of it. | |
| return true; | |
| } | |
| match candidate_newtype.base(db) { | |
| NewTypeBase::NewType(base_newtype) => candidate_newtype = base_newtype, | |
| // Classes can't inherit from NewTypes, so if we reach the base `ClassType` without | |
| // seeing `other`, then we can't be a subtype of it. | |
| NewTypeBase::ClassType(_) => return false, | |
| } | |
| } | |
| if self == other { | |
| // Either `self` is `other`, or it's a (transitive) NewType wrapper of it. | |
| return true; | |
| } | |
| match self.base(db) { | |
| NewTypeBase::NewType(base_newtype) => base_new_type.is_subtype_of(db, other), | |
| // Classes can't inherit from NewTypes, so if we reach the base `ClassType` without | |
| // seeing `other`, then we can't be a subtype of it. | |
| NewTypeBase::ClassType(_) => false, | |
| } |
| } | ||
|
|
||
| impl<'db> NewTypeBase<'db> { | ||
| fn to_type(self, db: &'db dyn Db) -> Type<'db> { |
There was a problem hiding this comment.
Suggested change
| fn to_type(self, db: &'db dyn Db) -> Type<'db> { | |
| fn to_instance(self, db: &'db dyn Db) -> Type<'db> { |
to make it clear that this method creates the type that represents "the set containing all possible instances of this newtypebase", not the type "the singleton set that only contains the runtime object representing this newtypebase itself"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
astral-sh/ty#742
This is an initial draft of a working implementation of
NewType. New test cases are innew_types.md, in particular:@AlexWaygood suggested a substantially different approach involving a new
Type::NewTypeInstancevariant rather than what this PR currently does, which is adding a newNominalInstanceType::NewTypevariant. I haven't yet tried that, and I don't have much intuition of my own about which is better, but I had this close to working, and I wanted to get some early feedback on it before I tried to do an overhaul.That said, my baby daughter was born this evening (happy, healthy ❤️), so it will be a few days or weeks before I get back to this.