diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B008_extended.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B008_extended.py index a9a6f14963..0d7175daa0 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B008_extended.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B008_extended.py @@ -35,3 +35,14 @@ def okay(obj=Class()): def error(obj=OtherClass()): ... + + +# https://github.com/astral-sh/ruff/issues/12717 + +from typing import NewType + +N = NewType("N", int) +L = NewType("L", list[str]) + +def okay(obj = N()): ... +def error(obj = L()): ... diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs index ab20364ac9..4db64c4438 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/function_call_in_argument_default.rs @@ -8,7 +8,7 @@ use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; use ruff_python_semantic::analyze::typing::{ - is_immutable_annotation, is_immutable_func, is_mutable_func, + is_immutable_annotation, is_immutable_func, is_immutable_newtype_call, is_mutable_func, }; use ruff_python_semantic::SemanticModel; @@ -22,12 +22,12 @@ use crate::checkers::ast::Checker; /// once, at definition time. The returned value will then be reused by all /// calls to the function, which can lead to unexpected behaviour. /// -/// Calls can be marked as an exception to this rule with the -/// [`lint.flake8-bugbear.extend-immutable-calls`] configuration option. +/// Parameters with immutable type annotations will be ignored by this rule. +/// Those whose default arguments are `NewType` calls where the original type +/// is immutable are also ignored. /// -/// Arguments with immutable type annotations will be ignored by this rule. -/// Types outside of the standard library can be marked as immutable with the -/// [`lint.flake8-bugbear.extend-immutable-calls`] configuration option as well. +/// Calls and types outside of the standard library can be marked as an exception +/// to this rule with the [`lint.flake8-bugbear.extend-immutable-calls`] configuration option. /// /// ## Example /// @@ -105,6 +105,9 @@ impl Visitor<'_> for ArgumentDefaultVisitor<'_, '_> { Expr::Call(ast::ExprCall { func, .. }) => { if !is_mutable_func(func, self.semantic) && !is_immutable_func(func, self.semantic, self.extend_immutable_calls) + && !func.as_name_expr().is_some_and(|name| { + is_immutable_newtype_call(name, self.semantic, self.extend_immutable_calls) + }) { self.diagnostics.push(( FunctionCallInDefaultArgument { diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_immutable_calls_arg_default.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_immutable_calls_arg_default.snap index bf2ac94ea0..611467e0f2 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_immutable_calls_arg_default.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__extend_immutable_calls_arg_default.snap @@ -1,6 +1,5 @@ --- source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs -snapshot_kind: text --- B008_extended.py:24:51: B008 Do not perform function call `Depends` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable | @@ -15,3 +14,10 @@ B008_extended.py:36:15: B008 Do not perform function call `OtherClass` in argume | ^^^^^^^^^^^^ B008 37 | ... | + +B008_extended.py:48:17: B008 Do not perform function call `L` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable + | +47 | def okay(obj = N()): ... +48 | def error(obj = L()): ... + | ^^^ B008 + | diff --git a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs index 502f448afe..60efdddfc3 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/function_call_in_dataclass_default.rs @@ -1,10 +1,9 @@ -use ruff_python_ast::{self as ast, Expr, ExprCall, Stmt, StmtAssign}; +use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, ViolationMetadata}; use ruff_python_ast::name::{QualifiedName, UnqualifiedName}; -use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_immutable_func}; -use ruff_python_semantic::SemanticModel; +use ruff_python_semantic::analyze::typing::{is_immutable_func, is_immutable_newtype_call}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -25,6 +24,9 @@ use crate::rules::ruff::rules::helpers::{ /// If a field needs to be initialized with a mutable object, use the /// `field(default_factory=...)` pattern. /// +/// Attributes whose default arguments are `NewType` calls +/// where the original type is immutable are ignored. +/// /// ## Examples /// ```python /// from dataclasses import dataclass @@ -138,7 +140,9 @@ pub(crate) fn function_call_in_dataclass_default( || is_class_var_annotation(annotation, checker.semantic()) || is_immutable_func(func, checker.semantic(), &extend_immutable_calls) || is_descriptor_class(func, checker.semantic()) - || is_immutable_newtype_call(func, checker.semantic(), &extend_immutable_calls) + || func.as_name_expr().is_some_and(|name| { + is_immutable_newtype_call(name, checker.semantic(), &extend_immutable_calls) + }) { continue; } @@ -158,46 +162,3 @@ fn any_annotated(class_body: &[Stmt]) -> bool { .iter() .any(|stmt| matches!(stmt, Stmt::AnnAssign(..))) } - -fn is_immutable_newtype_call( - func: &Expr, - semantic: &SemanticModel, - extend_immutable_calls: &[QualifiedName], -) -> bool { - let Expr::Name(name) = func else { - return false; - }; - - let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else { - return false; - }; - - if !binding.kind.is_assignment() { - return false; - } - - let Some(Stmt::Assign(StmtAssign { value, .. })) = binding.statement(semantic) else { - return false; - }; - - let Expr::Call(ExprCall { - func, arguments, .. - }) = value.as_ref() - else { - return false; - }; - - if !semantic.match_typing_expr(func, "NewType") { - return false; - } - - if arguments.len() != 2 { - return false; - } - - let Some(original_type) = arguments.find_argument_value("tp", 1) else { - return false; - }; - - is_immutable_annotation(original_type, semantic, extend_immutable_calls) -} diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 3ecea1a1ea..f8ba9296b8 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -2,7 +2,9 @@ use ruff_python_ast::helpers::{any_over_expr, is_const_false, map_subscript}; use ruff_python_ast::name::QualifiedName; -use ruff_python_ast::{self as ast, Expr, Int, Operator, ParameterWithDefault, Parameters, Stmt}; +use ruff_python_ast::{ + self as ast, Expr, ExprCall, Int, Operator, ParameterWithDefault, Parameters, Stmt, StmtAssign, +}; use ruff_python_stdlib::typing::{ as_pep_585_generic, has_pep_585_generic, is_immutable_generic_type, is_immutable_non_generic_type, is_immutable_return_type, is_literal_member, @@ -301,6 +303,56 @@ pub fn is_immutable_func( }) } +/// Return `true` if `name` is bound to the `typing.NewType` call where the original type is +/// immutable. +/// +/// For example: +/// ```python +/// from typing import NewType +/// +/// UserId = NewType("UserId", int) +/// ``` +/// +/// Here, `name` would be `UserId`. +pub fn is_immutable_newtype_call( + name: &ast::ExprName, + semantic: &SemanticModel, + extend_immutable_calls: &[QualifiedName], +) -> bool { + let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else { + return false; + }; + + if !binding.kind.is_assignment() { + return false; + } + + let Some(Stmt::Assign(StmtAssign { value, .. })) = binding.statement(semantic) else { + return false; + }; + + let Expr::Call(ExprCall { + func, arguments, .. + }) = value.as_ref() + else { + return false; + }; + + if !semantic.match_typing_expr(func, "NewType") { + return false; + } + + if arguments.len() != 2 { + return false; + } + + let Some(original_type) = arguments.find_argument_value("tp", 1) else { + return false; + }; + + is_immutable_annotation(original_type, semantic, extend_immutable_calls) +} + /// Return `true` if `func` is a function that returns a mutable value. pub fn is_mutable_func(func: &Expr, semantic: &SemanticModel) -> bool { semantic