Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show more precise messages in invalid type expressions #16850

Conversation

MatthewMckee4
Copy link
Contributor

Summary

Some error messages are not very specific and could be improved

Test Plan

Not yet sure where to put some tests, but will just test invalid type expressions

@AlexWaygood AlexWaygood added the red-knot Multi-file analysis & type inference label Mar 19, 2025
@MatthewMckee4
Copy link
Contributor Author

Im not fully sure where to put tests for the currently unsupported special forms and type qualifiers

Copy link
Contributor

github-actions bot commented Mar 19, 2025

mypy_primer results

No ecosystem changes detected ✅

@AlexWaygood
Copy link
Member

@MatthewMckee4
Copy link
Contributor Author

Yeah, i thought that, but we does providing error messages not imply that we are supporting them partially?

@AlexWaygood
Copy link
Member

True. We'll need comprehensive test suites covering each special form eventually, so you can create new files for each now if you like! But equally, I don't think it matters too much right now; I'd be inclined to just adjust the prose a little bit in those files saying that we don't fully support them, but we do detect some invalid uses when they appear in type expressions.

@MatthewMckee4
Copy link
Contributor Author

Okay, will do that then, thanks

@MatthewMckee4
Copy link
Contributor Author

MatthewMckee4 commented Mar 19, 2025

Tests failing because

def _(
    a: Final
) -> None: ...

raises no error

Ill change it to Final | int.

@AlexWaygood why does this happen exactly?

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 19, 2025

@AlexWaygood why does this happen exactly?

That's because we currently only validate that parameter annotations are valid annotation expressions. Bare Final is a valid annotation expression, even though it's not a valid type expression.

You can take a look at the typing spec page here for an explanation of the difference between the two concepts: https://typing.python.org/en/latest/spec/annotations.html#type-and-annotation-expressions. Final is a valid annotation expression but not a valid type expression; Final | int is neither a valid annotation expression nor a valid type expression.

Now, is Final a valid parameter annotation? No! 😄 But the reason it's invalid isn't relevant to the changes you're making here, and we shouldn't attempt to detect the invalidity as part of the Type::in_type_expression() method. At some point, we'll need to add some extra validation specifically for parameter annotations that checks exactly whether the annotation expression is one of the expressions that are disallowed in the specific context of parameter annotations. But we haven't implemented that additional check yet :-)

Copy link
Contributor

github-actions bot commented Mar 19, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MatthewMckee4
Copy link
Contributor Author

Okay understood. What function infers the type of parameter annotations?

@MatthewMckee4 MatthewMckee4 marked this pull request as ready for review March 19, 2025 16:19
@carljm
Copy link
Contributor

carljm commented Mar 19, 2025

What function infers the type of parameter annotations?

TypeInferenceBuilder::infer_parameter and TypeInferenceBuilder::infer_parameter_with_default both call into TypeInferenceBuilder::infer_optional_annotation_expression to infer the annotated type.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

MatthewMckee4 and others added 4 commits March 19, 2025 16:33
…nsupported_special_forms.md

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
…nsupported_special_forms.md

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Thanks, this is great!! I pushed a commit to make the match a bit more readable, since it was getting a bit big and the KnownInstance branches were interspersed with some of the other branches

@AlexWaygood AlexWaygood enabled auto-merge (squash) March 19, 2025 16:58
@AlexWaygood AlexWaygood merged commit 4ed93b4 into astral-sh:main Mar 19, 2025
22 checks passed
@MatthewMckee4 MatthewMckee4 deleted the in-type-expression-more-precise-messages branch March 19, 2025 17:29
dcreager added a commit that referenced this pull request Mar 21, 2025
* main: (26 commits)
  Use the common `OperatorPrecedence` for the parser (#16747)
  [red-knot] Check subtype relation between callable types (#16804)
  [red-knot] Check whether two callable types are equivalent (#16698)
  [red-knot] Ban most `Type::Instance` types in type expressions (#16872)
  Special-case value-expression inference of special form subscriptions (#16877)
  [syntax-errors] Fix star annotation before Python 3.11 (#16878)
  Recognize `SyntaxError:` as an error code for ecosystem checks (#16879)
  [red-knot] add test cases result in false positive errors (#16856)
  Bump 0.11.1 (#16871)
  Allow discovery of venv in VIRTUAL_ENV env variable (#16853)
  Split git pathspecs in change determination onto separate lines (#16869)
  Use the correct base commit for change determination (#16857)
  Separate `BitXorOr` into `BitXor` and `BitOr` precedence (#16844)
  Server: Allow `FixAll` action in presence of version-specific syntax errors (#16848)
  [`refurb`] Fix starred expressions fix (`FURB161`) (#16550)
  [`flake8-executable`] Add pytest and uv run to help message for `shebang-missing-python` (`EXE003`) (#16855)
  Show more precise messages in invalid type expressions (#16850)
  [`flake8-executables`] Allow `uv run` in shebang line for `shebang-missing-python` (`EXE003`) (#16849)
  Add `--exit-non-zero-on-format` (#16009)
  [red-knot] Ban list literals in most contexts in type expressions (#16847)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants