Skip to content

[ty] Tighten up a few edge cases in Concatenate type-expression parsing#24172

Merged
AlexWaygood merged 10 commits intomainfrom
alex/paramspec-error-message
Apr 9, 2026
Merged

[ty] Tighten up a few edge cases in Concatenate type-expression parsing#24172
AlexWaygood merged 10 commits intomainfrom
alex/paramspec-error-message

Conversation

@AlexWaygood
Copy link
Copy Markdown
Member

@AlexWaygood AlexWaygood commented Mar 25, 2026 •
edited
Loading

Summary

There were:

  • a couple of places where we emitted two diagnostics where one would do
  • a few places where we fell back to overly precise inferred types in error cases (inferring types like (Unknown, /) -> None where (...) -> None would arguably be better)
  • a few places where we correctly emitted a diagnostic, but didn't have an ideal error message

This PR just makes some minor cleanups in that area

Test Plan

new mdtests

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Mar 25, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Mar 25, 2026 •
edited
Loading

Typing conformance results

The percentage of diagnostics emitted that were expected errors held steady at 87.72%. The percentage of expected errors that received a diagnostic held steady at 82.85%. The number of fully passing files held steady at 74/132.

Summary

How are test cases classified?

Each test case represents one expected error annotation or a group of annotations sharing a tag. Counts are per test case, not per diagnostic — multiple diagnostics on the same line count as one. Required annotations (E) are true positives when ty flags the expected location and false negatives when it does not. Optional annotations (E?) are true positives when flagged but true negatives (not false negatives) when not. Tagged annotations (E[tag]) require ty to flag exactly one of the tagged lines; tagged multi-annotations (E[tag+]) allow any number up to the tag count. Flagging unexpected locations counts as a false positive.

Metric Old New Diff Outcome
True Positives 879 879 +0
False Positives 123 123 +0
False Negatives 182 182 +0
Total Diagnostics 1052 1052 +0
Precision 87.72% 87.72% +0.00%
Recall 82.85% 82.85% +0.00%
Passing Files 74/132 74/132 +0

True positives changed (4)

4 diagnostics
Test case Diff

generics_paramspec_basic.py:15

 
-error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type expression
+error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type alias value

generics_paramspec_basic.py:23

 
-error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type expression
-error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type expression
+error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a parameter annotation
+error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a return type annotation

generics_paramspec_basic.py:35

 
-error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type expression
+error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a parameter annotation

generics_paramspec_basic.py:39

 
-error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type expression
-error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a type expression
+error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a parameter annotation
+error[invalid-type-form] Bare ParamSpec `P` is not valid in this context in a parameter annotation

@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Mar 25, 2026 •
edited
Loading

Memory usage report

Memory usage unchanged ✅

@AlexWaygood AlexWaygood changed the title [ty] Tighten up a few edge cases in ParamSpec type expression parsing [ty] Tighten up a few edge cases in ParamSpec/Concatenate type expression parsing Mar 25, 2026
@AlexWaygood AlexWaygood changed the title [ty] Tighten up a few edge cases in ParamSpec/Concatenate type expression parsing [ty] Tighten up a few edge cases in ParamSpec/Concatenate type-expression parsing Mar 25, 2026
@astral-sh-bot
Copy link
Copy Markdown

astral-sh-bot Bot commented Mar 25, 2026 •
edited
Loading

ecosystem-analyzer results

No diagnostic changes detected ✅

Full report with detailed diff (timing results)

@AlexWaygood

This comment was marked as outdated.

Sign in to view
@AlexWaygood AlexWaygood marked this pull request as ready for review March 25, 2026 13:56
@AlexWaygood AlexWaygood assigned dhruvmanila and unassigned sharkdp Mar 25, 2026
@AlexWaygood AlexWaygood changed the title [ty] Tighten up a few edge cases in ParamSpec/Concatenate type-expression parsing [ty] Tighten up a few edge cases in Concatenate type-expression parsing Mar 25, 2026
dhruvmanila
dhruvmanila reviewed Mar 25, 2026
View reviewed changes
Comment thread crates/ty_python_semantic/src/types/special_form.rs Outdated
@carljm carljm removed their request for review March 25, 2026 18:34
dhruvmanila
dhruvmanila reviewed Mar 26, 2026
View reviewed changes
Comment thread crates/ty_python_semantic/resources/mdtest/generics/pep695/concatenate.md Outdated
Comment thread crates/ty_python_semantic/src/types/infer/builder/type_expression.rs
Comment thread crates/ty_python_semantic/resources/mdtest/generics/pep695/concatenate.md Outdated
@AlexWaygood AlexWaygood force-pushed the alex/paramspec-error-message branch from 4dccb3a to a145edc Compare March 30, 2026 13:28
@AlexWaygood AlexWaygood marked this pull request as draft March 30, 2026 13:32
@AlexWaygood
Copy link
Copy Markdown
Member Author

AlexWaygood commented Apr 8, 2026

Thanks for the review, you raised some great points! I've revised the approach, and also added snapshots to make it easier to see what the diagnostics in the various error cases will actually look like

@AlexWaygood AlexWaygood marked this pull request as ready for review April 8, 2026 14:06
@astral-sh-bot astral-sh-bot Bot requested a review from dhruvmanila April 8, 2026 14:06
dhruvmanila
dhruvmanila approved these changes Apr 9, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thank you!

@AlexWaygood AlexWaygood merged commit 99d97bd into main Apr 9, 2026
53 checks passed
@AlexWaygood AlexWaygood deleted the alex/paramspec-error-message branch April 9, 2026 08:43
ibraheemdev pushed a commit that referenced this pull request Apr 15, 2026
@AlexWaygood @ibraheemdev
[ty] Tighten up a few edge cases in Concatenate type-expression par…
5ff5f5b 
…sing (#24172)

## Summary

There were:
- a couple of places where we emitted two diagnostics where one would do
- a few places where we fell back to overly precise inferred types in
error cases (inferring types like `(Unknown, /) -> None` where `(...) ->
None` would arguably be better)
- a few places where we correctly emitted a diagnostic, but didn't have
an ideal error message

This PR just makes some minor cleanups in that area

## Test Plan

new mdtests
This was referenced Apr 27, 2026
Merged
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

@dhruvmanila dhruvmanila dhruvmanila approved these changes

@sharkdp sharkdp Awaiting requested review from sharkdp sharkdp is a code owner

@dcreager dcreager Awaiting requested review from dcreager dcreager is a code owner

@ibraheemdev ibraheemdev Awaiting requested review from ibraheemdev ibraheemdev is a code owner

Assignees

@dhruvmanila dhruvmanila

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AlexWaygood @charliermarsh @dhruvmanila @sharkdp