fix Feature: support @field.default decorator for attrs #2012#2494
fix Feature: support @field.default decorator for attrs #2012#2494asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support in Pyrefly’s attrs handling for the @<field>.default decorator pattern so that defaults provided via attrs’ decorator API are reflected in synthesized __init__ signatures (fixes #2012).
Changes:
- Detect
@<field>.defaultdecorators in the binder and propagate the target field name through decorator metadata. - Apply the decorated method’s return type as the field default for attrs classes when no explicit default is present.
- Update attrs test coverage to expect no errors for the
@a.defaultdefault-factory pattern.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyrefly/lib/test/attrs/fields.rs | Updates attrs tests to validate @field.default behavior (no error + __init__ no longer requires the field arg). |
| pyrefly/lib/binding/expr.rs | Detects @<field>.default in class scope and attaches attrs_default_field metadata to decorator bindings. |
| pyrefly/lib/binding/binding.rs | Extends BindingDecorator to carry attrs_default_field through the binding layer. |
| pyrefly/lib/alt/types/decorated_function.rs | Extends decorator metadata/types to track attrs default-field linkage and introduces a SpecialDecorator case. |
| pyrefly/lib/alt/traits.rs | Initializes the new decorator metadata field when promoting recursive decorator answers. |
| pyrefly/lib/alt/special_calls.rs | Ensures synthesized Decorator instances default attrs_default_field to None. |
| pyrefly/lib/alt/solve.rs | Suppresses decorator-expression errors for recognized attrs default decorators and propagates metadata into Decorator. |
| pyrefly/lib/alt/function.rs | Treats attrs default decorators as a special decorator affecting function flags. |
| pyrefly/lib/alt/class/class_field.rs | Uses @<field>.default method return types as defaults for attrs fields during dataclass/attrs field processing. |
| crates/pyrefly_types/src/types.rs | Exposes Type::attrs_default_field() accessor to query function metadata. |
| crates/pyrefly_types/src/callable.rs | Adds attrs_default_field to FuncFlags to persist this metadata in callable/function types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn is_attrs_field_specifier(&self, expr: &Expr) -> bool { | ||
| let Expr::Call(call) = expr else { | ||
| return false; | ||
| }; | ||
| match &*call.func { | ||
| Expr::Name(name) => matches!(name.id.as_str(), "field" | "attrib" | "ib"), | ||
| Expr::Attribute(attr) => { | ||
| let attr_name = attr.attr.id.as_str(); | ||
| if !matches!(attr_name, "field" | "attrib" | "ib") { | ||
| return false; | ||
| } | ||
| if let Expr::Name(base) = &*attr.value { | ||
| matches!(base.id.as_str(), "attr" | "attrs") | ||
| } else { | ||
| false | ||
| } | ||
| } | ||
| _ => false, | ||
| } |
There was a problem hiding this comment.
is_attrs_field_specifier currently treats any call to a function named field/attrib/ib as an attrs field specifier, without verifying that the callee actually comes from the attr/attrs modules. This can incorrectly mark unrelated field() usages as attrs fields, which then (a) suppresses decorator type errors via error_swallower() and (b) sets attrs_default_field metadata unexpectedly. Consider checking the binding for the callee (e.g., FlowStyle::Import/ImportAs with ModuleName::attr()/ModuleName::attrs()) to avoid false positives, similar to how attrs classes are detected in class metadata.
| fn attrs_default_for_field(&self, cls: &Class, field_name: &Name) -> Option<Type> { | ||
| for name in cls.class_body_fields() { | ||
| if name == field_name { | ||
| continue; | ||
| } | ||
| let Some(field) = self.get_non_synthesized_field_from_current_class_only(cls, name) | ||
| else { | ||
| continue; | ||
| }; | ||
| if !matches!(&field.0, ClassFieldInner::Method { .. }) { | ||
| continue; | ||
| } | ||
| let ty = field.ty(); | ||
| if ty | ||
| .attrs_default_field() | ||
| .is_some_and(|default_name| default_name == field_name) | ||
| { | ||
| return ty | ||
| .callable_return_type(self.heap) | ||
| .or_else(|| Some(self.heap.mk_any_implicit())); | ||
| } | ||
| } |
There was a problem hiding this comment.
attrs_default_for_field linearly scans all class_body_fields() and runs several lookups to find a matching @<field>.default method. Since get_dataclass_member is called in loops over all fields (e.g. dataclass synthesis/validation), this introduces an O(n^2) walk for attrs classes with many fields. Consider precomputing a map of {field_name -> default_return_type} once per class (e.g. during class metadata/field synthesis) or otherwise caching the result to avoid repeated scans.
| @@ -17,11 +16,11 @@ from attrs import define, field | |||
| class C: | |||
| a: dict = field() | |||
| @a.default # E: Object of class `dict` has no attribute `default` | |||
| @a.default | |||
| def _default_a(self): | |||
| return {} | |||
| c = C() # E: Missing argument `a` in function `C.__init__` | |||
| c = C() | |||
| "#, | |||
There was a problem hiding this comment.
The new @field.default support is only covered by the from attrs import field form. Since the implementation also has branches for attr.ib/attr.attrib/attrs.field-style calls (and should avoid false positives when field() is not from attrs), it would be good to add tests for at least one import attr; x = attr.ib() case and one negative case where a user-defined field() is used and @x.default should still error.
|
Diff from mypy_primer, showing the effect of this PR on open source code: core (https://github.com/home-assistant/core)
- ERROR homeassistant/helpers/entity_registry.py:220:6-20: Object of class `str` has no attribute `default` [missing-attribute]
- ERROR homeassistant/helpers/entity_registry.py:510:6-20: Object of class `str` has no attribute `default` [missing-attribute]
|
1 participant
Summary
Fixes #2012
Test Plan