fix Narrowing for isinstance and type[T] is unsound #713 #2284
+44
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
Fixes unsound negative narrowing for isinstance(x, cls) when cls has type type[T] by only performing negative subtraction when the classinfo is a concrete class object.
Changes:
- Restrict negative
isinstancenarrowing to only subtract whenclassinfois a concreteClassDef(or an alias to one). - Add helper methods to detect concrete classinfo for negative narrowing.
- Add a regression test ensuring the
elsebranch does not narrow forcls: type[int].
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pyrefly/lib/alt/narrow.rs | Adjusts negative isinstance narrowing to avoid subtracting for non-concrete classinfo like type[T]. |
| pyrefly/lib/test/narrow.rs | Adds regression coverage to ensure type[T] does not trigger unsound negative narrowing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
|
Diff from mypy_primer, showing the effect of this PR on open source code: pytest-robotframework (https://github.com/detachhead/pytest-robotframework)
- ERROR pytest_robotframework/__init__.py:136:35-37: `TracebackType | None` is not assignable to attribute `__traceback__` with type `Never` [bad-assignment]
- ::error file=pytest_robotframework/__init__.py,line=136,col=35,endLine=136,endColumn=37,title=Pyrefly bad-assignment::`TracebackType | None` is not assignable to attribute `__traceback__` with type `Never`
kopf (https://github.com/nolar/kopf)
- ERROR kopf/_cogs/structs/dicts.py:259:14-38: Runtime checkable protocol `Iterable` has an unsafe overlap with type `Never` [unsafe-overlap]
- ::error file=kopf/_cogs/structs/dicts.py,line=259,col=14,endLine=259,endColumn=38,title=Pyrefly unsafe-overlap::Runtime checkable protocol `Iterable` has an unsafe overlap with type `Never`%0A Attribute `__iter__` has incompatible types: `Never.__iter__` has type `Never`, which is not consistent with `BoundMethod[Never, (self: Never) -> Iterator[@_]]` in `Iterable.__iter__` (the type of read-write attributes cannot be changed)
porcupine (https://github.com/Akuli/porcupine)
- ERROR porcupine/plugins/highlight/pygments_highlighter.py:51:12-52: Object of class `type` has no attribute `get_tokens_unprocessed` [missing-attribute]
- ::error file=porcupine/plugins/highlight/pygments_highlighter.py,line=51,col=12,endLine=51,endColumn=52,title=Pyrefly missing-attribute::Object of class `type` has no attribute `get_tokens_unprocessed`
ibis (https://github.com/ibis-project/ibis)
- ::error file=ibis/expr/operations/window.py,line=50,col=9,endLine=50,endColumn=19,title=Pyrefly bad-override::Class member `WindowBoundary.__coerce__` overrides parent class `Value` in an inconsistent manner%0A `WindowBoundary.__coerce__` has type `BoundMethod[type[WindowBoundary[T, S]], (cls: type[WindowBoundary[T, S]], value: Unknown, **kwargs: Unknown) -> WindowBoundary[T, S] | WindowBoundary[Unknown, Any]]`, which is not assignable to `BoundMethod[type[WindowBoundary[T, S]], (cls: type[WindowBoundary[T, S]], value: Any, T: type[Any] | None = None, S: type[Any] | None = None) -> WindowBoundary[T, S]]`, the type of `Value.__coerce__`
+ ::error file=ibis/expr/operations/window.py,line=50,col=9,endLine=50,endColumn=19,title=Pyrefly bad-override::Class member `WindowBoundary.__coerce__` overrides parent class `Value` in an inconsistent manner%0A `WindowBoundary.__coerce__` has type `BoundMethod[type[WindowBoundary[T, S]], (cls: type[WindowBoundary[T, S]], value: Unknown, **kwargs: Unknown) -> WindowBoundary[Interval | Numeric, Any] | WindowBoundary[T, S] | WindowBoundary[Unknown, Scalar] | WindowBoundary[Unknown, Any]]`, which is not assignable to `BoundMethod[type[WindowBoundary[T, S]], (cls: type[WindowBoundary[T, S]], value: Any, T: type[Any] | None = None, S: type[Any] | None = None) -> WindowBoundary[T, S]]`, the type of `Value.__coerce__`
xarray (https://github.com/pydata/xarray)
+ ERROR xarray/core/dataarray.py:1491:51-57: Argument `Mapping[Any, T_ChunkDimFreq] | tuple[int, ...] | None` is not assignable to parameter `pos_kwargs` with type `Mapping[Any, T_ChunkDimFreq] | None` in function `xarray.namedarray.utils.either_dict_or_kwargs` [bad-argument-type]
+ ERROR xarray/namedarray/core.py:818:44-50: Argument `Mapping[Any, T_ChunkDim] | dict[Any, T_ChunkDim] | tuple[int, ...]` is not assignable to parameter `pos_kwargs` with type `Mapping[Any, T_ChunkDim] | None` in function `xarray.namedarray.utils.either_dict_or_kwargs` [bad-argument-type]
+ ::error file=xarray/core/dataarray.py,line=1491,col=51,endLine=1491,endColumn=57,title=Pyrefly bad-argument-type::Argument `Mapping[Any, T_ChunkDimFreq] | tuple[int, ...] | None` is not assignable to parameter `pos_kwargs` with type `Mapping[Any, T_ChunkDimFreq] | None` in function `xarray.namedarray.utils.either_dict_or_kwargs`
+ ::error file=xarray/namedarray/core.py,line=818,col=44,endLine=818,endColumn=50,title=Pyrefly bad-argument-type::Argument `Mapping[Any, T_ChunkDim] | dict[Any, T_ChunkDim] | tuple[int, ...]` is not assignable to parameter `pos_kwargs` with type `Mapping[Any, T_ChunkDim] | None` in function `xarray.namedarray.utils.either_dict_or_kwargs`
strawberry (https://github.com/strawberry-graphql/strawberry)
+ ERROR strawberry/http/__init__.py:28:26-39: Object of class `NoneType` has no attribute `errors` [missing-attribute]
+ ERROR strawberry/http/__init__.py:28:41-58: Object of class `NoneType` has no attribute `extensions` [missing-attribute]
+ ERROR strawberry/http/__init__.py:30:17-28: Object of class `NoneType` has no attribute `data` [missing-attribute]
+ ERROR strawberry/schema/schema.py:541:12-25: Object of class `NoneType` has no attribute `errors` [missing-attribute]
+ ERROR strawberry/schema/schema.py:542:44-57: `list[GraphQLError] | object | Unknown` is not assignable to attribute `pre_execution_errors` with type `list[GraphQLError] | None` [bad-assignment]
+ ERROR strawberry/schema/schema.py:544:38-51: Argument `list[GraphQLError] | object | Unknown` is not assignable to parameter `errors` with type `list[GraphQLError]` in function `strawberry.schema.base.BaseSchema._process_errors` [bad-argument-type]
+ ERROR strawberry/schema/schema.py:546:63-76: Argument `list[GraphQLError] | object | Unknown | None` is not assignable to parameter `errors` with type `list[GraphQLError] | None` in function `strawberry.types.execution.ExecutionResult.__init__` [bad-argument-type]
+ ERROR strawberry/schema/schema.py:547:9-26: Object of class `NoneType` has no attribute `extensions` [missing-attribute]
+ ERROR strawberry/schema/schema.py:549:16-22: Returned type `ExecutionResult | NoneType | Unknown` is not assignable to declared return type `ExecutionResult` [bad-return]
+ ::error file=strawberry/http/__init__.py,line=28,col=26,endLine=28,endColumn=39,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `errors`
+ ::error file=strawberry/http/__init__.py,line=28,col=41,endLine=28,endColumn=58,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `extensions`
+ ::error file=strawberry/http/__init__.py,line=30,col=17,endLine=30,endColumn=28,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `data`
+ ::error file=strawberry/schema/schema.py,line=541,col=12,endLine=541,endColumn=25,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `errors`
+ ::error file=strawberry/schema/schema.py,line=542,col=44,endLine=542,endColumn=57,title=Pyrefly bad-assignment::`list[GraphQLError] | object | Unknown` is not assignable to attribute `pre_execution_errors` with type `list[GraphQLError] | None`
+ ::error file=strawberry/schema/schema.py,line=544,col=38,endLine=544,endColumn=51,title=Pyrefly bad-argument-type::Argument `list[GraphQLError] | object | Unknown` is not assignable to parameter `errors` with type `list[GraphQLError]` in function `strawberry.schema.base.BaseSchema._process_errors`
+ ::error file=strawberry/schema/schema.py,line=546,col=63,endLine=546,endColumn=76,title=Pyrefly bad-argument-type::Argument `list[GraphQLError] | object | Unknown | None` is not assignable to parameter `errors` with type `list[GraphQLError] | None` in function `strawberry.types.execution.ExecutionResult.__init__`
+ ::error file=strawberry/schema/schema.py,line=547,col=9,endLine=547,endColumn=26,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `extensions`
+ ::error file=strawberry/schema/schema.py,line=549,col=16,endLine=549,endColumn=22,title=Pyrefly bad-return::Returned type `ExecutionResult | NoneType | Unknown` is not assignable to declared return type `ExecutionResult`
pandera (https://github.com/pandera-dev/pandera)
+ ERROR pandera/backends/ibis/base.py:168:17-36: Object of class `NoneType` has no attribute `rename` [missing-attribute]
+ ERROR pandera/backends/polars/base.py:57:34-50: Object of class `object` has no attribute `sample` [missing-attribute]
+ ERROR pandera/backends/polars/base.py:178:24-49: Object of class `NoneType` has no attribute `columns` [missing-attribute]
+ ERROR pandera/backends/polars/base.py:181:40-70: Object of class `NoneType` has no attribute `with_columns` [missing-attribute]
+ ERROR pandera/backends/polars/base.py:183:29-51: Object of class `NoneType` has no attribute `rows` [missing-attribute]
+ ERROR pandera/backends/polars/base.py:187:40-64: Object of class `NoneType` has no attribute `rename` [missing-attribute]
+ ERROR pandera/backends/polars/base.py:188:26-54: Cannot index into `object` [bad-index]
+ ::error file=pandera/backends/ibis/base.py,line=168,col=17,endLine=168,endColumn=36,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `rename`
+ ::error file=pandera/backends/polars/base.py,line=57,col=34,endLine=57,endColumn=50,title=Pyrefly missing-attribute::Object of class `object` has no attribute `sample`
+ ::error file=pandera/backends/polars/base.py,line=178,col=24,endLine=178,endColumn=49,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `columns`
+ ::error file=pandera/backends/polars/base.py,line=181,col=40,endLine=181,endColumn=70,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `with_columns`
+ ::error file=pandera/backends/polars/base.py,line=183,col=29,endLine=183,endColumn=51,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `rows`
+ ::error file=pandera/backends/polars/base.py,line=187,col=40,endLine=187,endColumn=64,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `rename`
+ ::error file=pandera/backends/polars/base.py,line=188,col=26,endLine=188,endColumn=54,title=Pyrefly bad-index::Cannot index into `object`%0A Object of class `object` has no attribute `__getitem__`
websockets (https://github.com/aaugustin/websockets)
+ ERROR src/websockets/sync/connection.py:497:34-42: Runtime checkable protocol `Iterable` has an unsafe overlap with type `Iterable[DataLike] | bytearray | bytes | memoryview[int]` [unsafe-overlap]
+ ::error file=src/websockets/sync/connection.py,line=497,col=34,endLine=497,endColumn=42,title=Pyrefly unsafe-overlap::Runtime checkable protocol `Iterable` has an unsafe overlap with type `Iterable[DataLike] | bytearray | bytes | memoryview[int]`%0A Attribute `__iter__` has incompatible types: `Iterable[DataLike] | bytearray | bytes | memoryview[int].__iter__` has type `BoundMethod[bytearray, (self: bytearray) -> Iterator[int]]`, which is not assignable to `BoundMethod[Iterable[DataLike] | bytearray | bytes | memoryview[int], (self: Iterable[DataLike] | bytearray | bytes | memoryview[int]) -> Iterator[@_]]`, the type of `Iterable.__iter__`
pylint (https://github.com/pycqa/pylint)
+ ERROR pylint/checkers/async_checker.py:74:25-41: Object of class `NoneType` has no attribute `getattr` [missing-attribute]
+ ERROR pylint/checkers/async_checker.py:75:25-41: Object of class `NoneType` has no attribute `getattr` [missing-attribute]
+ ERROR pylint/checkers/typecheck.py:1940:25-41: Object of class `NoneType` has no attribute `getattr` [missing-attribute]
+ ERROR pylint/checkers/typecheck.py:1941:25-41: Object of class `NoneType` has no attribute `getattr` [missing-attribute]
+ ERROR pylint/checkers/typecheck.py:1957:69-82: Object of class `NoneType` has no attribute `name` [missing-attribute]
+ ::error file=pylint/checkers/async_checker.py,line=74,col=25,endLine=74,endColumn=41,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `getattr`
+ ::error file=pylint/checkers/async_checker.py,line=75,col=25,endLine=75,endColumn=41,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `getattr`
+ ::error file=pylint/checkers/typecheck.py,line=1940,col=25,endLine=1940,endColumn=41,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `getattr`
+ ::error file=pylint/checkers/typecheck.py,line=1941,col=25,endLine=1941,endColumn=41,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `getattr`
+ ::error file=pylint/checkers/typecheck.py,line=1957,col=69,endLine=1957,endColumn=82,title=Pyrefly missing-attribute::Object of class `NoneType` has no attribute `name`
stone (https://github.com/dropbox/stone)
- ERROR stone/backends/python_rsrc/stone_base.py:105:75-90: Expected class object, got `Never` [invalid-argument]
- ERROR stone/backends/python_rsrc/stone_base.py:155:69-83: Expected class object, got `Never` [invalid-argument]
- ::error file=stone/backends/python_rsrc/stone_base.py,line=105,col=75,endLine=105,endColumn=90,title=Pyrefly invalid-argument::Expected class object, got `Never`
- ::error file=stone/backends/python_rsrc/stone_base.py,line=155,col=69,endLine=155,endColumn=83,title=Pyrefly invalid-argument::Expected class object, got `Never`
core (https://github.com/home-assistant/core)
+ ERROR homeassistant/components/deconz/entity.py:141:39-61: Object of class `object` has no attribute `reachable` [missing-attribute]
+ ERROR homeassistant/components/litterrobot/entity.py:57:61-69: Object of class `object` has no attribute `id` [missing-attribute]
+ ERROR homeassistant/components/shelly/coordinator.py:246:62-82: Object of class `object` has no attribute `settings` [missing-attribute]
- ERROR homeassistant/components/ssdp/scanner.py:538:23-37: Cannot set item in `dict[Never, Never]` [unsupported-operation]
- ERROR homeassistant/components/ssdp/scanner.py:538:41-44: Cannot set item in `dict[Never, Never]` [unsupported-operation]
- ERROR homeassistant/components/ssdp/scanner.py:549:14-23: Argument `dict[Unknown, Unknown] | dict[Never, Never]` is not assignable to parameter `upnp` with type `Mapping[str, Any]` in function `homeassistant.helpers.service_info.ssdp.SsdpServiceInfo.__init__` [bad-argument-type]
+ ::error file=homeassistant/components/deconz/entity.py,line=141,col=39,endLine=141,endColumn=61,title=Pyrefly missing-attribute::Object of class `object` has no attribute `reachable`
+ ::error file=homeassistant/components/litterrobot/entity.py,line=57,col=61,endLine=57,endColumn=69,title=Pyrefly missing-attribute::Object of class `object` has no attribute `id`
+ ::error file=homeassistant/components/shelly/coordinator.py,line=246,col=62,endLine=246,endColumn=82,title=Pyrefly missing-attribute::Object of class `object` has no attribute `settings`
- ::error file=homeassistant/components/ssdp/scanner.py,line=538,col=23,endLine=538,endColumn=37,title=Pyrefly unsupported-operation::Cannot set item in `dict[Never, Never]`%0A Argument `Literal['UDN']` is not assignable to parameter `key` with type `Never` in function `dict.__setitem__`
- ::error file=homeassistant/components/ssdp/scanner.py,line=538,col=41,endLine=538,endColumn=44,title=Pyrefly unsupported-operation::Cannot set item in `dict[Never, Never]`%0A Argument `str` is not assignable to parameter `value` with type `Never` in function `dict.__setitem__`
- ::error file=homeassistant/components/ssdp/scanner.py,line=549,col=14,endLine=549,endColumn=23,title=Pyrefly bad-argument-type::Argument `dict[Unknown, Unknown] | dict[Never, Never]` is not assignable to parameter `upnp` with type `Mapping[str, Any]` in function `homeassistant.helpers.service_info.ssdp.SsdpServiceInfo.__init__`
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+302
to
+307
| let allow_unwrap = matches!(right, Type::ClassDef(_)) | ||
| || (allow_type_form_classinfo | ||
| && matches!(&right, Type::Type(box Type::ClassType(_)))); | ||
| if allow_unwrap | ||
| && let Some((tparams, right)) = self.unwrap_class_object_silently(&right) | ||
| { |
There was a problem hiding this comment.
matches!(right, Type::ClassDef(_)) pattern-matches by value and will move right, but right is then borrowed again in unwrap_class_object_silently(&right), which will not compile. Use a borrowed match (e.g., match on &right) so right isn't moved when computing allow_unwrap.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #713
Adjusted isinstance negative narrowing so we only subtract when the classinfo is a concrete class object (a class def or alias to one), avoiding the unsound narrowing for type[T].
Adjusted negative isinstance narrowing so we only treat type[...] classinfo as concrete when the classinfo expression is a precise literal (class defs combined with | or tuple literals).
Test Plan
Added a regression test that mirrors the issue case to ensure the else branch stays at int | str.