mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-26 20:10:09 +00:00
[flake8-bugbear
] Exempt NewType
calls where the original type is immutable (B008
) (#15765)
## Summary Resolves #12717. This change incorporates the logic added in #15588. ## Test Plan `cargo nextest run` and `cargo insta test`. --------- Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
This commit is contained in:
parent
6090408f65
commit
4bec8ba731
5 changed files with 88 additions and 55 deletions
|
@ -35,3 +35,14 @@ def okay(obj=Class()):
|
||||||
|
|
||||||
def error(obj=OtherClass()):
|
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()): ...
|
||||||
|
|
|
@ -8,7 +8,7 @@ use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
|
||||||
use ruff_python_ast::visitor;
|
use ruff_python_ast::visitor;
|
||||||
use ruff_python_ast::visitor::Visitor;
|
use ruff_python_ast::visitor::Visitor;
|
||||||
use ruff_python_semantic::analyze::typing::{
|
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;
|
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
|
/// once, at definition time. The returned value will then be reused by all
|
||||||
/// calls to the function, which can lead to unexpected behaviour.
|
/// calls to the function, which can lead to unexpected behaviour.
|
||||||
///
|
///
|
||||||
/// Calls can be marked as an exception to this rule with the
|
/// Parameters with immutable type annotations will be ignored by this rule.
|
||||||
/// [`lint.flake8-bugbear.extend-immutable-calls`] configuration option.
|
/// 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.
|
/// Calls and types outside of the standard library can be marked as an exception
|
||||||
/// Types outside of the standard library can be marked as immutable with the
|
/// to this rule with the [`lint.flake8-bugbear.extend-immutable-calls`] configuration option.
|
||||||
/// [`lint.flake8-bugbear.extend-immutable-calls`] configuration option as well.
|
|
||||||
///
|
///
|
||||||
/// ## Example
|
/// ## Example
|
||||||
///
|
///
|
||||||
|
@ -105,6 +105,9 @@ impl Visitor<'_> for ArgumentDefaultVisitor<'_, '_> {
|
||||||
Expr::Call(ast::ExprCall { func, .. }) => {
|
Expr::Call(ast::ExprCall { func, .. }) => {
|
||||||
if !is_mutable_func(func, self.semantic)
|
if !is_mutable_func(func, self.semantic)
|
||||||
&& !is_immutable_func(func, self.semantic, self.extend_immutable_calls)
|
&& !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((
|
self.diagnostics.push((
|
||||||
FunctionCallInDefaultArgument {
|
FunctionCallInDefaultArgument {
|
||||||
|
|
|
@ -1,6 +1,5 @@
|
||||||
---
|
---
|
||||||
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
|
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
|
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
|
| ^^^^^^^^^^^^ B008
|
||||||
37 | ...
|
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
|
||||||
|
|
|
||||||
|
|
|
@ -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_diagnostics::{Diagnostic, Violation};
|
||||||
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
||||||
use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
|
use ruff_python_ast::name::{QualifiedName, UnqualifiedName};
|
||||||
use ruff_python_semantic::analyze::typing::{is_immutable_annotation, is_immutable_func};
|
use ruff_python_semantic::analyze::typing::{is_immutable_func, is_immutable_newtype_call};
|
||||||
use ruff_python_semantic::SemanticModel;
|
|
||||||
use ruff_text_size::Ranged;
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
use crate::checkers::ast::Checker;
|
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
|
/// If a field needs to be initialized with a mutable object, use the
|
||||||
/// `field(default_factory=...)` pattern.
|
/// `field(default_factory=...)` pattern.
|
||||||
///
|
///
|
||||||
|
/// Attributes whose default arguments are `NewType` calls
|
||||||
|
/// where the original type is immutable are ignored.
|
||||||
|
///
|
||||||
/// ## Examples
|
/// ## Examples
|
||||||
/// ```python
|
/// ```python
|
||||||
/// from dataclasses import dataclass
|
/// from dataclasses import dataclass
|
||||||
|
@ -138,7 +140,9 @@ pub(crate) fn function_call_in_dataclass_default(
|
||||||
|| is_class_var_annotation(annotation, checker.semantic())
|
|| is_class_var_annotation(annotation, checker.semantic())
|
||||||
|| is_immutable_func(func, checker.semantic(), &extend_immutable_calls)
|
|| is_immutable_func(func, checker.semantic(), &extend_immutable_calls)
|
||||||
|| is_descriptor_class(func, checker.semantic())
|
|| 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;
|
continue;
|
||||||
}
|
}
|
||||||
|
@ -158,46 +162,3 @@ fn any_annotated(class_body: &[Stmt]) -> bool {
|
||||||
.iter()
|
.iter()
|
||||||
.any(|stmt| matches!(stmt, Stmt::AnnAssign(..)))
|
.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)
|
|
||||||
}
|
|
||||||
|
|
|
@ -2,7 +2,9 @@
|
||||||
|
|
||||||
use ruff_python_ast::helpers::{any_over_expr, is_const_false, map_subscript};
|
use ruff_python_ast::helpers::{any_over_expr, is_const_false, map_subscript};
|
||||||
use ruff_python_ast::name::QualifiedName;
|
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::{
|
use ruff_python_stdlib::typing::{
|
||||||
as_pep_585_generic, has_pep_585_generic, is_immutable_generic_type,
|
as_pep_585_generic, has_pep_585_generic, is_immutable_generic_type,
|
||||||
is_immutable_non_generic_type, is_immutable_return_type, is_literal_member,
|
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.
|
/// Return `true` if `func` is a function that returns a mutable value.
|
||||||
pub fn is_mutable_func(func: &Expr, semantic: &SemanticModel) -> bool {
|
pub fn is_mutable_func(func: &Expr, semantic: &SemanticModel) -> bool {
|
||||||
semantic
|
semantic
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue