Avoid RUF008 if field annotation is immutable (#4039)

This commit is contained in:
Dhruv Manilawala 2023-04-21 01:32:12 +05:30 committed by GitHub
parent 7fd44a3e12
commit ba98149022
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 147 additions and 132 deletions

View file

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

View file

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

View file

@ -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,17 +175,27 @@ 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), ..
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, .. } = &statement.node
{
}
StmtKind::Assign { value, .. } => {
if is_mutable_expr(value) {
checker
.diagnostics
.push(Diagnostic::new(MutableDataclassDefault, Range::from(value)));
}
}
_ => (),
}
}
}

View file

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

View file

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

View file

@ -225,3 +225,44 @@ pub static SIMPLE_MAGIC_RETURN_TYPES: Lazy<FxHashMap<&'static str, &'static str>
("__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"],
];