From 60f18bd0ef7a0c469343b93a96414caba556076e Mon Sep 17 00:00:00 2001 From: epenet <6771947+epenet@users.noreply.github.com> Date: Mon, 8 Jul 2024 13:43:41 +0000 Subject: [PATCH] [flake8-return] Exempt properties (RET501) --- .../test/fixtures/flake8_return/RET501.py | 5 ++++ .../src/checkers/ast/analyze/statement.rs | 7 ++++- .../src/rules/flake8_return/rules/function.rs | 30 ++++++++++++++++--- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py b/crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py index 9bde6810cd7a1..57e814d70d6bd 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py @@ -12,3 +12,8 @@ def get(self, key: str) -> str | None: def get(self, key: str) -> None: print(f"{key} not found") return None + + @property + def prop(self) -> None: + print("Property not found") + return None diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index e92fee7e8d45e..433653c92ecba 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -223,7 +223,12 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { Rule::SuperfluousElseContinue, 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) { pylint::rules::useless_return( diff --git a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs index 124abdfee953e..a1f0f400fe30e 100644 --- a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs +++ b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs @@ -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::visitor::Visitor; 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_index::Indexer; use ruff_python_semantic::SemanticModel; @@ -364,7 +364,16 @@ impl Violation for SuperfluousElseBreak { } /// RET501 -fn unnecessary_return_none(checker: &mut Checker, stack: &Stack) { +fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator], stack: &Stack) { + // Skip properties. + let semantic = checker.semantic(); + if decorator_list + .iter() + .any(|decorator| semantic.match_builtin_expr(&decorator.expression, "property")) + { + return; + } + for stmt in &stack.returns { let Some(expr) = stmt.value.as_deref() else { continue; @@ -731,7 +740,12 @@ fn superfluous_elif_else(checker: &mut Checker, stack: &Stack) { } /// 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. let Some(last_stmt) = body.last() else { // Skip empty functions. @@ -785,9 +799,17 @@ pub(crate) fn function(checker: &mut Checker, body: &[Stmt], returns: Option<&Ex } } else { if checker.enabled(Rule::UnnecessaryReturnNone) { + // Skip properties. + let semantic = checker.semantic(); + if decorator_list + .iter() + .any(|decorator| semantic.match_builtin_expr(&decorator.expression, "property")) + { + return; + } // Skip functions that have a return annotation that is not `None`. if returns.map_or(true, Expr::is_none_literal_expr) { - unnecessary_return_none(checker, &stack); + unnecessary_return_none(checker, decorator_list, &stack); } } }