Skip to content

Commit

Permalink
Extend RET504 to with statements
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jun 10, 2023
1 parent 02b8ce8 commit 8f6912a
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 8 deletions.
37 changes: 33 additions & 4 deletions crates/ruff/resources/test/fixtures/flake8_return/RET504.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,20 +276,25 @@ def str_to_bool(val):

# Mixed assignments
def function_assignment(x):
def f(): ...
def f():
...

return f


def class_assignment(x):
class Foo: ...
class Foo:
...

return Foo


def mixed_function_assignment(x):
if x:
def f(): ...

def f():
...

else:
f = 42

Expand All @@ -298,8 +303,32 @@ def f(): ...

def mixed_class_assignment(x):
if x:
class Foo: ...

class Foo:
...

else:
Foo = 42

return Foo


# `with` statements
def foo():
with open("foo.txt", "r") as f:
x = f.read()
return x


def foo():
with open("foo.txt", "r") as f:
x = f.read()
print(x)
return x


def foo():
with open("foo.txt", "r") as f:
x = f.read()
print(x)
return x
2 changes: 1 addition & 1 deletion crates/ruff/src/rules/flake8_return/rules/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ fn implicit_return(checker: &mut Checker, stmt: &Stmt) {

/// RET504
fn unnecessary_assign(checker: &mut Checker, stack: &Stack) {
for (stmt_assign, stmt_return) in &stack.assignments {
for (stmt_assign, stmt_return) in &stack.assignment_return {
// Identify, e.g., `return x`.
let Some(value) = stmt_return.value.as_ref() else {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,12 @@ RET504.py:268:12: RET504 Unnecessary assignment to `val` before `return` stateme
| ^^^ RET504
|

RET504.py:320:12: RET504 Unnecessary assignment to `x` before `return` statement
|
318 | with open("foo.txt", "r") as f:
319 | x = f.read()
320 | return x
| ^ RET504
|


34 changes: 31 additions & 3 deletions crates/ruff/src/rules/flake8_return/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub(crate) struct Stack<'a> {
/// Whether the current function is a generator.
pub(crate) is_generator: bool,
/// The `assignment`-to-`return` statement pairs in the current function.
pub(crate) assignments: Vec<(&'a ast::StmtAssign, &'a ast::StmtReturn)>,
pub(crate) assignment_return: Vec<(&'a ast::StmtAssign, &'a ast::StmtReturn)>,
}

#[derive(Default)]
Expand Down Expand Up @@ -81,8 +81,36 @@ impl<'a> Visitor<'a> for ReturnVisitor<'a> {
Stmt::Return(stmt_return) => {
// If the `return` statement is preceded by an `assignment` statement, then the
// `assignment` statement may be redundant.
if let Some(stmt_assign) = self.sibling.and_then(Stmt::as_assign_stmt) {
self.stack.assignments.push((stmt_assign, stmt_return));
if let Some(sibling) = self.sibling {
match sibling {
// Example:
// ```python
// def foo():
// x = 1
// return x
// ```
Stmt::Assign(stmt_assign) => {
self.stack
.assignment_return
.push((stmt_assign, stmt_return));
}
// Example:
// ```python
// def foo():
// with open("foo.txt", "r") as f:
// x = f.read()
// return x
// ```
Stmt::With(ast::StmtWith { body, .. })
| Stmt::AsyncWith(ast::StmtAsyncWith { body, .. }) => {
if let Some(stmt_assign) = body.last().and_then(Stmt::as_assign_stmt) {
self.stack
.assignment_return
.push((stmt_assign, stmt_return));
}
}
_ => {}
}
}

self.stack.returns.push(stmt_return);
Expand Down

0 comments on commit 8f6912a

Please sign in to comment.