Skip to content

[red-knot] Three-argument type-calls take 'str' as the first argument #17168

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 3, 2025
Merged

[red-knot] Three-argument type-calls take 'str' as the first argument #17168

merged 4 commits into from
Apr 3, 2025

Conversation

sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Apr 3, 2025

Summary

Similar to #17163, a minor fix in the signature of type(…).

Test Plan

New MD tests

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Apr 3, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 3, 2025 •
edited
Loading

mypy_primer results

Changes were detected when running on open source projects 
packaging (https://github.com/pypa/packaging)
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/packaging/src/packaging/tags.py:238:42: Object of type `tuple[@Todo(map_with_boundness: intersections with negative contributions), @Todo(generics)]` cannot be assigned to parameter 1 (`version`) of function `_version_nodot`; expected type `Sequence`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/packaging/src/packaging/tags.py:327:39: Object of type `tuple[@Todo(Support for `typing.TypeVar` instances in type expressions), @Todo(generics)]` cannot be assigned to parameter 1 (`version`) of function `_version_nodot`; expected type `Sequence`
- Found 97 diagnostics
+ Found 95 diagnostics

AlexWaygood
AlexWaygood approved these changes Apr 3, 2025
View reviewed changes
@sharkdp sharkdp force-pushed the david/first-type-argument-should-be-str branch from 5bab38e to 0d50952 Compare April 3, 2025 11:30
@sharkdp
Copy link
Contributor Author

sharkdp commented Apr 3, 2025 •
edited
Loading 

+ error[lint:no-matching-overload] /tmp/mypy_primer/projects/pyinstrument/pyinstrument/vendor/decorator.py:380:27: No overload of class `type` matches arguments

Hm, now we have a false positive, because we don't understand that tuple[Any] (or more specifically, tuple[@Todo, @Todo], in this example) is assignable to tuple. Maybe because we rely on subtyping in order for x -> tuple assignments to work? I'll look into it.

@AlexWaygood
Copy link
Member 

+ error[lint:no-matching-overload] /tmp/mypy_primer/projects/pyinstrument/pyinstrument/vendor/decorator.py:380:27: No overload of class `type` matches arguments

Hm, now we have a false positive, because we don't understand that tuple[Any] (or more specifically, tuple[@Todo, @Todo], in this example) is assignable to tuple. Maybe because we rely on subtyping in order for x -> tuple assignments to work? I'll look into it.

ah, it looks like we understand that tuple[int, int] is a subtype of tuple and therefore assignable to tuple, but perhaps we don't understand that tuple[@Todo, @Todo] is assignable to tuple (because it is not a subtype of tuple). So we probably need to add a fallback (Type::Tuple(_), _) => KnownClass::Tuple.to_instance(db).is_assignable_to(target)branch toType::is_assignable_to(), beneath the existing (Type::Tuple(), Type::Tuple())` branch

@sharkdp
Copy link
Contributor Author

sharkdp commented Apr 3, 2025 •
edited
Loading

So we probably need to add a fallback (Type::Tuple(_), _) => KnownClass::Tuple.to_instance(db).is_assignable_to(target)branch toType::is_assignable_to(), beneath the existing (Type::Tuple(), Type::Tuple())` branch

That's exactly what I did, yes 👍. The false positive is gone.

AlexWaygood
AlexWaygood approved these changes Apr 3, 2025
View reviewed changes
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.

LG! Don't merge right now, though, there's a patch release in progress 😄

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 3, 2025 •
edited
Loading 

- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/packaging/src/packaging/tags.py:238:42: Object of type `tuple[@Todo(map_with_boundness: intersections with negative contributions), @Todo(generics)]` cannot be assigned to parameter 1 (`version`) of function `_version_nodot`; expected type `Sequence`
- error[lint:invalid-argument-type] /tmp/mypy_primer/projects/packaging/src/packaging/tags.py:327:39: Object of type `tuple[@Todo(Support for `typing.TypeVar` instances in type expressions), @Todo(generics)]` cannot be assigned to parameter 1 (`version`) of function `_version_nodot`; expected type `Sequence`

and the new branch in Type::is_assignable_to() now gets rid of some false positives 🥳 because tuple is assignable to Sequence, so therefore tuple[@Todo, @Todo] is assignable to Sequence also

@sharkdp sharkdp merged commit 3f00010 into main Apr 3, 2025
23 checks passed
@sharkdp sharkdp deleted the david/first-type-argument-should-be-str branch April 3, 2025 13:45
dcreager added a commit that referenced this pull request Apr 3, 2025
@dcreager
Merge remote-tracking branch 'origin/main' into dcreager/typevar-type
39244dd 
* origin/main:
  [red-knot] Fix more [redundant-cast] false positives (#17170)
  [red-knot] Three-argument type-calls take 'str' as the first argument (#17168)
  Control flow: `return` and `raise` (#17121)
  Bump 0.11.3 (#17173)
  [red-knot] Improve `Debug` implementation for `semantic_index::SymbolTable` (#17172)
  [red-knot] Fix `str(…)` calls (#17163)
  [red-knot] visibility_constraint analysis for match cases (#17077)
  [red-knot] Fix playground crashes when diagnostics are stale (#17165)
dcreager added a commit that referenced this pull request Apr 3, 2025
@dcreager
Merge remote-tracking branch 'origin/main' into dcreager/special-class
5694b3d 
* origin/main: (82 commits)
  [red-knot] Fix more [redundant-cast] false positives (#17170)
  [red-knot] Three-argument type-calls take 'str' as the first argument (#17168)
  Control flow: `return` and `raise` (#17121)
  Bump 0.11.3 (#17173)
  [red-knot] Improve `Debug` implementation for `semantic_index::SymbolTable` (#17172)
  [red-knot] Fix `str(…)` calls (#17163)
  [red-knot] visibility_constraint analysis for match cases (#17077)
  [red-knot] Fix playground crashes when diagnostics are stale (#17165)
  [red-knot] Callable types are disjoint from literals (#17160)
  [red-knot] Fix inference for `pow` between two literal integers (#17161)
  [red-knot] Add GitHub PR annotations when mdtests fail in CI (#17150)
  [red-knot] Fix equivalence of differently ordered unions that contain `Callable` types (#17145)
  [red-knot] Add initial set of tests for unreachable code (#17159)
  [`airflow`] Move `AIR302` to `AIR301` and `AIR303` to `AIR302` (#17151)
  ruff_db: simplify lifetimes on `DiagnosticDisplay`
  [red-knot] Detect division-by-zero in unions and intersections (#17157)
  [`airflow`] Add autofix infrastructure to `AIR302` name checks (#16965)
  [`flake8-bandit`] Mark `str` and `list[str]` literals as trusted input (`S603`) (#17136)
  [`airflow`] Add autofix for `AIR302` attribute checks (#16977)
  [`airflow`] Extend `AIR302` with additional symbols (#17085)
  ...
maxmynter pushed a commit to maxmynter/ruff that referenced this pull request Apr 3, 2025
@sharkdp @maxmynter
[red-knot] Three-argument type-calls take 'str' as the first argument (…
0adc31c 
…astral-sh#17168)

## Summary

Similar to astral-sh#17163, a minor fix in the signature of `type(…)`.

## Test Plan

New MD tests
@AlexWaygood AlexWaygood mentioned this pull request Apr 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

@carljm carljm Awaiting requested review from carljm

@dcreager dcreager Awaiting requested review from dcreager

Assignees
No one assigned
Labels
ty Multi-file analysis & type inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sharkdp @AlexWaygood