[ty] Improve consistency and quality of diagnostics relating to invalid type forms#24325
[ty] Improve consistency and quality of diagnostics relating to invalid type forms#24325AlexWaygood merged 1 commit intomainfrom
Conversation
Typing conformance resultsThe percentage of diagnostics emitted that were expected errors held steady at 86.76%. The percentage of expected errors that received a diagnostic held steady at 81.53%. The number of fully passing files held steady at 70/132. SummaryHow 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 (
True positives changed (16)16 diagnostics
|
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
40 | 0 | 0 |
escape-character-in-forward-annotation |
0 | 0 | 7 |
invalid-argument-type |
0 | 1 | 0 |
invalid-return-type |
1 | 0 | 0 |
invalid-type-form |
0 | 0 | 1 |
| Total | 41 | 1 | 8 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
Raw diff:
egglog-python (https://github.com/egraphs-good/egglog-python)
- python/egglog/exp/array_api.py:2878:35 error[invalid-argument-type] Argument to function `multiset_flat_map` is incorrect: Expected `(MultiSet[Value], /) -> MultiSet[MultiSet[Value]]`, found `UnstableFn`
jinja (https://github.com/pallets/jinja)
- src/jinja2/defaults.py:24:19 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ src/jinja2/defaults.py:24:19 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
- src/jinja2/environment.py:307:27 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ src/jinja2/environment.py:307:27 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
- src/jinja2/environment.py:399:27 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ src/jinja2/environment.py:399:27 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
- src/jinja2/environment.py:1177:27 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ src/jinja2/environment.py:1177:27 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
spack (https://github.com/spack/spack)
- lib/spack/spack/vendor/jinja2/defaults.py:24:19 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ lib/spack/spack/vendor/jinja2/defaults.py:24:19 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
- lib/spack/spack/vendor/jinja2/environment.py:303:27 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ lib/spack/spack/vendor/jinja2/environment.py:303:27 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
- lib/spack/spack/vendor/jinja2/environment.py:1161:27 error[escape-character-in-forward-annotation] Type expressions cannot contain escape characters
+ lib/spack/spack/vendor/jinja2/environment.py:1161:27 error[escape-character-in-forward-annotation] Escape characters are not allowed in type expressions
sympy (https://github.com/sympy/sympy)
- sympy/vector/implicitregion.py:130:49 error[invalid-type-form] Int literals are not allowed in this context in a type expression
+ sympy/vector/implicitregion.py:130:49 error[invalid-type-form] Int literals are not allowed in this context in a type expression: Did you mean `typing.Literal[0]`?2755643 to
7c45fad
Compare
7c45fad to
e7e60af
Compare
|
It definitely seems to me like We are still in beta; there will never be a better time in future for error code changes. |
|
Okay, I can just get rid of those error codes! |
a18198c to
9ddda47
Compare
28b482c to
a19616d
Compare
|
The ecosystem report shows the new "Did you mean |
a19616d to
b89cadd
Compare
993948e to
8c3251f
Compare
| o: bar(), # error: [invalid-type-form] "Function calls are not allowed in type expressions" | ||
| # error: [unsupported-operator] | ||
| # error: [invalid-type-form] "F-strings are not allowed in type expressions" | ||
| # error: [invalid-type-form] "Type expressions cannot use f-strings" |
There was a problem hiding this comment.
I might say "include" instead of "use", but it looks like this is consistent with our existing diagnostics elsewhere, so feel free to ignore.
There was a problem hiding this comment.
I think there was some inconsistency here -- thank you! Tried to clean it up
| c: f"int", | ||
| # error: [invalid-type-form] "Type expressions cannot use f-strings" | ||
| d: list[f"int"], | ||
| # error: [invalid-type-form] "Bytes literals are not allowed in this context in a type expression" |
There was a problem hiding this comment.
Does the message differ here because bytes literals are sometimes allowed in type expressions (e.g., within Literal)?
There was a problem hiding this comment.
Yup, exactly. It's not true to say they're always invalid in type expressions -- they're just only valid in certain specific contexts
| | | ||
| 1 | def _( | ||
| 2 | x: {int: str}, # error: [invalid-type-form] | ||
| | ^^^^^^^^^^ Did you mean `dict[int, str]`? |
8c3251f to
246831b
Compare
* main: Add a "release-gate" step to the release workflow (#24365) Disallow starred expressions as values of starred expressions (#24280) [`pyupgrade`] Ignore strings with string-only escapes (`UP012`) (#16058) [ty] Improve consistency and quality of diagnostics relating to invalid type forms (#24325) [flake8-type-checking] Clarify import cycle wording for TC001/TC002/TC003 (#24322) [`flake8-errmsg`] Avoid shadowing existing `msg` in fix for `EM101` (#24363) `RUF072`: skip formfeeds on dedent (#24308) Replace unmaintained `unic-ucd-category` crate with `icu_properties` (#24344) [ty] Replace markdown hard line breaks in snapshot tests (#24361) [ty] Move snapshot for code action test with trailing whitespace to external file (#24359)
Summary
Currently this causes us to emit
byte-string-type-annotation:but this causes us to emit a different error code (
invalid-type-form):This is pretty inconsistent. The cause is that we try to handle these AST nodes in both
infer_annotation_expressionandinfer_type_expression, and the two code paths became inconsistent with each other. This PR adjusts our logic ininfer_annotation_expressionto just fallthrough toinfer_type_expressionfor these AST nodes, and has both of the above examples use theinvalid-type-formerror code.The exact same issue exists for our
fstring-type-annotationrule, as well; this is also fixed in this PR. The error codesfstring-type-annotationandbyte-string-type-annotationare now unused, so they are removed wholesale.Since I was touching the relevant part of the code anyway, I also added some more subdiagnostic hints for some of these diagnostics as part of this PR.
Test Plan
Mdtests and snapshots.