diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF008.py b/crates/ruff/resources/test/fixtures/ruff/RUF008.py index d19a9bdc64..4504714ae5 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF008.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF008.py @@ -1,4 +1,6 @@ +import typing from dataclasses import dataclass, field +from typing import Sequence KNOWINGLY_MUTABLE_DEFAULT = [] @@ -6,6 +8,7 @@ KNOWINGLY_MUTABLE_DEFAULT = [] @dataclass() class A: mutable_default: list[int] = [] + immutable_annotation: typing.Sequence[int] = [] without_annotation = [] ignored_via_comment: list[int] = [] # noqa: RUF008 correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT @@ -15,6 +18,7 @@ class A: @dataclass class B: mutable_default: list[int] = [] + immutable_annotation: Sequence[int] = [] without_annotation = [] ignored_via_comment: list[int] = [] # noqa: RUF008 correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs b/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs index 7a43a63b35..0f6fa75fac 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/mutable_argument_default.rs @@ -1,8 +1,9 @@ -use rustpython_parser::ast::{Arguments, Constant, Expr, ExprKind, Operator}; +use rustpython_parser::ast::{Arguments, Expr, ExprKind}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::types::Range; +use ruff_python_semantic::analyze::typing::is_immutable_annotation; use crate::checkers::ast::Checker; @@ -25,47 +26,6 @@ const MUTABLE_FUNCS: &[&[&str]] = &[ &["collections", "deque"], ]; -const IMMUTABLE_TYPES: &[&[&str]] = &[ - &["", "bool"], - &["", "bytes"], - &["", "complex"], - &["", "float"], - &["", "frozenset"], - &["", "int"], - &["", "object"], - &["", "range"], - &["", "str"], - &["collections", "abc", "Sized"], - &["typing", "LiteralString"], - &["typing", "Sized"], -]; - -const IMMUTABLE_GENERIC_TYPES: &[&[&str]] = &[ - &["", "tuple"], - &["collections", "abc", "ByteString"], - &["collections", "abc", "Collection"], - &["collections", "abc", "Container"], - &["collections", "abc", "Iterable"], - &["collections", "abc", "Mapping"], - &["collections", "abc", "Reversible"], - &["collections", "abc", "Sequence"], - &["collections", "abc", "Set"], - &["typing", "AbstractSet"], - &["typing", "ByteString"], - &["typing", "Callable"], - &["typing", "Collection"], - &["typing", "Container"], - &["typing", "FrozenSet"], - &["typing", "Iterable"], - &["typing", "Literal"], - &["typing", "Mapping"], - &["typing", "Never"], - &["typing", "NoReturn"], - &["typing", "Reversible"], - &["typing", "Sequence"], - &["typing", "Tuple"], -]; - pub fn is_mutable_func(checker: &Checker, func: &Expr) -> bool { checker .ctx @@ -90,60 +50,6 @@ fn is_mutable_expr(checker: &Checker, expr: &Expr) -> bool { } } -fn is_immutable_annotation(checker: &Checker, expr: &Expr) -> bool { - match &expr.node { - ExprKind::Name { .. } | ExprKind::Attribute { .. } => checker - .ctx - .resolve_call_path(expr) - .map_or(false, |call_path| { - IMMUTABLE_TYPES - .iter() - .chain(IMMUTABLE_GENERIC_TYPES) - .any(|target| call_path.as_slice() == *target) - }), - ExprKind::Subscript { value, slice, .. } => { - checker - .ctx - .resolve_call_path(value) - .map_or(false, |call_path| { - if IMMUTABLE_GENERIC_TYPES - .iter() - .any(|target| call_path.as_slice() == *target) - { - true - } else if call_path.as_slice() == ["typing", "Union"] { - if let ExprKind::Tuple { elts, .. } = &slice.node { - elts.iter().all(|elt| is_immutable_annotation(checker, elt)) - } else { - false - } - } else if call_path.as_slice() == ["typing", "Optional"] { - is_immutable_annotation(checker, slice) - } else if call_path.as_slice() == ["typing", "Annotated"] { - if let ExprKind::Tuple { elts, .. } = &slice.node { - elts.first() - .map_or(false, |elt| is_immutable_annotation(checker, elt)) - } else { - false - } - } else { - false - } - }) - } - ExprKind::BinOp { - left, - op: Operator::BitOr, - right, - } => is_immutable_annotation(checker, left) && is_immutable_annotation(checker, right), - ExprKind::Constant { - value: Constant::None, - .. - } => true, - _ => false, - } -} - /// B006 pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { // Scan in reverse order to right-align zip(). @@ -166,7 +72,7 @@ pub fn mutable_argument_default(checker: &mut Checker, arguments: &Arguments) { .node .annotation .as_ref() - .map_or(false, |expr| is_immutable_annotation(checker, expr)) + .map_or(false, |expr| is_immutable_annotation(&checker.ctx, expr)) { checker.diagnostics.push(Diagnostic::new( MutableArgumentDefault, diff --git a/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs b/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs index 69068eba95..cd6e9e05eb 100644 --- a/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs +++ b/crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs @@ -3,6 +3,7 @@ use rustpython_parser::ast::{Expr, ExprKind, Stmt, StmtKind}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{call_path::compose_call_path, helpers::map_callable, types::Range}; +use ruff_python_semantic::analyze::typing::is_immutable_annotation; use ruff_python_semantic::context::Context; use crate::checkers::ast::Checker; @@ -174,16 +175,26 @@ pub fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &[Stmt]) /// RUF008 pub fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) { for statement in body { - if let StmtKind::AnnAssign { - value: Some(value), .. - } - | StmtKind::Assign { value, .. } = &statement.node - { - if is_mutable_expr(value) { - checker - .diagnostics - .push(Diagnostic::new(MutableDataclassDefault, Range::from(value))); + match &statement.node { + StmtKind::AnnAssign { + annotation, + value: Some(value), + .. + } => { + if !is_immutable_annotation(&checker.ctx, annotation) && is_mutable_expr(value) { + checker + .diagnostics + .push(Diagnostic::new(MutableDataclassDefault, Range::from(value))); + } } + StmtKind::Assign { value, .. } => { + if is_mutable_expr(value) { + checker + .diagnostics + .push(Diagnostic::new(MutableDataclassDefault, Range::from(value))); + } + } + _ => (), } } } diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF008_RUF008.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF008_RUF008.py.snap index 3af7ffb0f4..653d496831 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF008_RUF008.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF008_RUF008.py.snap @@ -1,44 +1,44 @@ --- source: crates/ruff/src/rules/ruff/mod.rs --- -RUF008.py:8:34: RUF008 Do not use mutable default values for dataclass attributes +RUF008.py:10:34: RUF008 Do not use mutable default values for dataclass attributes | - 8 | @dataclass() - 9 | class A: -10 | mutable_default: list[int] = [] +10 | @dataclass() +11 | class A: +12 | mutable_default: list[int] = [] | ^^ RUF008 -11 | without_annotation = [] -12 | ignored_via_comment: list[int] = [] # noqa: RUF008 +13 | immutable_annotation: typing.Sequence[int] = [] +14 | without_annotation = [] | -RUF008.py:9:26: RUF008 Do not use mutable default values for dataclass attributes +RUF008.py:12:26: RUF008 Do not use mutable default values for dataclass attributes | - 9 | class A: -10 | mutable_default: list[int] = [] -11 | without_annotation = [] +12 | mutable_default: list[int] = [] +13 | immutable_annotation: typing.Sequence[int] = [] +14 | without_annotation = [] | ^^ RUF008 -12 | ignored_via_comment: list[int] = [] # noqa: RUF008 -13 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT +15 | ignored_via_comment: list[int] = [] # noqa: RUF008 +16 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT | -RUF008.py:17:34: RUF008 Do not use mutable default values for dataclass attributes +RUF008.py:20:34: RUF008 Do not use mutable default values for dataclass attributes | -17 | @dataclass -18 | class B: -19 | mutable_default: list[int] = [] +20 | @dataclass +21 | class B: +22 | mutable_default: list[int] = [] | ^^ RUF008 -20 | without_annotation = [] -21 | ignored_via_comment: list[int] = [] # noqa: RUF008 +23 | immutable_annotation: Sequence[int] = [] +24 | without_annotation = [] | -RUF008.py:18:26: RUF008 Do not use mutable default values for dataclass attributes +RUF008.py:22:26: RUF008 Do not use mutable default values for dataclass attributes | -18 | class B: -19 | mutable_default: list[int] = [] -20 | without_annotation = [] +22 | mutable_default: list[int] = [] +23 | immutable_annotation: Sequence[int] = [] +24 | without_annotation = [] | ^^ RUF008 -21 | ignored_via_comment: list[int] = [] # noqa: RUF008 -22 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT +25 | ignored_via_comment: list[int] = [] # noqa: RUF008 +26 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT | diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 23018322e6..189b061e86 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -1,7 +1,10 @@ -use rustpython_parser::ast::{Expr, ExprKind}; +use rustpython_parser::ast::{Constant, Expr, ExprKind, Operator}; use ruff_python_ast::call_path::{from_unqualified_name, CallPath}; -use ruff_python_stdlib::typing::{PEP_585_BUILTINS_ELIGIBLE, PEP_593_SUBSCRIPTS, SUBSCRIPTS}; +use ruff_python_stdlib::typing::{ + IMMUTABLE_GENERIC_TYPES, IMMUTABLE_TYPES, PEP_585_BUILTINS_ELIGIBLE, PEP_593_SUBSCRIPTS, + SUBSCRIPTS, +}; use crate::context::Context; @@ -68,3 +71,53 @@ pub fn is_pep585_builtin(expr: &Expr, context: &Context) -> bool { PEP_585_BUILTINS_ELIGIBLE.contains(&call_path.as_slice()) }) } + +pub fn is_immutable_annotation(context: &Context, expr: &Expr) -> bool { + match &expr.node { + ExprKind::Name { .. } | ExprKind::Attribute { .. } => { + context.resolve_call_path(expr).map_or(false, |call_path| { + IMMUTABLE_TYPES + .iter() + .chain(IMMUTABLE_GENERIC_TYPES) + .any(|target| call_path.as_slice() == *target) + }) + } + ExprKind::Subscript { value, slice, .. } => { + context.resolve_call_path(value).map_or(false, |call_path| { + if IMMUTABLE_GENERIC_TYPES + .iter() + .any(|target| call_path.as_slice() == *target) + { + true + } else if call_path.as_slice() == ["typing", "Union"] { + if let ExprKind::Tuple { elts, .. } = &slice.node { + elts.iter().all(|elt| is_immutable_annotation(context, elt)) + } else { + false + } + } else if call_path.as_slice() == ["typing", "Optional"] { + is_immutable_annotation(context, slice) + } else if call_path.as_slice() == ["typing", "Annotated"] { + if let ExprKind::Tuple { elts, .. } = &slice.node { + elts.first() + .map_or(false, |elt| is_immutable_annotation(context, elt)) + } else { + false + } + } else { + false + } + }) + } + ExprKind::BinOp { + left, + op: Operator::BitOr, + right, + } => is_immutable_annotation(context, left) && is_immutable_annotation(context, right), + ExprKind::Constant { + value: Constant::None, + .. + } => true, + _ => false, + } +} diff --git a/crates/ruff_python_stdlib/src/typing.rs b/crates/ruff_python_stdlib/src/typing.rs index e1abeba65f..313ff0a8fd 100644 --- a/crates/ruff_python_stdlib/src/typing.rs +++ b/crates/ruff_python_stdlib/src/typing.rs @@ -225,3 +225,44 @@ pub static SIMPLE_MAGIC_RETURN_TYPES: Lazy ("__subclasscheck__", "bool"), ]) }); + +pub const IMMUTABLE_TYPES: &[&[&str]] = &[ + &["", "bool"], + &["", "bytes"], + &["", "complex"], + &["", "float"], + &["", "frozenset"], + &["", "int"], + &["", "object"], + &["", "range"], + &["", "str"], + &["collections", "abc", "Sized"], + &["typing", "LiteralString"], + &["typing", "Sized"], +]; + +pub const IMMUTABLE_GENERIC_TYPES: &[&[&str]] = &[ + &["", "tuple"], + &["collections", "abc", "ByteString"], + &["collections", "abc", "Collection"], + &["collections", "abc", "Container"], + &["collections", "abc", "Iterable"], + &["collections", "abc", "Mapping"], + &["collections", "abc", "Reversible"], + &["collections", "abc", "Sequence"], + &["collections", "abc", "Set"], + &["typing", "AbstractSet"], + &["typing", "ByteString"], + &["typing", "Callable"], + &["typing", "Collection"], + &["typing", "Container"], + &["typing", "FrozenSet"], + &["typing", "Iterable"], + &["typing", "Literal"], + &["typing", "Mapping"], + &["typing", "Never"], + &["typing", "NoReturn"], + &["typing", "Reversible"], + &["typing", "Sequence"], + &["typing", "Tuple"], +];