Add required space when fixing SIM300

crates/ruff/resources/test/fixtures/flake8_simplify/SIM300.py

@@ -14,6 +14,8 @@
 JediOrder.YODA == age  # SIM300
 0 < (number - 100)  # SIM300
 SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
+B<A[0][0]or B
+B or(B)<A[0][0]
 
 # OK
 compare == "yoda"

crates/ruff/src/cst/helpers.rs

@@ -1,4 +1,4 @@
-use libcst_native::{Expression, NameOrAttribute};
+use libcst_native::{Expression, NameOrAttribute, ParenthesizableWhitespace, SimpleWhitespace};
 
 fn compose_call_path_inner<'a>(expr: &'a Expression, parts: &mut Vec<&'a str>) {
     match expr {
@@ -36,3 +36,17 @@ pub(crate) fn compose_module_path(module: &NameOrAttribute) -> String {
         }
     }
 }
+
+/// Return a [`ParenthesizableWhitespace`] containing a single space.
+pub(crate) fn space() -> ParenthesizableWhitespace<'static> {
+    ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" "))
+}
+
+/// Ensure that a [`ParenthesizableWhitespace`] contains at least one space.
+pub(crate) fn or_space(whitespace: ParenthesizableWhitespace) -> ParenthesizableWhitespace {
+    if whitespace == ParenthesizableWhitespace::default() {
+        space()
+    } else {
+        whitespace
+    }
+}

crates/ruff/src/rules/flake8_comprehensions/fixes.rs

@@ -15,6 +15,7 @@ use ruff_python_codegen::Stylist;
 use ruff_source_file::Locator;
 
 use crate::autofix::codemods::CodegenStylist;
