diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py index 3562a5a989..84f05533ea 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B009_B010.py @@ -79,3 +79,10 @@ setattr(foo, "__debug__", 0) # Example: the long s character "ſ" normalizes to "s" under NFKC. getattr(foo, "ſ") setattr(foo, "ſ", 1) + + +getattr( + obj, + # text + "foo", +) \ No newline at end of file diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs index 9271d2b01a..cc5209bde0 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs +++ b/crates/ruff_linter/src/rules/flake8_bugbear/rules/getattr_with_constant.rs @@ -36,6 +36,9 @@ use crate::{AlwaysFixableViolation, Edit, Fix}; /// (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. /// +/// Additionally, the fix is marked as unsafe if the expression contains comments, +/// as the replacement may remove comments attached to the original `getattr` call. +/// /// For example, the long s character `"ſ"` normalizes to `"s"` under NFKC, so: /// ```python /// # This accesses an attribute with the exact name "ſ" (if it exists) @@ -89,7 +92,8 @@ pub(crate) fn getattr_with_constant(checker: &Checker, expr: &Expr, func: &Expr, // 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::() != attr_name; + let has_comments = checker.comment_ranges().intersects(expr.range()); + let is_unsafe = attr_name.nfkc().collect::() != attr_name || has_comments; let mut diagnostic = checker.report_diagnostic(GetAttrWithConstant, expr.range()); let edit = Edit::range_replacement( diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap index febc145dd7..58d09c27af 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap @@ -377,4 +377,28 @@ help: Replace `getattr` with attribute access - getattr(foo, "ſ") 80 + foo.ſ 81 | setattr(foo, "ſ", 1) +82 | +83 | +note: This is an unsafe fix and may change runtime behavior + +B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + --> B009_B010.py:84:1 + | +84 | / getattr( +85 | | obj, +86 | | # text +87 | | "foo", +88 | | ) + | |_^ + | +help: Replace `getattr` with attribute access +81 | setattr(foo, "ſ", 1) +82 | +83 | + - getattr( + - obj, + - # text + - "foo", + - ) +84 + obj.foo note: This is an unsafe fix and may change runtime behavior diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap index 8dab4338a1..3770c7f6a4 100644 --- a/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap +++ b/crates/ruff_linter/src/rules/flake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap @@ -133,4 +133,7 @@ help: Replace `setattr` with assignment 80 | getattr(foo, "ſ") - setattr(foo, "ſ", 1) 81 + foo.ſ = 1 +82 | +83 | +84 | getattr( note: This is an unsafe fix and may change runtime behavior