diff --git a/conformance/third_party/conformance.exp b/conformance/third_party/conformance.exp index 9bc65076af..13fdfd4791 100644 --- a/conformance/third_party/conformance.exp +++ b/conformance/third_party/conformance.exp @@ -12975,9 +12975,9 @@ "code": -2, "column": 4, "concise_description": "Class member `Y1.x` overrides parent class `X1` in an inconsistent manner", - "description": "Class member `Y1.x` overrides parent class `X1` in an inconsistent manner\n `Y1.x` has type `int`, which is not consistent with `str` in `X1.x` (the type of read-write attributes cannot be changed)", + "description": "Class member `Y1.x` overrides parent class `X1` in an inconsistent manner\n `Y1.x` has type `int`, which is not assignable to `str`, the type of `X1.x`", "line": 55, - "name": "bad-override-mutable-attribute", + "name": "bad-override", "severity": "error", "stop_column": 5, "stop_line": 55 diff --git a/crates/pyrefly_config/src/migration/pyright.rs b/crates/pyrefly_config/src/migration/pyright.rs index 10f9eb46c5..fc38a8d378 100644 --- a/crates/pyrefly_config/src/migration/pyright.rs +++ b/crates/pyrefly_config/src/migration/pyright.rs @@ -558,6 +558,10 @@ impl RuleOverrides { self.report_implicit_override, ErrorKind::MissingOverrideDecorator, ); + // Narrowing unannotated overrides report + // `bad-override-mutable-attribute`. Widening and incompatible pairs + // report plain `bad-override`, but that kind also covers method + // overrides and parameter names, so this knob must not touch it. add( self.report_incompatible_unannotated_override, ErrorKind::BadOverrideMutableAttribute, @@ -859,6 +863,25 @@ include = ["basedpyright.py"] Ok(()) } + #[test] + fn test_incompatible_unannotated_override_maps_to_mutable_attribute_only() { + let overrides = RuleOverrides { + report_incompatible_unannotated_override: Some(Severity::Warn), + ..Default::default() + }; + let errors = overrides.to_config().expect("expected error config"); + assert_eq!( + errors.severity(ErrorKind::BadOverrideMutableAttribute), + Severity::Warn + ); + // Plain `bad-override` also covers method overrides and parameter + // names, so this knob must leave it at its default. + assert_eq!( + errors.severity(ErrorKind::BadOverride), + ErrorKind::BadOverride.default_severity() + ); + } + #[test] fn test_report_trailing_commas() -> anyhow::Result<()> { let raw_file = r#" diff --git a/pyrefly/lib/alt/attr.rs b/pyrefly/lib/alt/attr.rs index 03fce7a9f3..6982a2fe67 100644 --- a/pyrefly/lib/alt/attr.rs +++ b/pyrefly/lib/alt/attr.rs @@ -119,6 +119,10 @@ pub enum AttrSubsetError { got: Type, want: Type, subset_error: SubsetError, + // Whether the read direction (`got <: want`, any-branch for overloads) + // failed. When false, only the write direction failed, i.e. the + // override narrows and the failure is purely about mutability. + read_failed: bool, }, // `want` is not a subtype of `got` // applies to property setters @@ -205,6 +209,7 @@ impl AttrSubsetError { got, want, subset_error: _, + read_failed: _, } => { format!( "`{child_class}.{attr_name}` has type `{}`, which is not consistent with `{}` in `{parent_class}.{attr_name}` (the type of read-write attributes cannot be changed)", diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index 5d5a830c2a..c32ddb990d 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -4826,9 +4826,30 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } Err(error) => { let mut diff_lines = Vec::new(); - // Invariant = ReadWrite vs ReadWrite type mismatch. - // Covariant or contravariant failures between a ReadWrite attribute and a - // Property are mutable attribute override violations. + // An Invariant failure is only about mutability when the override + // narrows: the read direction holds and only the write direction + // fails. Widening and mutually incompatible pairs fail the read + // direction too (as recorded by the engine, using the same + // any-branch overload semantics as the original check), so + // report those as plain `bad-override` with a covariant message. + let error = match *error { + AttrSubsetError::Invariant { + got, + want, + subset_error, + read_failed: true, + } => Box::new(AttrSubsetError::Covariant { + got, + want, + got_is_property: false, + want_is_property: false, + subset_error, + }), + error => Box::new(error), + }; + // A surviving Invariant is a narrowing ReadWrite vs ReadWrite + // mismatch. Covariant or contravariant failures between a ReadWrite + // attribute and a Property are mutable attribute override violations. let is_mutable_attribute = matches!( &*error, AttrSubsetError::Invariant { .. } @@ -6286,6 +6307,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { .expect("`Overloaded` has at least one branch"), _ => is_subset(got, want), }; + let read_failed = assignable.is_err(); let subset_error = assignable .map_or_else(Some, |_| is_subset(want, got).map_or_else(Some, |_| None)); if let Some(subset_error) = subset_error { @@ -6293,6 +6315,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { got: got.clone(), want: want.clone(), subset_error, + read_failed, })) } else { Ok(()) diff --git a/pyrefly/lib/error/signature_diff.rs b/pyrefly/lib/error/signature_diff.rs index 694a72a5e3..03208adfd0 100644 --- a/pyrefly/lib/error/signature_diff.rs +++ b/pyrefly/lib/error/signature_diff.rs @@ -433,7 +433,7 @@ class B(A): ); assert_eq!(messages.len(), 1, "Expected one error, got {messages:?}"); let expected = r#"Class member `B.foo` overrides parent class `A` in an inconsistent manner - `B.foo` has type `(self: Unknown) -> None`, which is not consistent with `(x: int) -> int` in `A.foo` (the type of read-write attributes cannot be changed) + `B.foo` has type `(self: Unknown) -> None`, which is not assignable to `(x: int) -> int`, the type of `A.foo` Signature mismatch: expected: def foo(self: B, x: int) -> int: ... ^^^^^^^^^ ^^^ return type diff --git a/pyrefly/lib/test/attrs/define.rs b/pyrefly/lib/test/attrs/define.rs index 7a9337f013..453be9a6d6 100644 --- a/pyrefly/lib/test/attrs/define.rs +++ b/pyrefly/lib/test/attrs/define.rs @@ -690,7 +690,7 @@ class Base: @define class Sub(Base): z: bool - x: float # E: not consistent with `int` + x: float # E: `Sub.x` has type `float`, which is not assignable to `int`, the type of `Base.x` reveal_type(Sub.__init__) # E: revealed type: (self: Sub, y: str, z: bool, x: float) -> None "#, diff --git a/pyrefly/lib/test/class_overrides.rs b/pyrefly/lib/test/class_overrides.rs index 0529a7224c..2818b2dc29 100644 --- a/pyrefly/lib/test/class_overrides.rs +++ b/pyrefly/lib/test/class_overrides.rs @@ -101,7 +101,7 @@ class InferredClassVarChild(ParameterizedParent): def get_incompatible_value() -> Callable[[str], int]: ... class IncompatibleChild(ParameterizedParent): - x: ClassVar[Callable[[str], int]] = get_incompatible_value() # E: is not consistent with + x: ClassVar[Callable[[str], int]] = get_incompatible_value() # E: `IncompatibleChild.x` has type `(str) -> int`, which is not assignable to `(object) -> int`, the type of `ParameterizedParent.x` "#, ); @@ -1639,7 +1639,7 @@ testcase!( class A: x = 1 class B(A): - x = "oops" # E: `B.x` has type `str`, which is not consistent with `int` + x = "oops" # E: `B.x` has type `str`, which is not assignable to `int`, the type of `A.x` "#, ); @@ -2712,6 +2712,81 @@ class ChildNarrowedSuppressed(Base): "#, ); +// Overriding with a mutually incompatible type is not a mutability failure, +// so it reports plain bad-override. https://github.com/facebook/pyrefly/issues/4720 +testcase!( + test_override_incompatible_attribute_is_plain_bad_override, + r#" +class A: + x: float + y: str | None + +class C(A): + x: str # E: Class member `C.x` overrides parent class `A` in an inconsistent manner # !E: (the type of read-write attributes cannot be changed) + y: int # E: `C.y` has type `int`, which is not assignable to `str | None`, the type of `A.y` # !E: (the type of read-write attributes cannot be changed) + "#, +); + +// Widening a read-write attribute fails the read direction, so it is not a +// mutability-only failure and reports plain bad-override. +testcase!( + test_override_widened_attribute_is_plain_bad_override, + r#" +class A: + p: int + +class B(A): + p: int | str # E: `B.p` has type `int | str`, which is not assignable to `int`, the type of `A.p` # !E: (the type of read-write attributes cannot be changed) + "#, +); + +// A narrowing override of an overloaded attribute keeps the mutable-attribute +// kind: the read check accepts any matching branch, so the direction recorded +// by the engine (not a plain all-branches recheck) decides the classification. +testcase!( + test_override_narrowed_overloaded_attribute_keeps_mutable_kind, + r#" +from typing import Callable, overload + +def project[A, R](f: Callable[[A], R]) -> tuple[A, R]: ... + +@overload +def f(x: int) -> str: ... +@overload +def f(x: str) -> int: ... +def f(x) -> str | int: ... + +class A: + x = project(f) + +class B(A): + x: tuple[bool, str] = (True, "s") # E: (the type of read-write attributes cannot be changed) + +class C(A): + x: tuple[bool, str] = (True, "s") # pyrefly: ignore[bad-override-mutable-attribute] + "#, +); + +// Kind probes for #4720: the parent kind suppresses the reclassified cases, +// but the mutable-attribute sub-kind must not. +testcase!( + test_override_reclassified_attribute_suppression_probes, + r#" +class A: + x: float + w: int + +class CChildSuppressed(A): + x: str # pyrefly: ignore[bad-override] + +class CChildNotSuppressed(A): + x: str # pyrefly: ignore[bad-override-mutable-attribute] # E: Class member `CChildNotSuppressed.x` overrides parent class `A` in an inconsistent manner + +class WChildNotSuppressed(A): + w: int | str # pyrefly: ignore[bad-override-mutable-attribute] # E: Class member `WChildNotSuppressed.w` overrides parent class `A` in an inconsistent manner + "#, +); + // Overriding a readwrite property with a plain attribute that narrows the type // is a mutable attribute override error: the parent's setter accepts A, but the // child's ReadWrite(B) only accepts B. diff --git a/pyrefly/lib/test/typed_dict.rs b/pyrefly/lib/test/typed_dict.rs index 75cff21dfa..fd17b6ca79 100644 --- a/pyrefly/lib/test/typed_dict.rs +++ b/pyrefly/lib/test/typed_dict.rs @@ -1334,7 +1334,7 @@ from typing import TypedDict class A(TypedDict): x: int class B(A): - x: str # E: `B.x` has type `str`, which is not consistent with `int` in `A.x` + x: str # E: `B.x` has type `str`, which is not assignable to `int`, the type of `A.x` "#, ); diff --git a/website/docs/configuration.mdx b/website/docs/configuration.mdx index cacf50e4e2..f8b318102c 100644 --- a/website/docs/configuration.mdx +++ b/website/docs/configuration.mdx @@ -846,7 +846,7 @@ A looser, less-strict preset intended for codebases migrating from mypy. This is - Sets [`check-unannotated-defs`](#check-unannotated-defs) `= false`, [`infer-return-types`](#infer-return-types) `= "never"`, [`legacy-overload-expansion`](#legacy-overload-expansion) `= true`, [`replace-untyped-imports-with-any`](#replace-untyped-imports-with-any) `= ["*"]` - Sets [`type-ignore-unknown-tag-behavior`](#type-ignore-unknown-tag-behavior) `= "suppress"` so existing mypy-coded ignores retain their legacy blanket-suppression behavior. -- Disables: [`bad-override-mutable-attribute`](./error-kinds.mdx#bad-override-mutable-attribute), [`bad-override-param-name`](./error-kinds.mdx#bad-override-param-name), [`unbound-name`](./error-kinds.mdx#unbound-name) +- Disables: [`bad-override-mutable-attribute`](./error-kinds.mdx#bad-override-mutable-attribute), [`bad-override-param-name`](./error-kinds.mdx#bad-override-param-name), [`unbound-name`](./error-kinds.mdx#unbound-name). Note that `bad-override-mutable-attribute` covers only narrowing overrides; widening or incompatible attribute overrides report plain `bad-override`, which this preset leaves enabled. #### Preset: `default` diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 48cc306b71..5c40697736 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -376,12 +376,13 @@ That means everywhere that we call `Base.f` can safely call `Sub.f`. The opposite case, where `Base.f` takes `float` and `Sub.f` takes `int`, is an error because `Sub.f` cannot accept every `float` value. -Note that bad overrides caused by inconsistent parameter names are separately reported as [bad-override-param-name](#bad-override-param-name), and bad overrides caused by mutable attribute type changes are separately reported as [bad-override-mutable-attribute](#bad-override-mutable-attribute). +Note that bad overrides caused by inconsistent parameter names are separately reported as [bad-override-param-name](#bad-override-param-name), and bad overrides caused by mutable attribute type changes are separately reported as [bad-override-mutable-attribute](#bad-override-mutable-attribute). The mutable-attribute kind is reserved for overrides that fail only because of mutability (narrowing); an attribute override that widens the parent's type, or whose type is incompatible with the parent's in both directions, is reported as plain `bad-override`. ## bad-override-mutable-attribute -Arises when a subclass overrides a mutable (read-write) attribute of a parent class with an incompatible type. +Arises when a subclass overrides a mutable (read-write) attribute of a parent class with a type that differs only in a way mutability forbids (narrowing). Mutable attributes require invariant types — the child's type must be exactly compatible with the parent's — because the attribute can be both read and written through a reference to the parent class. +If the child's type widens the parent's, or is incompatible with it in both directions, the override is reported as [bad-override](#bad-override) instead, so that disabling this error kind cannot mask the worse incompatibility. ```python class Base: