Skip to content

Commit

Permalink
Avoid panic when fixing inlined else blocks
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 27, 2024
1 parent 157d5ba commit ecd6b0a
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 5 deletions.
14 changes: 14 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 @@ -162,6 +162,20 @@ def bar7():
: # comment
pass


def bar8():
if True:
return
else: pass


def bar9():
if True:
return
else:\
pass


x = 0

if x == 1:
Expand Down
19 changes: 16 additions & 3 deletions crates/ruff_linter/src/rules/flake8_return/rules/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,9 +756,7 @@ pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Ex

fn remove_else(checker: &mut Checker, elif_else: &ElifElseClause) -> Result<Fix> {
if elif_else.test.is_some() {
// it's an elif, so we can just make it an if

// delete "el" from "elif"
// Ex) `elif` -> `if`
Ok(Fix::safe_edit(Edit::deletion(
elif_else.start(),
elif_else.start() + TextSize::from(2),
Expand All @@ -783,6 +781,21 @@ fn remove_else(checker: &mut Checker, elif_else: &ElifElseClause) -> Result<Fix>
return Err(anyhow::anyhow!("Compound statement cannot be inlined"));
};

// If the statement is on the same line as the `else`, just remove the `else: `.
// Ex) `else: return True` -> `return True`
let Some(first) = elif_else.body.first() else {
return Err(anyhow::anyhow!("`else` statement has no body"));
};
if checker
.indexer()
.in_multi_statement_line(first, checker.locator())
{
return Ok(Fix::safe_edit(Edit::deletion(
elif_else.start(),
first.start(),
)));
}

// we're deleting the `else`, and it's Colon, and the rest of the line(s) they're on,
// so here we get the last position of the line the Colon is on
let else_colon_end = checker.locator().full_line_end(else_colon.end());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,23 @@ RET505.py:161:5: RET505 Unnecessary `else` after `return` statement
|
= help: Remove unnecessary `else`

RET505.py:169:5: RET505 Unnecessary `else` after `return` statement
|
167 | if True:
168 | return
169 | else: pass
| ^^^^ RET505
|
= help: Remove unnecessary `else`

RET505.py:175:5: RET505 Unnecessary `else` after `return` statement
|
173 | if True:
174 | return
175 | else:\
| ^^^^ RET505
176 | pass
|
= help: Remove unnecessary `else`


Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,47 @@ RET505.py:161:5: RET505 [*] Unnecessary `else` after `return` statement
161 |+ # comment
162 |+ pass
164 163 |
165 164 | x = 0
166 165 |
165 164 |
166 165 | def bar8():

RET505.py:169:5: RET505 [*] Unnecessary `else` after `return` statement
|
167 | if True:
168 | return
169 | else: pass
| ^^^^ RET505
|
= help: Remove unnecessary `else`

Safe fix
166 166 | def bar8():
167 167 | if True:
168 168 | return
169 |- else: pass
169 |+ pass
170 170 |
171 171 |
172 172 | def bar9():

RET505.py:175:5: RET505 [*] Unnecessary `else` after `return` statement
|
173 | if True:
174 | return
175 | else:\
| ^^^^ RET505
176 | pass
|
= help: Remove unnecessary `else`

Safe fix
172 172 | def bar9():
173 173 | if True:
174 174 | return
175 |- else:\
176 |- pass
175 |+ pass
177 176 |
178 177 |
179 178 | x = 0


0 comments on commit ecd6b0a

Please sign in to comment.