Parenthesize expressins when converting to B009 (#7091)

astral-sh · Sep 3, 2023 · 9c3b2c3 · 9c3b2c3
1 parent 37d244d
commit 9c3b2c3
Show file tree

Hide file tree

Showing 4 changed files with 161 additions and 92 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
@@ -29,6 +29,10 @@
 getattr(1.0, "real")
 getattr(1j, "real")
 getattr(True, "real")
+getattr(x := 1, "real")
+getattr(x + y, "real")
+getattr("foo"
+        "bar", "real")
 
 
 # Valid setattr usage

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
@@ -82,16 +82,14 @@ pub(crate) fn getattr_with_constant(
         diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
             if matches!(
                 obj,
-                Expr::Constant(ast::ExprConstant {
-                    value: Constant::Int(_),
-                    ..
-                })
+                Expr::Name(_) | Expr::Attribute(_) | Expr::Subscript(_) | Expr::Call(_)
             ) {
-                // 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)
+            } else {
+                // Defensively parenthesize any other expressions. For example, attribute accesses
+                // on `int` literals must be parenthesized, e.g., `getattr(1, "real")` becomes
+                // `(1).real`. The same is true for named expressions and others.
+                format!("({}).{}", checker.locator().slice(obj), value)
             },
             expr.range(),
         )));

diff --git a/...rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B009_B009_B010.py.snap b/...rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B009_B009_B010.py.snap
@@ -184,7 +184,7 @@ B009_B010.py:28:1: B009 [*] Do not call `getattr` with a constant attribute valu
 26 26 |     pass
 27 27 | getattr(1, "real")
 28    |-getattr(1., "real")
-   28 |+1..real
+   28 |+(1.).real
 29 29 | getattr(1.0, "real")
 30 30 | getattr(1j, "real")
 31 31 | getattr(True, "real")
@@ -205,10 +205,10 @@ B009_B010.py:29:1: B009 [*] Do not call `getattr` with a constant attribute valu
 27 27 | getattr(1, "real")
 28 28 | getattr(1., "real")
 29    |-getattr(1.0, "real")
-   29 |+1.0.real
+   29 |+(1.0).real
 30 30 | getattr(1j, "real")
 31 31 | getattr(True, "real")
-32 32 | 
+32 32 | getattr(x := 1, "real")
 
 B009_B010.py:30:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
    |
@@ -217,6 +217,7 @@ B009_B010.py:30:1: B009 [*] Do not call `getattr` with a constant attribute valu
 30 | getattr(1j, "real")
    | ^^^^^^^^^^^^^^^^^^^ B009
 31 | getattr(True, "real")
+32 | getattr(x := 1, "real")
    |
    = help: Replace `getattr` with attribute access
 
@@ -225,17 +226,19 @@ B009_B010.py:30:1: B009 [*] Do not call `getattr` with a constant attribute valu
 28 28 | getattr(1., "real")
 29 29 | getattr(1.0, "real")
 30    |-getattr(1j, "real")
-   30 |+1j.real
+   30 |+(1j).real
 31 31 | getattr(True, "real")
-32 32 | 
-33 33 | 
+32 32 | getattr(x := 1, "real")
+33 33 | getattr(x + y, "real")
 
 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
+32 | getattr(x := 1, "real")
+33 | getattr(x + y, "real")
    |
    = help: Replace `getattr` with attribute access
 
@@ -244,9 +247,73 @@ B009_B010.py:31:1: B009 [*] Do not call `getattr` with a constant attribute valu
 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
+   31 |+(True).real
+32 32 | getattr(x := 1, "real")
+33 33 | getattr(x + y, "real")
+34 34 | getattr("foo"
+
+B009_B010.py:32:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
+   |
+30 | getattr(1j, "real")
+31 | getattr(True, "real")
+32 | getattr(x := 1, "real")
+   | ^^^^^^^^^^^^^^^^^^^^^^^ B009
+33 | getattr(x + y, "real")
+34 | getattr("foo"
+   |
+   = help: Replace `getattr` with attribute access
+
+ℹ Suggested fix
+29 29 | getattr(1.0, "real")
+30 30 | getattr(1j, "real")
+31 31 | getattr(True, "real")
+32    |-getattr(x := 1, "real")
+   32 |+(x := 1).real
+33 33 | getattr(x + y, "real")
+34 34 | getattr("foo"
+35 35 |         "bar", "real")
+
+B009_B010.py:33:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
+   |
+31 | getattr(True, "real")
+32 | getattr(x := 1, "real")
+33 | getattr(x + y, "real")
+   | ^^^^^^^^^^^^^^^^^^^^^^ B009
+34 | getattr("foo"
+35 |         "bar", "real")
+   |
+   = help: Replace `getattr` with attribute access
+
+ℹ Suggested fix
+30 30 | getattr(1j, "real")
+31 31 | getattr(True, "real")
+32 32 | getattr(x := 1, "real")
+33    |-getattr(x + y, "real")
+   33 |+(x + y).real
+34 34 | getattr("foo"
+35 35 |         "bar", "real")
+36 36 | 
+
+B009_B010.py:34:1: B009 [*] Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
+   |
+32 |   getattr(x := 1, "real")
+33 |   getattr(x + y, "real")
+34 | / getattr("foo"
+35 | |         "bar", "real")
+   | |______________________^ B009
+   |
+   = help: Replace `getattr` with attribute access
+
+ℹ Suggested fix
+31 31 | getattr(True, "real")
+32 32 | getattr(x := 1, "real")
+33 33 | getattr(x + y, "real")
+34    |-getattr("foo"
+35    |-        "bar", "real")
+   34 |+("foo"
+   35 |+        "bar").real
+36 36 | 
+37 37 | 
+38 38 | # Valid setattr usage
 
 
diff --git a/...rules/flake8_bugbear/snapshots/ruff__rules__flake8_bugbear__tests__B010_B009_B010.py.snap b/...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:46: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.
    |
-45 | # Invalid usage
-46 | setattr(foo, "bar", None)
+49 | # Invalid usage
+50 | setattr(foo, "bar", None)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^ B010
-47 | setattr(foo, "_123abc", None)
-48 | setattr(foo, "__123abc__", None)
+51 | setattr(foo, "_123abc", None)
+52 | setattr(foo, "__123abc__", None)
    |
    = help: Replace `setattr` with assignment
 
 ℹ Suggested fix
-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)
+47 47 |     pass
+48 48 | 
+49 49 | # Invalid usage
+50    |-setattr(foo, "bar", None)
+   50 |+foo.bar = None
+51 51 | setattr(foo, "_123abc", None)
+52 52 | setattr(foo, "__123abc__", None)
+53 53 | setattr(foo, "abc123", None)
 
-B009_B010.py:47: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.
    |
-45 | # Invalid usage
-46 | setattr(foo, "bar", None)
-47 | setattr(foo, "_123abc", None)
+49 | # Invalid usage
+50 | setattr(foo, "bar", None)
+51 | setattr(foo, "_123abc", None)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010
-48 | setattr(foo, "__123abc__", None)
-49 | setattr(foo, "abc123", None)
+52 | setattr(foo, "__123abc__", None)
+53 | setattr(foo, "abc123", None)
    |
    = help: Replace `setattr` with assignment
 
 ℹ Suggested fix
-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)
+48 48 | 
+49 49 | # Invalid usage
+50 50 | setattr(foo, "bar", None)
+51    |-setattr(foo, "_123abc", None)
+   51 |+foo._123abc = None
+52 52 | setattr(foo, "__123abc__", None)
+53 53 | setattr(foo, "abc123", None)
+54 54 | setattr(foo, r"abc123", None)
 
