[ty] make Type::BoundMethod include instances of same-named methods bound to a subclass#24039
[ty] make Type::BoundMethod include instances of same-named methods bound to a subclass#24039oconnor663 merged 3 commits intomainfrom
Type::BoundMethod include instances of same-named methods bound to a subclass#24039Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 86.46%. The percentage of expected errors that received a diagnostic held steady at 80.68%. The number of fully passing files held steady at 67/132. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownprefect
|
|
I think this also fixes astral-sh/ty#770? |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-await |
0 | 40 | 0 |
invalid-argument-type |
24 | 0 | 1 |
invalid-assignment |
18 | 0 | 1 |
invalid-return-type |
2 | 1 | 5 |
no-matching-overload |
8 | 0 | 0 |
possibly-missing-attribute |
4 | 0 | 0 |
unresolved-attribute |
3 | 0 | 0 |
unsupported-operator |
2 | 0 | 0 |
invalid-yield |
0 | 0 | 1 |
not-iterable |
1 | 0 | 0 |
possibly-unresolved-reference |
1 | 0 | 0 |
too-many-positional-arguments |
1 | 0 | 0 |
| Total | 64 | 41 | 8 |
Changes in flaky projects detected. Raw diff output excludes flaky projects; see the HTML report for details.
Raw diff (57 changes)
altair (https://github.com/vega/altair)
+ tests/test_transformed_data.py:191:16 error[invalid-argument-type] Argument to function `len` is incorrect: Expected `Sized`, found `DataFrameLike`
+ tests/test_transformed_data.py:192:12 error[unsupported-operator] Operator `in` is not supported between objects of type `Literal["wheat_start"]` and `DataFrameLike`
+ tests/test_transformed_data.py:193:16 error[invalid-argument-type] Argument to function `len` is incorrect: Expected `Sized`, found `DataFrameLike`
+ tests/test_transformed_data.py:194:12 error[unsupported-operator] Operator `in` is not supported between objects of type `Literal["mean_wheat"]` and `DataFrameLike`
core (https://github.com/home-assistant/core)
+ homeassistant/auth/permissions/util.py:113:12 error[invalid-return-type] Return type does not match returned value: expected `bool`, found `ValueType`
- homeassistant/auth/permissions/merge.py:63:43 error[invalid-argument-type] Argument to function `_merge_policies` is incorrect: Expected `list[CategoryType]`, found `list[Never]`
+ homeassistant/auth/permissions/merge.py:63:43 error[invalid-argument-type] Argument to function `_merge_policies` is incorrect: Expected `list[CategoryType]`, found `list[SubCategoryType | None]`
- homeassistant/components/asuswrt/helpers.py:44:16 error[invalid-return-type] Return type does not match returned value: expected `T@translate_to_legacy`, found `dict[Unknown, Unknown]`
+ homeassistant/components/asuswrt/helpers.py:44:16 error[invalid-return-type] Return type does not match returned value: expected `T@translate_to_legacy`, found `dict[Unknown, object]`
+ homeassistant/components/asuswrt/helpers.py:44:17 error[no-matching-overload] No overload of bound method `get` matches arguments
dd-trace-py (https://github.com/DataDog/dd-trace-py)
+ ddtrace/appsec/_asm_request_context.py:241:32 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int`, found `Any | str | int | None`
+ ddtrace/appsec/_asm_request_context.py:241:32 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `int`, found `Any | str | int | None`
+ ddtrace/appsec/_asm_request_context.py:241:32 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `str`, found `Any | str | int | None`
+ ddtrace/appsec/_asm_request_context.py:241:32 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `str`, found `Any | str | int | None`
+ ddtrace/appsec/_asm_request_context.py:241:32 error[invalid-argument-type] Argument to bound method `__init__` is incorrect: Expected `str`, found `Any | str | int | None`
meson (https://github.com/mesonbuild/meson)
+ mesonbuild/modules/sourceset.py:243:29 error[no-matching-overload] No overload of bound method `get` matches arguments
+ mesonbuild/modules/sourceset.py:243:50 error[too-many-positional-arguments] Too many positional arguments to bound method `get`: expected 2, got 3
pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/arrays/datetimes.py:3036:21 error[invalid-assignment] Object of type `Timestamp` is not assignable to `_TimestampNoneT1@_maybe_normalize_endpoints`
+ pandas/core/arrays/datetimes.py:3039:19 error[invalid-assignment] Object of type `Timestamp` is not assignable to `_TimestampNoneT2@_maybe_normalize_endpoints`
+ pandas/core/dtypes/missing.py:253:18 error[invalid-assignment] Object of type `ExtensionArray | ndarray[tuple[Any, ...], dtype[Any]] | Any` is not assignable to `ndarray[tuple[Any, ...], dtype[numpy.bool[builtins.bool]]] | NDFrame`
+ pandas/core/indexes/base.py:4736:17 error[invalid-argument-type] Argument to function `restore_dropped_levels_multijoin` is incorrect: Expected `ndarray[tuple[Any, ...], dtype[signedinteger[_64Bit]]]`, found `ndarray[tuple[Any, ...], dtype[signedinteger[_64Bit]]] | None`
+ pandas/core/indexes/base.py:4737:17 error[invalid-argument-type] Argument to function `restore_dropped_levels_multijoin` is incorrect: Expected `ndarray[tuple[Any, ...], dtype[signedinteger[_64Bit]]]`, found `ndarray[tuple[Any, ...], dtype[signedinteger[_64Bit]]] | None`
+ pandas/core/indexes/base.py:5138:13 error[unresolved-attribute] Attribute `flags` is not defined on `ExtensionArray` in union `ExtensionArray | ndarray[tuple[Any, ...], dtype[Any]]`
+ pandas/core/internals/blocks.py:2369:9 error[unresolved-attribute] Attribute `flags` is not defined on `ExtensionArray` in union `ExtensionArray | ndarray[tuple[Any, ...], dtype[Any]]`
- pandas/core/resample.py:3191:12 error[invalid-return-type] Return type does not match returned value: expected `FreqIndexT@_asfreq_compat`, found `DatetimeIndex | TimedeltaIndex`
+ pandas/core/resample.py:3191:12 error[invalid-return-type] Return type does not match returned value: expected `FreqIndexT@_asfreq_compat`, found `PeriodIndex | DatetimeIndex | TimedeltaIndex`
schema_salad (https://github.com/common-workflow-language/schema_salad)
+ schema_salad/metaschema.py:1045:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `Never`, found `Literal["$namespaces"]`
+ schema_salad/metaschema.py:1045:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `int`, found `Literal["$namespaces"]`
+ schema_salad/metaschema.py:1047:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `Never`, found `Literal["$schemas"]`
+ schema_salad/metaschema.py:1047:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `int`, found `Literal["$schemas"]`
+ schema_salad/metaschema.py:1049:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `Never`, found `Literal["$base"]`
+ schema_salad/metaschema.py:1049:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `int`, found `Literal["$base"]`
+ schema_salad/python_codegen_support.py:1042:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `Never`, found `Literal["$namespaces"]`
+ schema_salad/python_codegen_support.py:1042:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `int`, found `Literal["$namespaces"]`
+ schema_salad/python_codegen_support.py:1044:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `Never`, found `Literal["$schemas"]`
+ schema_salad/python_codegen_support.py:1044:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `int`, found `Literal["$schemas"]`
+ schema_salad/python_codegen_support.py:1046:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `Never`, found `Literal["$base"]`
+ schema_salad/python_codegen_support.py:1046:22 error[invalid-argument-type] Argument to bound method `pop` is incorrect: Expected `int`, found `Literal["$base"]`
scikit-build-core (https://github.com/scikit-build/scikit-build-core)
+ src/scikit_build_core/metadata/__init__.py:108:51 error[unresolved-attribute] Object of type `object` has no attribute `items`
+ src/scikit_build_core/metadata/__init__.py:89:24 error[invalid-argument-type] Argument is incorrect: Expected `str`, found `object`
+ src/scikit_build_core/metadata/__init__.py:89:35 error[invalid-argument-type] Argument is incorrect: Expected `str`, found `object`
- src/scikit_build_core/metadata/__init__.py:107:16 error[invalid-return-type] Return type does not match returned value: expected `T@_process_dynamic_metadata`, found `dict[Unknown, dict[str, str]]`
+ src/scikit_build_core/metadata/__init__.py:107:16 error[invalid-return-type] Return type does not match returned value: expected `T@_process_dynamic_metadata`, found `dict[object, dict[str, str]]`
- src/scikit_build_core/metadata/__init__.py:117:16 error[invalid-return-type] Return type does not match returned value: expected `T@_process_dynamic_metadata`, found `dict[Unknown, list[str]]`
+ src/scikit_build_core/metadata/__init__.py:117:16 error[invalid-return-type] Return type does not match returned value: expected `T@_process_dynamic_metadata`, found `dict[object, list[str]]`
+ src/scikit_build_core/metadata/__init__.py:117:40 error[not-iterable] Object of type `object` is not iterable
scipy (https://github.com/scipy/scipy)
+ scipy/signal/_ltisys.py:3753:15 warning[possibly-unresolved-reference] Name `h` used when possibly not defined
werkzeug (https://github.com/pallets/werkzeug)
- src/werkzeug/test.py:167:20 error[invalid-yield] Send type `Never` does not match annotated send type `None`
+ src/werkzeug/test.py:167:20 error[invalid-yield] Yield type `tuple[object, object]` does not match annotated yield type `tuple[str, Any]`
xarray (https://github.com/pydata/xarray)
+ xarray/coding/cftime_offsets.py:1520:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1522:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1524:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1530:20 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1532:20 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1535:20 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1538:20 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1540:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1542:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1544:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1546:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/coding/cftime_offsets.py:1548:16 error[invalid-assignment] Object of type `str` is not assignable to `T_FreqStr@_legacy_to_new_freq`
+ xarray/computation/rolling.py:1216:20 error[invalid-return-type] Return type does not match returned value: expected `T_Xarray@Coarsen`, found `DataArray`
- xarray/core/indexes.py:1924:23 error[invalid-assignment] Object of type `PandasIndex` is not assignable to `T_PandasOrXarrayIndex@Indexes`
+ xarray/core/indexes.py:1924:23 error[invalid-assignment] Object of type `Index` is not assignable to `T_PandasOrXarrayIndex@Indexes`There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 060afcaa19
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.always() | ||
| } else { | ||
| // If the names match, then disjointness depends on whether the class types are | ||
| // disjoint. | ||
| self.check_type_pair(db, a.self_instance(db), b.self_instance(db)) |
There was a problem hiding this comment.
Don't treat every same-named bound method pair as overlapping
When two overlapping classes define methods with the same name but different implementations, this branch now keeps their bound-method types non-disjoint purely because the names match and the instance types overlap. That is too permissive: class A: foo(self, x: int) -> int, class B: foo(self, x: str) -> str, class D(A, B): pass, and class E(B, A): pass all inhabit Intersection[A, B], but D().foo(1) and E().foo(1) do not both succeed because MRO exposes only one implementation. Since BindingsElement::retain_successful in crates/ty_python_semantic/src/types/call/bind.rs drops failing intersection branches once any branch matches, x: Intersection[A, B]; x.foo(1) becomes accepted after this change even though it is invalid for some inhabitants.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think we should (though currently don't) reject both class D(A, B): pass and class E(B, A): pass as Liskov-violating, so I'm not sure this is an issue.
I think enforcing Liskov is the thing that makes it OK for this PR to not consider the actual signatures of the methods. That may be worth a comment here.
|
I think the ecosystem hits in class A:
def method(self) -> object:
return None
class B:
def method(self) -> list[object]:
return []
def test1(obj: A) -> None:
assert isinstance(obj, B)
# before: Never
# after: (bound method A.method() -> object) & (bound method B.method() -> list[object])
reveal_type(obj.method)
def test2(obj: Any | A) -> None:
assert isinstance(obj, B)
# before: Any & list[object]
# after: list[object]
reveal_type(obj.method()) |
carljm
left a comment
There was a problem hiding this comment.
Very nice! This does make sense, and I think is much less work than I was expecting.
One note: it doesn't fix the full problem in the linked issue. We also infer function literal types for accessing unbound methods off type[] (SubclassOf) types, and this has the same issue (not accounting for subclass overrides). Definitely lower priority than the problem you fixed here, though. That problem won't be fixable in the same way, because we can't redefine the meaning of a function literal type, I think we need to upcast to callable type.
| self.always() | ||
| } else { | ||
| // If the names match, then disjointness depends on whether the class types are | ||
| // disjoint. | ||
| self.check_type_pair(db, a.self_instance(db), b.self_instance(db)) |
There was a problem hiding this comment.
I think we should (though currently don't) reject both class D(A, B): pass and class E(B, A): pass as Liskov-violating, so I'm not sure this is an issue.
I think enforcing Liskov is the thing that makes it OK for this PR to not consider the actual signatures of the methods. That may be worth a comment here.
|
|
||
| // A `BoundMethod` type includes instances of the same method bound to a | ||
| // subtype/subclass of the self type. | ||
| (Type::BoundMethod(a), Type::BoundMethod(b)) => { |
There was a problem hiding this comment.
Do we need to also consider method finality here (not just class finality, which is implicitly considered by the self-type disjointness check)? A @final method cannot be overridden in a subclass.
There was a problem hiding this comment.
Playing with this a bit, there are some corner cases. Here's an example where a non-final method and a @final method are not disjoint, because the latter inherits from the former:
class A:
def f(self): ...
class B(A):
@final
def f(self): ...Multiple inheritance can also come into play here:
class A:
def f(self): ...
class B:
@final
def f(self): ...
class C(B, A): ...The @final method wins in the MRO in that case, so it presumably doesn't violate the rules for @final, but the opposite inheritance order presumably would violate those rules? On the other hand only Mypy currently gives a diagnostic for that.
That said, if both sides are @final (and they're not literally the same method), it does seem fair to conclude that they should be disjoint?
… bound to a subclass Fixes astral-sh/ty#2428.
060afca to
ce83cfc
Compare
* main: (40 commits) [ty] resolve union-likes in emitting union attribute errors (#24263) [ty] Improve support for `Callable` type context (#23888) [ty] Propagate type context through `await` expressions (#24256) [`pyflakes`] Flag annotated variable redeclarations as `F811` in preview mode (#24244) [ty] Preserve `Divergent` when materializing recursive aliases (#24245) Fix W391 fixes for consecutive empty notebook cells (#24236) [flake8-bugbear] Clarify RUF071 fix safety for non-path string comparisons (#24149) [ty] Ban type qualifiers in PEP-695 type aliases (#24242) [ty] Include keyword-prefixed symbols in completions for attributes (#24232) [ty] Add tests for TypedDict method overloads on unions (#24230) [ty] report unused bindings as unnecessary hint diagnostics (#23305) Remove unused `non_root` variable (#24238) Extend F507 to flag %-format strings with zero placeholders (#24215) [`flake8-simplify`] Suppress `SIM105` for `except*` before Python 3.12 (#23869) Ignore pre-initialization references in SIM113 (#24235) Parenthesize expression in RUF050 fix (#24234) Publish playgrounds using the `release-playground` environment (#24223) [ty] Fix instance-attribute lookup in methods of protocol classes (#24213) [ty] Used shared expression cache during generic call inference (#24219) [ty] make `Type::BoundMethod` include instances of same-named methods bound to a subclass (#24039) ...
Fixes astral-sh/ty#2428.
@carljm it sounded in the issue like you were picturing a new
Typevariant, but it seemed simpler to adjust howBoundMethodbehaves. In that case, the go-to-definition problem just doesn't come up? Please let me know if I'm going about this the wrong way or leaving out any important test cases.