Skip to content

Conversation

MatthewMckee4
Copy link
Contributor

@MatthewMckee4 MatthewMckee4 commented Mar 20, 2025

Summary

Catch Some Instances, but raise type error for the rest of them
Fixes #16851

Test Plan

Extend invalid.md in annotations

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Mar 20, 2025
Copy link
Contributor

github-actions bot commented Mar 20, 2025

mypy_primer results

Changes were detected when running on open source projects
isort (https://github.com/pycqa/isort)
+ error[lint:invalid-type-form] /tmp/mypy_primer/projects/isort/isort/wrap.py:16:33: Variable of type `Enum` is not allowed in a type expression
+ error[lint:invalid-type-form] /tmp/mypy_primer/projects/isort/isort/wrap_modes.py:12:33: Variable of type `Enum` is not allowed in a type expression
- Found 79 diagnostics
+ Found 81 diagnostics

MatthewMckee4 and others added 2 commits March 20, 2025 15:08
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@AlexWaygood
Copy link
Member

From the mypy_primer diff, it looks like we'll also need to add special cases to avoid false positives on NewType and GenericAlias. I think we already have a KnownClass variant for GenericAlias, but I don't think we have one for NewType yet.

Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Nice!

@carljm
Copy link
Contributor

carljm commented Mar 20, 2025

Oh, reviewed the code without looking at the mypy-primer and tomllib results. We're still seeing a lot of "Variable of type 'object' not allowed" which look like false positives, probably need to get that resolved before landing this.

AlexWaygood added a commit that referenced this pull request Mar 20, 2025
…#16877)

## Summary

Currently for something like `X = typing.Tuple[str, str]`, we infer the
value of `X` as `object`. That's because `Tuple` (like many of the
symbols in the typing module) is annotated as a `_SpecialForm` instance
in typeshed's stubs:


https://github.com/astral-sh/ruff/blob/23382f5f8c7b4e356368cdeb1049b8c1910baff3/crates/red_knot_vendored/vendor/typeshed/stdlib/typing.pyi#L215

and we don't understand implicit type aliases yet, and the stub for
`_SpecialForm.__getitem__` says it always returns `object`:


https://github.com/astral-sh/ruff/blob/23382f5f8c7b4e356368cdeb1049b8c1910baff3/crates/red_knot_vendored/vendor/typeshed/stdlib/typing.pyi#L198-L200

We have existing false positives in our test suite due to this:


https://github.com/astral-sh/ruff/blob/23382f5f8c7b4e356368cdeb1049b8c1910baff3/crates/red_knot_python_semantic/resources/mdtest/annotations/annotated.md?plain=1#L76-L78

and it's causing _many_ new false positives in #16872, which tries to
make our annotation-expression parsing stricter in some ways.

This PR therefore adds some small special casing for `KnownInstanceType`
variants that fallback to `_SpecialForm`, so that these false positives
can be avoided.

## Test Plan

Existing mdtest altered.

Cc. @MatthewMckee4
@AlexWaygood AlexWaygood reopened this Mar 20, 2025
@carljm
Copy link
Contributor

carljm commented Mar 20, 2025

Looks good to me now! The only new false positive we see now in mypy-primer is with the edge case of creating an enum type imperatively by calling the enum.Enum constructor. I think it's fine to leave this false positive in place for now.

@carljm carljm merged commit 63e78b4 into astral-sh:main Mar 20, 2025
44 of 45 checks passed
@carljm
Copy link
Contributor

carljm commented Mar 20, 2025

Thanks for the PR!

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)
  ...
@MatthewMckee4 MatthewMckee4 deleted the ban-most-instance-type-in-type-expression branch March 21, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ty Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[red-knot] Ban most Type::Instance types in type expressions
3 participants