From 71ff6f911dab2a9c1a79f878af2cecce8d54a33e Mon Sep 17 00:00:00 2001 From: Justin Prieto Date: Sat, 2 Sep 2023 07:45:35 -0400 Subject: [PATCH] Fix `getattr` calls on int literals (#7057) --- .../test/fixtures/flake8_bugbear/B009_B010.py | 10 +- .../rules/getattr_with_constant.rs | 17 +- ...ke8_bugbear__tests__B009_B009_B010.py.snap | 109 ++++++++++++- ...ke8_bugbear__tests__B010_B009_B010.py.snap | 148 +++++++++--------- 4 files changed, 202 insertions(+), 82 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_bugbear/B009_B010.py b/crates/ruff/resources/test/fixtures/flake8_bugbear/B009_B010.py index 623ddc6c96..b5a8450e90 100644 --- a/crates/ruff/resources/test/fixtures/flake8_bugbear/B009_B010.py +++ b/crates/ruff/resources/test/fixtures/flake8_bugbear/B009_B010.py @@ -1,7 +1,7 @@ """ Should emit: -B009 - Line 19, 20, 21, 22, 23, 24 -B010 - Line 40, 41, 42, 43, 44, 45 +B009 - Lines 19-31 +B010 - Lines 40-45 """ # Valid getattr usage @@ -24,6 +24,12 @@ getattr(foo, r"abc123") _ = lambda x: getattr(x, "bar") if getattr(x, "bar"): pass +getattr(1, "real") +getattr(1., "real") +getattr(1.0, "real") +getattr(1j, "real") +getattr(True, "real") + # Valid setattr usage setattr(foo, bar, None) diff --git a/crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs b/crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs index 572bc139e8..3c4bcf6cd9 100644 --- a/crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs +++ b/crates/ruff/src/rules/flake8_bugbear/rules/getattr_with_constant.rs @@ -1,7 +1,6 @@ -use ruff_python_ast::{self as ast, Constant, Expr}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Constant, Expr}; use ruff_python_stdlib::identifiers::{is_identifier, is_mangled_private}; use ruff_text_size::Ranged; @@ -81,7 +80,19 @@ pub(crate) fn getattr_with_constant( let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range()); if checker.patch(diagnostic.kind.rule()) { diagnostic.set_fix(Fix::suggested(Edit::range_replacement( - format!("{}.{}", checker.locator().slice(obj), value), + if matches!( + obj, + Expr::Constant(ast::ExprConstant { + value: Constant::Int(_), + .. + }) + ) { + // Attribute accesses on `int` literals must be parenthesized, e.g., + // `getattr(1, "real")` becomes `(1).real`. + format!("({}).{}", checker.locator().slice(obj), value) + } else { + format!("{}.{}", checker.locator().slice(obj), value) + }, expr.range(), ))); } diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B009_B009_B010.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B009_B009_B010.py.snap index 94bc6e4483..c05f0f6b9d 100644 --- a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B009_B009_B010.py.snap +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B009_B009_B010.py.snap @@ -124,7 +124,7 @@ B009_B010.py:24:15: B009 [*] Do not call `getattr` with a constant attribute val 24 |+_ = lambda x: x.bar 25 25 | if getattr(x, "bar"): 26 26 | pass -27 27 | +27 27 | getattr(1, "real") B009_B010.py:25:4: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. | @@ -133,6 +133,7 @@ B009_B010.py:25:4: B009 [*] Do not call `getattr` with a constant attribute valu 25 | if getattr(x, "bar"): | ^^^^^^^^^^^^^^^^^ B009 26 | pass +27 | getattr(1, "real") | = help: Replace `getattr` with attribute access @@ -143,7 +144,109 @@ B009_B010.py:25:4: B009 [*] Do not call `getattr` with a constant attribute valu 25 |-if getattr(x, "bar"): 25 |+if x.bar: 26 26 | pass -27 27 | -28 28 | # Valid setattr usage +27 27 | getattr(1, "real") +28 28 | getattr(1., "real") + +B009_B010.py:27:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + | +25 | if getattr(x, "bar"): +26 | pass +27 | getattr(1, "real") + | ^^^^^^^^^^^^^^^^^^ B009 +28 | getattr(1., "real") +29 | getattr(1.0, "real") + | + = help: Replace `getattr` with attribute access + +ℹ Suggested fix +24 24 | _ = lambda x: getattr(x, "bar") +25 25 | if getattr(x, "bar"): +26 26 | pass +27 |-getattr(1, "real") + 27 |+(1).real +28 28 | getattr(1., "real") +29 29 | getattr(1.0, "real") +30 30 | getattr(1j, "real") + +B009_B010.py:28:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + | +26 | pass +27 | getattr(1, "real") +28 | getattr(1., "real") + | ^^^^^^^^^^^^^^^^^^^ B009 +29 | getattr(1.0, "real") +30 | getattr(1j, "real") + | + = help: Replace `getattr` with attribute access + +ℹ Suggested fix +25 25 | if getattr(x, "bar"): +26 26 | pass +27 27 | getattr(1, "real") +28 |-getattr(1., "real") + 28 |+1..real +29 29 | getattr(1.0, "real") +30 30 | getattr(1j, "real") +31 31 | getattr(True, "real") + +B009_B010.py:29:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + | +27 | getattr(1, "real") +28 | getattr(1., "real") +29 | getattr(1.0, "real") + | ^^^^^^^^^^^^^^^^^^^^ B009 +30 | getattr(1j, "real") +31 | getattr(True, "real") + | + = help: Replace `getattr` with attribute access + +ℹ Suggested fix +26 26 | pass +27 27 | getattr(1, "real") +28 28 | getattr(1., "real") +29 |-getattr(1.0, "real") + 29 |+1.0.real +30 30 | getattr(1j, "real") +31 31 | getattr(True, "real") +32 32 | + +B009_B010.py:30:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + | +28 | getattr(1., "real") +29 | getattr(1.0, "real") +30 | getattr(1j, "real") + | ^^^^^^^^^^^^^^^^^^^ B009 +31 | getattr(True, "real") + | + = help: Replace `getattr` with attribute access + +ℹ Suggested fix +27 27 | getattr(1, "real") +28 28 | getattr(1., "real") +29 29 | getattr(1.0, "real") +30 |-getattr(1j, "real") + 30 |+1j.real +31 31 | getattr(True, "real") +32 32 | +33 33 | + +B009_B010.py:31:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access. + | +29 | getattr(1.0, "real") +30 | getattr(1j, "real") +31 | getattr(True, "real") + | ^^^^^^^^^^^^^^^^^^^^^ B009 + | + = help: Replace `getattr` with attribute access + +ℹ Suggested fix +28 28 | getattr(1., "real") +29 29 | getattr(1.0, "real") +30 30 | getattr(1j, "real") +31 |-getattr(True, "real") + 31 |+True.real +32 32 | +33 33 | +34 34 | # Valid setattr usage diff --git a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B010_B009_B010.py.snap b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B010_B009_B010.py.snap index e8b728cd8f..7f27fb0c82 100644 --- a/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B010_B009_B010.py.snap +++ b/crates/ruff/src/rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B010_B009_B010.py.snap @@ -1,120 +1,120 @@ --- source: crates/ruff/src/rules/flake8_bugbear/mod.rs --- -B009_B010.py:40:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. +B009_B010.py:46:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | -39 | # Invalid usage -40 | setattr(foo, "bar", None) +45 | # Invalid usage +46 | setattr(foo, "bar", None) | ^^^^^^^^^^^^^^^^^^^^^^^^^ B010 -41 | setattr(foo, "_123abc", None) -42 | setattr(foo, "__123abc__", None) +47 | setattr(foo, "_123abc", None) +48 | setattr(foo, "__123abc__", None) | = help: Replace `setattr` with assignment ℹ Suggested fix -37 37 | pass -38 38 | -39 39 | # Invalid usage -40 |-setattr(foo, "bar", None) - 40 |+foo.bar = None -41 41 | setattr(foo, "_123abc", None) -42 42 | setattr(foo, "__123abc__", None) -43 43 | setattr(foo, "abc123", None) +43 43 | pass +44 44 | +45 45 | # Invalid usage +46 |-setattr(foo, "bar", None) + 46 |+foo.bar = None +47 47 | setattr(foo, "_123abc", None) +48 48 | setattr(foo, "__123abc__", None) +49 49 | setattr(foo, "abc123", None) -B009_B010.py:41:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. +B009_B010.py:47:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | -39 | # Invalid usage -40 | setattr(foo, "bar", None) -41 | setattr(foo, "_123abc", None) +45 | # Invalid usage +46 | setattr(foo, "bar", None) +47 | setattr(foo, "_123abc", None) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010 -42 | setattr(foo, "__123abc__", None) -43 | setattr(foo, "abc123", None) +48 | setattr(foo, "__123abc__", None) +49 | setattr(foo, "abc123", None) | = help: Replace `setattr` with assignment ℹ Suggested fix -38 38 | -39 39 | # Invalid usage -40 40 | setattr(foo, "bar", None) -41 |-setattr(foo, "_123abc", None) - 41 |+foo._123abc = None -42 42 | setattr(foo, "__123abc__", None) -43 43 | setattr(foo, "abc123", None) -44 44 | setattr(foo, r"abc123", None) +44 44 | +45 45 | # Invalid usage +46 46 | setattr(foo, "bar", None) +47 |-setattr(foo, "_123abc", None) + 47 |+foo._123abc = None +48 48 | setattr(foo, "__123abc__", None) +49 49 | setattr(foo, "abc123", None) +50 50 | setattr(foo, r"abc123", None) -B009_B010.py:42:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. +B009_B010.py:48:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | -40 | setattr(foo, "bar", None) -41 | setattr(foo, "_123abc", None) -42 | setattr(foo, "__123abc__", None) +46 | setattr(foo, "bar", None) +47 | setattr(foo, "_123abc", None) +48 | setattr(foo, "__123abc__", None) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010 -43 | setattr(foo, "abc123", None) -44 | setattr(foo, r"abc123", None) +49 | setattr(foo, "abc123", None) +50 | setattr(foo, r"abc123", None) | = help: Replace `setattr` with assignment ℹ Suggested fix -39 39 | # Invalid usage -40 40 | setattr(foo, "bar", None) -41 41 | setattr(foo, "_123abc", None) -42 |-setattr(foo, "__123abc__", None) - 42 |+foo.__123abc__ = None -43 43 | setattr(foo, "abc123", None) -44 44 | setattr(foo, r"abc123", None) -45 45 | setattr(foo.bar, r"baz", None) +45 45 | # Invalid usage +46 46 | setattr(foo, "bar", None) +47 47 | setattr(foo, "_123abc", None) +48 |-setattr(foo, "__123abc__", None) + 48 |+foo.__123abc__ = None +49 49 | setattr(foo, "abc123", None) +50 50 | setattr(foo, r"abc123", None) +51 51 | setattr(foo.bar, r"baz", None) -B009_B010.py:43:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. +B009_B010.py:49:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | -41 | setattr(foo, "_123abc", None) -42 | setattr(foo, "__123abc__", None) -43 | setattr(foo, "abc123", None) +47 | setattr(foo, "_123abc", None) +48 | setattr(foo, "__123abc__", None) +49 | setattr(foo, "abc123", None) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010 -44 | setattr(foo, r"abc123", None) -45 | setattr(foo.bar, r"baz", None) +50 | setattr(foo, r"abc123", None) +51 | setattr(foo.bar, r"baz", None) | = help: Replace `setattr` with assignment ℹ Suggested fix -40 40 | setattr(foo, "bar", None) -41 41 | setattr(foo, "_123abc", None) -42 42 | setattr(foo, "__123abc__", None) -43 |-setattr(foo, "abc123", None) - 43 |+foo.abc123 = None -44 44 | setattr(foo, r"abc123", None) -45 45 | setattr(foo.bar, r"baz", None) +46 46 | setattr(foo, "bar", None) +47 47 | setattr(foo, "_123abc", None) +48 48 | setattr(foo, "__123abc__", None) +49 |-setattr(foo, "abc123", None) + 49 |+foo.abc123 = None +50 50 | setattr(foo, r"abc123", None) +51 51 | setattr(foo.bar, r"baz", None) -B009_B010.py:44:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. +B009_B010.py:50:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | -42 | setattr(foo, "__123abc__", None) -43 | setattr(foo, "abc123", None) -44 | setattr(foo, r"abc123", None) +48 | setattr(foo, "__123abc__", None) +49 | setattr(foo, "abc123", None) +50 | setattr(foo, r"abc123", None) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010 -45 | setattr(foo.bar, r"baz", None) +51 | setattr(foo.bar, r"baz", None) | = help: Replace `setattr` with assignment ℹ Suggested fix -41 41 | setattr(foo, "_123abc", None) -42 42 | setattr(foo, "__123abc__", None) -43 43 | setattr(foo, "abc123", None) -44 |-setattr(foo, r"abc123", None) - 44 |+foo.abc123 = None -45 45 | setattr(foo.bar, r"baz", None) +47 47 | setattr(foo, "_123abc", None) +48 48 | setattr(foo, "__123abc__", None) +49 49 | setattr(foo, "abc123", None) +50 |-setattr(foo, r"abc123", None) + 50 |+foo.abc123 = None +51 51 | setattr(foo.bar, r"baz", None) -B009_B010.py:45:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. +B009_B010.py:51:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access. | -43 | setattr(foo, "abc123", None) -44 | setattr(foo, r"abc123", None) -45 | setattr(foo.bar, r"baz", None) +49 | setattr(foo, "abc123", None) +50 | setattr(foo, r"abc123", None) +51 | setattr(foo.bar, r"baz", None) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010 | = help: Replace `setattr` with assignment ℹ Suggested fix -42 42 | setattr(foo, "__123abc__", None) -43 43 | setattr(foo, "abc123", None) -44 44 | setattr(foo, r"abc123", None) -45 |-setattr(foo.bar, r"baz", None) - 45 |+foo.bar.baz = None +48 48 | setattr(foo, "__123abc__", None) +49 49 | setattr(foo, "abc123", None) +50 50 | setattr(foo, r"abc123", None) +51 |-setattr(foo.bar, r"baz", None) + 51 |+foo.bar.baz = None