-
Notifications
You must be signed in to change notification settings - Fork 280
fix Feature: support @field.default decorator for attrs #2012 #2494
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,7 @@ use ruff_python_ast::ExprYieldFrom; | |
| use ruff_python_ast::Identifier; | ||
| use ruff_python_ast::Operator; | ||
| use ruff_python_ast::StringLiteral; | ||
| use ruff_python_ast::name::Name; | ||
| use ruff_text_size::Ranged; | ||
| use ruff_text_size::TextRange; | ||
| use starlark_map::Hashed; | ||
|
|
@@ -58,6 +59,7 @@ use crate::binding::bindings::NameLookupResult; | |
| use crate::binding::narrow::AtomicNarrowOp; | ||
| use crate::binding::narrow::NarrowOps; | ||
| use crate::binding::narrow::NarrowSource; | ||
| use crate::binding::scope::FlowStyle; | ||
| use crate::binding::scope::Scope; | ||
| use crate::config::error_kind::ErrorKind; | ||
| use crate::error::context::ErrorInfo; | ||
|
|
@@ -1085,13 +1087,65 @@ impl<'a> BindingsBuilder<'a> { | |
| ) -> Vec<Idx<KeyDecorator>> { | ||
| let mut decorator_keys = Vec::with_capacity(decorators.len()); | ||
| for mut x in decorators { | ||
| let attrs_default_field = self.attrs_default_decorator_field(&x.expression); | ||
| self.ensure_expr(&mut x.expression, usage); | ||
| let k = self.insert_binding( | ||
| KeyDecorator(x.range), | ||
| BindingDecorator { expr: x.expression }, | ||
| BindingDecorator { | ||
| expr: x.expression, | ||
| attrs_default_field, | ||
| }, | ||
| ); | ||
| decorator_keys.push(k); | ||
| } | ||
| decorator_keys | ||
| } | ||
|
|
||
| fn attrs_default_decorator_field(&self, expr: &Expr) -> Option<Name> { | ||
| if self.scopes.enclosing_class_and_metadata_keys().is_none() { | ||
| return None; | ||
| } | ||
| let Expr::Attribute(attr) = expr else { | ||
| return None; | ||
| }; | ||
| if attr.attr.id.as_str() != "default" { | ||
| return None; | ||
| } | ||
| let Expr::Name(base_name) = &*attr.value else { | ||
| return None; | ||
| }; | ||
| let (_, style) = self.scopes.binding_idx_for_name(&base_name.id)?; | ||
| let FlowStyle::ClassField { | ||
| initial_value: Some(initial_value), | ||
| } = style | ||
| else { | ||
| return None; | ||
| }; | ||
| if self.is_attrs_field_specifier(&initial_value) { | ||
| Some(base_name.id.clone()) | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| 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, | ||
| } | ||
|
Comment on lines
+1131
to
+1149
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,6 @@ | |
| use crate::attrs_testcase; | ||
|
|
||
| attrs_testcase!( | ||
| bug = "Correctly recognize field and default decorator", | ||
| field_default_decorator, | ||
| r#" | ||
| from attrs import define, field | ||
|
|
@@ -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() | ||
| "#, | ||
|
Comment on lines
10
to
24
|
||
| ); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attrs_default_for_fieldlinearly scans allclass_body_fields()and runs several lookups to find a matching@<field>.defaultmethod. Sinceget_dataclass_memberis 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.