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

Expand RUF008 to all classes, but to a new code (RUF012) #4390

Merged
merged 37 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
43dc99b
RUF008/9 for non-dataclasses as well
adampauls Apr 25, 2023
941d47a
Update crates/ruff/src/checkers/ast/mod.rs
adampauls Apr 26, 2023
87da6cb
Update crates/ruff/src/rules/ruff/mod.rs
adampauls Apr 26, 2023
86dfc75
fmt
adampauls Apr 26, 2023
261d418
Merge remote-tracking branch 'ruff/main'
adampauls Apr 26, 2023
e7423ad
merge
adampauls Apr 26, 2023
12aff2d
bump ruleset size
adampauls Apr 27, 2023
3234dac
Merge remote-tracking branch 'ruff/main'
adampauls Apr 28, 2023
03483eb
fixup
adampauls Apr 28, 2023
e1a2919
that is one opinionated linter
adampauls Apr 28, 2023
4c451d0
Merge remote-tracking branch 'ruff/main'
adampauls May 9, 2023
1937e6a
removing RUF011
adampauls May 12, 2023
fc298dd
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 12, 2023
673939a
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 12, 2023
5c8ad65
fix EOL
adampauls May 12, 2023
09fce64
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 12, 2023
a804567
Merge branch 'main' into only-mutable-fields
adampauls May 12, 2023
5497b98
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 16, 2023
4e8459d
Merge branch 'only-mutable-fields' of github.com:adampauls/ruff into …
adampauls May 16, 2023
c693a35
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 17, 2023
e511717
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 25, 2023
4eaa172
Split into new code.
adampauls May 28, 2023
fe8c1d4
Split into new code.
adampauls May 28, 2023
805d9cc
Split into new code.
adampauls May 28, 2023
181126c
Copy over tests for ClassVar
adampauls May 28, 2023
50b6a4d
fix cargo test
adampauls May 28, 2023
b50c7fd
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls May 30, 2023
08beb0f
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls Jun 6, 2023
3ba6c7a
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls Jun 8, 2023
537bb81
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls Jun 11, 2023
2d1ff5e
ruff.schema.json
adampauls Jun 11, 2023
f7ae3bf
ruff.schema.json
adampauls Jun 11, 2023
d3d5a9e
.snap
adampauls Jun 11, 2023
82458cd
Merge remote-tracking branch 'ruff/main' into only-mutable-fields
adampauls Jun 12, 2023
618a703
Merge branch 'main' into only-mutable-fields
charliermarsh Jun 12, 2023
0c1d088
Tweak docs
charliermarsh Jun 12, 2023
987926b
Fix docs
charliermarsh Jun 12, 2023
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
22 changes: 22 additions & 0 deletions crates/ruff/resources/test/fixtures/ruff/RUF012.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import typing
from typing import ClassVar, Sequence

KNOWINGLY_MUTABLE_DEFAULT = []


class A:
mutable_default: list[int] = []
immutable_annotation: typing.Sequence[int] = []
without_annotation = []
ignored_via_comment: list[int] = [] # noqa: RUF012
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
class_variable: typing.ClassVar[list[int]] = []


