Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[flake8-bugbear] Mark fix as unsafe for non-NFKC attribute names (B009, B010) (#21131)

Browse Source
This commit is contained in:
Dan Parizher
2025-11-03 09:45:23 -05:00
committed by GitHub
parent e017b039df
commit 6ddfb51d71
5 changed files with 105 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py vendored
View File

@@ -70,3 +70,12 @@ builtins.getattr(foo, "bar")
# Regression test for: https://github.com/astral-sh/ruff/issues/18353
setattr(foo, "__debug__", 0)
# Regression test for: https://github.com/astral-sh/ruff/issues/21126
# Non-NFKC attribute names should be marked as unsafe. Python normalizes identifiers in
# attribute access (obj.attr) using NFKC, but does not normalize string
# arguments passed to getattr/setattr. Rewriting `getattr(ns, "ſ")` to
# `ns.ſ` would be interpreted as `ns.s` at runtime, changing behavior.
# Example: the long s character "ſ" normalizes to "s" under NFKC.
getattr(foo, "ſ")
setattr(foo, "ſ", 1)

32
crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs
View File

@@ -3,6 +3,7 @@ use ruff_python_ast::{self as ast, Expr};
use ruff_python_stdlib::identifiers::{is_identifier, is_mangled_private};
use ruff_source_file::LineRanges;
use ruff_text_size::Ranged;
use unicode_normalization::UnicodeNormalization;
use crate::checkers::ast::Checker;
use crate::fix::edits::pad;
@@ -29,6 +30,21 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
/// obj.foo
/// ```
///
/// ## Fix safety
/// The fix is marked as unsafe for attribute names that are not in NFKC (Normalization Form KC)
/// normalization. Python normalizes identifiers using NFKC when using attribute access syntax
/// (e.g., `obj.attr`), but does not normalize string arguments passed to `getattr`. Rewriting
/// `getattr(obj, "ſ")` to `obj.ſ` would be interpreted as `obj.s` at runtime, changing behavior.
///
/// For example, the long s character `"ſ"` normalizes to `"s"` under NFKC, so:
/// ```python
/// # This accesses an attribute with the exact name "ſ" (if it exists)
/// value = getattr(obj, "ſ")
///
/// # But this would normalize to "s" and access a different attribute
/// obj.ſ # This is interpreted as obj.s, not obj.ſ
/// ```
///
/// ## References
/// - [Python documentation: `getattr`](https://docs.python.org/3/library/functions.html#getattr)
#[derive(ViolationMetadata)]
@@ -69,8 +85,14 @@ pub(crate) fn getattr_with_constant(checker: &Checker, expr: &Expr, func: &Expr,
return;
}
// Mark fixes as unsafe for non-NFKC attribute names. Python normalizes identifiers using NFKC, so using
// attribute syntax (e.g., `obj.attr`) would normalize the name and potentially change
// program behavior.
let attr_name = value.to_str();
let is_unsafe = attr_name.nfkc().collect::<String>() != attr_name;
let mut diagnostic = checker.report_diagnostic(GetAttrWithConstant, expr.range());
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
let edit = Edit::range_replacement(
pad(
if matches!(
obj,
@@ -88,5 +110,11 @@ pub(crate) fn getattr_with_constant(checker: &Checker, expr: &Expr, func: &Expr,
checker.locator(),
),
expr.range(),
)));
);
let fix = if is_unsafe {
Fix::unsafe_edit(edit)
} else {
Fix::safe_edit(edit)
};
diagnostic.set_fix(fix);
}

34
crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs
View File

