Promote some rules to "always" fixable (#7840)

## Summary This PR upgrades some rules from "sometimes" to "always" fixes, now that we're getting ready to ship support in the CLI. The focus here was on identifying rules for which the diagnostic itself is high-confidence, and the fix itself is too (assuming that the diagnostic is correct). This is _unlike_ rules that _may_ be a false positive, like those that (e.g.) assume an object is a dictionary when you call `.values()` on it. Specifically, I upgraded: - A bunch of rules that only apply to `.pyi` files. - Rules that rewrite deprecated imports or aliases. - Some other misc. rules, like: `empty-print-string`, `unused-noqa`, `getattr-with-constant`. Open to feedback on any of these.
astral-sh · Oct 10, 2023 · ec7395b · ec7395b
1 parent d8c0360
commit ec7395b
Show file tree

Hide file tree

Showing 63 changed files with 419 additions and 422 deletions.
diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs
@@ -111,7 +111,7 @@ pub(crate) fn check_noqa(
                             Diagnostic::new(UnusedNOQA { codes: None }, directive.range());
                         if settings.rules.should_fix(diagnostic.kind.rule()) {
                             diagnostic
-                                .set_fix(Fix::unsafe_edit(delete_noqa(directive.range(), locator)));
+                                .set_fix(Fix::safe_edit(delete_noqa(directive.range(), locator)));
                         }
                         diagnostics.push(diagnostic);
                     }
@@ -175,12 +175,12 @@ pub(crate) fn check_noqa(
                         );
                         if settings.rules.should_fix(diagnostic.kind.rule()) {
                             if valid_codes.is_empty() {
-                                diagnostic.set_fix(Fix::unsafe_edit(delete_noqa(
+                                diagnostic.set_fix(Fix::safe_edit(delete_noqa(
                                     directive.range(),
                                     locator,
                                 )));
                             } else {
-                                diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+                                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
                                     format!("# noqa: {}", valid_codes.join(", ")),
                                     directive.range(),
                                 )));

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
@@ -86,7 +86,7 @@ pub(crate) fn getattr_with_constant(
 
     let mut diagnostic = Diagnostic::new(GetAttrWithConstant, expr.range());
     if checker.patch(diagnostic.kind.rule()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
             pad(
                 if matches!(
                     obj,

diff --git a/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs b/crates/ruff_linter/src/rules/flake8_bugbear/rules/setattr_with_constant.rs
@@ -109,7 +109,7 @@ pub(crate) fn setattr_with_constant(
         if expr == child.as_ref() {
             let mut diagnostic = Diagnostic::new(SetAttrWithConstant, expr.range());
             if checker.patch(diagnostic.kind.rule()) {
-                diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+                diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
                     assignment(obj, name, value, checker.generator()),
                     expr.range(),
                 )));

diff --git a/...lake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap b/...lake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B009_B009_B010.py.snap
@@ -11,7 +11,7 @@ B009_B010.py:19:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 16 16 | getattr(foo, "__123abc")
 17 17 | 
 18 18 | # Invalid usage
@@ -32,7 +32,7 @@ B009_B010.py:20:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 17 17 | 
 18 18 | # Invalid usage
 19 19 | getattr(foo, "bar")
@@ -53,7 +53,7 @@ B009_B010.py:21:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 18 18 | # Invalid usage
 19 19 | getattr(foo, "bar")
 20 20 | getattr(foo, "_123abc")
@@ -74,7 +74,7 @@ B009_B010.py:22:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 19 19 | getattr(foo, "bar")
 20 20 | getattr(foo, "_123abc")
 21 21 | getattr(foo, "__123abc__")
@@ -95,7 +95,7 @@ B009_B010.py:23:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 20 20 | getattr(foo, "_123abc")
 21 21 | getattr(foo, "__123abc__")
 22 22 | getattr(foo, "abc123")
@@ -116,7 +116,7 @@ B009_B010.py:24:15: B009 [*] Do not call `getattr` with a constant attribute val
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 21 21 | getattr(foo, "__123abc__")
 22 22 | getattr(foo, "abc123")
 23 23 | getattr(foo, r"abc123")
@@ -137,7 +137,7 @@ B009_B010.py:25:4: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 22 22 | getattr(foo, "abc123")
 23 23 | getattr(foo, r"abc123")
 24 24 | _ = lambda x: getattr(x, "bar")
@@ -158,7 +158,7 @@ B009_B010.py:27:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 24 24 | _ = lambda x: getattr(x, "bar")
 25 25 | if getattr(x, "bar"):
 26 26 |     pass
@@ -179,7 +179,7 @@ B009_B010.py:28:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 25 25 | if getattr(x, "bar"):
 26 26 |     pass
 27 27 | getattr(1, "real")
@@ -200,7 +200,7 @@ B009_B010.py:29:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 26 26 |     pass
 27 27 | getattr(1, "real")
 28 28 | getattr(1., "real")
@@ -221,7 +221,7 @@ B009_B010.py:30:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 27 27 | getattr(1, "real")
 28 28 | getattr(1., "real")
 29 29 | getattr(1.0, "real")
@@ -242,7 +242,7 @@ B009_B010.py:31:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 28 28 | getattr(1., "real")
 29 29 | getattr(1.0, "real")
 30 30 | getattr(1j, "real")
@@ -263,7 +263,7 @@ B009_B010.py:32:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 29 29 | getattr(1.0, "real")
 30 30 | getattr(1j, "real")
 31 31 | getattr(True, "real")
@@ -284,7 +284,7 @@ B009_B010.py:33:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 30 30 | getattr(1j, "real")
 31 31 | getattr(True, "real")
 32 32 | getattr(x := 1, "real")
@@ -304,7 +304,7 @@ B009_B010.py:34:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 31 31 | getattr(True, "real")
 32 32 | getattr(x := 1, "real")
 33 33 | getattr(x + y, "real")
@@ -326,7 +326,7 @@ B009_B010.py:58:8: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 55 55 | setattr(foo.bar, r"baz", None)
 56 56 | 
 57 57 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1722458885
@@ -345,7 +345,7 @@ B009_B010.py:65:1: B009 [*] Do not call `getattr` with a constant attribute valu
    |
    = help: Replace `getattr` with attribute access
 
-ℹ Suggested fix
+ℹ Fix
 62 62 | setattr(*foo, "bar", None)
 63 63 | 
 64 64 | # Regression test for: https://github.com/astral-sh/ruff/issues/7455#issuecomment-1739800901

diff --git a/...lake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap b/...lake8_bugbear/snapshots/ruff_linter__rules__flake8_bugbear__tests__B010_B009_B010.py.snap
@@ -11,7 +11,7 @@ B009_B010.py:50:1: B010 [*] Do not call `setattr` with a constant attribute valu
    |
    = help: Replace `setattr` with assignment
 
-ℹ Suggested fix
+ℹ Fix
 47 47 |     pass
 48 48 | 
 49 49 | # Invalid usage
@@ -32,7 +32,7 @@ B009_B010.py:51:1: B010 [*] Do not call `setattr` with a constant attribute valu
    |
    = help: Replace `setattr` with assignment
 
-ℹ Suggested fix
+ℹ Fix
 48 48 | 
 49 49 | # Invalid usage
 50 50 | setattr(foo, "bar", None)
@@ -53,7 +53,7 @@ B009_B010.py:52:1: B010 [*] Do not call `setattr` with a constant attribute valu
    |
    = help: Replace `setattr` with assignment
 
-ℹ Suggested fix
+ℹ Fix
 49 49 | # Invalid usage
 50 50 | setattr(foo, "bar", None)
 51 51 | setattr(foo, "_123abc", None)
@@ -74,7 +74,7 @@ B009_B010.py:53:1: B010 [*] Do not call `setattr` with a constant attribute valu
    |
    = help: Replace `setattr` with assignment
 
-ℹ Suggested fix
+ℹ Fix
 50 50 | setattr(foo, "bar", None)
 51 51 | setattr(foo, "_123abc", None)
 52 52 | setattr(foo, "__123abc__", None)
@@ -94,7 +94,7 @@ B009_B010.py:54:1: B010 [*] Do not call `setattr` with a constant attribute valu
    |
    = help: Replace `setattr` with assignment
 
-ℹ Suggested fix
+ℹ Fix
 51 51 | setattr(foo, "_123abc", None)
 52 52 | setattr(foo, "__123abc__", None)
 53 53 | setattr(foo, "abc123", None)
@@ -115,7 +115,7 @@ B009_B010.py:55:1: B010 [*] Do not call `setattr` with a constant attribute valu
    |
    = help: Replace `setattr` with assignment
 
-ℹ Suggested fix
+ℹ Fix
 52 52 | setattr(foo, "__123abc__", None)
 53 53 | setattr(foo, "abc123", None)
 54 54 | setattr(foo, r"abc123", None)

diff --git a/crates/ruff_linter/src/rules/flake8_logging/rules/undocumented_warn.rs b/crates/ruff_linter/src/rules/flake8_logging/rules/undocumented_warn.rs
@@ -63,7 +63,7 @@ pub(crate) fn undocumented_warn(checker: &mut Checker, expr: &Expr) {
                     checker.semantic(),
                 )?;
                 let reference_edit = Edit::range_replacement(binding, expr.range());
-                Ok(Fix::unsafe_edits(import_edit, [reference_edit]))
+                Ok(Fix::safe_edits(import_edit, [reference_edit]))
             });
         }
         checker.diagnostics.push(diagnostic);

diff --git a/...flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG009_LOG009.py.snap b/...flake8_logging/snapshots/ruff_linter__rules__flake8_logging__tests__LOG009_LOG009.py.snap
@@ -11,7 +11,7 @@ LOG009.py:3:1: LOG009 [*] Use of undocumented `logging.WARN` constant
   |
   = help: Replace `logging.WARN` with `logging.WARNING`
 
-ℹ Suggested fix
+ℹ Fix
 1 1 | import logging
 2 2 | 
 3   |-logging.WARN  # LOG009
@@ -30,7 +30,7 @@ LOG009.py:8:1: LOG009 [*] Use of undocumented `logging.WARN` constant
   |
   = help: Replace `logging.WARN` with `logging.WARNING`
 
-ℹ Suggested fix
+ℹ Fix
 5 5 | 
 6 6 | from logging import WARN, WARNING
 7 7 | 

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/non_empty_stub_body.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/non_empty_stub_body.rs
@@ -70,7 +70,7 @@ pub(crate) fn non_empty_stub_body(checker: &mut Checker, body: &[Stmt]) {
 
     let mut diagnostic = Diagnostic::new(NonEmptyStubBody, stmt.range());
     if checker.patch(Rule::NonEmptyStubBody) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
             format!("..."),
             stmt.range(),
         )));

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/numeric_literal_too_long.rs
@@ -51,7 +51,7 @@ pub(crate) fn numeric_literal_too_long(checker: &mut Checker, expr: &Expr) {
 
     let mut diagnostic = Diagnostic::new(NumericLiteralTooLong, expr.range());
     if checker.patch(diagnostic.kind.rule()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
             "...".to_string(),
             expr.range(),
         )));

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/simple_defaults.rs
@@ -535,7 +535,7 @@ pub(crate) fn typed_argument_simple_defaults(checker: &mut Checker, parameters:
                 let mut diagnostic = Diagnostic::new(TypedArgumentDefaultInStub, default.range());
 
                 if checker.patch(diagnostic.kind.rule()) {
-                    diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+                    diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
                         "...".to_string(),
                         default.range(),
                     )));
