Fix redundant cast detection for generic calls - #21801
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Sorry but this seems wrong. Why are we discarding type context? |
|
Good point. I reproduced the original issue by inspecting the types: with the existing Any context, the source of cast(list[int], identity(xs)) is inferred as list[Any], while identity(xs) on its own is inferred as list[int]. My intention with the second context-free pass was only to avoid that Any-driven inference for the redundant-cast comparison; the normal cast check still uses the existing context. But I see the concern: type_context=None does not necessarily model removing the cast, since the expression may still receive context from its surrounding expression. I'll investigate whether the fix should instead preserve that surrounding context or happen in generic inference itself. |
|
Follow-up: I changed the speculative redundant-cast inference so it no longer discards the incoming type context. The normal cast source check is unchanged and still uses the existing cast_context = self.type_context[-1]
inferred_source_type = self.accept(
expr.expr,
type_context=cast_context,
allow_none_return=True,
always_allow_any=True,
)So instead of forcing I also updated the regression test to match the original issue more closely: def f(xs: list[int]) -> list[int]:
return cast(list[int], identity(xs))Verification after the change:
So the fix now preserves the surrounding type context while still avoiding the Would you mind having another look when you have a chance? Sorry for the confusion with the earlier approach, and thanks for pointing this out. |
|
Diff from mypy_primer, showing the effect of this PR on open source code: steam.py (https://github.com/Gobot1234/steam.py)
+ steam/manifest.py:1200: error: Redundant cast to "list[DepotID]" [redundant-cast]
+ steam/state.py:728: error: Redundant cast to "list[int]" [redundant-cast]
openlibrary (https://github.com/internetarchive/openlibrary)
+ openlibrary/solr/updater/work.py: note: In member "update_key" of class "WorkSolrUpdater":
+ openlibrary/solr/updater/work.py:115: error: Redundant cast to "WorkSeriesEdge[SeriesDict]" [redundant-cast]
prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/__init__.py:46: error: Redundant cast to "VersionInfo" [redundant-cast]
+ src/prefect/input/run_input.py:527: error: Redundant cast to "type[AutomaticRunInput[T]]" [redundant-cast]
scrapy (https://github.com/scrapy/scrapy)
- scrapy/extensions/feedexport.py:563: error: Redundant cast to "list[Any]" [redundant-cast]
+ scrapy/extensions/feedexport.py:559: error: Redundant cast to "list[Task[None]]" [redundant-cast]
pandas-stubs (https://github.com/pandas-dev/pandas-stubs)
+ tests/series/test_add.py:45: error: Unused "type: ignore" comment [unused-ignore]
+ tests/indexes/test_indexes.py:1071: error: Redundant cast to "list[Index[Any]]" [redundant-cast]
cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/builder.py: note: In member "generate_arg" of class "Builder":
+ cwltool/builder.py:636:28: error: Redundant cast to "list[str]" [redundant-cast]
+ cwltool/cwlprov/ro.py: note: In member "add_data_file" of class "ResearchObject":
+ cwltool/cwlprov/ro.py:572:47: error: Redundant cast to "Aggregate" [redundant-cast]
rotki (https://github.com/rotki/rotki)
+ rotkehlchen/accounting/history_base_entries.py:176: error: Redundant cast to "HistoryBaseEntry[Any]" [redundant-cast]
+ rotkehlchen/accounting/history_base_entries.py:186: error: Redundant cast to "HistoryBaseEntry[Any]" [redundant-cast]
anyio (https://github.com/agronholm/anyio)
+ src/anyio/_backends/_asyncio.py:2782: error: Redundant cast to "tuple[Transport, StreamProtocol]" [redundant-cast]
meson (https://github.com/mesonbuild/meson)
+ mesonbuild/utils/universal.py:1894:16: error: Redundant cast to "list[_T]" [redundant-cast]
+ mesonbuild/build.py:1457:33: error: Redundant cast to "list[str | None]" [redundant-cast]
paasta (https://github.com/yelp/paasta)
+ paasta_tools/utils.py:4220: error: Redundant cast to "list[str]" [redundant-cast]
+ paasta_tools/spark_tools.py:120: error: Redundant cast to "list[DockerVolume]" [redundant-cast]
mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ pymongo/synchronous/encryption.py:149: error: Redundant cast to "Collection[RawBSONDocument]" [redundant-cast]
+ pymongo/asynchronous/encryption.py:150: error: Redundant cast to "AsyncCollection[RawBSONDocument]" [redundant-cast]
|
|
I still don't think this is right, since it feels like this duplicates code. Why not simply pass through type context for the |
|
I tried passing the incoming context directly to the original The focused redundant-cast tests still pass:
However, In cast(Graph, {name: object() for name in (graph or set())})then produces: The outer So it looks like the existing I agree that doing a second Also, thanks for taking the time to reply and point me in the right direction. I really appreciate the review and the context here. |
Fixes #21796
visit_cast_expr()currently checks the source expression with anAnytype context. For generic calls, that context participates in type argument inference, so a call such asidentity(xs)is inferred aslist[Any]insidecast()even though it is independently inferred aslist[int].This prevents the redundant-cast check from recognizing that the source type and target type are the same.
Check the cast source expression without a type context so its type is inferred independently before comparing it with the cast target.
Also add a regression test covering redundant casts around generic return types.
Tests:
python runtests.py testRedundantCastGenericReturnpython runtests.py check-warnings.testpython runtests.py selfpython runtests.py lint