Skip to content

Commit

Permalink
Respect msgspec.Struct default-copy semantics (#7786)
Browse files Browse the repository at this point in the history
## Summary

The carve-out we have in `RUF012` for Pydantic classes also applies to
`msgspec.Struct`.

Closes #7785.
  • Loading branch information
charliermarsh committed Oct 3, 2023
1 parent 37d21c0 commit 90c259b
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 8 deletions.
11 changes: 11 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF012.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]] = []
12 changes: 9 additions & 3 deletions crates/ruff_linter/src/rules/ruff/rules/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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"]
)
})
})
Expand Down
10 changes: 5 additions & 5 deletions crates/ruff_linter/src/rules/ruff/rules/mutable_class_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down

0 comments on commit 90c259b

Please sign in to comment.