@@ -4,6 +4,7 @@ use ruff_text_size::{Ranged, TextRange};
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_codegen::Generator;
use ruff_python_stdlib::identifiers::{is_identifier, is_mangled_private};
use unicode_normalization::UnicodeNormalization;
use crate::checkers::ast::Checker;
use crate::{AlwaysFixableViolation, Edit, Fix};
@@ -28,6 +29,23 @@ use crate::{AlwaysFixableViolation, Edit, Fix};
/// obj.foo = 42
/// ```
///
/// ## Fix safety
/// The fix is marked as unsafe for attribute names that are not in NFKC (Normalization Form KC)
/// normalization. Python normalizes identifiers using NFKC when using attribute access syntax
/// (e.g., `obj.attr = value`), but does not normalize string arguments passed to `setattr`.
/// Rewriting `setattr(obj, "ſ", 1)` to `obj.ſ = 1` would be interpreted as `obj.s = 1` at
/// runtime, changing behavior.
///
/// For example, the long s character `"ſ"` normalizes to `"s"` under NFKC, so:
/// ```python
/// # This creates an attribute with the exact name "ſ"
/// setattr(obj, "ſ", 1)
/// getattr(obj, "ſ") # Returns 1
///
/// # But this would normalize to "s" and set a different attribute
/// obj.ſ = 1 # This is interpreted as obj.s = 1, not obj.ſ = 1
/// ```
///
/// ## References
/// - [Python documentation: `setattr`](https://docs.python.org/3/library/functions.html#setattr)
#[derive(ViolationMetadata)]
@@ -89,6 +107,12 @@ pub(crate) fn setattr_with_constant(checker: &Checker, expr: &Expr, func: &Expr,
return;
}
// Mark fixes as unsafe for non-NFKC attribute names. Python normalizes identifiers using NFKC, so using
// attribute syntax (e.g., `obj.attr = value`) would normalize the name and potentially change
// program behavior.
let attr_name = name.to_str();
let is_unsafe = attr_name.nfkc().collect::<String>() != attr_name;
// We can only replace a `setattr` call (which is an `Expr`) with an assignment
// (which is a `Stmt`) if the `Expr` is already being used as a `Stmt`
// (i.e., it's directly within an `Stmt::Expr`).
@@ -100,10 +124,16 @@ pub(crate) fn setattr_with_constant(checker: &Checker, expr: &Expr, func: &Expr,
{
if expr == child.as_ref() {
let mut diagnostic = checker.report_diagnostic(SetAttrWithConstant, expr.range());
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
let edit = Edit::range_replacement(
assignment(obj, name.to_str(), value, checker.generator()),
expr.range(),
)));
);
let fix = if is_unsafe {
Fix::unsafe_edit(edit)
} else {
Fix::safe_edit(edit)
};
diagnostic.set_fix(fix);
}
}
}

18
crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap
View File

@@ -360,3 +360,21 @@ help: Replace `getattr` with attribute access
70 |
71 | # Regression test for: https://github.com/astral-sh/ruff/issues/18353
72 | setattr(foo, "__debug__", 0)
B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
--> B009_B010.py:80:1
|
78 | # `ns.ſ` would be interpreted as `ns.s` at runtime, changing behavior.
79 | # Example: the long s character "ſ" normalizes to "s" under NFKC.
80 | getattr(foo, "ſ")
| ^^^^^^^^^^^^^^^^^
81 | setattr(foo, "ſ", 1)
|
help: Replace `getattr` with attribute access
77 | # arguments passed to getattr/setattr. Rewriting `getattr(ns, "ſ")` to
78 | # `ns.ſ` would be interpreted as `ns.s` at runtime, changing behavior.
79 | # Example: the long s character "ſ" normalizes to "s" under NFKC.
- getattr(foo, "ſ")
80 + foo.ſ
81 | setattr(foo, "ſ", 1)
note: This is an unsafe fix and may change runtime behavior

16
crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap
View File

@@ -118,3 +118,19 @@ help: Replace `setattr` with assignment
56 |
57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
58 | assert getattr(func, '_rpc')is True
B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
--> B009_B010.py:81:1
|
79 | # Example: the long s character "ſ" normalizes to "s" under NFKC.
80 | getattr(foo, "ſ")
81 | setattr(foo, "ſ", 1)
| ^^^^^^^^^^^^^^^^^^^^
|
help: Replace `setattr` with assignment
78 | # `ns.ſ` would be interpreted as `ns.s` at runtime, changing behavior.
79 | # Example: the long s character "ſ" normalizes to "s" under NFKC.
80 | getattr(foo, "ſ")
- setattr(foo, "ſ", 1)
81 + foo.ſ = 1
note: This is an unsafe fix and may change runtime behavior