Skip to content
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

[ruff] Handle attrs's auto_attribs correctly (RUF009) #14520

Merged
merged 2 commits into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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()
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -84,6 +85,28 @@ 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 => {
if any_annotated(&class_def.body) {
Some(AttrsAutoAttribs::True)
} else {
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<QualifiedName> = checker
.settings
.flake8_bugbear
Expand All @@ -101,13 +124,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;
Expand All @@ -121,3 +152,10 @@ pub(crate) fn function_call_in_dataclass_default(
checker.diagnostics.push(diagnostic);
}
}

#[inline]
fn any_annotated(class_body: &[Stmt]) -> bool {
class_body
.iter()
.any(|stmt| matches!(stmt, Stmt::AnnAssign(..)))
}
61 changes: 55 additions & 6 deletions crates/ruff_linter/src/rules/ruff/rules/helpers.rs
Original file line number Diff line number Diff line change
@@ -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__`.
Expand Down Expand Up @@ -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),
}
}
Expand All @@ -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 {
Expand All @@ -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,
Expand All @@ -112,7 +129,39 @@ 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 = if *func == "s" {
AttrsAutoAttribs::False
} else {
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 {
InSyncWithFoo marked this conversation as resolved.
Show resolved Hide resolved
Expr::BooleanLiteral(literal) => {
if literal.value {
AttrsAutoAttribs::True
} else {
AttrsAutoAttribs::False
}
}
Expr::NoneLiteral(..) => AttrsAutoAttribs::None,
_ => AttrsAutoAttribs::Unknown,
};

return Some(DataclassKind::Attrs(auto_attribs));
}
["dataclasses", "dataclass"] => return Some(DataclassKind::Stdlib),
_ => continue,
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading