mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-29 13:25:17 +00:00
Preserve trivia (i.e. comments) in PLR5501 (#13573)
Closes https://github.com/astral-sh/ruff/issues/13545 As described in the issue, we move comments before the inner `if` statement to before the newly constructed `elif` statement (previously `else`).
This commit is contained in:
parent
fdd0a22c03
commit
cc1f766622
3 changed files with 225 additions and 23 deletions
|
@ -97,3 +97,50 @@ def not_ok5():
|
||||||
if 2:
|
if 2:
|
||||||
pass
|
pass
|
||||||
else: 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
|
||||||
|
|
|
@ -108,7 +108,11 @@ fn convert_to_elif(
|
||||||
let inner_if_line_start = locator.line_start(first.start());
|
let inner_if_line_start = locator.line_start(first.start());
|
||||||
let inner_if_line_end = locator.line_end(first.end());
|
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 {
|
let Some(indentation) = indentation(locator, else_clause) else {
|
||||||
return Err(anyhow::anyhow!("`else` is expected to be on its own line"));
|
return Err(anyhow::anyhow!("`else` is expected to be on its own line"));
|
||||||
};
|
};
|
||||||
|
@ -122,15 +126,30 @@ fn convert_to_elif(
|
||||||
stylist,
|
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.
|
// 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 {
|
let Some(unindented) = indented.strip_prefix(indentation) else {
|
||||||
return Err(anyhow::anyhow!("indented block to start with indentation"));
|
return Err(anyhow::anyhow!("indented block to start with indentation"));
|
||||||
};
|
};
|
||||||
let indented = format!("{indentation}el{unindented}");
|
let indented = format!("{indentation}el{unindented}");
|
||||||
|
|
||||||
Ok(Fix::safe_edit(Edit::replacement(
|
Ok(Fix::safe_edits(
|
||||||
indented,
|
Edit::replacement(
|
||||||
locator.line_start(else_clause.start()),
|
indented,
|
||||||
inner_if_line_end,
|
locator.line_start(else_clause.start()),
|
||||||
)))
|
inner_if_line_end,
|
||||||
|
),
|
||||||
|
trivia,
|
||||||
|
))
|
||||||
}
|
}
|
||||||
|
|
|
@ -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():
|
52 52 | def not_ok1_with_comments():
|
||||||
53 53 | if 1:
|
53 53 | if 1:
|
||||||
54 54 | pass
|
54 54 | pass
|
||||||
55 |+ elif 2:
|
55 |+ # inner comment
|
||||||
56 |+ pass
|
56 |+ elif 2:
|
||||||
55 57 | else:
|
57 |+ pass
|
||||||
|
55 58 | else:
|
||||||
56 |- # inner comment
|
56 |- # inner comment
|
||||||
57 |- if 2:
|
57 |- if 2:
|
||||||
58 |- pass
|
58 |- pass
|
||||||
59 |- else:
|
59 |- else:
|
||||||
60 |- pass # final pass comment
|
60 |- pass # final pass comment
|
||||||
58 |+ pass # final pass comment
|
59 |+ pass # final pass comment
|
||||||
61 59 |
|
61 60 |
|
||||||
62 60 |
|
62 61 |
|
||||||
63 61 | # Regression test for https://github.com/apache/airflow/blob/f1e1cdcc3b2826e68ba133f350300b5065bbca33/airflow/models/dag.py#L1737
|
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
|
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`
|
= help: Convert to `elif`
|
||||||
|
|
||||||
ℹ Safe fix
|
ℹ Safe fix
|
||||||
93 93 | def not_ok5():
|
93 93 | def not_ok5():
|
||||||
94 94 | if 1:
|
94 94 | if 1:
|
||||||
95 95 | pass
|
95 95 | pass
|
||||||
96 |- else:
|
96 |- else:
|
||||||
97 |- if 2:
|
97 |- if 2:
|
||||||
98 |- pass
|
98 |- pass
|
||||||
99 |- else: pass
|
99 |- else: pass
|
||||||
96 |+ elif 2:
|
96 |+ elif 2:
|
||||||
97 |+ pass
|
97 |+ pass
|
||||||
98 |+ else: 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
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue