diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF009.py b/crates/ruff/resources/test/fixtures/ruff/RUF009.py index 9a8a9e6ee2..53c6a0598e 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF009.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF009.py @@ -1,5 +1,8 @@ +import datetime +import re import typing -from dataclasses import dataclass +from dataclasses import dataclass, field +from pathlib import Path from typing import ClassVar, NamedTuple @@ -17,6 +20,12 @@ class A: class_variable: typing.ClassVar[list[int]] = default_function() another_class_var: ClassVar[list[int]] = default_function() + fine_path: Path = Path() + fine_date: datetime.date = datetime.date(2042, 1, 1) + fine_timedelta: datetime.timedelta = datetime.timedelta(hours=7) + fine_tuple: tuple[int] = tuple([1]) + fine_regex: re.Pattern = re.compile(r".*") + DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES = ImmutableType(40) DEFAULT_A_FOR_ALL_DATACLASSES = A([1, 2, 3]) @@ -29,3 +38,5 @@ class B: not_optimal: ImmutableType = ImmutableType(20) good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES + + fine_dataclass_function: list[int] = field(default_factory=list) diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs b/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs index d255bcf9d0..c873bc4bd2 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs @@ -1,3 +1,4 @@ +use ruff_python_semantic::analyze::typing::is_immutable_func; use ruff_text_size::TextRange; use rustpython_parser::ast::{Arguments, Constant, Expr, ExprKind}; @@ -13,6 +14,45 @@ use crate::checkers::ast::Checker; use super::mutable_argument_default::is_mutable_func; +/// ## What it does +/// Checks for function calls in function defaults. +/// +/// ## Why is it bad? +/// The function calls in the defaults are only performed once, at definition +/// time. The returned value is then reused by all calls to the function. +/// +/// ## Options +/// - `flake8-bugbear.extend-immutable-calls` +/// +/// ## Examples: +/// ```python +/// def create_list() -> list[int]: +/// return [1, 2, 3] +/// +/// def mutable_default(arg: list[int] = create_list()) -> list[int]: +/// arg.append(4) +/// return arg +/// ``` +/// +/// Use instead: +/// ```python +/// def better(arg: list[int] | None = None) -> list[int]: +/// if arg is None: +/// arg = create_list() +/// +/// arg.append(4) +/// return arg +/// ``` +/// +/// Alternatively, if you _want_ the shared behaviour, make it more obvious +/// by assigning it to a module-level variable: +/// ```python +/// I_KNOW_THIS_IS_SHARED_STATE = create_list() +/// +/// def mutable_default(arg: list[int] = I_KNOW_THIS_IS_SHARED_STATE) -> list[int]: +/// arg.append(4) +/// return arg +/// ``` #[violation] pub struct FunctionCallInDefaultArgument { pub name: Option, @@ -30,35 +70,6 @@ impl Violation for FunctionCallInDefaultArgument { } } -const IMMUTABLE_FUNCS: &[&[&str]] = &[ - &["", "tuple"], - &["", "frozenset"], - &["datetime", "date"], - &["datetime", "datetime"], - &["datetime", "timedelta"], - &["decimal", "Decimal"], - &["operator", "attrgetter"], - &["operator", "itemgetter"], - &["operator", "methodcaller"], - &["pathlib", "Path"], - &["types", "MappingProxyType"], - &["re", "compile"], -]; - -fn is_immutable_func(checker: &Checker, func: &Expr, extend_immutable_calls: &[CallPath]) -> bool { - checker - .ctx - .resolve_call_path(func) - .map_or(false, |call_path| { - IMMUTABLE_FUNCS - .iter() - .any(|target| call_path.as_slice() == *target) - || extend_immutable_calls - .iter() - .any(|target| call_path == *target) - }) -} - struct ArgumentDefaultVisitor<'a> { checker: &'a Checker<'a>, diagnostics: Vec<(DiagnosticKind, TextRange)>, @@ -73,7 +84,7 @@ where match &expr.node { ExprKind::Call { func, args, .. } => { if !is_mutable_func(self.checker, func) - && !is_immutable_func(self.checker, func, &self.extend_immutable_calls) + && !is_immutable_func(&self.checker.ctx, func, &self.extend_immutable_calls) && !is_nan_or_infinity(func, args) { self.diagnostics.push(( diff --git a/crates/ruff/src/rules/flake8_bugbear/settings.rs b/crates/ruff/src/rules/flake8_bugbear/settings.rs index c9a436fe02..566aa1519d 100644 --- a/crates/ruff/src/rules/flake8_bugbear/settings.rs +++ b/crates/ruff/src/rules/flake8_bugbear/settings.rs @@ -23,7 +23,8 @@ pub struct Options { "# )] /// Additional callable functions to consider "immutable" when evaluating, - /// e.g., the `no-mutable-default-argument` rule (`B006`). + /// e.g., the `no-mutable-default-argument` rule (`B006`) or + /// `no-function-call-in-dataclass-defaults` rule (`RUF009`). pub extend_immutable_calls: Option>, } 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 68c2c1bfec..3fe3e13b44 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 @@ -1,10 +1,13 @@ +use ruff_python_ast::call_path::{from_qualified_name, CallPath}; 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}; -use ruff_python_semantic::analyze::typing::is_immutable_annotation; -use ruff_python_semantic::context::Context; +use ruff_python_semantic::{ + analyze::typing::{is_immutable_annotation, is_immutable_func}, + context::Context, +}; use crate::checkers::ast::Checker; @@ -64,6 +67,9 @@ impl Violation for MutableDataclassDefault { /// Function calls are only performed once, at definition time. The returned /// value is then reused by all instances of the dataclass. /// +/// ## Options +/// - `flake8-bugbear.extend-immutable-calls` +/// /// ## Examples: /// ```python /// from dataclasses import dataclass @@ -141,11 +147,11 @@ fn is_mutable_expr(expr: &Expr) -> bool { ) } -const ALLOWED_FUNCS: &[&[&str]] = &[&["dataclasses", "field"]]; +const ALLOWED_DATACLASS_SPECIFIC_FUNCTIONS: &[&[&str]] = &[&["dataclasses", "field"]]; -fn is_allowed_func(context: &Context, func: &Expr) -> bool { +fn is_allowed_dataclass_function(context: &Context, func: &Expr) -> bool { context.resolve_call_path(func).map_or(false, |call_path| { - ALLOWED_FUNCS + ALLOWED_DATACLASS_SPECIFIC_FUNCTIONS .iter() .any(|target| call_path.as_slice() == *target) }) @@ -161,6 +167,14 @@ fn is_class_var_annotation(context: &Context, annotation: &Expr) -> bool { /// RUF009 pub fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &[Stmt]) { + let extend_immutable_calls: Vec = checker + .settings + .flake8_bugbear + .extend_immutable_calls + .iter() + .map(|target| from_qualified_name(target)) + .collect(); + for statement in body { if let StmtKind::AnnAssign { annotation, @@ -172,7 +186,9 @@ pub fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &[Stmt]) continue; } if let ExprKind::Call { func, .. } = &expr.node { - if !is_allowed_func(&checker.ctx, func) { + if !is_immutable_func(&checker.ctx, func, &extend_immutable_calls) + && !is_allowed_dataclass_function(&checker.ctx, func) + { checker.diagnostics.push(Diagnostic::new( FunctionCallInDataclassDefaultArgument { name: compose_call_path(func), diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap index 4f95c1be16..096591e7f9 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap @@ -1,44 +1,44 @@ --- source: crates/ruff/src/rules/ruff/mod.rs --- -RUF009.py:16:41: RUF009 Do not perform function call `default_function` in dataclass defaults +RUF009.py:19:41: RUF009 Do not perform function call `default_function` in dataclass defaults | -16 | @dataclass() -17 | class A: -18 | hidden_mutable_default: list[int] = default_function() +19 | @dataclass() +20 | class A: +21 | hidden_mutable_default: list[int] = default_function() | ^^^^^^^^^^^^^^^^^^ RUF009 -19 | class_variable: typing.ClassVar[list[int]] = default_function() -20 | another_class_var: ClassVar[list[int]] = default_function() +22 | class_variable: typing.ClassVar[list[int]] = default_function() +23 | another_class_var: ClassVar[list[int]] = default_function() | -RUF009.py:27:41: RUF009 Do not perform function call `default_function` in dataclass defaults +RUF009.py:36:41: RUF009 Do not perform function call `default_function` in dataclass defaults | -27 | @dataclass -28 | class B: -29 | hidden_mutable_default: list[int] = default_function() +36 | @dataclass +37 | class B: +38 | hidden_mutable_default: list[int] = default_function() | ^^^^^^^^^^^^^^^^^^ RUF009 -30 | another_dataclass: A = A() -31 | not_optimal: ImmutableType = ImmutableType(20) +39 | another_dataclass: A = A() +40 | not_optimal: ImmutableType = ImmutableType(20) | -RUF009.py:28:28: RUF009 Do not perform function call `A` in dataclass defaults +RUF009.py:37:28: RUF009 Do not perform function call `A` in dataclass defaults | -28 | class B: -29 | hidden_mutable_default: list[int] = default_function() -30 | another_dataclass: A = A() +37 | class B: +38 | hidden_mutable_default: list[int] = default_function() +39 | another_dataclass: A = A() | ^^^ RUF009 -31 | not_optimal: ImmutableType = ImmutableType(20) -32 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +40 | not_optimal: ImmutableType = ImmutableType(20) +41 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES | -RUF009.py:29:34: RUF009 Do not perform function call `ImmutableType` in dataclass defaults +RUF009.py:38:34: RUF009 Do not perform function call `ImmutableType` in dataclass defaults | -29 | hidden_mutable_default: list[int] = default_function() -30 | another_dataclass: A = A() -31 | not_optimal: ImmutableType = ImmutableType(20) +38 | hidden_mutable_default: list[int] = default_function() +39 | another_dataclass: A = A() +40 | not_optimal: ImmutableType = ImmutableType(20) | ^^^^^^^^^^^^^^^^^ RUF009 -32 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES -33 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES +41 | good_variant: ImmutableType = DEFAULT_IMMUTABLETYPE_FOR_ALL_DATACLASSES +42 | okay_variant: A = DEFAULT_A_FOR_ALL_DATACLASSES | diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 3c45adb901..850e1c03bc 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -122,3 +122,33 @@ pub fn is_immutable_annotation(context: &Context, expr: &Expr) -> bool { _ => false, } } + +const IMMUTABLE_FUNCS: &[&[&str]] = &[ + &["", "tuple"], + &["", "frozenset"], + &["datetime", "date"], + &["datetime", "datetime"], + &["datetime", "timedelta"], + &["decimal", "Decimal"], + &["operator", "attrgetter"], + &["operator", "itemgetter"], + &["operator", "methodcaller"], + &["pathlib", "Path"], + &["types", "MappingProxyType"], + &["re", "compile"], +]; + +pub fn is_immutable_func( + context: &Context, + func: &Expr, + extend_immutable_calls: &[CallPath], +) -> bool { + context.resolve_call_path(func).map_or(false, |call_path| { + IMMUTABLE_FUNCS + .iter() + .any(|target| call_path.as_slice() == *target) + || extend_immutable_calls + .iter() + .any(|target| call_path == *target) + }) +} diff --git a/ruff.schema.json b/ruff.schema.json index 2d126f00f9..6e9f2d2ef9 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -669,7 +669,7 @@ "type": "object", "properties": { "extend-immutable-calls": { - "description": "Additional callable functions to consider \"immutable\" when evaluating, e.g., the `no-mutable-default-argument` rule (`B006`).", + "description": "Additional callable functions to consider \"immutable\" when evaluating, e.g., the `no-mutable-default-argument` rule (`B006`) or `no-function-call-in-dataclass-defaults` rule (`RUF009`).", "type": [ "array", "null"