Skip to content

Commit be5db3c

Browse files
Bortlesboatclaudeamyreese
authored
[flake8-bugbear] Implement delattr-with-constant (B043) (#23737)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Amethyst Reese <amethyst@n7.gg>
1 parent fbfd873 commit be5db3c

8 files changed

Lines changed: 337 additions & 0 deletions

File tree

‎crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B043.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
"""
2+
Should emit:
3+
B043 - Lines 16-20
4+
"""
5+
6+
# Valid delattr usage
7+
delattr(foo, bar)
8+
delattr(foo, "bar{foo}".format(foo="a"))
9+
delattr(foo, "123abc")
10+
delattr(foo, "__123abc")
11+
delattr(foo, r"123\abc")
12+
delattr(foo, "except")
13+
_ = lambda x: delattr(x, "bar")
14+
if delattr(x, "bar"):
15+
pass
16+
17+
# Invalid usage
18+
delattr(foo, "bar")
19+
delattr(foo, "_123abc")
20+
delattr(foo, "__123abc__")
21+
delattr(foo, "abc123")
22+
delattr(foo, r"abc123")
23+
24+
# Starred argument
25+
delattr(*foo, "bar")
26+
27+
# Non-NFKC attribute name (unsafe fix)
28+
delattr(foo, "\u017f")
29+
30+
# Comment in expression (unsafe fix)
31+
delattr(
32+
obj,
33+
# text
34+
"foo",
35+
)
36+
37+
import builtins
38+
builtins.delattr(foo, "bar")

‎crates/ruff_linter/src/checkers/ast/analyze/expression.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -739,6 +739,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
739739
if checker.is_rule_enabled(Rule::GetAttrWithConstant) {
740740
flake8_bugbear::rules::getattr_with_constant(checker, expr, func, args);
741741
}
742+
if checker.is_rule_enabled(Rule::DelAttrWithConstant) {
743+
flake8_bugbear::rules::delattr_with_constant(checker, expr, func, args);
744+
}
742745
if checker.is_rule_enabled(Rule::SetAttrWithConstant) {
743746
flake8_bugbear::rules::setattr_with_constant(checker, expr, func, args);
744747
}

‎crates/ruff_linter/src/codes.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
393393
(Flake8Bugbear, "034") => rules::flake8_bugbear::rules::ReSubPositionalArgs,
394394
(Flake8Bugbear, "035") => rules::flake8_bugbear::rules::StaticKeyDictComprehension,
395395
(Flake8Bugbear, "039") => rules::flake8_bugbear::rules::MutableContextvarDefault,
396+
(Flake8Bugbear, "043") => rules::flake8_bugbear::rules::DelAttrWithConstant,
396397
(Flake8Bugbear, "901") => rules::flake8_bugbear::rules::ReturnInGenerator,
397398
(Flake8Bugbear, "903") => rules::flake8_bugbear::rules::ClassAsDataStructure,
398399
(Flake8Bugbear, "904") => rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept,

‎crates/ruff_linter/src/rules/flake8_bugbear/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ mod tests {
5757
#[test_case(Rule::ReSubPositionalArgs, Path::new("B034.py"))]
5858
#[test_case(Rule::RedundantTupleInExceptionHandler, Path::new("B013.py"))]
5959
#[test_case(Rule::ReuseOfGroupbyGenerator, Path::new("B031.py"))]
60+
#[test_case(Rule::DelAttrWithConstant, Path::new("B043.py"))]
6061
#[test_case(Rule::SetAttrWithConstant, Path::new("B009_B010.py"))]
6162
#[test_case(Rule::StarArgUnpackingAfterKeywordArg, Path::new("B026.py"))]
6263
#[test_case(Rule::StaticKeyDictComprehension, Path::new("B035.py"))]

‎crates/ruff_linter/src/rules/flake8_bugbear/rules/delattr_with_constant.rs‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
use ruff_python_ast::{self as ast, Expr, ExprContext, Identifier, Stmt};
2+
use ruff_text_size::{Ranged, TextRange};
3+
4+
use ruff_macros::{ViolationMetadata, derive_message_formats};
5+
use ruff_python_codegen::Generator;
6+
use ruff_python_stdlib::identifiers::{is_identifier, is_mangled_private};
7+
use unicode_normalization::UnicodeNormalization;
8+
9+
use crate::checkers::ast::Checker;
10+
use crate::{AlwaysFixableViolation, Applicability, Edit, Fix};
11+
12+
/// ## What it does
13+
/// Checks for uses of `delattr` that take a constant attribute value as an
14+
/// argument (e.g., `delattr(obj, "foo")`).
15+
///
16+
/// ## Why is this bad?
17+
/// `delattr` is used to delete attributes dynamically. If the attribute is
18+
/// defined as a constant, it is no safer than a typical property deletion.
19+
/// When possible, prefer `del` statements over `delattr` calls, as the
20+
/// former is more concise and idiomatic.
21+
///
22+
/// ## Example
23+
/// ```python
24+
/// delattr(obj, "foo")
25+
/// ```
26+
///
27+
/// Use instead:
28+
/// ```python
29+
/// del obj.foo
30+
/// ```
31+
///
32+
/// ## Fix safety
33+
/// The fix is marked as unsafe for attribute names that are not in NFKC
34+
/// (Normalization Form KC) normalization. Python normalizes identifiers using
35+
/// NFKC when using attribute access syntax (e.g., `del obj.attr`), but does
36+
/// not normalize string arguments passed to `delattr`. Rewriting
37+
/// `delattr(obj, "ſ")` to `del obj.ſ` would be interpreted as `del obj.s`
38+
/// at runtime, changing behavior.
39+
///
40+
/// Additionally, the fix is marked as unsafe if the expression contains
41+
/// comments, as the replacement may remove comments attached to the original
42+
/// `delattr` call.
43+
///
44+
/// ## References
45+
/// - [Python documentation: `delattr`](https://docs.python.org/3/library/functions.html#delattr)
46+
#[derive(ViolationMetadata)]
47+
#[violation_metadata(preview_since = "NEXT_RUFF_VERSION")]
48+
pub(crate) struct DelAttrWithConstant;
49+
50+
impl AlwaysFixableViolation for DelAttrWithConstant {
51+
#[derive_message_formats]
52+
fn message(&self) -> String {
53+
"Do not call `delattr` with a constant attribute value. It is not any safer than \
54+
normal property deletion."
55+
.to_string()
56+
}
57+
58+
fn fix_title(&self) -> String {
59+
"Replace `delattr` with `del` statement".to_string()
60+
}
61+
}
62+
63+
/// B043
64+
pub(crate) fn delattr_with_constant(checker: &Checker, expr: &Expr, func: &Expr, args: &[Expr]) {
65+
if !checker.semantic().match_builtin_expr(func, "delattr") {
66+
return;
67+
}
68+
69+
let [
70+
obj,
71+
Expr::StringLiteral(ast::ExprStringLiteral { value: name, .. }),
72+
] = args
73+
else {
74+
return;
75+
};
76+
if obj.is_starred_expr() {
77+
return;
78+
}
79+
80+
let attr_name = name.to_str();
81+
82+
// Ignore if the attribute name is `__debug__` (syntax error), not a valid
83+
// identifier (eg, keywords), or otherwise a name that would be mangled.
84+
if !is_identifier(attr_name) || attr_name == "__debug__" || is_mangled_private(attr_name) {
85+
return;
86+
}
87+
88+
let has_comments = checker.comment_ranges().intersects(expr.range());
89+
let applicability = if attr_name.nfkc().collect::<String>() != attr_name || has_comments {
90+
Applicability::Unsafe
91+
} else {
92+
Applicability::Safe
93+
};
94+
95+
// We can only replace a `delattr` call (which is an `Expr`) with a `del`
96+
// statement (which is a `Stmt`) if the `Expr` is already being used as a
97+
// `Stmt` (i.e., it's directly within an `Stmt::Expr`).
98+
if let Stmt::Expr(ast::StmtExpr {
99+
value: child,
100+
range: _,
101+
node_index: _,
102+
}) = checker.semantic().current_statement()
103+
{
104+
if expr == child.as_ref() {
105+
let mut diagnostic = checker.report_diagnostic(DelAttrWithConstant, expr.range());
106+
let edit = Edit::range_replacement(
107+
generate_del_statement(obj, attr_name, checker.generator()),
108+
expr.range(),
109+
);
110+
diagnostic.set_fix(Fix::applicable_edit(edit, applicability));
111+
}
112+
}
113+
}
114+
115+
fn generate_del_statement(obj: &Expr, attr_name: &str, generator: Generator) -> String {
116+
let stmt = Stmt::Delete(ast::StmtDelete {
117+
targets: vec![Expr::Attribute(ast::ExprAttribute {
118+
value: Box::new(obj.clone()),
119+
attr: Identifier::new(attr_name.to_string(), TextRange::default()),
120+
ctx: ExprContext::Del,
121+
range: TextRange::default(),
122+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
123+
})],
124+
range: TextRange::default(),
125+
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
126+
});
127+
generator.stmt(&stmt)
128+
}

‎crates/ruff_linter/src/rules/flake8_bugbear/rules/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ pub(crate) use assignment_to_os_environ::*;
55
pub(crate) use batched_without_explicit_strict::*;
66
pub(crate) use cached_instance_method::*;
77
pub(crate) use class_as_data_structure::*;
8+
pub(crate) use delattr_with_constant::*;
89
pub(crate) use duplicate_exceptions::*;
910
pub(crate) use duplicate_value::*;
1011
pub(crate) use except_with_empty_tuple::*;
@@ -46,6 +47,7 @@ mod assignment_to_os_environ;
4647
mod batched_without_explicit_strict;
4748
mod cached_instance_method;
4849
mod class_as_data_structure;
50+
mod delattr_with_constant;
4951
mod duplicate_exceptions;
5052
mod duplicate_value;
5153
mod except_with_empty_tuple;

‎crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B043_B043.py.snap‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
---
2+
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
3+
---
4+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
5+
--> B043.py:18:1
6+
|
7+
17 | # Invalid usage
8+
18 | delattr(foo, "bar")
9+
| ^^^^^^^^^^^^^^^^^^^
10+
19 | delattr(foo, "_123abc")
11+
20 | delattr(foo, "__123abc__")
12+
|
13+
help: Replace `delattr` with `del` statement
14+
15 | pass
15+
16 |
16+
17 | # Invalid usage
17+
- delattr(foo, "bar")
18+
18 + del foo.bar
19+
19 | delattr(foo, "_123abc")
20+
20 | delattr(foo, "__123abc__")
21+
21 | delattr(foo, "abc123")
22+
23+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
24+
--> B043.py:19:1
25+
|
26+
17 | # Invalid usage
27+
18 | delattr(foo, "bar")
28+
19 | delattr(foo, "_123abc")
29+
| ^^^^^^^^^^^^^^^^^^^^^^^
30+
20 | delattr(foo, "__123abc__")
31+
21 | delattr(foo, "abc123")
32+
|
33+
help: Replace `delattr` with `del` statement
34+
16 |
35+
17 | # Invalid usage
36+
18 | delattr(foo, "bar")
37+
- delattr(foo, "_123abc")
38+
19 + del foo._123abc
39+
20 | delattr(foo, "__123abc__")
40+
21 | delattr(foo, "abc123")
41+
22 | delattr(foo, r"abc123")
42+
43+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
44+
--> B043.py:20:1
45+
|
46+
18 | delattr(foo, "bar")
47+
19 | delattr(foo, "_123abc")
48+
20 | delattr(foo, "__123abc__")
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
50+
21 | delattr(foo, "abc123")
51+
22 | delattr(foo, r"abc123")
52+
|
53+
help: Replace `delattr` with `del` statement
54+
17 | # Invalid usage
55+
18 | delattr(foo, "bar")
56+
19 | delattr(foo, "_123abc")
57+
- delattr(foo, "__123abc__")
58+
20 + del foo.__123abc__
59+
21 | delattr(foo, "abc123")
60+
22 | delattr(foo, r"abc123")
61+
23 |
62+
63+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
64+
--> B043.py:21:1
65+
|
66+
19 | delattr(foo, "_123abc")
67+
20 | delattr(foo, "__123abc__")
68+
21 | delattr(foo, "abc123")
69+
| ^^^^^^^^^^^^^^^^^^^^^^
70+
22 | delattr(foo, r"abc123")
71+
|
72+
help: Replace `delattr` with `del` statement
73+
18 | delattr(foo, "bar")
74+
19 | delattr(foo, "_123abc")
75+
20 | delattr(foo, "__123abc__")
76+
- delattr(foo, "abc123")
77+
21 + del foo.abc123
78+
22 | delattr(foo, r"abc123")
79+
23 |
80+
24 | # Starred argument
81+
82+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
83+
--> B043.py:22:1
84+
|
85+
20 | delattr(foo, "__123abc__")
86+
21 | delattr(foo, "abc123")
87+
22 | delattr(foo, r"abc123")
88+
| ^^^^^^^^^^^^^^^^^^^^^^^
89+
23 |
90+
24 | # Starred argument
91+
|
92+
help: Replace `delattr` with `del` statement
93+
19 | delattr(foo, "_123abc")
94+
20 | delattr(foo, "__123abc__")
95+
21 | delattr(foo, "abc123")
96+
- delattr(foo, r"abc123")
97+
22 + del foo.abc123
98+
23 |
99+
24 | # Starred argument
100+
25 | delattr(*foo, "bar")
101+
102+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
103+
--> B043.py:28:1
104+
|
105+
27 | # Non-NFKC attribute name (unsafe fix)
106+
28 | delattr(foo, "\u017f")
107+
| ^^^^^^^^^^^^^^^^^^^^^^
108+
29 |
109+
30 | # Comment in expression (unsafe fix)
110+
|
111+
help: Replace `delattr` with `del` statement
112+
25 | delattr(*foo, "bar")
113+
26 |
114+
27 | # Non-NFKC attribute name (unsafe fix)
115+
- delattr(foo, "\u017f")
116+
28 + del foo.ſ
117+
29 |
118+
30 | # Comment in expression (unsafe fix)
119+
31 | delattr(
120+
note: This is an unsafe fix and may change runtime behavior
121+
122+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
123+
--> B043.py:31:1
124+
|
125+
30 | # Comment in expression (unsafe fix)
126+
31 | / delattr(
127+
32 | | obj,
128+
33 | | # text
129+
34 | | "foo",
130+
35 | | )
131+
| |_^
132+
36 |
133+
37 | import builtins
134+
|
135+
help: Replace `delattr` with `del` statement
136+
28 | delattr(foo, "\u017f")
137+
29 |
138+
30 | # Comment in expression (unsafe fix)
139+
- delattr(
140+
- obj,
141+
- # text
142+
- "foo",
143+
- )
144+
31 + del obj.foo
145+
32 |
146+
33 | import builtins
147+
34 | builtins.delattr(foo, "bar")
148+
note: This is an unsafe fix and may change runtime behavior
149+
150+
B043 [*] Do not call `delattr` with a constant attribute value. It is not any safer than normal property deletion.
151+
--> B043.py:38:1
152+
|
153+
37 | import builtins
154+
38 | builtins.delattr(foo, "bar")
155+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
156+
|
157+
help: Replace `delattr` with `del` statement
158+
35 | )
159+
36 |
160+
37 | import builtins
161+
- builtins.delattr(foo, "bar")
162+
38 + del foo.bar

‎ruff.schema.json‎

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
⚡