Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint] Implement useless-return (R1711) #3116

Merged
merged 16 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 21 additions & 0 deletions crates/ruff/resources/test/fixtures/pylint/useless_return.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
def some_fun():
print(5)
return None

def some_other_fun():
print(5)
return

class SomeClass:
def incredible(self):
print(42)
return None

def bare_return_not_final_statement():
if 2 * 2 == 4:
return
print(5)

def tricky():
if 2 * 2 == 4:
return None
3 changes: 3 additions & 0 deletions crates/ruff/src/checkers/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,9 @@ where
if self.settings.rules.enabled(&Rule::ReturnInInit) {
pylint::rules::return_in_init(self, stmt);
}
if self.settings.rules.enabled(&Rule::UselessReturn) {
pylint::rules::useless_return(self, stmt);
}
}
StmtKind::ClassDef {
name,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Pylint, "R0913") => Rule::TooManyArguments,
(Pylint, "R0912") => Rule::TooManyBranches,
(Pylint, "R0915") => Rule::TooManyStatements,
(Pylint, "R1711") => Rule::UselessReturn,
(Pylint, "W2901") => Rule::RedefinedLoopName,

// flake8-builtins
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ ruff_macros::register_rules!(
rules::pyflakes::rules::UnusedAnnotation,
rules::pyflakes::rules::RaiseNotImplemented,
// pylint
rules::pylint::rules::UselessReturn,
rules::pylint::rules::YieldInInit,
rules::pylint::rules::InvalidAllObject,
rules::pylint::rules::InvalidAllFormat,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ mod tests {
#[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"); "PLE2502")]
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"); "PLE01310")]
#[test_case(Rule::YieldInInit, Path::new("yield_in_init.py"); "PLE0100")]
#[test_case(Rule::UselessReturn, Path::new("useless_return.py"); "PLR1711")]
#[test_case(Rule::RedefinedLoopName, Path::new("redefined_loop_name.py"); "PLW2901")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ pub use used_prior_global_declaration::{
};
pub use useless_else_on_loop::{useless_else_on_loop, UselessElseOnLoop};
pub use useless_import_alias::{useless_import_alias, UselessImportAlias};
pub use useless_return::{useless_return, UselessReturn};
pub use yield_in_init::{yield_in_init, YieldInInit};

mod await_outside_async;
Expand Down Expand Up @@ -54,4 +55,5 @@ mod use_from_import;
mod used_prior_global_declaration;
mod useless_else_on_loop;
mod useless_import_alias;
mod useless_return;
mod yield_in_init;
86 changes: 86 additions & 0 deletions crates/ruff/src/rules/pylint/rules/useless_return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use log::error;

use rustpython_parser::ast::{Stmt, StmtKind};

use ruff_macros::{define_violation, derive_message_formats};

use crate::{
ast::{helpers::is_const_none, types::Range},
autofix::helpers::delete_stmt,
checkers::ast::Checker,
registry::Diagnostic,
violation::AlwaysAutofixableViolation,
};

define_violation!(
/// ## What it does
/// Checks if the last statement of a function or a method is `return` or
/// `return None`.
/// ## Why is this bad?
///
/// Python implicitly assumes `None` return value at the end of a function.
/// This statement can be safely removed.
///
/// ## Example
/// ```python
/// def some_fun():
/// print(5)
/// return None
/// ```
pub struct UselessReturn;
);

impl AlwaysAutofixableViolation for UselessReturn {
#[derive_message_formats]
fn message(&self) -> String {
format!("Useless return at end of function or method")
}

fn autofix_title(&self) -> String {
format!("Remove useless return statement at the end of the function")
}
}

pub fn useless_return(checker: &mut Checker, stmt: &Stmt) {
let StmtKind::Return { value} = &stmt.node else {
return;
};
let parent_statement: Option<&Stmt> =
checker.current_stmt_parent().map(std::convert::Into::into);

let belongs_to_function_scope = match parent_statement {
Some(node) => matches!(node.node, StmtKind::FunctionDef { .. }),
None => false,
};
let is_last_function_statement =
checker.current_sibling_stmt().is_none() && belongs_to_function_scope;
if !is_last_function_statement {
return;
}

let is_bare_return_or_none = match value {
None => true,
Some(loc_expr) => is_const_none(loc_expr),
};
if is_bare_return_or_none {
let mut diagnostic = Diagnostic::new(UselessReturn, Range::from_located(stmt));
if checker.patch(diagnostic.kind.rule()) {
match delete_stmt(
stmt,
None,
&[],
checker.locator,
checker.indexer,
checker.stylist,
) {
Ok(fix) => {
diagnostic.amend(fix);
}
Err(e) => {
error!("Failed to delete `return` statement: {}", e);
}
};
}
checker.diagnostics.push(diagnostic);
}
tomecki marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
UselessReturn: ~
location:
row: 3
column: 4
end_location:
row: 3
column: 15
fix:
content: ""
location:
row: 3
column: 0
end_location:
row: 4
column: 0
parent: ~
- kind:
UselessReturn: ~
location:
row: 7
column: 4
end_location:
row: 7
column: 10
fix:
content: ""
location:
row: 7
column: 0
end_location:
row: 8
column: 0
parent: ~
- kind:
UselessReturn: ~
location:
row: 12
column: 8
end_location:
row: 12
column: 19
fix:
content: ""
location:
row: 12
column: 0
end_location:
row: 13
column: 0
parent: ~

2 changes: 2 additions & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.