Skip to content

Commit cc1f766

Browse files
zaniebzanieb
authored
Preserve trivia (i.e. comments) in PLR5501 (#13573)
Closes #13545 As described in the issue, we move comments before the inner `if` statement to before the newly constructed `elif` statement (previously `else`).
1 parent fdd0a22 commit cc1f766

3 files changed

Lines changed: 225 additions & 23 deletions

File tree

‎crates/ruff_linter/resources/test/fixtures/pylint/collapsible_else_if.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,50 @@ def not_ok5():
9797
if 2:
9898
pass
9999
else: pass
100+
101+
102+
def not_ok1_with_multiline_comments():
103+
if 1:
104+
pass
105+
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
112+
113+
114+
def not_ok1_with_deep_indented_comments():
115+
if 1:
116+
pass
117+
else:
118+
# inner comment which happens to be overly indented
119+
if 2:
120+
pass
121+
else:
122+
pass # final pass comment
123+
124+
125+
def not_ok1_with_shallow_indented_comments():
126+
if 1:
127+
pass
128+
else:
129+
# inner comment which happens to be under indented
130+
if 2:
131+
pass
132+
else:
133+
pass # final pass comment
134+
135+
136+
def not_ok1_with_mixed_indented_comments():
137+
if 1:
138+
pass
139+
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

‎crates/ruff_linter/src/rules/pylint/rules/collapsible_else_if.rs‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,11 @@ fn convert_to_elif(
108108
let inner_if_line_start = locator.line_start(first.start());
109109
let inner_if_line_end = locator.line_end(first.end());
110110

111-
// Identify the indentation of the loop itself (e.g., the `while` or `for`).
111+
// Capture the trivia between the `else` and the `if`.
112+
let else_line_end = locator.full_line_end(else_clause.start());
113+
let trivia_range = TextRange::new(else_line_end, inner_if_line_start);
114+
115+
// Identify the indentation of the outer clause
112116
let Some(indentation) = indentation(locator, else_clause) else {
113117
return Err(anyhow::anyhow!("`else` is expected to be on its own line"));
114118
};
@@ -122,15 +126,30 @@ fn convert_to_elif(
122126
stylist,
123127
)?;
124128

129+
// If there's trivia, restore it
130+
let trivia = if trivia_range.is_empty() {
131+
None
132+
} else {
133+
let indented_trivia =
134+
adjust_indentation(trivia_range, indentation, locator, indexer, stylist)?;
135+
Some(Edit::insertion(
136+
indented_trivia,
137+
locator.line_start(else_clause.start()),
138+
))
139+
};
140+
125141
// Strip the indent from the first line of the `if` statement, and add `el` to the start.
126142
let Some(unindented) = indented.strip_prefix(indentation) else {
127143
return Err(anyhow::anyhow!("indented block to start with indentation"));
128144
};
129145
let indented = format!("{indentation}el{unindented}");
130146

131-
Ok(Fix::safe_edit(Edit::replacement(
132-
indented,
133-
locator.line_start(else_clause.start()),
134-
inner_if_line_end,
135-
)))
147+
Ok(Fix::safe_edits(
148+
Edit::replacement(
149+
indented,
150+
locator.line_start(else_clause.start()),
151+
inner_if_line_end,
152+
),
153+
trivia,
154+
))
136155
}

‎crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR5501_collapsible_else_if.py.snap‎

Lines changed: 153 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,19 @@ collapsible_else_if.py:55:5: PLR5501 [*] Use `elif` instead of `else` then `if`,
7373
52 52 | def not_ok1_with_comments():
7474
53 53 | if 1:
7575
54 54 | pass
76-
55 |+ elif 2:
77-
56 |+ pass
78-
55 57 | else:
76+
55 |+ # inner comment
77+
56 |+ elif 2:
78+
57 |+ pass
79+
55 58 | else:
7980
56 |- # inner comment
8081
57 |- if 2:
8182
58 |- pass
8283
59 |- else:
8384
60 |- pass # final pass comment
84-
58 |+ pass # final pass comment
85-
61 59 |
86-
62 60 |
87-
63 61 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737
85+
59 |+ pass # final pass comment
86+
61 60 |
87+
62 61 |
88+
63 62 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737
8889

8990
collapsible_else_if.py:69:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
9091
|
@@ -181,15 +182,150 @@ collapsible_else_if.py:96:5: PLR5501 [*] Use `elif` instead of `else` then `if`,
181182
= help: Convert to `elif`
182183

183184
ℹ Safe fix
184-
93 93 | def not_ok5():
185-
94 94 | if 1:
186-
95 95 | pass
187-
96 |- else:
188-
97 |- if 2:
189-
98 |- pass
190-
99 |- else: pass
191-
96 |+ elif 2:
192-
97 |+ pass
193-
98 |+ else: pass
185+
93 93 | def not_ok5():
186+
94 94 | if 1:
187+
95 95 | pass
188+
96 |- else:
189+
97 |- if 2:
190+
98 |- pass
191+
99 |- else: pass
192+
96 |+ elif 2:
193+
97 |+ pass
194+
98 |+ else: pass
195+
100 99 |
196+
101 100 |
197+
102 101 | def not_ok1_with_multiline_comments():
194198

199+
collapsible_else_if.py:105:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
200+
|
201+
103 | if 1:
202+
104 | pass
203+
105 | else:
204+
| _____^
205+
106 | | # inner comment which happens
206+
107 | | # to be longer than one line
207+
108 | | if 2:
208+
| |________^ PLR5501
209+
109 | pass
210+
110 | else:
211+
|
212+
= help: Convert to `elif`
195213

214+
ℹ Safe fix
215+
102 102 | def not_ok1_with_multiline_comments():
216+
103 103 | if 1:
217+
104 104 | pass
218+
105 |+ # inner comment which happens
219+
106 |+ # to be longer than one line
220+
107 |+ elif 2:
221+
108 |+ pass
222+
105 109 | else:
223+
106 |- # inner comment which happens
224+
107 |- # to be longer than one line
225+
108 |- if 2:
226+
109 |- pass
227+
110 |- else:
228+
111 |- pass # final pass comment
229+
110 |+ pass # final pass comment
230+
112 111 |
231+
113 112 |
232+
114 113 | def not_ok1_with_deep_indented_comments():
233+
234+
collapsible_else_if.py:117:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
235+
|
236+
115 | if 1:
237+
116 | pass
238+
117 | else:
239+
| _____^
240+
118 | | # inner comment which happens to be overly indented
241+
119 | | if 2:
242+
| |________^ PLR5501
243+
120 | pass
244+
121 | else:
245+
|
246+
= help: Convert to `elif`
247+
248+
ℹ Safe fix
249+
114 114 | def not_ok1_with_deep_indented_comments():
250+
115 115 | if 1:
251+
116 116 | pass
252+
117 |+ # inner comment which happens to be overly indented
253+
118 |+ elif 2:
254+
119 |+ pass
255+
117 120 | else:
256+
118 |- # inner comment which happens to be overly indented
257+
119 |- if 2:
258+
120 |- pass
259+
121 |- else:
260+
122 |- pass # final pass comment
261+
121 |+ pass # final pass comment
262+
123 122 |
263+
124 123 |
264+
125 124 | def not_ok1_with_shallow_indented_comments():
265+
266+
collapsible_else_if.py:128:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
267+
|
268+
126 | if 1:
269+
127 | pass
270+
128 | else:
271+
| _____^
272+
129 | | # inner comment which happens to be under indented
273+
130 | | if 2:
274+
| |________^ PLR5501
275+
131 | pass
276+
132 | else:
277+
|
278+
= help: Convert to `elif`
279+
280+
ℹ Safe fix
281+
125 125 | def not_ok1_with_shallow_indented_comments():
282+
126 126 | if 1:
283+
127 127 | pass
284+
128 |- else:
285+
129 128 | # inner comment which happens to be under indented
286+
130 |- if 2:
287+
131 |- pass
288+
132 |- else:
289+
133 |- pass # final pass comment
290+
129 |+ elif 2:
291+
130 |+ pass
292+
131 |+ else:
293+
132 |+ pass # final pass comment
294+
134 133 |
295+
135 134 |
296+
136 135 | def not_ok1_with_mixed_indented_comments():
297+
298+
collapsible_else_if.py:139:5: PLR5501 [*] Use `elif` instead of `else` then `if`, to reduce indentation
299+
|
300+
137 | if 1:
301+
138 | pass
302+
139 | else:
303+
| _____^
304+
140 | | # inner comment which has mixed
305+
141 | | # indentation levels
306+
142 | | # which is pretty weird
307+
143 | | if 2:
308+
| |________^ PLR5501
309+
144 | pass
310+
145 | else:
311+
|
312+
= help: Convert to `elif`
313+
314+
ℹ Safe fix
315+
136 136 | def not_ok1_with_mixed_indented_comments():
316+
137 137 | if 1:
317+
138 138 | pass
318+
139 |+ # inner comment which has mixed
319+
140 |+ # indentation levels
320+
141 |+ # which is pretty weird
321+
142 |+ elif 2:
322+
143 |+ pass
323+
139 144 | else:
324+
140 |- # inner comment which has mixed
325+
141 |- # indentation levels
326+
142 |- # which is pretty weird
327+
143 |- if 2:
328+
144 |- pass
329+
145 |- else:
330+
146 |- pass # final pass comment
331+
145 |+ pass # final pass comment

0 commit comments

Comments
 (0)