[ty] Validate constructor arguments when a class is used as a decorator#22377
Merged
charliermarsh merged 4 commits intomainfrom Jan 17, 2026
Merged
[ty] Validate constructor arguments when a class is used as a decorator#22377charliermarsh merged 4 commits intomainfrom
charliermarsh merged 4 commits intomainfrom
Conversation
Typing conformance resultsNo changes detected ✅ |
|
charliermarsh
commented
Jan 4, 2026
charliermarsh
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
Member
|
this looks like it might conflict quite badly with #22124, though I have no idea how close that PR is to landing |
charliermarsh
force-pushed
the
charlie/class-decorator-validate-args
branch
from
aa17527 to
10d4e56
Compare
Contributor
#22124 is currently ready for review, but with a few questions in need of some guidance, so I would not say it's going to land very soon |
This was referenced Jan 7, 2026
AlexWaygood
reviewed
Jan 13, 2026
Member
AlexWaygood
left a comment
AlexWaygood
left a comment
There was a problem hiding this comment.
This isn't a full review but it looks like I got halfway through reviewing last week, so here's a partial review
AlexWaygood
changed the title
dcreager
approved these changes
Jan 17, 2026
Member
There was a problem hiding this comment.
I did not review the tests very thoroughly since @AlexWaygood already did.
On the code side, I have a mild concern that we're duplicating the "class constructors should not use try_call" logic in so many places, but that fact isn't introduced by this PR, so I don't consider that a blocker here. If we don't have an open issue for that (making try_call work for all call sites and callables) we should open one.
Comment on lines
7255
to
7258
| Err(err) => { | ||
| err.report_diagnostic(&self.context, decorator_ty, decorator_node.into()); | ||
| err.return_type() | ||
| } |
Member
There was a problem hiding this comment.
This error arm is slightly different than the one below. Do they produce different diagnostics? If so, is that on purpose? If we could consolidate these two arms to be identical, then we could collapse this down overall to something like
let call_arguments = CallArguments::positional([decorated_ty]);
let call_result = if use_constructor_call {
decorator_ty.try_call_constructor(
self.db(),
|_| call_arguments,
TypeContext::default(),
)
} else {
decorator_ty
.try_call(self.db(), &call_arguments)
.map(|bindings| bindings.return_type(self.db()))
};
match call_result {
Ok(return_ty) => ...
Err(err) => ...
}
Member
Author
There was a problem hiding this comment.
I think the error types are slightly different -- ConstructorCallError vs. CallError -- but I tried to unify more.
charliermarsh
force-pushed
the
charlie/class-decorator-validate-args
branch
2 times, most recently
from
a304a6f to
e527848
Compare
charliermarsh
force-pushed
the
charlie/class-decorator-validate-args
branch
from
e527848 to
9408474
Compare
Member
Author
|
(Reviewing ecosystem results...) |
Member
Author
|
I believe the WerkZeug diagnostics are true positives because we now "see" the cached property decorator. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Summary
If a class is used as a decorator, we now use the class constructor.
Closes astral-sh/ty#2232.