+use crate::cst::helpers::space;
 use crate::rules::flake8_comprehensions::rules::ObjectType;
 use crate::{
     checkers::ast::Checker,
@@ -123,7 +124,7 @@ pub(crate) fn fix_unnecessary_generator_dict(checker: &Checker, expr: &Expr) ->
         lpar: vec![],
         rpar: vec![],
         whitespace_before_colon: ParenthesizableWhitespace::default(),
-        whitespace_after_colon: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
+        whitespace_after_colon: space(),
     }));
 
     Ok(Edit::range_replacement(
@@ -195,7 +196,7 @@ pub(crate) fn fix_unnecessary_list_comprehension_dict(
         value: Box::new(value.clone()),
         for_in: list_comp.for_in.clone(),
         whitespace_before_colon: ParenthesizableWhitespace::default(),
-        whitespace_after_colon: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
+        whitespace_after_colon: space(),
         lbrace: LeftCurlyBrace {
             whitespace_after: call.whitespace_before_args.clone(),
         },
@@ -926,14 +927,10 @@ pub(crate) fn fix_unnecessary_map(
             ifs: vec![],
             inner_for_in: None,
             asynchronous: None,
-            whitespace_before: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
-            whitespace_after_for: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(
-                " ",
-            )),
-            whitespace_before_in: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(
-                " ",
-            )),
-            whitespace_after_in: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
+            whitespace_before: space(),
+            whitespace_after_for: space(),
+            whitespace_before_in: space(),
+            whitespace_after_in: space(),
         });
 
         match object_type {

crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs

@@ -3,9 +3,8 @@ use std::borrow::Cow;
 use anyhow::Result;
 use anyhow::{bail, Context};
 use libcst_native::{
-    self, Assert, BooleanOp, CompoundStatement, Expression, ParenthesizableWhitespace,
-    ParenthesizedNode, SimpleStatementLine, SimpleWhitespace, SmallStatement, Statement,
-    TrailingWhitespace, UnaryOperation,
+    self, Assert, BooleanOp, CompoundStatement, Expression, ParenthesizedNode, SimpleStatementLine,
+    SimpleWhitespace, SmallStatement, Statement, TrailingWhitespace, UnaryOperation,
 };
 
 use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
@@ -22,6 +21,7 @@ use ruff_text_size::Ranged;
 
 use crate::autofix::codemods::CodegenStylist;
 use crate::checkers::ast::Checker;
+use crate::cst::helpers::space;
 use crate::cst::matchers::match_indented_block;
 use crate::cst::matchers::match_module;
 use crate::importer::ImportRequest;
@@ -576,7 +576,7 @@ fn negate<'a>(expression: &Expression<'a>) -> Expression<'a> {
     }
     Expression::UnaryOperation(Box::new(UnaryOperation {
         operator: libcst_native::UnaryOp::Not {
-            whitespace_after: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
+            whitespace_after: space(),
         },
         expression: Box::new(expression.clone()),
         lpar: vec![],

crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs

@@ -2,17 +2,18 @@ use std::borrow::Cow;
 
 use anyhow::{bail, Result};
 use libcst_native::{
-    BooleanOp, BooleanOperation, CompoundStatement, Expression, If, LeftParen,
-    ParenthesizableWhitespace, ParenthesizedNode, RightParen, SimpleWhitespace, Statement, Suite,
+    BooleanOp, BooleanOperation, CompoundStatement, Expression, If, LeftParen, ParenthesizedNode,
+    RightParen, Statement, Suite,
 };
-use ruff_text_size::TextRange;
 
 use ruff_diagnostics::Edit;
 use ruff_python_ast::whitespace;
 use ruff_python_codegen::Stylist;
 use ruff_source_file::Locator;
+use ruff_text_size::TextRange;
 
 use crate::autofix::codemods::CodegenStylist;
+use crate::cst::helpers::space;
 use crate::cst::matchers::{match_function_def, match_if, match_indented_block, match_statement};
 
 fn parenthesize_and_operand(expr: Expression) -> Expression {
@@ -102,8 +103,8 @@ pub(crate) fn fix_nested_if_statements(
     outer_if.test = Expression::BooleanOperation(Box::new(BooleanOperation {
         left: Box::new(parenthesize_and_operand(outer_if.test.clone())),
         operator: BooleanOp::And {
-            whitespace_before: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
-            whitespace_after: ParenthesizableWhitespace::SimpleWhitespace(SimpleWhitespace(" ")),
+            whitespace_before: space(),
+            whitespace_after: space(),
         },
         right: Box::new(parenthesize_and_operand(inner_if.test.clone())),
         lpar: vec![],

crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs

@@ -7,10 +7,11 @@ use ruff_python_ast::{self as ast, CmpOp, Expr, UnaryOp};
 use ruff_python_codegen::Stylist;
 use ruff_python_stdlib::str::{self};
 use ruff_source_file::Locator;
-use ruff_text_size::Ranged;
+use ruff_text_size::{Ranged, TextRange};
 
 use crate::autofix::snippet::SourceCodeSnippet;
 use crate::checkers::ast::Checker;
+use crate::cst::helpers::or_space;
 use crate::cst::matchers::{match_comparison, transform_expression};
 use crate::registry::AsRule;
 
@@ -116,43 +117,43 @@ fn reverse_comparison(expr: &Expr, locator: &Locator, stylist: &Stylist) -> Resu
                 whitespace_before,
                 whitespace_after,
             } => CompOp::GreaterThan {
-                whitespace_before,
-                whitespace_after,
+                whitespace_before: or_space(whitespace_before),
+                whitespace_after: or_space(whitespace_after),
             },
             CompOp::GreaterThan {
                 whitespace_before,
                 whitespace_after,
             } => CompOp::LessThan {
-                whitespace_before,
-                whitespace_after,
+                whitespace_before: or_space(whitespace_before),
+                whitespace_after: or_space(whitespace_after),
             },
             CompOp::LessThanEqual {
                 whitespace_before,
                 whitespace_after,
             } => CompOp::GreaterThanEqual {
-                whitespace_before,
-                whitespace_after,
+                whitespace_before: or_space(whitespace_before),
+                whitespace_after: or_space(whitespace_after),
             },
             CompOp::GreaterThanEqual {
                 whitespace_before,
                 whitespace_after,
             } => CompOp::LessThanEqual {
-                whitespace_before,
-                whitespace_after,
+                whitespace_before: or_space(whitespace_before),
+                whitespace_after: or_space(whitespace_after),
             },
             CompOp::Equal {
                 whitespace_before,
                 whitespace_after,
             } => CompOp::Equal {
-                whitespace_before,
-                whitespace_after,
+                whitespace_before: or_space(whitespace_before),
+                whitespace_after: or_space(whitespace_after),
             },
             CompOp::NotEqual {
                 whitespace_before,
                 whitespace_after,
             } => CompOp::NotEqual {
-                whitespace_before,
-                whitespace_after,
+                whitespace_before: or_space(whitespace_before),
+                whitespace_after: or_space(whitespace_after),
             },
             _ => panic!("Expected comparison operator"),
         };
@@ -193,7 +194,7 @@ pub(crate) fn yoda_conditions(
         );
         if checker.patch(diagnostic.kind.rule()) {
             diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
-                suggestion,
+                pad(suggestion, expr.range(), checker.locator()),
                 expr.range(),
             )));
         }
@@ -205,3 +206,29 @@ pub(crate) fn yoda_conditions(
         ));
     }
 }
+
+/// Add leading or trailing whitespace to a snippet, if it's immediately preceded or followed by
+/// an identifier or keyword.
+fn pad(mut content: String, range: TextRange, locator: &Locator) -> String {
+    // Ex) `A or(B)>1`. To avoid `A or1<(B)`, insert a space before the fix, to achieve `A or 1<(B)`.
+    if locator
+        .up_to(range.start())
+        .chars()
+        .last()
+        .is_some_and(|char| char.is_ascii_alphabetic())
+    {
+        content.insert(0, ' ');
+    }
+
+    // Ex) `1>(B)or A`. To avoid `(B)<1or A`, insert a space after the fix, to achieve `(B)<1 or A`.
+    if locator
+        .after(range.end())
+        .chars()
+        .next()
+        .is_some_and(|char| char.is_ascii_alphabetic())
+    {
+        content.push(' ');
+    }
+
+    content
+}

crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap

@@ -268,7 +268,7 @@ SIM300.py:14:1: SIM300 [*] Yoda conditions are discouraged, use `age == JediOrde
    14 |+age == JediOrder.YODA  # SIM300
 15 15 | 0 < (number - 100)  # SIM300
 16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
-17 17 | 
+17 17 | B<A[0][0]or B
 
 SIM300.py:15:1: SIM300 [*] Yoda conditions are discouraged, use `(number - 100) > 0` instead
    |
@@ -277,6 +277,7 @@ SIM300.py:15:1: SIM300 [*] Yoda conditions are discouraged, use `(number - 100)
 15 | 0 < (number - 100)  # SIM300
    | ^^^^^^^^^^^^^^^^^^ SIM300
 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
+17 | B<A[0][0]or B
    |
    = help: Replace Yoda condition with `(number - 100) > 0`
 
@@ -287,17 +288,17 @@ SIM300.py:15:1: SIM300 [*] Yoda conditions are discouraged, use `(number - 100)
 15    |-0 < (number - 100)  # SIM300
    15 |+(number - 100) > 0  # SIM300
 16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
-17 17 | 
-18 18 | # OK
+17 17 | B<A[0][0]or B
+18 18 | B or(B)<A[0][0]
 
 SIM300.py:16:1: SIM300 [*] Yoda conditions are discouraged
    |
 14 | JediOrder.YODA == age  # SIM300
 15 | 0 < (number - 100)  # SIM300
 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM300
-17 | 
-18 | # OK
+17 | B<A[0][0]or B
+18 | B or(B)<A[0][0]
    |
    = help: Replace Yoda condition
 
@@ -307,8 +308,49 @@ SIM300.py:16:1: SIM300 [*] Yoda conditions are discouraged
 15 15 | 0 < (number - 100)  # SIM300
 16    |-SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
    16 |+(60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE  # SIM300
-17 17 | 
-18 18 | # OK
-19 19 | compare == "yoda"
+17 17 | B<A[0][0]or B
+18 18 | B or(B)<A[0][0]
+19 19 | 
+
+SIM300.py:17:1: SIM300 [*] Yoda conditions are discouraged, use `A[0][0] > B` instead
+   |
+15 | 0 < (number - 100)  # SIM300
+16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
+17 | B<A[0][0]or B
+   | ^^^^^^^^^ SIM300
+18 | B or(B)<A[0][0]
+   |
+   = help: Replace Yoda condition with `A[0][0] > B`
+
+ℹ Fix
+14 14 | JediOrder.YODA == age  # SIM300
+15 15 | 0 < (number - 100)  # SIM300
+16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
+17    |-B<A[0][0]or B
+   17 |+A[0][0] > B or B
+18 18 | B or(B)<A[0][0]
+19 19 | 
+20 20 | # OK
+
+SIM300.py:18:5: SIM300 [*] Yoda conditions are discouraged, use `A[0][0] > (B)` instead
+   |
+16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
+17 | B<A[0][0]or B
+18 | B or(B)<A[0][0]
+   |     ^^^^^^^^^^^ SIM300
+19 | 
+20 | # OK
+   |
+   = help: Replace Yoda condition with `A[0][0] > (B)`
+
+ℹ Fix
+15 15 | 0 < (number - 100)  # SIM300
+16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60)  # SIM300
+17 17 | B<A[0][0]or B
+18    |-B or(B)<A[0][0]
+   18 |+B or A[0][0] > (B)
+19 19 | 
+20 20 | # OK
+21 21 | compare == "yoda"