diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/collapsible_else_if.py b/crates/ruff_linter/resources/test/fixtures/pylint/collapsible_else_if.py index 7848b2c05725d..0b73d1fe5c943 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/collapsible_else_if.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/collapsible_else_if.py @@ -97,3 +97,50 @@ def not_ok5(): if 2: pass else: pass + + +def not_ok1_with_multiline_comments(): + if 1: + pass + else: + # inner comment which happens + # to be longer than one line + if 2: + pass + else: + pass # final pass comment + + +def not_ok1_with_deep_indented_comments(): + if 1: + pass + else: + # inner comment which happens to be overly indented + if 2: + pass + else: + pass # final pass comment + + +def not_ok1_with_shallow_indented_comments(): + if 1: + pass + else: + # inner comment which happens to be under indented + if 2: + pass + else: + pass # final pass comment + + +def not_ok1_with_mixed_indented_comments(): + if 1: + pass + else: + # inner comment which has mixed + # indentation levels + # which is pretty weird + if 2: + pass + else: + pass # final pass comment 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 5e491b6c53bb6..aae432959fa84 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 @@ -108,7 +108,11 @@ fn convert_to_elif( let inner_if_line_start = locator.line_start(first.start()); let inner_if_line_end = locator.line_end(first.end()); - // Identify the indentation of the loop itself (e.g., the `while` or `for`). + // Capture the trivia between the `else` and the `if`. + let else_line_end = locator.full_line_end(else_clause.start()); + let trivia_range = TextRange::new(else_line_end, inner_if_line_start); + + // Identify the indentation of the outer clause let Some(indentation) = indentation(locator, else_clause) else { return Err(anyhow::anyhow!("`else` is expected to be on its own line")); }; @@ -122,15 +126,30 @@ fn convert_to_elif( stylist, )?; + // If there's trivia, restore it + let trivia = if trivia_range.is_empty() { + None + } else { + let indented_trivia = + adjust_indentation(trivia_range, indentation, locator, indexer, stylist)?; + Some(Edit::insertion( + indented_trivia, + locator.line_start(else_clause.start()), + )) + }; + // Strip the indent from the first line of the `if` statement, and add `el` to the start. let Some(unindented) = indented.strip_prefix(indentation) else { return Err(anyhow::anyhow!("indented block to start with indentation")); }; let indented = format!("{indentation}el{unindented}"); - Ok(Fix::safe_edit(Edit::replacement( - indented, - locator.line_start(else_clause.start()), - inner_if_line_end, - ))) + Ok(Fix::safe_edits( + Edit::replacement( + indented, + locator.line_start(else_clause.start()), + inner_if_line_end, + ), + trivia, + )) } diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap index 4d13e11126c9a..d5b95a89eaa68 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap @@ -73,18 +73,19 @@ collapsible_else_if.py:55:5: PLR5501 [*] Use `elif` instead of `else` then `if`, 52 52 | def not_ok1_with_comments(): 53 53 | if 1: 54 54 | pass - 55 |+ elif 2: - 56 |+ pass -55 57 | else: + 55 |+ # inner comment + 56 |+ elif 2: + 57 |+ pass +55 58 | else: 56 |- # inner comment 57 |- if 2: 58 |- pass 59 |- else: 60 |- pass # final pass comment - 58 |+ pass # final pass comment -61 59 | -62 60 | -63 61 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737 + 59 |+ pass # final pass comment +61 60 | +62 61 | +63 62 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737 collapsible_else_if.py:69:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation | @@ -181,15 +182,150 @@ collapsible_else_if.py:96:5: PLR5501 [*] Use `elif` instead of `else` then `if`, = help: Convert to `elif` ℹ Safe fix -93 93 | def not_ok5(): -94 94 | if 1: -95 95 | pass -96 |- else: -97 |- if 2: -98 |- pass -99 |- else: pass - 96 |+ elif 2: - 97 |+ pass - 98 |+ else: pass +93 93 | def not_ok5(): +94 94 | if 1: +95 95 | pass +96 |- else: +97 |- if 2: +98 |- pass +99 |- else: pass + 96 |+ elif 2: + 97 |+ pass + 98 |+ else: pass +100 99 | +101 100 | +102 101 | def not_ok1_with_multiline_comments(): +collapsible_else_if.py:105:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation + | +103 | if 1: +104 | pass +105 | else: + | _____^ +106 | | # inner comment which happens +107 | | # to be longer than one line +108 | | if 2: + | |________^ PLR5501 +109 | pass +110 | else: + | + = help: Convert to `elif` +ℹ Safe fix +102 102 | def not_ok1_with_multiline_comments(): +103 103 | if 1: +104 104 | pass + 105 |+ # inner comment which happens + 106 |+ # to be longer than one line + 107 |+ elif 2: + 108 |+ pass +105 109 | else: +106 |- # inner comment which happens +107 |- # to be longer than one line +108 |- if 2: +109 |- pass +110 |- else: +111 |- pass # final pass comment + 110 |+ pass # final pass comment +112 111 | +113 112 | +114 113 | def not_ok1_with_deep_indented_comments(): + +collapsible_else_if.py:117:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation + | +115 | if 1: +116 | pass +117 | else: + | _____^ +118 | | # inner comment which happens to be overly indented +119 | | if 2: + | |________^ PLR5501 +120 | pass +121 | else: + | + = help: Convert to `elif` + +ℹ Safe fix +114 114 | def not_ok1_with_deep_indented_comments(): +115 115 | if 1: +116 116 | pass + 117 |+ # inner comment which happens to be overly indented + 118 |+ elif 2: + 119 |+ pass +117 120 | else: +118 |- # inner comment which happens to be overly indented +119 |- if 2: +120 |- pass +121 |- else: +122 |- pass # final pass comment + 121 |+ pass # final pass comment +123 122 | +124 123 | +125 124 | def not_ok1_with_shallow_indented_comments(): + +collapsible_else_if.py:128:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation + | +126 | if 1: +127 | pass +128 | else: + | _____^ +129 | | # inner comment which happens to be under indented +130 | | if 2: + | |________^ PLR5501 +131 | pass +132 | else: + | + = help: Convert to `elif` + +ℹ Safe fix +125 125 | def not_ok1_with_shallow_indented_comments(): +126 126 | if 1: +127 127 | pass +128 |- else: +129 128 | # inner comment which happens to be under indented +130 |- if 2: +131 |- pass +132 |- else: +133 |- pass # final pass comment + 129 |+ elif 2: + 130 |+ pass + 131 |+ else: + 132 |+ pass # final pass comment +134 133 | +135 134 | +136 135 | def not_ok1_with_mixed_indented_comments(): + +collapsible_else_if.py:139:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation + | +137 | if 1: +138 | pass +139 | else: + | _____^ +140 | | # inner comment which has mixed +141 | | # indentation levels +142 | | # which is pretty weird +143 | | if 2: + | |________^ PLR5501 +144 | pass +145 | else: + | + = help: Convert to `elif` + +ℹ Safe fix +136 136 | def not_ok1_with_mixed_indented_comments(): +137 137 | if 1: +138 138 | pass + 139 |+ # inner comment which has mixed + 140 |+ # indentation levels + 141 |+ # which is pretty weird + 142 |+ elif 2: + 143 |+ pass +139 144 | else: +140 |- # inner comment which has mixed +141 |- # indentation levels +142 |- # which is pretty weird +143 |- if 2: +144 |- pass +145 |- else: +146 |- pass # final pass comment + 145 |+ pass # final pass comment