Skip to content

Commit 20b13fd

Browse files
committed
Account for other property-like decorators as well
1 parent ac94d4a commit 20b13fd

File tree

3 files changed

+69
-42
lines changed

3 files changed

+69
-42
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_return/RET501.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,3 @@
1-
from functools import cached_property
2-
3-
41
def x(y):
52
if not y:
63
return
@@ -21,7 +18,27 @@ def prop(self) -> None:
2118
print("Property not found")
2219
return None
2320

21+
22+
import abc
23+
import enum
24+
import types
25+
from functools import cached_property
26+
27+
28+
class BaseCache2:
2429
@cached_property
2530
def prop(self) -> None:
2631
print("Property not found")
2732
return None
33+
34+
@abc.abstractproperty
35+
def prop2(self) -> None:
36+
return None
37+
38+
@types.DynamicClassAttribute
39+
def prop3(self) -> None:
40+
return None
41+
42+
@enum.property
43+
def prop4(self) -> None:
44+
return None

crates/ruff_linter/src/rules/flake8_return/rules/function.rs

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use anyhow::Result;
55
use ruff_diagnostics::{AlwaysFixableViolation, FixAvailability, Violation};
66
use ruff_diagnostics::{Diagnostic, Edit, Fix};
77
use ruff_macros::{derive_message_formats, violation};
8-
use ruff_python_ast::helpers::{is_const_false, is_const_true, map_callable};
8+
use ruff_python_ast::helpers::{is_const_false, is_const_true};
99
use ruff_python_ast::stmt_if::elif_else_range;
1010
use ruff_python_ast::visitor::Visitor;
1111
use ruff_python_ast::whitespace::indentation;
@@ -373,18 +373,14 @@ fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator],
373373
continue;
374374
}
375375

376-
// Skip properties and cached properties.
377-
if decorator_list.iter().any(|decorator| {
378-
checker
379-
.semantic()
380-
.resolve_qualified_name(map_callable(&decorator.expression))
381-
.is_some_and(|qualified_name| {
382-
matches!(
383-
qualified_name.segments(),
384-
["" | "builtins", "property"] | ["functools", "cached_property"]
385-
)
386-
})
387-
}) {
376+
// Skip properties and other stdlib property-like decorators.
377+
//
378+
// TODO: be more principled here once we have type inference;
379+
// hardcoding these is a little hacky.
380+
if decorator_list
381+
.iter()
382+
.any(|decorator| is_property_like_decorator(decorator, checker.semantic()))
383+
{
388384
return;
389385
}
390386

@@ -397,6 +393,20 @@ fn unnecessary_return_none(checker: &mut Checker, decorator_list: &[Decorator],
397393
}
398394
}
399395

396+
fn is_property_like_decorator(decorator: &Decorator, semantic: &SemanticModel) -> bool {
397+
semantic
398+
.resolve_qualified_name(&decorator.expression)
399+
.is_some_and(|qualified_name| {
400+
matches!(
401+
qualified_name.segments(),
402+
["" | "builtins" | "enum", "property"]
403+
| ["functools", "cached_property"]
404+
| ["abc", "abstractproperty"]
405+
| ["types", "DynamicClassAttribute"]
406+
)
407+
})
408+
}
409+
400410
/// RET502
401411
fn implicit_return_value(checker: &mut Checker, stack: &Stack) {
402412
for stmt in &stack.returns {
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,42 @@
11
---
22
source: crates/ruff_linter/src/rules/flake8_return/mod.rs
33
---
4-
RET501.py:7:5: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value
4+
RET501.py:4:5: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value
55
|
6-
5 | if not y:
7-
6 | return
8-
7 | return None # error
6+
2 | if not y:
7+
3 | return
8+
4 | return None # error
99
| ^^^^^^^^^^^ RET501
1010
|
1111
= help: Remove explicit `return None`
1212

1313
ℹ Safe fix
14-
4 4 | def x(y):
15-
5 5 | if not y:
16-
6 6 | return
17-
7 |- return None # error
18-
7 |+ return # error
19-
8 8 |
20-
9 9 |
21-
10 10 | class BaseCache:
14+
1 1 | def x(y):
15+
2 2 | if not y:
16+
3 3 | return
17+
4 |- return None # error
18+
4 |+ return # error
19+
5 5 |
20+
6 6 |
21+
7 7 | class BaseCache:
2222

23-
RET501.py:17:9: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value
23+
RET501.py:14:9: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value
2424
|
25-
15 | def get(self, key: str) -> None:
26-
16 | print(f"{key} not found")
27-
17 | return None
25+
12 | def get(self, key: str) -> None:
26+
13 | print(f"{key} not found")
27+
14 | return None
2828
| ^^^^^^^^^^^ RET501
29-
18 |
30-
19 | @property
29+
15 |
30+
16 | @property
3131
|
3232
= help: Remove explicit `return None`
3333

3434
ℹ Safe fix
35-
14 14 |
36-
15 15 | def get(self, key: str) -> None:
37-
16 16 | print(f"{key} not found")
38-
17 |- return None
39-
17 |+ return
40-
18 18 |
41-
19 19 | @property
42-
20 20 | def prop(self) -> None:
35+
11 11 |
36+
12 12 | def get(self, key: str) -> None:
37+
13 13 | print(f"{key} not found")
38+
14 |- return None
39+
14 |+ return
40+
15 15 |
41+
16 16 | @property
42+
17 17 | def prop(self) -> None:

0 commit comments

Comments
 (0)