From b37057834a4627a4baf37c8478620549359aab35 Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Fri, 22 Nov 2024 03:15:32 +0000 Subject: [PATCH 1/2] [`ruff`] Handle `attrs`'s `auto_attribs` correctly (`RUF009`) --- .../ruff/RUF009_attrs_auto_attribs.py | 101 +++++++++++++++++ crates/ruff_linter/src/rules/ruff/mod.rs | 4 + .../function_call_in_dataclass_default.rs | 41 ++++++- .../src/rules/ruff/rules/helpers.rs | 57 ++++++++-- ...ests__preview__RUF009_RUF009_attrs.py.snap | 10 -- ...__RUF009_RUF009_attrs_auto_attribs.py.snap | 102 ++++++++++++++++++ 6 files changed, 296 insertions(+), 19 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs_auto_attribs.py create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs_auto_attribs.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs_auto_attribs.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs_auto_attribs.py new file mode 100644 index 0000000000000..6366304fdf3b1 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF009_attrs_auto_attribs.py @@ -0,0 +1,101 @@ +import attr +from attrs import define, field, frozen, mutable + + +foo = int + + +@define # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@define() # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@define(auto_attribs=None) # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@frozen # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@frozen() # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@frozen(auto_attribs=None) # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@mutable # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@mutable() # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@mutable(auto_attribs=None) # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@attr.s # auto_attribs = False +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@attr.s() # auto_attribs = False +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() + + +@attr.s(auto_attribs=None) # auto_attribs = None => True +class C: + a: str = 0 + b = field() + c: int = foo() + d = list() diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index befec2e77beb8..4cec7109a8fae 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -396,6 +396,10 @@ mod tests { Rule::FunctionCallInDataclassDefaultArgument, Path::new("RUF009_attrs.py") )] + #[test_case( + Rule::FunctionCallInDataclassDefaultArgument, + Path::new("RUF009_attrs_auto_attribs.py") + )] #[test_case(Rule::MutableDataclassDefault, Path::new("RUF008_attrs.py"))] #[test_case(Rule::MapIntVersionParsing, Path::new("RUF048.py"))] #[test_case(Rule::MapIntVersionParsing, Path::new("RUF048_1.py"))] diff --git a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs index 9e40f9a1fecca..bf9d38f33c2ee 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs @@ -9,6 +9,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::rules::ruff::rules::helpers::{ dataclass_kind, is_class_var_annotation, is_dataclass_field, is_descriptor_class, + AttrsAutoAttribs, DataclassKind, }; /// ## What it does @@ -84,6 +85,25 @@ pub(crate) fn function_call_in_dataclass_default( return; } + let attrs_auto_attribs = match dataclass_kind { + DataclassKind::Stdlib => None, + + DataclassKind::Attrs(attrs_auto_attribs) => match attrs_auto_attribs { + AttrsAutoAttribs::Unknown => return, + + AttrsAutoAttribs::None => match any_annotated(&class_def.body) { + true => Some(AttrsAutoAttribs::True), + false => Some(AttrsAutoAttribs::False), + }, + + _ => Some(attrs_auto_attribs), + }, + }; + let dataclass_kind = match attrs_auto_attribs { + None => DataclassKind::Stdlib, + Some(attrs_auto_attribs) => DataclassKind::Attrs(attrs_auto_attribs), + }; + let extend_immutable_calls: Vec = checker .settings .flake8_bugbear @@ -101,13 +121,21 @@ pub(crate) fn function_call_in_dataclass_default( else { continue; }; - let Expr::Call(ast::ExprCall { func, .. }) = &**expr else { + let Expr::Call(ast::ExprCall { func, .. }) = expr.as_ref() else { continue; }; - if is_class_var_annotation(annotation, checker.semantic()) + let is_field = is_dataclass_field(func, checker.semantic(), dataclass_kind); + + // Non-explicit fields in an `attrs` dataclass + // with `auto_attribs=False` are class variables. + if matches!(attrs_auto_attribs, Some(AttrsAutoAttribs::False)) && !is_field { + continue; + } + + if is_field + || is_class_var_annotation(annotation, checker.semantic()) || is_immutable_func(func, checker.semantic(), &extend_immutable_calls) - || is_dataclass_field(func, checker.semantic(), dataclass_kind) || is_descriptor_class(func, checker.semantic()) { continue; @@ -121,3 +149,10 @@ pub(crate) fn function_call_in_dataclass_default( checker.diagnostics.push(diagnostic); } } + +#[inline] +fn any_annotated(class_body: &Vec) -> bool { + class_body + .iter() + .any(|stmt| matches!(stmt, Stmt::AnnAssign(..))) +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs index 3e255d60d0ecb..a6f3816a624e7 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs @@ -1,5 +1,5 @@ use ruff_python_ast::helpers::{map_callable, map_subscript}; -use ruff_python_ast::{self as ast, Expr}; +use ruff_python_ast::{self as ast, Expr, ExprCall}; use ruff_python_semantic::{analyze, BindingKind, Modules, SemanticModel}; /// Return `true` if the given [`Expr`] is a special class attribute, like `__slots__`. @@ -49,7 +49,7 @@ pub(super) fn is_dataclass_field( dataclass_kind: DataclassKind, ) -> bool { match dataclass_kind { - DataclassKind::Attrs => is_attrs_field(func, semantic), + DataclassKind::Attrs(..) => is_attrs_field(func, semantic), DataclassKind::Stdlib => is_stdlib_dataclass_field(func, semantic), } } @@ -76,13 +76,29 @@ pub(super) fn is_final_annotation(annotation: &Expr, semantic: &SemanticModel) - semantic.match_typing_expr(map_subscript(annotation), "Final") } +/// Values that [`attrs`'s `auto_attribs`][1] accept. +/// +/// [1]: https://www.attrs.org/en/stable/api.html#attrs.define +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub(super) enum AttrsAutoAttribs { + /// `a: str = ...` are automatically converted to fields. + True, + /// Only `attrs.field()`/`attr.ib()` calls are considered fields. + False, + /// `True` if any attributes are annotated (and no unannotated `attrs.field`s are found). + /// `False` otherwise. + None, + /// The provided value is not a literal. + Unknown, +} + /// Enumeration of various kinds of dataclasses recognised by Ruff #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub(super) enum DataclassKind { /// dataclasses created by the stdlib `dataclasses` module Stdlib, /// dataclasses created by the third-party `attrs` library - Attrs, + Attrs(AttrsAutoAttribs), } impl DataclassKind { @@ -91,11 +107,12 @@ impl DataclassKind { } pub(super) const fn is_attrs(self) -> bool { - matches!(self, DataclassKind::Attrs) + matches!(self, DataclassKind::Attrs(..)) } } -/// Return the kind of dataclass this class definition is (stdlib or `attrs`), or `None` if the class is not a dataclass. +/// Return the kind of dataclass this class definition is (stdlib or `attrs`), +/// or `None` if the class is not a dataclass. pub(super) fn dataclass_kind( class_def: &ast::StmtClassDef, semantic: &SemanticModel, @@ -112,7 +129,35 @@ pub(super) fn dataclass_kind( }; match qualified_name.segments() { - ["attrs", "define" | "frozen"] | ["attr", "s"] => return Some(DataclassKind::Attrs), + ["attrs", func @ ("define" | "frozen" | "mutable")] | ["attr", func @ "s"] => { + // `.define`, `.frozen` and `.mutable` all default `auto_attribs` to `None`, + // whereas `@attr.s` implicitly sets `auto_attribs=False`. + // https://www.attrs.org/en/stable/api.html#attrs.define + // https://www.attrs.org/en/stable/api-attr.html#attr.s + let Expr::Call(ExprCall { arguments, .. }) = &decorator.expression else { + let auto_attribs = match func.as_ref() { + "s" => AttrsAutoAttribs::False, + _ => AttrsAutoAttribs::None, + }; + + return Some(DataclassKind::Attrs(auto_attribs)); + }; + + let Some(auto_attribs) = arguments.find_keyword("auto_attribs") else { + return Some(DataclassKind::Attrs(AttrsAutoAttribs::None)); + }; + + let auto_attribs = match &auto_attribs.value { + Expr::BooleanLiteral(literal) => match literal.value { + true => AttrsAutoAttribs::True, + false => AttrsAutoAttribs::False, + }, + Expr::NoneLiteral(..) => AttrsAutoAttribs::None, + _ => AttrsAutoAttribs::Unknown, + }; + + return Some(DataclassKind::Attrs(auto_attribs)); + } ["dataclasses", "dataclass"] => return Some(DataclassKind::Stdlib), _ => continue, } diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap index 771b3601feb69..5ed5adb316ebb 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs.py.snap @@ -2,16 +2,6 @@ source: crates/ruff_linter/src/rules/ruff/mod.rs snapshot_kind: text --- -RUF009_attrs.py:23:41: RUF009 Do not perform function call `default_function` in dataclass defaults - | -21 | @attr.s -22 | class A: -23 | hidden_mutable_default: list[int] = default_function() - | ^^^^^^^^^^^^^^^^^^ RUF009 -24 | class_variable: typing.ClassVar[list[int]] = default_function() -25 | another_class_var: ClassVar[list[int]] = default_function() - | - RUF009_attrs.py:46:41: RUF009 Do not perform function call `default_function` in dataclass defaults | 44 | @define diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs_auto_attribs.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs_auto_attribs.py.snap new file mode 100644 index 0000000000000..6324b9b5e06c7 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__preview__RUF009_RUF009_attrs_auto_attribs.py.snap @@ -0,0 +1,102 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +snapshot_kind: text +--- +RUF009_attrs_auto_attribs.py:12:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +10 | a: str = 0 +11 | b = field() +12 | c: int = foo() + | ^^^^^ RUF009 +13 | d = list() + | + +RUF009_attrs_auto_attribs.py:20:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +18 | a: str = 0 +19 | b = field() +20 | c: int = foo() + | ^^^^^ RUF009 +21 | d = list() + | + +RUF009_attrs_auto_attribs.py:28:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +26 | a: str = 0 +27 | b = field() +28 | c: int = foo() + | ^^^^^ RUF009 +29 | d = list() + | + +RUF009_attrs_auto_attribs.py:36:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +34 | a: str = 0 +35 | b = field() +36 | c: int = foo() + | ^^^^^ RUF009 +37 | d = list() + | + +RUF009_attrs_auto_attribs.py:44:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +42 | a: str = 0 +43 | b = field() +44 | c: int = foo() + | ^^^^^ RUF009 +45 | d = list() + | + +RUF009_attrs_auto_attribs.py:52:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +50 | a: str = 0 +51 | b = field() +52 | c: int = foo() + | ^^^^^ RUF009 +53 | d = list() + | + +RUF009_attrs_auto_attribs.py:60:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +58 | a: str = 0 +59 | b = field() +60 | c: int = foo() + | ^^^^^ RUF009 +61 | d = list() + | + +RUF009_attrs_auto_attribs.py:68:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +66 | a: str = 0 +67 | b = field() +68 | c: int = foo() + | ^^^^^ RUF009 +69 | d = list() + | + +RUF009_attrs_auto_attribs.py:76:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +74 | a: str = 0 +75 | b = field() +76 | c: int = foo() + | ^^^^^ RUF009 +77 | d = list() + | + +RUF009_attrs_auto_attribs.py:92:14: RUF009 Do not perform function call `foo` in dataclass defaults + | +90 | a: str = 0 +91 | b = field() +92 | c: int = foo() + | ^^^^^ RUF009 +93 | d = list() + | + +RUF009_attrs_auto_attribs.py:100:14: RUF009 Do not perform function call `foo` in dataclass defaults + | + 98 | a: str = 0 + 99 | b = field() +100 | c: int = foo() + | ^^^^^ RUF009 +101 | d = list() + | From 7ca39811c644904d2249aca6545ccbe60604d21b Mon Sep 17 00:00:00 2001 From: InSyncWithFoo Date: Fri, 22 Nov 2024 03:29:25 +0000 Subject: [PATCH 2/2] Clippy --- .../function_call_in_dataclass_default.rs | 13 ++++++++----- .../src/rules/ruff/rules/helpers.rs | 18 +++++++++++------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs index bf9d38f33c2ee..5902584a71b9d 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs @@ -91,10 +91,13 @@ pub(crate) fn function_call_in_dataclass_default( DataclassKind::Attrs(attrs_auto_attribs) => match attrs_auto_attribs { AttrsAutoAttribs::Unknown => return, - AttrsAutoAttribs::None => match any_annotated(&class_def.body) { - true => Some(AttrsAutoAttribs::True), - false => Some(AttrsAutoAttribs::False), - }, + AttrsAutoAttribs::None => { + if any_annotated(&class_def.body) { + Some(AttrsAutoAttribs::True) + } else { + Some(AttrsAutoAttribs::False) + } + } _ => Some(attrs_auto_attribs), }, @@ -151,7 +154,7 @@ pub(crate) fn function_call_in_dataclass_default( } #[inline] -fn any_annotated(class_body: &Vec) -> bool { +fn any_annotated(class_body: &[Stmt]) -> bool { class_body .iter() .any(|stmt| matches!(stmt, Stmt::AnnAssign(..))) diff --git a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs index a6f3816a624e7..d6a743b677bf4 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs @@ -135,9 +135,10 @@ pub(super) fn dataclass_kind( // https://www.attrs.org/en/stable/api.html#attrs.define // https://www.attrs.org/en/stable/api-attr.html#attr.s let Expr::Call(ExprCall { arguments, .. }) = &decorator.expression else { - let auto_attribs = match func.as_ref() { - "s" => AttrsAutoAttribs::False, - _ => AttrsAutoAttribs::None, + let auto_attribs = if *func == "s" { + AttrsAutoAttribs::False + } else { + AttrsAutoAttribs::None }; return Some(DataclassKind::Attrs(auto_attribs)); @@ -148,10 +149,13 @@ pub(super) fn dataclass_kind( }; let auto_attribs = match &auto_attribs.value { - Expr::BooleanLiteral(literal) => match literal.value { - true => AttrsAutoAttribs::True, - false => AttrsAutoAttribs::False, - }, + Expr::BooleanLiteral(literal) => { + if literal.value { + AttrsAutoAttribs::True + } else { + AttrsAutoAttribs::False + } + } Expr::NoneLiteral(..) => AttrsAutoAttribs::None, _ => AttrsAutoAttribs::Unknown, };