From c964b2f7e746bfc4799eed8c29eb78c9dc39f24b Mon Sep 17 00:00:00 2001 From: Ching-Wei Kang Date: Fri, 11 Sep 2026 15:25:03 -0500 Subject: [PATCH 1/3] test: cover method assignment diagnostics Signed-off-by: Ching-Wei Kang --- test/args.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/args.md b/test/args.md index 244c1dcb95..a6f2630170 100644 --- a/test/args.md +++ b/test/args.md @@ -115,6 +115,15 @@ $ echo -e '[errors]\nimplicit-any = "ignore"' > $TMPDIR/pyrefly.toml && \ [0] ``` +## Strict mode rejects assigning to methods + +```scrut {output_stream: stdout} +$ echo -e 'class A:\n def f(self) -> None: ...\n\ndef replacement(self: A) -> None: ...\n\nA.f = replacement' > $TMPDIR/method_assign.py && \ +> $PYREFLY check $TMPDIR/method_assign.py --preset strict --output-format=min-text --summary=none +ERROR * Cannot assign to method `f` [method-assign] (glob) +[1] +``` + ## Scalar fields in the config file override `--preset` ```scrut {output_stream: stdout} From 2e7e1fe4b1e43ad6a6a12abf8520fe2557acbc15 Mon Sep 17 00:00:00 2001 From: Ching-Wei Kang Date: Fri, 11 Sep 2026 15:34:54 -0500 Subject: [PATCH 2/3] feat: diagnose assignments to methods Signed-off-by: Ching-Wei Kang --- crates/pyrefly_config/src/base.rs | 1 + crates/pyrefly_config/src/config.rs | 1 + crates/pyrefly_config/src/error_kind.rs | 3 + .../src/migration/mypy/pyproject.rs | 21 ++++++- .../pyrefly_config/src/migration/mypy/util.rs | 4 ++ pyrefly/lib/alt/attr.rs | 55 +++++++++++++++++++ pyrefly/lib/alt/class/class_field.rs | 7 +++ pyrefly/lib/test/attributes.rs | 39 ++++++++++++- pyrefly/lib/test/util.rs | 10 ++++ scripts/error_presets.json | 1 + website/docs/configuration.mdx | 2 +- website/docs/error-kinds.mdx | 17 ++++++ website/docs/migrate/mypy/error-codes.mdx | 2 +- website/docs/migrate/mypy/strict-mode.mdx | 2 + website/docs/migrate/pyright/strict-mode.mdx | 1 + 15 files changed, 161 insertions(+), 5 deletions(-) diff --git a/crates/pyrefly_config/src/base.rs b/crates/pyrefly_config/src/base.rs index 345af53372..df5c07f3fe 100644 --- a/crates/pyrefly_config/src/base.rs +++ b/crates/pyrefly_config/src/base.rs @@ -197,6 +197,7 @@ impl Preset { let errors = HashMap::from([ (ErrorKind::DirectAbstractBaseInstantiation, Severity::Error), (ErrorKind::ImplicitAny, Severity::Error), + (ErrorKind::MethodAssign, Severity::Error), (ErrorKind::MissingOverrideDecorator, Severity::Error), (ErrorKind::OpenUnpacking, Severity::Error), (ErrorKind::PotentialBadKeywordArgument, Severity::Error), diff --git a/crates/pyrefly_config/src/config.rs b/crates/pyrefly_config/src/config.rs index 30e2abc8fe..d727d54154 100644 --- a/crates/pyrefly_config/src/config.rs +++ b/crates/pyrefly_config/src/config.rs @@ -3394,6 +3394,7 @@ output-format = "omit-errors" errors.severity(ErrorKind::MissingOverrideDecorator), Severity::Error ); + assert_eq!(errors.severity(ErrorKind::MethodAssign), Severity::Error); assert_eq!(errors.severity(ErrorKind::OpenUnpacking), Severity::Error); // Pyrefly infers concrete return types in most cases, so we don't // ask users for an explicit annotation in strict mode. diff --git a/crates/pyrefly_config/src/error_kind.rs b/crates/pyrefly_config/src/error_kind.rs index 719edff13a..fd6a0b6520 100644 --- a/crates/pyrefly_config/src/error_kind.rs +++ b/crates/pyrefly_config/src/error_kind.rs @@ -284,6 +284,8 @@ pub enum ErrorKind { /// Attempting to use `yield` in a way that is not allowed. /// e.g. `yield from` with something that's not an iterable. InvalidYield, + /// Assigning to a method on a class object or instance. + MethodAssign, /// A file-level `# pyrefly: ignore-errors` (or `ignore-errors[code]`) directive /// appears after the first line of code, where it is silently inert. File-level /// suppressions are only honored in the preamble, at the top of the file. @@ -573,6 +575,7 @@ impl ErrorKind { ErrorKind::InvalidAbstractMethod => Severity::Ignore, ErrorKind::InvalidCast => Severity::Ignore, ErrorKind::InvalidDecorator => Severity::Warn, + ErrorKind::MethodAssign => Severity::Ignore, ErrorKind::MisplacedIgnore => Severity::Warn, ErrorKind::MissingAttributePatchTarget => Severity::Warn, ErrorKind::MissingOverrideDecorator => Severity::Ignore, diff --git a/crates/pyrefly_config/src/migration/mypy/pyproject.rs b/crates/pyrefly_config/src/migration/mypy/pyproject.rs index 80d32b6ed0..3f05fd2464 100644 --- a/crates/pyrefly_config/src/migration/mypy/pyproject.rs +++ b/crates/pyrefly_config/src/migration/mypy/pyproject.rs @@ -482,8 +482,27 @@ allow_redefinition = true #[test] fn test_strict_checks_unannotated_defs() -> anyhow::Result<()> { - let cfg = parse_pyproject_config("[tool.mypy]\nstrict = true\n")?; + let mut cfg = parse_pyproject_config("[tool.mypy]\nstrict = true\n")?; assert_eq!(cfg.root.check_unannotated_defs, Some(true)); + cfg.configure(); + assert_eq!( + cfg.errors(Path::new(".")).severity(ErrorKind::MethodAssign), + Severity::Error + ); + Ok(()) + } + + #[test] + fn test_enable_method_assign_error() -> anyhow::Result<()> { + let src = r#"[tool.mypy] +enable_error_code = ["method-assign"] +"#; + let mut cfg = parse_pyproject_config(src)?; + cfg.configure(); + assert_eq!( + cfg.errors(Path::new(".")).severity(ErrorKind::MethodAssign), + Severity::Error + ); Ok(()) } diff --git a/crates/pyrefly_config/src/migration/mypy/util.rs b/crates/pyrefly_config/src/migration/mypy/util.rs index ee078677ea..a6a3099bdd 100644 --- a/crates/pyrefly_config/src/migration/mypy/util.rs +++ b/crates/pyrefly_config/src/migration/mypy/util.rs @@ -171,6 +171,9 @@ pub fn make_error_config( if disallow_any_generics || strict { errors.insert(ErrorKind::ImplicitAny, Severity::Error); } + if strict { + errors.insert(ErrorKind::MethodAssign, Severity::Error); + } if disallow_any_explicit { errors.insert(ErrorKind::ExplicitAny, Severity::Error); } @@ -210,6 +213,7 @@ fn code_to_kind(errors: HashMap) -> HashMap add(severity, ErrorKind::MissingAttribute), "arg-type" => add(severity, ErrorKind::BadArgumentType), "assignment" => add(severity, ErrorKind::BadAssignment), + "method-assign" => add(severity, ErrorKind::MethodAssign), "call-arg" => add(severity, ErrorKind::BadArgumentCount), "call-overload" => add(severity, ErrorKind::NoMatchingOverload), "index" => { diff --git a/pyrefly/lib/alt/attr.rs b/pyrefly/lib/alt/attr.rs index d0772b38fe..2b512ef89c 100644 --- a/pyrefly/lib/alt/attr.rs +++ b/pyrefly/lib/alt/attr.rs @@ -1043,6 +1043,18 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { }; let (lookup_found, lookup_not_found, lookup_error) = self.lookup_attr(attr_base.clone(), attr_name).decompose(); + if lookup_found + .iter() + .any(|(_, found_on)| self.attribute_base_has_method(found_on, attr_name)) + { + self.error_with_context( + errors, + range, + ErrorKind::MethodAssign, + format!("Cannot assign to method `{attr_name}`"), + context, + ); + } for e in lookup_error { e.add_to(errors, range, attr_name, todo_ctx); should_narrow = false; @@ -1151,6 +1163,49 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } } + fn attribute_base_has_method(&self, base: &AttributeBase1, attr_name: &Name) -> bool { + let class_has_method = |class: &Class| { + self.get_class_member(class, attr_name) + .is_some_and(|field| field.is_method()) + }; + match base { + AttributeBase1::ClassInstance(class) + | AttributeBase1::SelfType(class) + | AttributeBase1::Quantified(_, class) => class_has_method(class.class_object()), + AttributeBase1::ClassObject(class) | AttributeBase1::GenericAlias(class) => { + class_has_method(class.class_object()) + } + AttributeBase1::EnumLiteral(literal) => class_has_method(literal.class.class_object()), + AttributeBase1::LiteralString => class_has_method(self.stdlib.str().class_object()), + AttributeBase1::QuantifiedValue(quantified) => { + class_has_method(quantified.class_type(self.stdlib).class_object()) + } + AttributeBase1::SuperInstance(_, obj) => match obj { + SuperObj::Instance(class) | SuperObj::Class(class) => { + class_has_method(class.class_object()) + } + }, + AttributeBase1::ShapedArrayInstance(tensor) => { + class_has_method(tensor.base_class.class_object()) + } + AttributeBase1::ProtocolSubset(inner) => { + self.attribute_base_has_method(inner, attr_name) + } + AttributeBase1::Intersect(options, fallback) => options + .iter() + .chain(fallback) + .any(|base| self.attribute_base_has_method(base, attr_name)), + AttributeBase1::Any(_) + | AttributeBase1::Never + | AttributeBase1::TypeAny(_) + | AttributeBase1::TypeNever + | AttributeBase1::Module(_) + | AttributeBase1::Property(_) + | AttributeBase1::BoundMethod(_) + | AttributeBase1::TypedDict(_) => false, + } + } + pub fn check_set_read_write_and_infer_narrow( &self, attr_ty: Type, diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index b7156ca951..835ba14ab7 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -1052,6 +1052,13 @@ impl ClassField { } } + pub(crate) fn is_method(&self) -> bool { + matches!( + &self.0, + ClassFieldInner::Method { .. } | ClassFieldInner::ProxyMethod { .. } + ) + } + pub fn is_final(&self) -> bool { match &self.0 { ClassFieldInner::Property { ty, .. } => ty.has_final_decoration(), diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index 0e32b0c722..f814c8bfdd 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -262,8 +262,7 @@ Child.shared ); testcase!( - bug = "Example of how making methods read-write but not invariant is unsound", - test_method_assign, + test_method_assign_disabled_by_default, r#" from typing import Protocol class X(Protocol): @@ -280,6 +279,42 @@ y.foo() # result is "hi" "#, ); +testcase!( + test_method_assign, + TestEnv::new().enable_method_assign_error(), + r#" +from collections.abc import Callable + +class A: + def method(self) -> None: ... + + @staticmethod + def static_method() -> None: ... + + @classmethod + def class_method(cls) -> None: ... + + callback: Callable[[], None] = lambda: None + +class B(A): + pass + +def replacement(self: A) -> None: ... + +a = A() +A.method = replacement # E: Cannot assign to method `method` +a.method = lambda: None # E: Cannot assign to method `method` +A.static_method = lambda: None # E: Cannot assign to method `static_method` +a.static_method = lambda: None # E: Cannot assign to method `static_method` +A.class_method = lambda: None # E: Cannot assign to method `class_method` +a.class_method = lambda: None # E: Cannot assign to method `class_method` +B.method = replacement # E: Cannot assign to method `method` +B().method = lambda: None # E: Cannot assign to method `method` +A.callback = lambda: None +a.callback = lambda: None + "#, +); + testcase!( test_attribute_union, r#" diff --git a/pyrefly/lib/test/util.rs b/pyrefly/lib/test/util.rs index e7d729eb95..4666f21035 100644 --- a/pyrefly/lib/test/util.rs +++ b/pyrefly/lib/test/util.rs @@ -160,6 +160,7 @@ pub struct TestEnv { unknown_attribute_type_error: bool, implicit_abstract_class_error: bool, open_unpacking_error: bool, + method_assign_error: bool, missing_override_decorator_error: bool, missing_super_call_error: bool, not_required_key_access_error: bool, @@ -218,6 +219,7 @@ impl TestEnv { unknown_attribute_type_error: false, implicit_abstract_class_error: false, open_unpacking_error: false, + method_assign_error: false, missing_override_decorator_error: false, missing_super_call_error: false, not_required_key_access_error: false, @@ -395,6 +397,11 @@ impl TestEnv { self } + pub fn enable_method_assign_error(mut self) -> Self { + self.method_assign_error = true; + self + } + pub fn enable_missing_override_decorator_error(mut self) -> Self { self.missing_override_decorator_error = true; self @@ -665,6 +672,9 @@ impl TestEnv { if self.open_unpacking_error { errors.set_error_severity(ErrorKind::OpenUnpacking, Severity::Error); } + if self.method_assign_error { + errors.set_error_severity(ErrorKind::MethodAssign, Severity::Error); + } if self.missing_override_decorator_error { errors.set_error_severity(ErrorKind::MissingOverrideDecorator, Severity::Error); } diff --git a/scripts/error_presets.json b/scripts/error_presets.json index 7a8d0d25d4..f3ec713253 100644 --- a/scripts/error_presets.json +++ b/scripts/error_presets.json @@ -77,6 +77,7 @@ "invalid-type-var-tuple": ["legacy", "default", "strict", "all"], "invalid-variance": ["legacy", "default", "strict", "all"], "invalid-yield": ["legacy", "default", "strict", "all"], + "method-assign": ["strict", "all"], "misplaced-ignore": ["legacy", "default", "strict", "all"], "missing-argument": ["legacy", "default", "strict", "all"], "missing-attribute": ["legacy", "default", "strict", "all"], diff --git a/website/docs/configuration.mdx b/website/docs/configuration.mdx index 3d3fc76d09..42b75e3dd8 100644 --- a/website/docs/configuration.mdx +++ b/website/docs/configuration.mdx @@ -707,7 +707,7 @@ The default Pyrefly configuration. Equivalent to having no preset at all. Enables additional error codes on top of the default for stricter checking. - Sets [`strict-callable-subtyping`](#strict-callable-subtyping) `= true` -- Enables (as errors): [`direct-abstract-base-instantiation`](./error-kinds.mdx#direct-abstract-base-instantiation), [`implicit-any`](./error-kinds.mdx#implicit-any) (covers every implicit-`Any` sub-kind), [`missing-override-decorator`](./error-kinds.mdx#missing-override-decorator), [`open-unpacking`](./error-kinds.mdx#open-unpacking), [`potential-bad-keyword-argument`](./error-kinds.mdx#potential-bad-keyword-argument), [`unused-ignore`](./error-kinds.mdx#unused-ignore) +- Enables (as errors): [`direct-abstract-base-instantiation`](./error-kinds.mdx#direct-abstract-base-instantiation), [`implicit-any`](./error-kinds.mdx#implicit-any) (covers every implicit-`Any` sub-kind), [`method-assign`](./error-kinds.mdx#method-assign), [`missing-override-decorator`](./error-kinds.mdx#missing-override-decorator), [`open-unpacking`](./error-kinds.mdx#open-unpacking), [`potential-bad-keyword-argument`](./error-kinds.mdx#potential-bad-keyword-argument), [`unused-ignore`](./error-kinds.mdx#unused-ignore) #### Preset: `all` diff --git a/website/docs/error-kinds.mdx b/website/docs/error-kinds.mdx index 91cc775ae1..30b448a208 100644 --- a/website/docs/error-kinds.mdx +++ b/website/docs/error-kinds.mdx @@ -1244,6 +1244,23 @@ def bad_yield_from() -> Generator[int, None, None]: yield from 1 ``` +## method-assign + +Default severity: `ignore` + +Assigning to a method on a class object or instance is ambiguous because Python's +type system cannot distinguish the bound and unbound callable types that the +replacement will produce. This check is enabled by the `strict` preset. + +```python +class Service: + def run(self) -> None: ... + +def replacement(self: Service) -> None: ... + +Service.run = replacement # method-assign +``` + ## misplaced-ignore Default severity: `warn` diff --git a/website/docs/migrate/mypy/error-codes.mdx b/website/docs/migrate/mypy/error-codes.mdx index 5de834fd38..dedf8185e7 100644 --- a/website/docs/migrate/mypy/error-codes.mdx +++ b/website/docs/migrate/mypy/error-codes.mdx @@ -60,7 +60,7 @@ kind does not make every diagnostic identical to mypy's. | `return` | `bad-return` | Automatic | Broad | Mypy uses this for missing-return paths; Pyrefly groups incompatible and missing returns under `bad-return`. | | `return-value` | `bad-return` | Automatic | Direct-ish | | | `assignment` | `bad-assignment` | Automatic | Direct-ish | | -| `method-assign` | `bad-assignment` or `read-only` | Manual | Related | Depends on whether the failure is type incompatibility or attempted mutation. | +| `method-assign` | `method-assign` | Automatic | Direct | | | `type-arg` | `implicit-any-type-argument` | Automatic | Direct-ish | | | `type-var` | `bad-specialization` | Automatic | Partial | Pyrefly uses `invalid-type-var` for many TypeVar definition and usage errors. | | `union-attr` | `missing-attribute` | Automatic | Direct-ish | | diff --git a/website/docs/migrate/mypy/strict-mode.mdx b/website/docs/migrate/mypy/strict-mode.mdx index b87024fe5d..097eaf0407 100644 --- a/website/docs/migrate/mypy/strict-mode.mdx +++ b/website/docs/migrate/mypy/strict-mode.mdx @@ -53,6 +53,7 @@ The [`strict` preset](../../configuration.mdx#preset-strict) sets `strict-callable-subtyping = true` and enables [`implicit-any`](../../error-kinds.mdx#implicit-any) (which covers every implicit-`Any` sub-kind), +[`method-assign`](../../error-kinds.mdx#method-assign), [`missing-override-decorator`](../../error-kinds.mdx#missing-override-decorator), [`potential-bad-keyword-argument`](../../error-kinds.mdx#potential-bad-keyword-argument), [`unused-ignore`](../../error-kinds.mdx#unused-ignore), and @@ -66,6 +67,7 @@ implicit-`Any` sub-kind), | Redundant casts | `redundant-cast = "warn"` | Generated at warning severity | | Untyped or incompletely typed definitions | `implicit-any-parameter` + `unannotated-return` | Two Pyrefly policies stand in for several mypy flags | | Missing generic arguments | `implicit-any = "error"` | Broader than `implicit-any-type-argument` alone | +| Assigning to methods | `method-assign = "error"` | Direct mapping, and enabled by Pyrefly's strict preset | | Checking unannotated function bodies | Not expanded by the converter | Set `check-unannotated-defs = true` yourself | | Explicit `Any` | `explicit-any` | Only when `disallow_any_explicit` is set; mypy `strict` alone does not enable it | | Unused ignores | `unused-type-ignore` / `unused-ignore` | Not part of the mypy converter map; add by hand | diff --git a/website/docs/migrate/pyright/strict-mode.mdx b/website/docs/migrate/pyright/strict-mode.mdx index 8514b0dffe..6039ae583a 100644 --- a/website/docs/migrate/pyright/strict-mode.mdx +++ b/website/docs/migrate/pyright/strict-mode.mdx @@ -53,6 +53,7 @@ several linter-like checks. Pyrefly's - [`implicit-any`](../../error-kinds.mdx#implicit-any), which covers every implicit-`Any` sub-kind; +- [`method-assign`](../../error-kinds.mdx#method-assign); - [`missing-override-decorator`](../../error-kinds.mdx#missing-override-decorator); - [`potential-bad-keyword-argument`](../../error-kinds.mdx#potential-bad-keyword-argument); - [`unused-ignore`](../../error-kinds.mdx#unused-ignore); From fdddaaf63f4e2543e97b575663b608250e85782d Mon Sep 17 00:00:00 2001 From: Ching-Wei Kang <164879897+WilliamK112@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:50:13 -0500 Subject: [PATCH 3/3] Track resolved method members during attribute lookup --- .../src/migration/mypy/pyproject.rs | 8 +- pyrefly/lib/alt/attr.rs | 215 +++++++++--------- pyrefly/lib/alt/class/class_field.rs | 95 ++++++-- pyrefly/lib/alt/class/enums.rs | 26 ++- pyrefly/lib/test/attributes.rs | 41 ++++ 5 files changed, 262 insertions(+), 123 deletions(-) diff --git a/crates/pyrefly_config/src/migration/mypy/pyproject.rs b/crates/pyrefly_config/src/migration/mypy/pyproject.rs index 3f05fd2464..88d01def4d 100644 --- a/crates/pyrefly_config/src/migration/mypy/pyproject.rs +++ b/crates/pyrefly_config/src/migration/mypy/pyproject.rs @@ -482,8 +482,14 @@ allow_redefinition = true #[test] fn test_strict_checks_unannotated_defs() -> anyhow::Result<()> { - let mut cfg = parse_pyproject_config("[tool.mypy]\nstrict = true\n")?; + let cfg = parse_pyproject_config("[tool.mypy]\nstrict = true\n")?; assert_eq!(cfg.root.check_unannotated_defs, Some(true)); + Ok(()) + } + + #[test] + fn test_strict_enables_method_assign() -> anyhow::Result<()> { + let mut cfg = parse_pyproject_config("[tool.mypy]\nstrict = true\n")?; cfg.configure(); assert_eq!( cfg.errors(Path::new(".")).severity(ErrorKind::MethodAssign), diff --git a/pyrefly/lib/alt/attr.rs b/pyrefly/lib/alt/attr.rs index 2b512ef89c..e0a2042a46 100644 --- a/pyrefly/lib/alt/attr.rs +++ b/pyrefly/lib/alt/attr.rs @@ -71,8 +71,9 @@ use crate::types::types::Type; /// since each union member is looked up separately. #[derive(Debug)] struct LookupResult { - /// The lookup was successful and an attribute was found. - pub found: Vec<(Attribute, AttributeBase1)>, + /// The lookup was successful and an attribute was found. The boolean records + /// whether the resolved class member is a method. + pub found: Vec<(Attribute, AttributeBase1, bool)>, /// The attribute was not found. Callers can use fallback behavior, for /// example looking up a different attribute. pub not_found: Vec, @@ -372,12 +373,17 @@ impl LookupResult { } fn found(&mut self, attr: Attribute, on: AttributeBase1) { - self.found.push((attr, on)) + self.found.push((attr, on, false)) } - fn found_class_attribute(&mut self, class_attr: ClassAttribute, on: AttributeBase1) { + fn found_class_attribute( + &mut self, + class_attr: ClassAttribute, + on: AttributeBase1, + is_method: bool, + ) { self.found - .push((Attribute::class_attribute(class_attr), on)) + .push((Attribute::class_attribute(class_attr), on, is_method)) } fn not_found(&mut self, not_found: NotFoundOn) { @@ -395,7 +401,7 @@ impl LookupResult { fn decompose( self, ) -> ( - Vec<(Attribute, AttributeBase1)>, + Vec<(Attribute, AttributeBase1, bool)>, Vec, Vec, ) { @@ -621,7 +627,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { if class == self.stdlib.none_type().class_object() ) }); - for (attr, _) in found { + for (attr, _, _) in found { match self.resolve_get_access(attr_name, attr, range, errors, context) { Ok(ty) => types.push(ty), Err(err) => { @@ -814,7 +820,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } let suppress_errors = ErrorCollector::new(errors.module().clone(), ErrorStyle::Never); let mut types = vec![self.heap.mk_any_implicit()]; - for (found_attr, _) in lookup_result.found { + for (found_attr, _, _) in lookup_result.found { if let Ok(ty) = self.resolve_get_access(attr_name, found_attr, range, &suppress_errors, None) { @@ -860,7 +866,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } } }; - for (attr, _) in lookup_result.found { + for (attr, _, _) in lookup_result.found { attr_tys.push( self.resolve_get_access(attr_name, attr, range, errors, context) .unwrap_or_else(|e| { @@ -934,7 +940,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let (setattr_found, setattr_not_found, setattr_error) = self .lookup_magic_dunder_attr(attr_base, &dunder::SETATTR) .decompose(); - for (setattr_attr, _) in setattr_found { + for (setattr_attr, _, _) in setattr_found { let result = self .resolve_get_access(attr_name, setattr_attr, range, errors, context) .map(|setattr_ty| { @@ -983,7 +989,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let (delattr_found, delattr_not_found, delattr_error) = self .lookup_magic_dunder_attr(attr_base, &dunder::DELATTR) .decompose(); - for (delattr_attr, _) in delattr_found { + for (delattr_attr, _, _) in delattr_found { let result = self .resolve_get_access(attr_name, delattr_attr, range, errors, context) .map(|delattr_ty| { @@ -1043,10 +1049,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { }; let (lookup_found, lookup_not_found, lookup_error) = self.lookup_attr(attr_base.clone(), attr_name).decompose(); - if lookup_found - .iter() - .any(|(_, found_on)| self.attribute_base_has_method(found_on, attr_name)) - { + if lookup_found.iter().any(|(_, _, is_method)| *is_method) { self.error_with_context( errors, range, @@ -1077,7 +1080,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { ); should_narrow = false; } - for (attr, found_on) in lookup_found { + for (attr, found_on, _) in lookup_found { match attr { // Attribute setting bypasses `__getattr__` lookup and checks `__setattr__` // If the attribute is not found, we fall back to `__setattr__` @@ -1163,47 +1166,9 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } } - fn attribute_base_has_method(&self, base: &AttributeBase1, attr_name: &Name) -> bool { - let class_has_method = |class: &Class| { - self.get_class_member(class, attr_name) - .is_some_and(|field| field.is_method()) - }; - match base { - AttributeBase1::ClassInstance(class) - | AttributeBase1::SelfType(class) - | AttributeBase1::Quantified(_, class) => class_has_method(class.class_object()), - AttributeBase1::ClassObject(class) | AttributeBase1::GenericAlias(class) => { - class_has_method(class.class_object()) - } - AttributeBase1::EnumLiteral(literal) => class_has_method(literal.class.class_object()), - AttributeBase1::LiteralString => class_has_method(self.stdlib.str().class_object()), - AttributeBase1::QuantifiedValue(quantified) => { - class_has_method(quantified.class_type(self.stdlib).class_object()) - } - AttributeBase1::SuperInstance(_, obj) => match obj { - SuperObj::Instance(class) | SuperObj::Class(class) => { - class_has_method(class.class_object()) - } - }, - AttributeBase1::ShapedArrayInstance(tensor) => { - class_has_method(tensor.base_class.class_object()) - } - AttributeBase1::ProtocolSubset(inner) => { - self.attribute_base_has_method(inner, attr_name) - } - AttributeBase1::Intersect(options, fallback) => options - .iter() - .chain(fallback) - .any(|base| self.attribute_base_has_method(base, attr_name)), - AttributeBase1::Any(_) - | AttributeBase1::Never - | AttributeBase1::TypeAny(_) - | AttributeBase1::TypeNever - | AttributeBase1::Module(_) - | AttributeBase1::Property(_) - | AttributeBase1::BoundMethod(_) - | AttributeBase1::TypedDict(_) => false, - } + fn class_member_is_method(&self, class: &Class, name: &Name) -> bool { + self.get_class_member(class, name) + .is_some_and(|field| field.is_method()) } pub fn check_set_read_write_and_infer_narrow( @@ -1390,7 +1355,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { for error in lookup_error { error.add_to(errors, range, attr_name, todo_ctx); } - for (attr, _) in lookup_found { + for (attr, _, _) in lookup_found { match attr { // Attribute deletion bypasses `__getattr__` lookup and checks `__delattr__` // If the attribute is not found, we fall back to `__delattr__` @@ -1500,7 +1465,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { if (!got_attrs.is_empty()) && let Some(want) = self.get_protocol_attribute(protocol, got.clone(), attr_name) { - for (got_attr, _) in got_attrs.iter() { + for (got_attr, _, _) in got_attrs.iter() { // A `__getattr__` fallback on a class object is not evidence that the // missing member exists: `__getattr__` governs instance attribute // access, not attributes of the class object itself. Reject rather @@ -1688,9 +1653,9 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { /// Fold one attribute per base into a single attribute, or `None` when they cannot be combined. fn fold_attribute_candidates( &self, - candidates: &[Vec<(Attribute, AttributeBase1)>], - ) -> Option<(Attribute, AttributeBase1)> { - let [(_, found_on)] = candidates.first()?.as_slice() else { + candidates: &[Vec<(Attribute, AttributeBase1, bool)>], + ) -> Option<(Attribute, AttributeBase1, bool)> { + let [(_, found_on, _)] = candidates.first()?.as_slice() else { return None; }; let found_on = found_on.clone(); @@ -1698,10 +1663,12 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let mut types = Vec::with_capacity(candidates.len()); let mut read_only_reason = None; let mut is_class_attribute = false; + let mut is_method = false; for base_results in candidates { - let [(attribute, _)] = base_results.as_slice() else { + let [(attribute, _, candidate_is_method)] = base_results.as_slice() else { return None; }; + is_method |= candidate_is_method; let ty = match attribute { Attribute::Simple(ty) => ty, Attribute::ClassAttribute(ClassAttribute::ReadWrite(ty)) => { @@ -1730,7 +1697,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } else { Attribute::simple(combined) }; - Some((attribute, found_on)) + Some((attribute, found_on, is_method)) } /// Look up an attribute on a single `AttributeBase1` using only class field declarations. @@ -1751,7 +1718,11 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { acc.found_type(Lit::Str(e.member.as_str().into()).to_implicit_type(), base) } AttributeBase1::LiteralString => match self.get_literal_string_attribute(attr_name) { - Some(attr) => acc.found_class_attribute(attr, base), + Some(attr) => acc.found_class_attribute( + attr, + base, + self.class_member_is_method(self.stdlib.str().class_object(), attr_name), + ), None => acc.not_found(NotFoundOn::ClassInstance( self.stdlib.str().class_object().dupe(), base, @@ -1760,7 +1731,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { AttributeBase1::ShapedArrayInstance(tensor) => { if attr_name.as_str() == "shape" { if let Some(attr) = self.get_shaped_array_attribute(tensor, attr_name) { - acc.found_class_attribute(attr, base); + acc.found_class_attribute(attr, base, false); return; } let shape = if tensor.tuple_carrier_shape_arg_index().is_some() { @@ -1776,7 +1747,11 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { // handles Self-type substitution via InstanceKind::ShapedArray. let metadata = self.get_metadata_for_class(tensor.base_class.class_object()); match self.get_shaped_array_attribute(tensor, attr_name) { - Some(attr) => acc.found_class_attribute(attr, base), + Some(attr) => acc.found_class_attribute( + attr, + base.clone(), + self.class_member_is_method(tensor.base_class.class_object(), attr_name), + ), None if metadata.has_base_any() => acc.found_type(Type::any_implicit(), base), None => acc.not_found(NotFoundOn::ClassInstance( tensor.base_class.class_object().dupe(), @@ -1787,16 +1762,16 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { AttributeBase1::ClassInstance(class) => { // Special handling for nn.ModuleDict with TypedDict type argument if let Some(attr) = self.try_nn_module_dict_attr(class, attr_name) { - acc.found_class_attribute(attr, base); + acc.found_class_attribute(attr, base, false); return; } // Normal class instance attribute lookup let metadata = self.get_metadata_for_class(class.class_object()); let attr_lookup_result = - self.get_enum_or_instance_attribute(class, metadata, attr_name); + self.get_enum_or_instance_attribute_with_method(class, metadata, attr_name); match attr_lookup_result { - Some(attr) => acc.found_class_attribute(attr, base), + Some((attr, is_method)) => acc.found_class_attribute(attr, base, is_method), None if metadata.has_base_any() => { acc.found_type(self.heap.mk_any_implicit(), base) } @@ -1813,10 +1788,10 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } AttributeBase1::EnumLiteral(lit @ LitEnum { class, .. }) => { let metadata = self.get_metadata_for_class(class.class_object()); - let attr_lookup_result = - self.get_enum_literal_or_instance_attribute(lit, metadata, attr_name); + let attr_lookup_result = self + .get_enum_literal_or_instance_attribute_with_method(lit, metadata, attr_name); match attr_lookup_result { - Some(attr) => acc.found_class_attribute(attr, base), + Some((attr, is_method)) => acc.found_class_attribute(attr, base, is_method), None if metadata.has_base_any() => { acc.found_type(self.heap.mk_any_implicit(), base) } @@ -1826,10 +1801,11 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } } AttributeBase1::SuperInstance(cls, obj) => { - match self.get_super_attribute(cls, obj, attr_name) { - Some(attr) => acc.found_class_attribute( + match self.get_super_attribute_with_method(cls, obj, attr_name) { + Some((attr, is_method)) => acc.found_class_attribute( attr.read_only_equivalent(ReadOnlyReason::Super), base, + is_method, ), None if let SuperObj::Instance(cls) = obj && self.extends_any(cls.class_object()) => @@ -1840,6 +1816,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { ReadOnlyReason::Super, ), base, + false, ) } None if let SuperObj::Class(cls) = obj @@ -1851,6 +1828,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { ReadOnlyReason::Super, ), base, + false, ) } None => { @@ -1880,7 +1858,14 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { // When looking up a magic dunder method as part of checking a class object // against a protocol, we prefer methods on the metaclass over methods on the // class object. See test::enums::test_iterate for why we need to do this. - acc.found_class_attribute(attr, base) + let metaclass = self + .get_metadata_for_class(class.class_object()) + .metaclass(self.stdlib); + acc.found_class_attribute( + attr, + base, + self.class_member_is_method(metaclass.class_object(), attr_name), + ) } else if let AttributeBase1::ClassObject(class) = &**protocol_base && self .get_class_member(class.class_object(), attr_name) @@ -1922,33 +1907,40 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { quantified.clone(), class, attr_name, - ), - _ => self.get_class_attribute(class, attr_name), + ) + .map(|attr| { + ( + attr, + self.class_member_is_method(class.class_object(), attr_name), + ) + }), + _ => self.get_class_attribute_with_method(class, attr_name), }; match attr { - Some( + Some(( no_access @ ClassAttribute::NoAccess( NoAccessReason::ClassUseOfInstanceAttribute(_), ), - ) => { + is_method, + )) => { // Instance-only attributes from `__slots__` produce slot descriptors. // The order of precedence is: data descriptors > slot descriptors > non-descriptor attributes // All attributes on `type` like `__name__` are considered data descriptors, despite not being annotated as such. let metadata = self.get_metadata_for_class(class.class_object()); let metaclass = metadata.metaclass(self.stdlib); let metaclass_attr = - self.get_metaclass_attribute(class, metaclass, attr_name); + self.get_metaclass_attribute_with_method(class, metaclass, attr_name); match metaclass_attr { - Some(meta_attr) + Some((meta_attr, meta_is_method)) if self.class_attribute_is_data_descriptor(&meta_attr) || metaclass.class_object().is_builtin("type") => { - acc.found_class_attribute(meta_attr, base) + acc.found_class_attribute(meta_attr, base, meta_is_method) } - _ => acc.found_class_attribute(no_access, base), + _ => acc.found_class_attribute(no_access, base, is_method), } } - Some(attr) => { + Some((attr, is_method)) => { // When the class defines a @property, class-level access converts it // to ReadWrite (the raw getter). Check the metaclass for a property // with the same name — it takes precedence per the descriptor protocol. @@ -1959,25 +1951,26 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let metadata = self.get_metadata_for_class(class.class_object()); let metaclass = metadata.metaclass(self.stdlib); if !metaclass.class_object().is_builtin("type") { - let metaclass_attr = - self.get_metaclass_attribute(class, metaclass, attr_name); + let metaclass_attr = self.get_metaclass_attribute_with_method( + class, metaclass, attr_name, + ); match metaclass_attr { - Some(meta_attr) + Some((meta_attr, meta_is_method)) if self.class_attribute_is_data_descriptor(&meta_attr) || matches!( meta_attr, ClassAttribute::Property(_, _, _) ) => { - acc.found_class_attribute(meta_attr, base) + acc.found_class_attribute(meta_attr, base, meta_is_method) } - _ => acc.found_class_attribute(attr, base), + _ => acc.found_class_attribute(attr, base, is_method), } } else { - acc.found_class_attribute(attr, base) + acc.found_class_attribute(attr, base, is_method) } } else { - acc.found_class_attribute(attr, base) + acc.found_class_attribute(attr, base, is_method) } } None => { @@ -1995,13 +1988,15 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { acc, ); } else { - let instance_attr = self.get_metaclass_attribute( + let instance_attr = self.get_metaclass_attribute_with_method( class, metadata.metaclass(self.stdlib), attr_name, ); match instance_attr { - Some(attr) => acc.found_class_attribute(attr, base), + Some((attr, is_method)) => { + acc.found_class_attribute(attr, base, is_method) + } None if metadata.has_base_any() => { // We can't immediately fall back to Any in this case -- `type[Any]` is actually a special // AttributeBase which requires additional lookup on `type` itself before the Any fallback. @@ -2027,7 +2022,11 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { }, AttributeBase1::Quantified(q, bound) => { match self.get_bounded_quantified_attribute(q.clone(), bound, attr_name) { - Some(attr) => acc.found_class_attribute(attr, base), + Some(attr) => acc.found_class_attribute( + attr, + base.clone(), + self.class_member_is_method(bound.class_object(), attr_name), + ), None => { acc.not_found(NotFoundOn::ClassInstance(bound.class_object().dupe(), base)) } @@ -2068,8 +2067,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { acc.found_type(getter, base) } else { let class = self.stdlib.property(); - match self.get_instance_attribute(class, attr_name) { - Some(attr) => acc.found_class_attribute(attr, base), + match self.get_instance_attribute_with_method(class, attr_name) { + Some((attr, is_method)) => acc.found_class_attribute(attr, base, is_method), None => acc.not_found(NotFoundOn::ClassInstance( class.class_object().dupe(), base, @@ -2078,8 +2077,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } } AttributeBase1::TypedDict(typed_dict) => { - match self.get_typed_dict_attribute(typed_dict, attr_name) { - Some(attr) => acc.found_class_attribute(attr, base), + match self.get_typed_dict_attribute_with_method(typed_dict, attr_name) { + Some((attr, is_method)) => acc.found_class_attribute(attr, base, is_method), None => acc.not_found(NotFoundOn::ClassInstance( typed_dict.class_object().dupe(), base, @@ -2087,7 +2086,11 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } } AttributeBase1::SelfType(cls) => match self.get_self_attribute(cls, attr_name) { - Some(attr) => acc.found_class_attribute(attr, base), + Some(attr) => acc.found_class_attribute( + attr, + base.clone(), + self.class_member_is_method(cls.class_object(), attr_name), + ), None => { let metadata = self.get_metadata_for_class(cls.class_object()); if metadata.has_base_any() { @@ -2184,8 +2187,8 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { )); return; } - match self.get_metaclass_attribute(class, metaclass, dunder_name) { - Some(attr) => acc.found_class_attribute(attr, base), + match self.get_metaclass_attribute_with_method(class, metaclass, dunder_name) { + Some((attr, is_method)) => acc.found_class_attribute(attr, base, is_method), None => acc.not_found(NotFoundOn::ClassInstance( metaclass.class_object().clone(), base, @@ -2304,7 +2307,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { ) .decompose(); if getattribute_not_found.is_empty() && getattribute_internal_error.is_empty() { - for (attr, found_on) in getattribute_found { + for (attr, found_on, _) in getattribute_found { result.found( Attribute::getattr(not_found.clone(), attr, attr_name.clone()), found_on, @@ -2317,7 +2320,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { .lookup_magic_dunder_attr(not_found.attr_base1().to_attr_base(), &dunder::GETATTR) .decompose(); if getattr_not_found.is_empty() && getattr_internal_error.is_empty() { - for (attr, found_on) in getattr_found { + for (attr, found_on, _) in getattr_found { result.found( Attribute::getattr(not_found.clone(), attr, attr_name.clone()), found_on, @@ -3033,7 +3036,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { }; let (found, not_found, internal_errors) = lookup_result.decompose(); let mut results = Vec::new(); - for (attr, _) in found { + for (attr, _, _) in found { let found_ty = match self.resolve_get_access(attr_name, attr, range, errors, None) { Err(..) => fall_back_to_object(), Ok(ty) => ty, @@ -3289,7 +3292,7 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { let mut is_deprecated = false; let found_types: Vec<_> = found_attrs .into_iter() - .filter_map(|(attr, _)| { + .filter_map(|(attr, _, _)| { match &attr { Attribute::ClassAttribute(ClassAttribute::ReadWrite(ty)) | Attribute::ClassAttribute(ClassAttribute::ReadOnly(ty, _)) diff --git a/pyrefly/lib/alt/class/class_field.rs b/pyrefly/lib/alt/class/class_field.rs index 835ba14ab7..66c0571179 100644 --- a/pyrefly/lib/alt/class/class_field.rs +++ b/pyrefly/lib/alt/class/class_field.rs @@ -4762,8 +4762,22 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } pub fn get_instance_attribute(&self, cls: &ClassType, name: &Name) -> Option { + self.get_instance_attribute_with_method(cls, name) + .map(|(attr, _)| attr) + } + + pub fn get_instance_attribute_with_method( + &self, + cls: &ClassType, + name: &Name, + ) -> Option<(ClassAttribute, bool)> { self.get_class_member(cls.class_object(), name) - .map(|field| self.as_instance_attribute(name, field.as_ref(), &Instance::of_class(cls))) + .map(|field| { + let is_method = field.is_method(); + let attr = + self.as_instance_attribute(name, field.as_ref(), &Instance::of_class(cls)); + (attr, is_method) + }) } pub fn get_self_attribute(&self, cls: &ClassType, name: &Name) -> Option { @@ -4832,7 +4846,18 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { metaclass: &ClassType, name: &Name, ) -> Option { + self.get_metaclass_attribute_with_method(cls, metaclass, name) + .map(|(attr, _)| attr) + } + + pub fn get_metaclass_attribute_with_method( + &self, + cls: &ClassBase, + metaclass: &ClassType, + name: &Name, + ) -> Option<(ClassAttribute, bool)> { let attr = self.get_class_member(metaclass.class_object(), name)?; + let is_method = attr.is_method(); let attr = self.as_instance_attribute( name, attr.as_ref(), @@ -4843,11 +4868,12 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { .class_object() .has_toplevel_qname(ModuleName::builtins().as_str(), "type") { - return Some(ClassAttribute::read_write( - self.constructor_to_callable(cls.class_type()), + return Some(( + ClassAttribute::read_write(self.constructor_to_callable(cls.class_type())), + is_method, )); } - Some(attr) + Some((attr, is_method)) } /// Returns true if this attribute represents a data descriptor @@ -4906,6 +4932,15 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { td: &TypedDictInner, name: &Name, ) -> Option { + self.get_typed_dict_attribute_with_method(td, name) + .map(|(attr, _)| attr) + } + + pub fn get_typed_dict_attribute_with_method( + &self, + td: &TypedDictInner, + name: &Name, + ) -> Option<(ClassAttribute, bool)> { if let Some(meta) = self .get_metadata_for_class(td.class_object()) .typed_dict_metadata() @@ -4927,15 +4962,20 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { } }) .map(|member| { - self.as_instance_attribute( + let is_method = member.value.is_method(); + let attr = self.as_instance_attribute( name, member.value.as_ref(), &Instance::of_typed_dict(td), - ) + ); + (attr, is_method) }); } self.get_class_member(td.class_object(), name).map(|field| { - self.as_instance_attribute(name, field.as_ref(), &Instance::of_typed_dict(td)) + let is_method = field.is_method(); + let attr = + self.as_instance_attribute(name, field.as_ref(), &Instance::of_typed_dict(td)); + (attr, is_method) }) } @@ -4980,31 +5020,45 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { super_obj: &SuperObj, name: &Name, ) -> Option { + self.get_super_attribute_with_method(start_lookup_cls, super_obj, name) + .map(|(attr, _)| attr) + } + + pub fn get_super_attribute_with_method( + &self, + start_lookup_cls: &ClassType, + super_obj: &SuperObj, + name: &Name, + ) -> Option<(ClassAttribute, bool)> { match super_obj { SuperObj::Instance(obj) => self .get_super_class_member(obj.class_object(), Some(start_lookup_cls), name) .map(|member| { + let is_method = member.value.is_method(); if let Some(reason) = self.super_method_needs_impl_reason(&member) { - ClassAttribute::no_access(reason) + (ClassAttribute::no_access(reason), is_method) } else { - self.as_instance_attribute( + let attr = self.as_instance_attribute( name, member.value.as_ref(), &Instance::of_self_type(obj), - ) + ); + (attr, is_method) } }), SuperObj::Class(obj) => self .get_super_class_member(obj.class_object(), Some(start_lookup_cls), name) .map(|member| { + let is_method = member.value.is_method(); if let Some(reason) = self.super_method_needs_impl_reason(&member) { - ClassAttribute::no_access(reason) + (ClassAttribute::no_access(reason), is_method) } else { - self.as_class_attribute( + let attr = self.as_class_attribute( name, member.value.as_ref(), &ClassBase::SelfType(obj.clone()), - ) + ); + (attr, is_method) } }), } @@ -5044,8 +5098,21 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { /// Access is disallowed for instance-only attributes and for attributes whose /// type contains a class-scoped type parameter - e.g., `class A[T]: x: T`. pub fn get_class_attribute(&self, cls: &ClassBase, name: &Name) -> Option { + self.get_class_attribute_with_method(cls, name) + .map(|(attr, _)| attr) + } + + pub fn get_class_attribute_with_method( + &self, + cls: &ClassBase, + name: &Name, + ) -> Option<(ClassAttribute, bool)> { self.get_class_member(cls.class_object(), name) - .map(|field| self.as_class_attribute(name, field.as_ref(), cls)) + .map(|field| { + let is_method = field.is_method(); + let attr = self.as_class_attribute(name, field.as_ref(), cls); + (attr, is_method) + }) } pub fn get_bounded_quantified_class_attribute( diff --git a/pyrefly/lib/alt/class/enums.rs b/pyrefly/lib/alt/class/enums.rs index f220d34a1d..83e3ae0ac0 100644 --- a/pyrefly/lib/alt/class/enums.rs +++ b/pyrefly/lib/alt/class/enums.rs @@ -154,8 +154,19 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { metadata: &ClassMetadata, attr_name: &Name, ) -> Option { + self.get_enum_or_instance_attribute_with_method(class, metadata, attr_name) + .map(|(attr, _)| attr) + } + + pub fn get_enum_or_instance_attribute_with_method( + &self, + class: &ClassType, + metadata: &ClassMetadata, + attr_name: &Name, + ) -> Option<(ClassAttribute, bool)> { self.special_case_enum_attr_lookup(class, None, metadata, attr_name) - .or_else(|| self.get_instance_attribute(class, attr_name)) + .map(|attr| (attr, false)) + .or_else(|| self.get_instance_attribute_with_method(class, attr_name)) } /// Checks for a special-cased enum attribute on an enum literal, falling back to a regular instance attribute lookup. @@ -165,9 +176,20 @@ impl<'ctx, 'answer, Ans: LookupAnswer> AnswersSolver<'ctx, 'answer, Ans> { metadata: &ClassMetadata, attr_name: &Name, ) -> Option { + self.get_enum_literal_or_instance_attribute_with_method(lit, metadata, attr_name) + .map(|(attr, _)| attr) + } + + pub fn get_enum_literal_or_instance_attribute_with_method( + &self, + lit: &LitEnum, + metadata: &ClassMetadata, + attr_name: &Name, + ) -> Option<(ClassAttribute, bool)> { let class = &lit.class; self.special_case_enum_attr_lookup(class, Some(lit), metadata, attr_name) - .or_else(|| self.get_instance_attribute(class, attr_name)) + .map(|attr| (attr, false)) + .or_else(|| self.get_instance_attribute_with_method(class, attr_name)) } /// Special-case enum attribute lookups. Dispatches to the appropriate helper diff --git a/pyrefly/lib/test/attributes.rs b/pyrefly/lib/test/attributes.rs index f814c8bfdd..16f471c71e 100644 --- a/pyrefly/lib/test/attributes.rs +++ b/pyrefly/lib/test/attributes.rs @@ -315,6 +315,47 @@ a.callback = lambda: None "#, ); +testcase!( + test_method_assign_lookup_precedence, + TestEnv::new().enable_method_assign_error(), + r#" +from collections.abc import Callable +from typing import TypedDict + +class Meta(type): + def f(cls) -> None: ... + +class C(metaclass=Meta): + pass + +C.f = C.f # E: Cannot assign to method `f` + +class D(TypedDict): + items: int + +d: D = {"items": 1} +d.items = d.items # E: Cannot assign to method `items` + +class Base: + def f(self) -> None: ... + +class Child(Base): + f: Callable[[], None] = lambda: None + + def test(self) -> None: + super().f = lambda: None # E: Cannot assign to method `f` # E: Cannot set field `f` + +class Parent: + f: Callable[[], None] = lambda: None + +class Derived(Parent): + def f(self) -> None: ... + + def test(self) -> None: + super().f = lambda: None # E: Cannot set field `f` + "#, +); + testcase!( test_attribute_union, r#"