From 90c259beb9244e49bec37cb6c27960d1001b04cd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 3 Oct 2023 12:51:25 -0400 Subject: [PATCH] Respect `msgspec.Struct` default-copy semantics (#7786) ## Summary The carve-out we have in `RUF012` for Pydantic classes also applies to `msgspec.Struct`. Closes https://github.com/astral-sh/ruff/issues/7785. --- .../resources/test/fixtures/ruff/RUF012.py | 11 +++++++++++ crates/ruff_linter/src/rules/ruff/rules/helpers.rs | 12 +++++++++--- .../src/rules/ruff/rules/mutable_class_default.rs | 10 +++++----- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py index 9f7349f740ae40..ef58e45df1cb48 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py @@ -37,3 +37,14 @@ class D(BaseModel): without_annotation = [] class_variable: ClassVar[list[int]] = [] final_variable: Final[list[int]] = [] + + +from msgspec import Struct + + +class E(Struct): + mutable_default: list[int] = [] + immutable_annotation: Sequence[int] = [] + without_annotation = [] + class_variable: ClassVar[list[int]] = [] + final_variable: Final[list[int]] = [] diff --git a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs index 8ccb0703769047..78caf8d18fb5d6 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/helpers.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/helpers.rs @@ -49,8 +49,14 @@ pub(super) fn is_dataclass(class_def: &ast::StmtClassDef, semantic: &SemanticMod }) } -/// Returns `true` if the given class is a Pydantic `BaseModel` or `BaseSettings` subclass. -pub(super) fn is_pydantic_model(class_def: &ast::StmtClassDef, semantic: &SemanticModel) -> bool { +/// Returns `true` if the given class has "default copy" semantics. +/// +/// For example, Pydantic `BaseModel` and `BaseSettings` subclassses copy attribute defaults on +/// instance creation. As such, the use of mutable default values is safe for such classes. +pub(super) fn has_default_copy_semantics( + class_def: &ast::StmtClassDef, + semantic: &SemanticModel, +) -> bool { let Some(Arguments { args: bases, .. }) = class_def.arguments.as_deref() else { return false; }; @@ -59,7 +65,7 @@ pub(super) fn is_pydantic_model(class_def: &ast::StmtClassDef, semantic: &Semant semantic.resolve_call_path(expr).is_some_and(|call_path| { matches!( call_path.as_slice(), - ["pydantic", "BaseModel" | "BaseSettings"] + ["pydantic", "BaseModel" | "BaseSettings"] | ["msgspec", "Struct"] ) }) }) diff --git a/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs b/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs index 85ac38c5fd2083..da1e2837512a48 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs @@ -7,7 +7,7 @@ use ruff_text_size::Ranged; use crate::checkers::ast::Checker; use crate::rules::ruff::rules::helpers::{ - is_class_var_annotation, is_dataclass, is_final_annotation, is_pydantic_model, + has_default_copy_semantics, is_class_var_annotation, is_dataclass, is_final_annotation, is_special_attribute, }; @@ -64,8 +64,8 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt && !is_immutable_annotation(annotation, checker.semantic(), &[]) && !is_dataclass(class_def, checker.semantic()) { - // Avoid Pydantic models, which end up copying defaults on instance creation. - if is_pydantic_model(class_def, checker.semantic()) { + // Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation. + if has_default_copy_semantics(class_def, checker.semantic()) { return; } @@ -78,8 +78,8 @@ pub(crate) fn mutable_class_default(checker: &mut Checker, class_def: &ast::Stmt if !targets.iter().all(is_special_attribute) && is_mutable_expr(value, checker.semantic()) { - // Avoid Pydantic models, which end up copying defaults on instance creation. - if is_pydantic_model(class_def, checker.semantic()) { + // Avoid, e.g., Pydantic and msgspec models, which end up copying defaults on instance creation. + if has_default_copy_semantics(class_def, checker.semantic()) { return; }