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

[red-knot] is_subtype_of fix for KnownInstance types #14750

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Dec 3, 2024

Summary

KnownInstance::instance_fallback may return instances of supertypes. For example, it returns an instance of _SpecialForm for Literal. This means it can't be used on the right-hand side of is_subtype_of relationships, because it might lead to false positives.

I can lead to false negatives on the left hand side of is_subtype_of, but this is at least a known limitation. False negatives are fine for most applications, but false positives can lead to wrong results in intersection-simplification, for example.

closes #14731

Test Plan

Added regression test

`KnownInstance::instance_fallback` may return instances of supertypes.
For example, it returns an instance of `_SpecialForm` for `Literal`.
This means it can't be used on the right-hand side of `is_subtype_of`
relationships, because it might lead to false positives.

I can lead to false negatives on the left hand side of `is_subtype_of`,
but this is at least a known limitation. False negatives are fine for
most applications, but false positives can lead to wrong results in
intersection-simplification, for example.
@sharkdp sharkdp added the red-knot Multi-file analysis & type inference label Dec 3, 2024
Copy link
Contributor

github-actions bot commented Dec 3, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@sharkdp sharkdp merged commit a255d79 into main Dec 3, 2024
21 checks passed
@sharkdp sharkdp deleted the david/fix-14731 branch December 3, 2024 11:03
dcreager added a commit that referenced this pull request Dec 3, 2024
* main:
  [`ruff`] Extend unnecessary-regular-expression to non-literal strings (`RUF055`) (#14679)
  Minor followups to RUF052 (#14755)
  [red-knot] Property tests (#14178)
  [red-knot] `is_subtype_of` fix for `KnownInstance` types (#14750)
  Improve docs for flake8-use-pathlib rules (#14741)
  [`ruff`] Implemented `used-dummy-variable` (`RUF052`) (#14611)
  [red-knot] Simplify tuples containing `Never` (#14744)
  Possible fix for flaky file watching test (#14543)
  [`flake8-import-conventions`] Improve syntax check for aliases supplied in configuration for `unconventional-import-alias (ICN001)` (#14745)
  [red-knot] Deeper understanding of `LiteralString` (#14649)
  red-knot: support narrowing for bool(E) (#14668)
  [`refurb`] Handle non-finite decimals in `verbose-decimal-constructor (FURB157)` (#14596)
  [red-knot] Re-enable linter corpus tests (#14736)
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.

[red-knot] Incorrect subtyping behavior of Literal instances and _SpecialForm
2 participants