Skip to content

Comments

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

Merged
sharkdp merged 4 commits intomainfrom
david/first-type-argument-should-be-str
Apr 3, 2025
Merged

[red-knot] Three-argument type-calls take 'str' as the first argument#17168
sharkdp merged 4 commits intomainfrom
david/first-type-argument-should-be-str

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
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

AlexWaygood commented Apr 3, 2025 

+ 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 reviewed Apr 3, 2025
View reviewed changes
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