diff --git a/crates/ruff/resources/test/fixtures/flake8_return/RET501.py b/crates/ruff/resources/test/fixtures/flake8_return/RET501.py index 75caa1a22f..9bde6810cd 100644 --- a/crates/ruff/resources/test/fixtures/flake8_return/RET501.py +++ b/crates/ruff/resources/test/fixtures/flake8_return/RET501.py @@ -2,3 +2,13 @@ def x(y): if not y: return return None # error + + +class BaseCache: + def get(self, key: str) -> str | None: + print(f"{key} not found") + return None + + def get(self, key: str) -> None: + print(f"{key} not found") + return None diff --git a/crates/ruff/resources/test/fixtures/pylint/useless_return.py b/crates/ruff/resources/test/fixtures/pylint/useless_return.py index 75da8d8beb..135aa27f94 100644 --- a/crates/ruff/resources/test/fixtures/pylint/useless_return.py +++ b/crates/ruff/resources/test/fixtures/pylint/useless_return.py @@ -48,3 +48,13 @@ def print_python_version(): """This function returns None.""" print(sys.version) return None # [useless-return] + + +class BaseCache: + def get(self, key: str) -> str | None: + print(f"{key} not found") + return None + + def get(self, key: str) -> None: + print(f"{key} not found") + return None diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index d72ccba5f2..b2d6ad052b 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -434,11 +434,20 @@ where Rule::SuperfluousElseContinue, Rule::SuperfluousElseBreak, ]) { - flake8_return::rules::function(self, body); + flake8_return::rules::function( + self, + body, + returns.as_ref().map(|expr| &**expr), + ); } if self.settings.rules.enabled(Rule::UselessReturn) { - pylint::rules::useless_return(self, stmt, body); + pylint::rules::useless_return( + self, + stmt, + body, + returns.as_ref().map(|expr| &**expr), + ); } if self.settings.rules.enabled(Rule::ComplexStructure) { diff --git a/crates/ruff/src/rules/flake8_annotations/helpers.rs b/crates/ruff/src/rules/flake8_annotations/helpers.rs index 519bb8c8a9..5d16e832d4 100644 --- a/crates/ruff/src/rules/flake8_annotations/helpers.rs +++ b/crates/ruff/src/rules/flake8_annotations/helpers.rs @@ -6,9 +6,7 @@ use ruff_python_ast::visibility; use crate::checkers::ast::Checker; use crate::docstrings::definition::{Definition, DefinitionKind}; -pub(super) fn match_function_def( - stmt: &Stmt, -) -> (&str, &Arguments, &Option>, &Vec) { +pub(super) fn match_function_def(stmt: &Stmt) -> (&str, &Arguments, Option<&Expr>, &Vec) { match &stmt.node { StmtKind::FunctionDef { name, @@ -23,7 +21,7 @@ pub(super) fn match_function_def( returns, body, .. - } => (name, args, returns, body), + } => (name, args, returns.as_ref().map(|expr| &**expr), body), _ => panic!("Found non-FunctionDef in match_name"), } } diff --git a/crates/ruff/src/rules/flake8_return/rules.rs b/crates/ruff/src/rules/flake8_return/rules.rs index 78a9bebdda..621e4f2bc5 100644 --- a/crates/ruff/src/rules/flake8_return/rules.rs +++ b/crates/ruff/src/rules/flake8_return/rules.rs @@ -4,6 +4,7 @@ use rustpython_parser::ast::{Constant, Expr, ExprKind, Location, Stmt, StmtKind} use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; use ruff_diagnostics::{Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::is_const_none; use ruff_python_ast::helpers::{elif_else_range, end_of_statement}; use ruff_python_ast::types::Range; use ruff_python_ast::visitor::Visitor; @@ -484,7 +485,7 @@ fn superfluous_else(checker: &mut Checker, stack: &Stack) -> bool { } /// Run all checks from the `flake8-return` plugin. -pub fn function(checker: &mut Checker, body: &[Stmt]) { +pub fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Expr>) { // Skip empty functions. if body.is_empty() { return; @@ -535,7 +536,10 @@ pub fn function(checker: &mut Checker, body: &[Stmt]) { if !result_exists(&stack.returns) { if checker.settings.rules.enabled(Rule::UnnecessaryReturnNone) { - unnecessary_return_none(checker, &stack); + // Skip functions that have a return annotation that is not `None`. + if returns.map_or(true, is_const_none) { + unnecessary_return_none(checker, &stack); + } } return; } diff --git a/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET501_RET501.py.snap b/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET501_RET501.py.snap index 650524d1a1..08e0c586da 100644 --- a/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET501_RET501.py.snap +++ b/crates/ruff/src/rules/flake8_return/snapshots/ruff__rules__flake8_return__tests__RET501_RET501.py.snap @@ -22,4 +22,24 @@ expression: diagnostics row: 4 column: 15 parent: ~ +- kind: + name: UnnecessaryReturnNone + body: "Do not explicitly `return None` in function if it is the only possible return value" + suggestion: "Remove explicit `return None`" + fixable: true + location: + row: 14 + column: 8 + end_location: + row: 14 + column: 19 + fix: + content: return + location: + row: 14 + column: 8 + end_location: + row: 14 + column: 19 + parent: ~ diff --git a/crates/ruff/src/rules/pylint/rules/useless_return.rs b/crates/ruff/src/rules/pylint/rules/useless_return.rs index 3be3b36795..896c7998d8 100644 --- a/crates/ruff/src/rules/pylint/rules/useless_return.rs +++ b/crates/ruff/src/rules/pylint/rules/useless_return.rs @@ -1,5 +1,5 @@ use log::error; -use rustpython_parser::ast::{Constant, ExprKind, Stmt, StmtKind}; +use rustpython_parser::ast::{Constant, Expr, ExprKind, Stmt, StmtKind}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_macros::{derive_message_formats, violation}; @@ -46,7 +46,17 @@ impl AlwaysAutofixableViolation for UselessReturn { } /// PLR1711 -pub fn useless_return<'a>(checker: &mut Checker<'a>, stmt: &'a Stmt, body: &'a [Stmt]) { +pub fn useless_return<'a>( + checker: &mut Checker<'a>, + stmt: &'a Stmt, + body: &'a [Stmt], + returns: Option<&'a Expr>, +) { + // Skip functions that have a return annotation that is not `None`. + if !returns.map_or(true, is_const_none) { + return; + } + // Skip empty functions. if body.is_empty() { return; diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1711_useless_return.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1711_useless_return.py.snap index 46d84a2169..4ef84cbe8f 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1711_useless_return.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLR1711_useless_return.py.snap @@ -102,4 +102,24 @@ expression: diagnostics row: 51 column: 0 parent: ~ +- kind: + name: UselessReturn + body: "Useless `return` statement at end of function" + suggestion: "Remove useless `return` statement" + fixable: true + location: + row: 60 + column: 8 + end_location: + row: 60 + column: 19 + fix: + content: "" + location: + row: 60 + column: 0 + end_location: + row: 61 + column: 0 + parent: ~