@@ -572,7 +572,7 @@ pub(crate) fn argument_simple_defaults(checker: &mut Checker, parameters: &Param
                 let mut diagnostic = Diagnostic::new(ArgumentDefaultInStub, default.range());
 
                 if checker.patch(diagnostic.kind.rule()) {
-                    diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+                    diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
                         "...".to_string(),
                         default.range(),
                     )));
@@ -607,7 +607,7 @@ pub(crate) fn assignment_default_in_stub(checker: &mut Checker, targets: &[Expr]
 
     let mut diagnostic = Diagnostic::new(AssignmentDefaultInStub, value.range());
     if checker.patch(diagnostic.kind.rule()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
             "...".to_string(),
             value.range(),
         )));
@@ -643,7 +643,7 @@ pub(crate) fn annotated_assignment_default_in_stub(
 
     let mut diagnostic = Diagnostic::new(AssignmentDefaultInStub, value.range());
     if checker.patch(diagnostic.kind.rule()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
             "...".to_string(),
             value.range(),
         )));
@@ -748,7 +748,7 @@ pub(crate) fn type_alias_without_annotation(checker: &mut Checker, value: &Expr,
                 target.start(),
                 checker.semantic(),
             )?;
-            Ok(Fix::unsafe_edits(
+            Ok(Fix::safe_edits(
                 Edit::range_replacement(format!("{id}: {binding}"), target.range()),
                 [import_edit],
             ))

diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/string_or_bytes_too_long.rs
@@ -68,7 +68,7 @@ pub(crate) fn string_or_bytes_too_long(checker: &mut Checker, expr: &Expr) {
 
     let mut diagnostic = Diagnostic::new(StringOrBytesTooLong, expr.range());
     if checker.patch(diagnostic.kind.rule()) {
-        diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement(
+        diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
             "...".to_string(),
             expr.range(),
         )));

diff --git a/.../rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI010_PYI010.pyi.snap b/.../rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI010_PYI010.pyi.snap
@@ -11,7 +11,7 @@ PYI010.pyi:6:5: PYI010 [*] Function body must contain only `...`
   |
   = help: Replace function body with `...`
 
-ℹ Suggested fix
+ℹ Fix
 3 3 |     """foo"""  # OK, docstrings are handled by another rule
 4 4 | 
 5 5 | def buzz():
@@ -31,7 +31,7 @@ PYI010.pyi:9:5: PYI010 [*] Function body must contain only `...`
    |
    = help: Replace function body with `...`
 
-ℹ Suggested fix
+ℹ Fix
 6  6  |     print("buzz")  # ERROR PYI010
 7  7  | 
 8  8  | def foo2():
@@ -51,7 +51,7 @@ PYI010.pyi:12:5: PYI010 [*] Function body must contain only `...`
    |
    = help: Replace function body with `...`
 
-ℹ Suggested fix
+ℹ Fix
 9  9  |     123  # ERROR PYI010
 10 10 | 
 11 11 | def bizz():