class B:
mutable_default: list[int] = []
immutable_annotation: Sequence[int] = []
without_annotation = []
ignored_via_comment: list[int] = [] # noqa: RUF012
correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
class_variable: ClassVar[list[int]] = []
13 changes: 8 additions & 5 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -794,13 +794,16 @@ where
if self.any_enabled(&[
Rule::MutableDataclassDefault,
Rule::FunctionCallInDataclassDefaultArgument,
]) && ruff::rules::is_dataclass(&self.semantic_model, decorator_list)
{
if self.enabled(Rule::MutableDataclassDefault) {
ruff::rules::mutable_dataclass_default(self, body);
Rule::MutableClassDefault,
]) {
let is_dataclass =
ruff::rules::is_dataclass(&self.semantic_model, decorator_list);
if self.any_enabled(&[Rule::MutableDataclassDefault, Rule::MutableClassDefault])
{
ruff::rules::mutable_class_default(self, body, is_dataclass);
}

if self.enabled(Rule::FunctionCallInDataclassDefaultArgument) {
if is_dataclass && self.enabled(Rule::FunctionCallInDataclassDefaultArgument) {
ruff::rules::function_call_in_dataclass_defaults(self, body);
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,6 +752,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "009") => (RuleGroup::Unspecified, rules::ruff::rules::FunctionCallInDataclassDefaultArgument),
(Ruff, "010") => (RuleGroup::Unspecified, rules::ruff::rules::ExplicitFStringTypeConversion),
(Ruff, "011") => (RuleGroup::Unspecified, rules::ruff::rules::StaticKeyDictComprehension),
(Ruff, "012") => (RuleGroup::Unspecified, rules::ruff::rules::MutableClassDefault),
(Ruff, "100") => (RuleGroup::Unspecified, rules::ruff::rules::UnusedNOQA),
(Ruff, "200") => (RuleGroup::Unspecified, rules::ruff::rules::InvalidPyprojectToml),

Expand Down
13 changes: 12 additions & 1 deletion crates/ruff/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,18 @@ mod tests {

#[test_case(Rule::MutableDataclassDefault, Path::new("RUF008.py"))]
#[test_case(Rule::FunctionCallInDataclassDefaultArgument, Path::new("RUF009.py"))]
fn mutable_defaults(rule_code: Rule, path: &Path) -> Result<()> {
fn mutable_dataclass_defaults(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("ruff").join(path).as_path(),
&settings::Settings::for_rule(rule_code),
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::MutableClassDefault, Path::new("RUF012.py"))]
fn mutable_class_defaults(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Path::new("ruff").join(path).as_path(),
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pub(crate) use asyncio_dangling_task::*;
pub(crate) use collection_literal_concatenation::*;
pub(crate) use explicit_f_string_type_conversion::*;
pub(crate) use invalid_pyproject_toml::InvalidPyprojectToml;
pub(crate) use mutable_defaults_in_dataclass_fields::*;
pub(crate) use mutable_defaults_in_class_fields::*;
pub(crate) use pairwise_over_zipped::*;
pub(crate) use unused_noqa::*;

Expand All @@ -15,7 +15,7 @@ mod collection_literal_concatenation;
mod confusables;
mod explicit_f_string_type_conversion;
mod invalid_pyproject_toml;
mod mutable_defaults_in_dataclass_fields;
mod mutable_defaults_in_class_fields;
mod pairwise_over_zipped;
mod static_key_dict_comprehension;
mod unused_noqa;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,18 @@ use ruff_python_semantic::{
use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for mutable default values in dataclasses without the use of
/// `dataclasses.field`.
/// Checks for mutable default values in dataclasses.
///
/// ## Why is this bad?
/// Mutable default values share state across all instances of the dataclass,
/// while not being obvious. This can lead to bugs when the attributes are
/// changed in one instance, as those changes will unexpectedly affect all
/// other instances.
///
/// ## Examples:
/// Instead of sharing mutable defaults, use the `field(default_factory=...)`
/// pattern.
///
/// ## Examples
/// ```python
/// from dataclasses import dataclass
///
Expand All @@ -40,26 +42,49 @@ use crate::checkers::ast::Checker;
/// class A:
/// mutable_default: list[int] = field(default_factory=list)
/// ```
#[violation]
pub struct MutableDataclassDefault;

impl Violation for MutableDataclassDefault {
#[derive_message_formats]
fn message(&self) -> String {
format!("Do not use mutable default values for dataclass attributes")
}
}

/// ## What it does
/// Checks for mutable default values in class attributes.
///
/// ## Why is this bad?
/// Mutable default values share state across all instances of the class,
/// while not being obvious. This can lead to bugs when the attributes are
/// changed in one instance, as those changes will unexpectedly affect all
/// other instances.
///
/// When mutable value are intended, they should be annotated with
/// `typing.ClassVar`.
///
/// Alternatively, if you _want_ shared behaviour, make it more obvious
/// by assigning to a module-level variable:
/// ## Examples
/// ```python
/// from dataclasses import dataclass
/// class A:
/// mutable_default: list[int] = []
/// ```
///
/// I_KNOW_THIS_IS_SHARED_STATE = [1, 2, 3, 4]
/// Use instead:
/// ```python
/// from typing import ClassVar
///
///
/// @dataclass
/// class A:
/// mutable_default: list[int] = I_KNOW_THIS_IS_SHARED_STATE
/// mutable_default: ClassVar[list[int]] = []
/// ```
#[violation]
pub struct MutableDataclassDefault;
pub struct MutableClassDefault;

impl Violation for MutableDataclassDefault {
impl Violation for MutableClassDefault {
#[derive_message_formats]
fn message(&self) -> String {
format!("Do not use mutable default values for dataclass attributes")
format!("Do not use mutable default values for class attributes")
}
}

Expand All @@ -73,7 +98,7 @@ impl Violation for MutableDataclassDefault {
/// ## Options
/// - `flake8-bugbear.extend-immutable-calls`
///
/// ## Examples:
/// ## Examples
/// ```python
/// from dataclasses import dataclass
///
Expand Down Expand Up @@ -214,8 +239,15 @@ pub(crate) fn function_call_in_dataclass_defaults(checker: &mut Checker, body: &
}
}

/// RUF008
pub(crate) fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) {
/// RUF008/RUF012
pub(crate) fn mutable_class_default(checker: &mut Checker, body: &[Stmt], is_dataclass: bool) {
fn diagnostic(is_dataclass: bool, value: &Expr) -> Diagnostic {
if is_dataclass {
Diagnostic::new(MutableDataclassDefault, value.range())
} else {
Diagnostic::new(MutableClassDefault, value.range())
}
}
for statement in body {
match statement {
Stmt::AnnAssign(ast::StmtAnnAssign {
Expand All @@ -227,16 +259,12 @@ pub(crate) fn mutable_dataclass_default(checker: &mut Checker, body: &[Stmt]) {
&& !is_immutable_annotation(checker.semantic_model(), annotation)
&& is_mutable_expr(value)
{
checker
.diagnostics
.push(Diagnostic::new(MutableDataclassDefault, value.range()));
checker.diagnostics.push(diagnostic(is_dataclass, value));
}
}
Stmt::Assign(ast::StmtAssign { value, .. }) => {
if is_mutable_expr(value) {
checker
.diagnostics
.push(Diagnostic::new(MutableDataclassDefault, value.range()));
checker.diagnostics.push(diagnostic(is_dataclass, value));
}
}
_ => (),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
source: crates/ruff/src/rules/ruff/mod.rs
---
RUF012.py:8:34: RUF012 Do not use mutable default values for class attributes
|
7 | class A:
8 | mutable_default: list[int] = []
| ^^ RUF012
9 | immutable_annotation: typing.Sequence[int] = []
10 | without_annotation = []
|

RUF012.py:10:26: RUF012 Do not use mutable default values for class attributes
|
8 | mutable_default: list[int] = []
9 | immutable_annotation: typing.Sequence[int] = []
10 | without_annotation = []
| ^^ RUF012
11 | ignored_via_comment: list[int] = [] # noqa: RUF012
12 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
|

RUF012.py:17:34: RUF012 Do not use mutable default values for class attributes
|
16 | class B:
17 | mutable_default: list[int] = []
| ^^ RUF012
18 | immutable_annotation: Sequence[int] = []
19 | without_annotation = []
|

RUF012.py:19:26: RUF012 Do not use mutable default values for class attributes
|
17 | mutable_default: list[int] = []
18 | immutable_annotation: Sequence[int] = []
19 | without_annotation = []
| ^^ RUF012
20 | ignored_via_comment: list[int] = [] # noqa: RUF012
21 | correct_code: list[int] = KNOWINGLY_MUTABLE_DEFAULT
|


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.