-B009_B010.py:48:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
+B009_B010.py:52:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
    |
-46 | setattr(foo, "bar", None)
-47 | setattr(foo, "_123abc", None)
-48 | setattr(foo, "__123abc__", None)
+50 | setattr(foo, "bar", None)
+51 | setattr(foo, "_123abc", None)
+52 | setattr(foo, "__123abc__", None)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010
-49 | setattr(foo, "abc123", None)
-50 | setattr(foo, r"abc123", None)
+53 | setattr(foo, "abc123", None)
+54 | setattr(foo, r"abc123", None)
    |
    = help: Replace `setattr` with assignment
 
 ℹ Suggested fix
-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)
+49 49 | # Invalid usage
+50 50 | setattr(foo, "bar", None)
+51 51 | setattr(foo, "_123abc", None)
+52    |-setattr(foo, "__123abc__", None)
+   52 |+foo.__123abc__ = None
+53 53 | setattr(foo, "abc123", None)
+54 54 | setattr(foo, r"abc123", None)
+55 55 | setattr(foo.bar, r"baz", None)
 
-B009_B010.py:49:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
+B009_B010.py:53:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
    |
-47 | setattr(foo, "_123abc", None)
-48 | setattr(foo, "__123abc__", None)
-49 | setattr(foo, "abc123", None)
+51 | setattr(foo, "_123abc", None)
+52 | setattr(foo, "__123abc__", None)
+53 | setattr(foo, "abc123", None)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010
-50 | setattr(foo, r"abc123", None)
-51 | setattr(foo.bar, r"baz", None)
+54 | setattr(foo, r"abc123", None)
+55 | setattr(foo.bar, r"baz", None)
    |
    = help: Replace `setattr` with assignment
 
 ℹ Suggested fix
-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)
+50 50 | setattr(foo, "bar", None)
+51 51 | setattr(foo, "_123abc", None)
+52 52 | setattr(foo, "__123abc__", None)
+53    |-setattr(foo, "abc123", None)
+   53 |+foo.abc123 = None
+54 54 | setattr(foo, r"abc123", None)
+55 55 | setattr(foo.bar, r"baz", None)
 
-B009_B010.py:50:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
+B009_B010.py:54:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
    |
-48 | setattr(foo, "__123abc__", None)
-49 | setattr(foo, "abc123", None)
-50 | setattr(foo, r"abc123", None)
+52 | setattr(foo, "__123abc__", None)
+53 | setattr(foo, "abc123", None)
+54 | setattr(foo, r"abc123", None)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010
-51 | setattr(foo.bar, r"baz", None)
+55 | setattr(foo.bar, r"baz", None)
    |
    = help: Replace `setattr` with assignment
 
 ℹ Suggested fix
-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)
+51 51 | setattr(foo, "_123abc", None)
+52 52 | setattr(foo, "__123abc__", None)
+53 53 | setattr(foo, "abc123", None)
+54    |-setattr(foo, r"abc123", None)
+   54 |+foo.abc123 = None
+55 55 | setattr(foo.bar, r"baz", None)
 
-B009_B010.py:51:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
+B009_B010.py:55:1: B010 [*] Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
    |
-49 | setattr(foo, "abc123", None)
-50 | setattr(foo, r"abc123", None)
-51 | setattr(foo.bar, r"baz", None)
+53 | setattr(foo, "abc123", None)
+54 | setattr(foo, r"abc123", None)
+55 | setattr(foo.bar, r"baz", None)
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B010
    |
    = help: Replace `setattr` with assignment
 
 ℹ Suggested fix
-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
+52 52 | setattr(foo, "__123abc__", None)
+53 53 | setattr(foo, "abc123", None)
+54 54 | setattr(foo, r"abc123", None)
+55    |-setattr(foo.bar, r"baz", None)
+   55 |+foo.bar.baz = None