From f96365eaa89242381b57db98d18b1ccf35cb7cf5 Mon Sep 17 00:00:00 2001 From: Steve C Date: Sun, 21 Jan 2024 13:23:23 -0500 Subject: [PATCH] don't retain comments on the `else` line! --- .../rules/pylint/rules/collapsible_else_if.rs | 19 +++++++------------ ...eview__PLR5501_collapsible_else_if.py.snap | 2 +- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs b/crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs index 96ab96535c7aa..2b993b61dbebb 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs @@ -1,6 +1,5 @@ use ast::whitespace::indentation; use ruff_python_ast::{self as ast, ElifElseClause, Stmt}; -use ruff_python_trivia::{SimpleTokenKind, SimpleTokenizer}; use ruff_text_size::{Ranged, TextRange}; use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, FixAvailability, Violation}; @@ -87,10 +86,11 @@ pub(crate) fn collapsible_else_if(checker: &mut Checker, stmt: &Stmt) { if checker.settings.preview.is_enabled() { let inner_if_line_start = checker.locator().line_start(first.start()); let inner_if_line_end = checker.locator().line_end(first.end()); - let inner_if_full_line_end = checker.locator().full_line_end(first.end()); + // Identify the indentation of the loop itself (e.g., the `while` or `for`). let desired_indentation = indentation(checker.locator(), else_clause).unwrap_or(""); + // Dedent the content from the end of the `else` to the end of the `if`. let indented = adjust_indentation( TextRange::new(inner_if_line_start, inner_if_line_end), desired_indentation, @@ -99,19 +99,14 @@ pub(crate) fn collapsible_else_if(checker: &mut Checker, stmt: &Stmt) { ) .unwrap(); + // Unindent the first line (which is the `if` and add `el` to the start) let fixed_indented = format!("el{}", indented.strip_prefix(desired_indentation).unwrap()); - let else_colon = - SimpleTokenizer::starts_at(else_clause.start(), checker.locator().contents()) - .find(|token| token.kind() == SimpleTokenKind::Colon) - .unwrap(); - - diagnostic.set_fix(Fix::applicable_edits( - Edit::deletion(inner_if_line_start, inner_if_full_line_end), - [Edit::range_replacement( + diagnostic.set_fix(Fix::applicable_edit( + Edit::range_replacement( fixed_indented, - TextRange::new(else_clause.start(), else_colon.end()), - )], + TextRange::new(else_clause.start(), inner_if_line_end), + ), Applicability::Safe, )); } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLR5501_collapsible_else_if.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLR5501_collapsible_else_if.py.snap index 35e5b27759c8d..c3fa401c86141 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLR5501_collapsible_else_if.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLR5501_collapsible_else_if.py.snap @@ -80,7 +80,7 @@ collapsible_else_if.py:55:5: PLR5501 [*] Use `elif` instead of `else` then `if`, 55 |+ elif 2: 56 |+ pass 57 |+ else: - 58 |+ pass # final pass comment # else comment + 58 |+ pass # final pass comment 60 59 | 61 60 | 62 61 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737