Report mutually-incompatible attribute overrides as bad-override

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

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

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)",

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,13 +6307,15 @@ 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 {
                     Err(Box::new(AttrSubsetError::Invariant {
                         got: got.clone(),
                         want: want.clone(),
                         subset_error,
+                        read_failed,
                     }))
                 } else {
                     Ok(())

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

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
 "#,

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.

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`
     "#,
 );
 

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`
 

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: