[flake8-return] Exempt properties from explicit return rule (RET501) (#12243)

First contribution - apologies if something is missing

Fixes #12197
This commit is contained in:
epenet 2024-07-09 04:39:30 +02:00 committed by GitHub
parent bf3d903939
commit 2041b0e5fb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 38 additions and 10 deletions

View file

@ -12,3 +12,8 @@ class BaseCache:
def get(self, key: str) -> None: def get(self, key: str) -> None:
print(f"{key} not found") print(f"{key} not found")
return None return None
@property
def prop(self) -> None:
print("Property not found")
return None

View file

@ -223,7 +223,12 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
Rule::SuperfluousElseContinue, Rule::SuperfluousElseContinue,
Rule::SuperfluousElseBreak, Rule::SuperfluousElseBreak,
]) { ]) {
flake8_return::rules::function(checker, body, returns.as_ref().map(AsRef::as_ref)); flake8_return::rules::function(
checker,
body,
decorator_list,
returns.as_ref().map(AsRef::as_ref),
);
} }
if checker.enabled(Rule::UselessReturn) { if checker.enabled(Rule::UselessReturn) {
pylint::rules::useless_return( pylint::rules::useless_return(

View file

@ -9,7 +9,7 @@ use ruff_python_ast::helpers::{is_const_false, is_const_true};
use ruff_python_ast::stmt_if::elif_else_range; use ruff_python_ast::stmt_if::elif_else_range;
use ruff_python_ast::visitor::Visitor; use ruff_python_ast::visitor::Visitor;
use ruff_python_ast::whitespace::indentation; use ruff_python_ast::whitespace::indentation;
use ruff_python_ast::{self as ast, ElifElseClause, Expr, Stmt}; use ruff_python_ast::{self as ast, Decorator, ElifElseClause, Expr, Stmt};
use ruff_python_codegen::Stylist; use ruff_python_codegen::Stylist;
use ruff_python_index::Indexer; use ruff_python_index::Indexer;
use ruff_python_semantic::SemanticModel; use ruff_python_semantic::SemanticModel;
@ -364,7 +364,7 @@ impl Violation for SuperfluousElseBreak {
} }
/// RET501 /// RET501
fn unnecessary_return_none(checker: &mut Checker, stack: &Stack) { fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator], stack: &Stack) {
for stmt in &stack.returns { for stmt in &stack.returns {
let Some(expr) = stmt.value.as_deref() else { let Some(expr) = stmt.value.as_deref() else {
continue; continue;
@ -372,7 +372,17 @@ fn unnecessary_return_none(checker: &mut Checker, stack: &Stack) {
if !expr.is_none_literal_expr() { if !expr.is_none_literal_expr() {
continue; continue;
} }
let mut diagnostic = Diagnostic::new(UnnecessaryReturnNone, stmt.range);
// Skip properties.
if decorator_list.iter().any(|decorator| {
checker
.semantic()
.match_builtin_expr(&decorator.expression, "property")
}) {
return;
}
let mut diagnostic = Diagnostic::new(UnnecessaryReturnNone, stmt.range());
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
"return".to_string(), "return".to_string(),
stmt.range(), stmt.range(),
@ -387,10 +397,10 @@ fn implicit_return_value(checker: &mut Checker, stack: &Stack) {
if stmt.value.is_some() { if stmt.value.is_some() {
continue; continue;
} }
let mut diagnostic = Diagnostic::new(ImplicitReturnValue, stmt.range); let mut diagnostic = Diagnostic::new(ImplicitReturnValue, stmt.range());
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
"return None".to_string(), "return None".to_string(),
stmt.range, stmt.range(),
))); )));
checker.diagnostics.push(diagnostic); checker.diagnostics.push(diagnostic);
} }
@ -731,7 +741,12 @@ fn superfluous_elif_else(checker: &mut Checker, stack: &Stack) {
} }
/// Run all checks from the `flake8-return` plugin. /// Run all checks from the `flake8-return` plugin.
pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Expr>) { pub(crate) fn function(
checker: &mut Checker,
body: &[Stmt],
decorator_list: &[Decorator],
returns: Option<&Expr>,
) {
// Find the last statement in the function. // Find the last statement in the function.
let Some(last_stmt) = body.last() else { let Some(last_stmt) = body.last() else {
// Skip empty functions. // Skip empty functions.
@ -787,7 +802,7 @@ pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Ex
if checker.enabled(Rule::UnnecessaryReturnNone) { if checker.enabled(Rule::UnnecessaryReturnNone) {
// Skip functions that have a return annotation that is not `None`. // Skip functions that have a return annotation that is not `None`.
if returns.map_or(true, Expr::is_none_literal_expr) { if returns.map_or(true, Expr::is_none_literal_expr) {
unnecessary_return_none(checker, &stack); unnecessary_return_none(checker, decorator_list, &stack);
} }
} }
} }

View file

@ -26,6 +26,8 @@ RET501.py:14:9: RET501 [*] Do not explicitly `return None` in function if it is
13 | print(f"{key} not found") 13 | print(f"{key} not found")
14 | return None 14 | return None
| ^^^^^^^^^^^ RET501 | ^^^^^^^^^^^ RET501
15 |
16 | @property
| |
= help: Remove explicit `return None` = help: Remove explicit `return None`
@ -35,5 +37,6 @@ RET501.py:14:9: RET501 [*] Do not explicitly `return None` in function if it is
13 13 | print(f"{key} not found") 13 13 | print(f"{key} not found")
14 |- return None 14 |- return None
14 |+ return 14 |+ return
15 15 |
16 16 | @property
17 17 | def prop(self) -> None: