diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM300.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM300.py index 25735ca70ca0c..a9cd7df87209e 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM300.py +++ b/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(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 + } +} diff --git a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs b/crates/ruff/src/rules/flake8_comprehensions/fixes.rs index d895f339d0a09..a69383836309a 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/fixes.rs +++ b/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 { diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs index 816348f18774c..c3727a840b3ee 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/assertion.rs +++ b/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![], diff --git a/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs b/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs index f2f69690bba43..e03b685abb5a2 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/fix_if.rs +++ b/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![], diff --git a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs index 99ae4f5a39970..d2618f8d47107 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/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 +} diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap index 50c2af9f8c7cc..997f098472ed5 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap +++ b/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 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 0` @@ -287,8 +288,8 @@ 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 (60 * 60) # SIM300 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ SIM300 -17 | -18 | # OK +17 | B (60 * 60) # SIM300 16 |+(60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE # SIM300 -17 17 | -18 18 | # OK -19 19 | compare == "yoda" +17 17 | B B` instead + | +15 | 0 < (number - 100) # SIM300 +16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 +17 | B 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 B or B +18 18 | B or(B) (B)` instead + | +16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 +17 | B (B)` + +ℹ Fix +15 15 | 0 < (number - 100) # SIM300 +16 16 | SomeClass().settings.SOME_CONSTANT_VALUE > (60 * 60) # SIM300 +17 17 | B (B) +19 19 | +20 20 | # OK +21 21 | compare == "yoda"