[ty] Validate bare ParamSpec usage in type annotations, and support stringified ParamSpecs as the first argument to Callable#23625
Merged
AlexWaygood merged 16 commits intomainfrom Mar 5, 2026
Conversation
Typing conformance resultsThe percentage of diagnostics emitted that were expected errors decreased from 85.74% to 85.73%. The percentage of expected errors that received a diagnostic increased from 78.13% to 78.45%. Summary
True positives addedDetails
False positives addedDetails
|
Memory usage reportMemory usage unchanged ✅ |
|
Callable
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
invalid-type-form |
5 | 0 | 0 |
invalid-return-type |
1 | 0 | 0 |
| Total | 46 | 0 | 0 |
7ba9ff6 to
0ffdea6
Compare
Add validation to reject `ParamSpec` usage in positions where it is not valid. A `ParamSpec` can only be used as the first argument to `Callable` or the last argument to `Concatenate`. This change detects and reports `invalid-type-form` diagnostics for the following invalid uses: - Bare `ParamSpec` as a variable/parameter annotation (e.g., `a: P`) - `ParamSpec` as a type argument to non-ParamSpec type variables (e.g., `list[P]`) - `ParamSpec` inside a `Callable` parameter list (e.g., `Callable[[P], int]`) - `ParamSpec` as the return type of a `Callable` (e.g., `Callable[..., P]`) - Bare `ParamSpec` as a type alias value (e.g., `type Alias = P`) - `ParamSpec` in function return type annotations (e.g., `-> P`) https://claude.ai/code/session_01HAr9mYARjM9pVP8B8JRHtF
- Shorten the primary diagnostic to "A bare `ParamSpec` is not valid in this context" and move the detailed guidance to an info subdiagnostic - Use `raise NotImplementedError` for `invalid_return` test bodies to avoid distracting `empty-body` diagnostics - Add `y: Any` parameter and assign from it to avoid `invalid-assignment` diagnostics in `invalid_variable_annotation` tests https://claude.ai/code/session_01HAr9mYARjM9pVP8B8JRHtF
- Change diagnostic to `Bare ParamSpec `P` is not valid in this context` to include the specific ParamSpec name - Move `Any` import to the top of the code block alongside other `typing` imports in the legacy paramspec test https://claude.ai/code/session_01HAr9mYARjM9pVP8B8JRHtF
…info message - Remove the `qualifiers.is_empty()` guard so that `ClassVar[P]`, `Final[P]`, etc. also trigger the bare ParamSpec diagnostic - Update the `.info()` message to mention type parameter/argument lists - Add tests for bare ParamSpec with type qualifiers (e.g. `Final[P]`) - Add tests for stringified ParamSpecs (e.g. `-> "P"`) - Update classvar.md to expect the new diagnostic on `ClassVar[P]` https://claude.ai/code/session_01HAr9mYARjM9pVP8B8JRHtF
Handle `Callable["P", int]` and `Callable[Concatenate[int, "P"], int]` by parsing the string annotation and recursively processing the inner expression in `infer_callable_parameter_types`. https://claude.ai/code/session_01HAr9mYARjM9pVP8B8JRHtF
8ee930d to
a32ae67
Compare
oconnor663
approved these changes
Mar 3, 2026
Contributor
oconnor663
left a comment
There was a problem hiding this comment.
My unbridled intellect Claude caught the following cases that aren't currently covered. Worth adding?
def g(_: P | int): ...
def g(_: Union[P, int]): ...
def g(_: Optional[P]): ...| /// `Concatenate`. In all other type expression positions, a bare `ParamSpec` is invalid. | ||
| /// | ||
| /// Returns `true` if the type was a `ParamSpec` and a diagnostic was reported. | ||
| pub(crate) fn check_for_bare_paramspec<'db>( |
Contributor
There was a problem hiding this comment.
Is "bare" a term of art here? I wonder if it would be clearer to call them "misplaced" ParamSpecs?
Member
Author
I think this actually calls the whole approach into question 😆 I'm going to try a rewrite. Thank you!! |
5608f33 to
0351ff5
Compare
AlexWaygood
commented
Mar 5, 2026
Comment on lines
2510
to
2528
| #[test] | ||
| fn hover_typevar_spec_binding() { | ||
| let test = cursor_test( | ||
| r#" | ||
| from typing import Callable | ||
| type Alias2[**AB = [int, str]] = Callable[A<CURSOR>B, tuple[AB]] | ||
| "#, | ||
| ); | ||
|
|
||
| // TODO: This should just be `**AB@Alias2 (<variance>)` | ||
| // https://github.com/astral-sh/ty/issues/1581 | ||
| assert_snapshot!(test.hover(), @" | ||
| (**AB@Alias2) -> tuple[AB@Alias2] | ||
| (**AB@Alias2) -> tuple[Unknown] | ||
| --------------------------------------------- | ||
| ```python | ||
| (**AB@Alias2) -> tuple[AB@Alias2] | ||
| (**AB@Alias2) -> tuple[Unknown] | ||
| ``` | ||
| --------------------------------------------- | ||
| info[hover]: Hovered content is |
Member
Author
There was a problem hiding this comment.
It confused me a bit at first that the snapshot changed here, but it's a good change. AB is a ParamSpec, so it's not a valid type argument to tuple. We now detect that this is invalid, so we infer the value of Alias2 as (**AB@Alias2) -> tuple[Unknown]. We obviously should show **AB@Alias2 instead of (**AB@Alias2) -> tuple[Unknown] when hovering over AB, but that's covered by the existing TODO comment.
1a911e9 to
394e4b1
Compare
Member
Author
|
The new false positive in the conformance suite report is due to missing support for |
carljm
added a commit
that referenced
this pull request
Mar 5, 2026
* main: Update conformance suite commit hash (#23746) conformance.py: Collapse the summary paragraph when nothing changed (#23745) [ty] Make inferred specializations line up with source types more better (#23715) Bump 0.15.5 (#23743) [ty] Render all changed diagnostics in conformance.py (#23613) [ty] Split deferred checks out of `types/infer/builder.rs` (#23740) Discover markdown files by default in preview mode (#23434) [ty] Use `HasOptionalDefinition` for `except` handlers (#23739) [ty] Fix precedence of `all` selector in TOML configurations (#23723) [ty] Make `all` selector case sensitive (#23713) [ty] Add a diagnostic if a `TypeVar` is used to specialize a `ParamSpec`, or vice versa (#23738) [ty] Override home directory in ty tests (#23724) [ty] More type-variable default validation (#23639) [ty] Validate bare ParamSpec usage in type annotations, and support stringified ParamSpecs as the first argument to `Callable` (#23625) [ty] Add `all` selector to ty.json's `schema` (#23721) [ty] Add quotes to related issues links (#23720) [ty] Fix panic on incomplete except handlers (#23708)
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
Add validation to detect and report errors when a bare
ParamSpecis used in invalid contexts. AParamSpecis only valid:CallableConcatenateGeneric[P],Protocol[P])The validation is applied to both PEP 695 style (
def foo[**P]) and legacy style (P = ParamSpec("P"))ParamSpecdeclarations.While checking whether this validation also worked for stringified ParamSpecs, I realised that we actually didn't support stringified ParamSpecs in some places where we should. So I fixed that as part of this PR, too.
I'm not wild about how we have to "manually" add this check in multiple places, but I don't see an obviously better way of doing this.^I found an obviously better way of doing this.
Test Plan
mdtests and snapshots