language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-08 12:48:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Add bugbear immutable functions as allowed in dataclasses (#4122)

Browse source
This commit is contained in:
Moritz Sauter 2023-04-28 03:23:06 +02:00 • committed by GitHub
parent 089b64e9c1
commit ee6d8f7467
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 132 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

13
crates/ruff/resources/test/fixtures/ruff/RUF009.py vendored
View file

@ -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)

71
crates/ruff/src/rules/flake8_bugbear/rules/function_call_argument_default.rs
View file

@ -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<String>,
@ -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((

3
crates/ruff/src/rules/flake8_bugbear/settings.rs
View file

@ -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<Vec<String>>,
}

28
crates/ruff/src/rules/ruff/rules/mutable_defaults_in_dataclass_fields.rs
View file

@ -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<CallPath> = 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),

48
crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF009_RUF009.py.snap
View file

@ -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
|

30
crates/ruff_python_semantic/src/analyze/typing.rs
View file

@ -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)
})
}

2
ruff.schema.json generated
View file

@ -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"