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 5728ebcbfe269c..ddf43c179534a8 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py @@ -1,6 +1,3 @@ -from functools import cached_property - - def x(y): if not y: return @@ -21,7 +18,27 @@ def prop(self) -> None: print("Property not found") return None + +import abc +import enum +import types +from functools import cached_property + + +class BaseCache2: @cached_property def prop(self) -> None: print("Property not found") return None + + @abc.abstractproperty + def prop2(self) -> None: + return None + + @types.DynamicClassAttribute + def prop3(self) -> None: + return None + + @enum.property + def prop4(self) -> None: + return None 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 ddfe2cf1afe3ee..5590947d0edd30 100644 --- a/crates/ruff_linter/src/rules/flake8_return/rules/function.rs +++ b/crates/ruff_linter/src/rules/flake8_return/rules/function.rs @@ -5,7 +5,7 @@ use anyhow::Result; use ruff_diagnostics::{AlwaysFixableViolation, FixAvailability, Violation}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::{is_const_false, is_const_true, map_callable}; +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; @@ -373,18 +373,14 @@ fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator], continue; } - // Skip properties and cached properties. - if decorator_list.iter().any(|decorator| { - checker - .semantic() - .resolve_qualified_name(map_callable(&decorator.expression)) - .is_some_and(|qualified_name| { - matches!( - qualified_name.segments(), - ["" | "builtins", "property"] | ["functools", "cached_property"] - ) - }) - }) { + // Skip properties and other stdlib property-like decorators. + // + // TODO: be more principled here once we have type inference; + // hardcoding these is a little hacky. + if decorator_list + .iter() + .any(|decorator| is_property_like_decorator(decorator, checker.semantic())) + { return; } @@ -397,6 +393,20 @@ fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator], } } +fn is_property_like_decorator(decorator: &Decorator, semantic: &SemanticModel) -> bool { + semantic + .resolve_qualified_name(&decorator.expression) + .is_some_and(|qualified_name| { + matches!( + qualified_name.segments(), + ["" | "builtins" | "enum", "property"] + | ["functools", "cached_property"] + | ["abc", "abstractproperty"] + | ["types", "DynamicClassAttribute"] + ) + }) +} + /// RET502 fn implicit_return_value(checker: &mut Checker, stack: &Stack) { for stmt in &stack.returns { diff --git a/crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET501_RET501.py.snap b/crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET501_RET501.py.snap index 2cece7ec25311c..889492cab66230 100644 --- a/crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET501_RET501.py.snap +++ b/crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET501_RET501.py.snap @@ -1,42 +1,42 @@ --- source: crates/ruff_linter/src/rules/flake8_return/mod.rs --- -RET501.py:7:5: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value +RET501.py:4:5: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value | -5 | if not y: -6 | return -7 | return None # error +2 | if not y: +3 | return +4 | return None # error | ^^^^^^^^^^^ RET501 | = help: Remove explicit `return None` ℹ Safe fix -4 4 | def x(y): -5 5 | if not y: -6 6 | return -7 |- return None # error - 7 |+ return # error -8 8 | -9 9 | -10 10 | class BaseCache: +1 1 | def x(y): +2 2 | if not y: +3 3 | return +4 |- return None # error + 4 |+ return # error +5 5 | +6 6 | +7 7 | class BaseCache: -RET501.py:17:9: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value +RET501.py:14:9: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value | -15 | def get(self, key: str) -> None: -16 | print(f"{key} not found") -17 | return None +12 | def get(self, key: str) -> None: +13 | print(f"{key} not found") +14 | return None | ^^^^^^^^^^^ RET501 -18 | -19 | @property +15 | +16 | @property | = help: Remove explicit `return None` ℹ Safe fix -14 14 | -15 15 | def get(self, key: str) -> None: -16 16 | print(f"{key} not found") -17 |- return None - 17 |+ return -18 18 | -19 19 | @property -20 20 | def prop(self) -> None: +11 11 | +12 12 | def get(self, key: str) -> None: +13 13 | print(f"{key} not found") +14 |- return None + 14 |+ return +15 15 | +16 16 | @property +17 17 | def prop(self) -> None: