Skip to content

Commit

Permalink
Skip empty lines when determining base indentation (#9795)
Browse files Browse the repository at this point in the history
## Summary

It turns out we saw a panic in cases when dedenting blocks like the `def
wrapper` here:

```python
def instrument_url(f: UrlFuncT) -> UrlFuncT:
    # TODO: Type this with ParamSpec to preserve the function signature.
    if not INSTRUMENTING:  # nocoverage -- option is always enabled; should we remove?
        return f
    else:

        def wrapper(
            self: "ZulipTestCase", url: str, info: object = {}, **kwargs: Union[bool, str]
        ) -> HttpResponseBase:
```

Since we relied on the first line to determine the indentation, instead
of the first non-empty line.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh authored Feb 2, 2024
1 parent e50603c commit ee5b07d
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 3 deletions.
40 changes: 40 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_return/RET505.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,3 +198,43 @@ def bar9():
def sb(self):
if self._sb is not None: return self._sb
else: self._sb = '\033[01;%dm'; self._sa = '\033[0;0m';


def indent(x, y, w, z):
if x: # [no-else-return]
a = 1
return y
else:

c = 3
return z


def indent(x, y, w, z):
if x: # [no-else-return]
a = 1
return y
else:
# comment
c = 3
return z


def indent(x, y, w, z):
if x: # [no-else-return]
a = 1
return y
else:
# comment
c = 3
return z


def indent(x, y, w, z):
if x: # [no-else-return]
a = 1
return y
else:
# comment
c = 3
return z
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,48 @@ RET505.py:200:5: RET505 Unnecessary `else` after `return` statement
|
= help: Remove unnecessary `else`

RET505.py:207:5: RET505 Unnecessary `else` after `return` statement
|
205 | a = 1
206 | return y
207 | else:
| ^^^^ RET505
208 |
209 | c = 3
|
= help: Remove unnecessary `else`

RET505.py:217:5: RET505 Unnecessary `else` after `return` statement
|
215 | a = 1
216 | return y
217 | else:
| ^^^^ RET505
218 | # comment
219 | c = 3
|
= help: Remove unnecessary `else`

RET505.py:227:5: RET505 Unnecessary `else` after `return` statement
|
225 | a = 1
226 | return y
227 | else:
| ^^^^ RET505
228 | # comment
229 | c = 3
|
= help: Remove unnecessary `else`

RET505.py:237:5: RET505 Unnecessary `else` after `return` statement
|
235 | a = 1
236 | return y
237 | else:
| ^^^^ RET505
238 | # comment
239 | c = 3
|
= help: Remove unnecessary `else`


Original file line number Diff line number Diff line change
Expand Up @@ -358,5 +358,107 @@ RET505.py:200:5: RET505 [*] Unnecessary `else` after `return` statement
199 199 | if self._sb is not None: return self._sb
200 |- else: self._sb = '\033[01;%dm'; self._sa = '\033[0;0m';
200 |+ self._sb = '\033[01;%dm'; self._sa = '\033[0;0m';
201 201 |
202 202 |
203 203 | def indent(x, y, w, z):

RET505.py:207:5: RET505 [*] Unnecessary `else` after `return` statement
|
205 | a = 1
206 | return y
207 | else:
| ^^^^ RET505
208 |
209 | c = 3
|
= help: Remove unnecessary `else`

Safe fix
204 204 | if x: # [no-else-return]
205 205 | a = 1
206 206 | return y
207 |- else:
208 207 |
209 |- c = 3
210 |- return z
208 |+ c = 3
209 |+ return z
211 210 |
212 211 |
213 212 | def indent(x, y, w, z):

RET505.py:217:5: RET505 [*] Unnecessary `else` after `return` statement
|
215 | a = 1
216 | return y
217 | else:
| ^^^^ RET505
218 | # comment
219 | c = 3
|
= help: Remove unnecessary `else`

Safe fix
214 214 | if x: # [no-else-return]
215 215 | a = 1
216 216 | return y
217 |- else:
218 |- # comment
219 |- c = 3
220 |- return z
217 |+ # comment
218 |+ c = 3
219 |+ return z
221 220 |
222 221 |
223 222 | def indent(x, y, w, z):

RET505.py:227:5: RET505 [*] Unnecessary `else` after `return` statement
|
225 | a = 1
226 | return y
227 | else:
| ^^^^ RET505
228 | # comment
229 | c = 3
|
= help: Remove unnecessary `else`

Safe fix
224 224 | if x: # [no-else-return]
225 225 | a = 1
226 226 | return y
227 |- else:
228 |- # comment
229 |- c = 3
230 |- return z
227 |+ # comment
228 |+ c = 3
229 |+ return z
231 230 |
232 231 |
233 232 | def indent(x, y, w, z):

RET505.py:237:5: RET505 [*] Unnecessary `else` after `return` statement
|
235 | a = 1
236 | return y
237 | else:
| ^^^^ RET505
238 | # comment
239 | c = 3
|
= help: Remove unnecessary `else`

Safe fix
234 234 | if x: # [no-else-return]
235 235 | a = 1
236 236 | return y
237 |- else:
238 237 | # comment
239 |- c = 3
240 |- return z
238 |+ c = 3
239 |+ return z


13 changes: 10 additions & 3 deletions crates/ruff_python_trivia/src/textwrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,18 @@ pub fn dedent(text: &str) -> Cow<'_, str> {
/// # Panics
/// If the first line is indented by less than the provided indent.
pub fn dedent_to(text: &str, indent: &str) -> String {
// Look at the indentation of the first line, to determine the "baseline" indentation.
// Look at the indentation of the first non-empty line, to determine the "baseline" indentation.
let existing_indent_len = text
.universal_newlines()
.next()
.map_or(0, |line| line.len() - line.trim_start().len());
.find_map(|line| {
let trimmed = line.trim_whitespace_start();
if trimmed.is_empty() || trimmed.starts_with('#') {
None
} else {
Some(line.len() - trimmed.len())
}
})
.unwrap_or_default();

// Determine the amount of indentation to remove.
let dedent_len = existing_indent_len - indent.len();
Expand Down

0 comments on commit ee5b07d

Please sign in to comment.