diff --git a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py index 91829bd01856d..dcf730f4a75ec 100644 --- a/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py +++ b/crates/ruff/resources/test/fixtures/flake8_simplify/SIM222.py @@ -152,3 +152,11 @@ def validate(self, value): if f(a or [1] or True or [2]): # SIM222 pass + +# Regression test for: https://github.com/astral-sh/ruff/issues/7099 +def secondToTime(s0: int) -> (int, int, int) or str: + m, s = divmod(s0, 60) + + +def secondToTime(s0: int) -> ((int, int, int) or str): + m, s = divmod(s0, 60) diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs index cbd348cc382f3..559a460c2d345 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_bool_op.rs @@ -11,6 +11,8 @@ use ruff_diagnostics::{AlwaysAutofixableViolation, AutofixKind, Diagnostic, Edit use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::comparable::ComparableExpr; use ruff_python_ast::helpers::{contains_effect, Truthiness}; +use ruff_python_ast::parenthesize::parenthesized_range; +use ruff_python_codegen::Generator; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -692,12 +694,12 @@ pub(crate) fn expr_or_not_expr(checker: &mut Checker, expr: &Expr) { } } -pub(crate) fn get_short_circuit_edit( +fn get_short_circuit_edit( expr: &Expr, range: TextRange, truthiness: Truthiness, in_boolean_test: bool, - checker: &Checker, + generator: Generator, ) -> Edit { let content = if in_boolean_test { match truthiness { @@ -708,9 +710,17 @@ pub(crate) fn get_short_circuit_edit( } } } else { - checker.generator().expr(expr) + generator.expr(expr) }; - Edit::range_replacement(content, range) + Edit::range_replacement( + if matches!(expr, Expr::Tuple(ast::ExprTuple { elts, ctx: _, range: _}) if !elts.is_empty()) + { + format!("({})", content) + } else { + content + }, + range, + ) } fn is_short_circuit( @@ -734,7 +744,7 @@ fn is_short_circuit( BoolOp::Or => Truthiness::Truthy, }; - let mut location = expr.start(); + let mut furthest = expr; let mut edit = None; let mut remove = None; @@ -749,7 +759,7 @@ fn is_short_circuit( && (!checker.semantic().in_boolean_test() || contains_effect(value, |id| checker.semantic().is_builtin(id))) { - location = next_value.start(); + furthest = next_value; continue; } @@ -758,17 +768,19 @@ fn is_short_circuit( // short-circuit expression is the first expression in the list; otherwise, we'll see it // as `next_value` before we see it as `value`. if value_truthiness == short_circuit_truthiness { - remove = Some(if location == value.start() { - ContentAround::After - } else { - ContentAround::Both - }); + remove = Some(ContentAround::After); + edit = Some(get_short_circuit_edit( value, - TextRange::new(location, expr.end()), + TextRange::new( + parenthesized_range(furthest.into(), expr.into(), checker.locator().contents()) + .unwrap_or(furthest.range()) + .start(), + expr.end(), + ), short_circuit_truthiness, checker.semantic().in_boolean_test(), - checker, + checker.generator(), )); break; } @@ -776,17 +788,22 @@ fn is_short_circuit( // If the next expression is a constant, and it matches the short-circuit value, then // we can return the location of the expression. if next_value_truthiness == short_circuit_truthiness { - remove = Some(if index == values.len() - 2 { + remove = Some(if index + 1 == values.len() - 1 { ContentAround::Before } else { ContentAround::Both }); edit = Some(get_short_circuit_edit( next_value, - TextRange::new(location, expr.end()), + TextRange::new( + parenthesized_range(furthest.into(), expr.into(), checker.locator().contents()) + .unwrap_or(furthest.range()) + .start(), + expr.end(), + ), short_circuit_truthiness, checker.semantic().in_boolean_test(), - checker, + checker.generator(), )); break; } diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM222_SIM222.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM222_SIM222.py.snap index 91a804d98af3a..a4bf8aa027c73 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM222_SIM222.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM222_SIM222.py.snap @@ -534,7 +534,7 @@ SIM222.py:85:6: SIM222 [*] Use `True` instead of `... or True` 87 87 | a or (1,) or True or (2,) # SIM222 88 88 | -SIM222.py:87:6: SIM222 [*] Use `1,` instead of `1, or ...` +SIM222.py:87:6: SIM222 [*] Use `(1,)` instead of `(1,) or ...` | 85 | a or tuple(()) or True # SIM222 86 | @@ -543,14 +543,14 @@ SIM222.py:87:6: SIM222 [*] Use `1,` instead of `1, or ...` 88 | 89 | a or tuple((1,)) or True or tuple((2,)) # SIM222 | - = help: Replace with `1,` + = help: Replace with `(1,)` ℹ Suggested fix 84 84 | 85 85 | a or tuple(()) or True # SIM222 86 86 | 87 |-a or (1,) or True or (2,) # SIM222 - 87 |+a or 1, # SIM222 + 87 |+a or (1,) # SIM222 88 88 | 89 89 | a or tuple((1,)) or True or tuple((2,)) # SIM222 90 90 | @@ -1003,5 +1003,42 @@ SIM222.py:153:11: SIM222 [*] Use `[1]` instead of `[1] or ...` 153 |-if f(a or [1] or True or [2]): # SIM222 153 |+if f(a or [1]): # SIM222 154 154 | pass +155 155 | +156 156 | # Regression test for: https://github.com/astral-sh/ruff/issues/7099 + +SIM222.py:157:30: SIM222 [*] Use `(int, int, int)` instead of `(int, int, int) or ...` + | +156 | # Regression test for: https://github.com/astral-sh/ruff/issues/7099 +157 | def secondToTime(s0: int) -> (int, int, int) or str: + | ^^^^^^^^^^^^^^^^^^^^^^ SIM222 +158 | m, s = divmod(s0, 60) + | + = help: Replace with `(int, int, int)` + +ℹ Suggested fix +154 154 | pass +155 155 | +156 156 | # Regression test for: https://github.com/astral-sh/ruff/issues/7099 +157 |-def secondToTime(s0: int) -> (int, int, int) or str: + 157 |+def secondToTime(s0: int) -> (int, int, int): +158 158 | m, s = divmod(s0, 60) +159 159 | +160 160 | + +SIM222.py:161:31: SIM222 [*] Use `(int, int, int)` instead of `(int, int, int) or ...` + | +161 | def secondToTime(s0: int) -> ((int, int, int) or str): + | ^^^^^^^^^^^^^^^^^^^^^^ SIM222 +162 | m, s = divmod(s0, 60) + | + = help: Replace with `(int, int, int)` + +ℹ Suggested fix +158 158 | m, s = divmod(s0, 60) +159 159 | +160 160 | +161 |-def secondToTime(s0: int) -> ((int, int, int) or str): + 161 |+def secondToTime(s0: int) -> ((int, int, int)): +162 162 | m, s = divmod(s0, 60) diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 4408ff9f05d5e..87c070bb96ed6 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -1,6 +1,5 @@ //! Generate Python source code from an abstract syntax tree (AST). -use ruff_python_ast::{ParameterWithDefault, TypeParams}; use std::ops::Deref; use ruff_python_ast::{ @@ -8,8 +7,8 @@ use ruff_python_ast::{ ExceptHandler, Expr, Identifier, MatchCase, Operator, Parameter, Parameters, Pattern, Stmt, Suite, TypeParam, TypeParamParamSpec, TypeParamTypeVar, TypeParamTypeVarTuple, WithItem, }; +use ruff_python_ast::{ParameterWithDefault, TypeParams}; use ruff_python_literal::escape::{AsciiEscape, Escape, UnicodeEscape}; - use ruff_source_file::LineEnding; use super::stylist::{Indentation, Quote, Stylist}; @@ -1381,12 +1380,12 @@ impl<'a> Generator<'a> { mod tests { use ruff_python_ast::{Mod, ModModule}; use ruff_python_parser::{self, parse_suite, Mode}; - use ruff_source_file::LineEnding; - use super::Generator; use crate::stylist::{Indentation, Quote}; + use super::Generator; + fn round_trip(contents: &str) -> String { let indentation = Indentation::default(); let quote = Quote::default();