Skip to content

Commit dcf0a8d

Browse files
Mark RET501 fix unsafe if comments are inside (astral-sh#18780)
Co-authored-by: Charlie Marsh <[email protected]>
1 parent e352a50 commit dcf0a8d

File tree

3 files changed

+45
-4
lines changed

3 files changed

+45
-4
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,13 @@ def prop3(self) -> None:
4949
def prop4(self) -> None:
5050
print("I've run out of things to say")
5151
return None
52+
53+
54+
# https://github.com/astral-sh/ruff/issues/18774
55+
class _:
56+
def foo(bar):
57+
if not bar:
58+
return
59+
return (
60+
None # comment
61+
)

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use anyhow::Result;
22

3+
use ruff_diagnostics::Applicability;
34
use ruff_macros::{ViolationMetadata, derive_message_formats};
45
use ruff_python_ast::helpers::{is_const_false, is_const_true};
56
use ruff_python_ast::stmt_if::elif_else_range;
@@ -50,6 +51,11 @@ use super::super::visitor::{ReturnVisitor, Stack};
5051
/// return
5152
/// return
5253
/// ```
54+
///
55+
/// ## Fix Safety
56+
/// This rule's fix is marked as unsafe for cases in which comments would be
57+
/// dropped from the `return` statement.
58+
///
5359
#[derive(ViolationMetadata)]
5460
pub(crate) struct UnnecessaryReturnNone;
5561

@@ -381,10 +387,15 @@ fn unnecessary_return_none(checker: &Checker, decorator_list: &[Decorator], stac
381387
}
382388

383389
let mut diagnostic = checker.report_diagnostic(UnnecessaryReturnNone, stmt.range());
384-
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
385-
"return".to_string(),
386-
stmt.range(),
387-
)));
390+
let edit = Edit::range_replacement("return".to_string(), stmt.range());
391+
diagnostic.set_fix(Fix::applicable_edit(
392+
edit,
393+
if checker.comment_ranges().intersects(stmt.range()) {
394+
Applicability::Unsafe
395+
} else {
396+
Applicability::Safe
397+
},
398+
));
388399
}
389400
}
390401

crates/ruff_linter/src/rules/flake8_return/snapshots/ruff_linter__rules__flake8_return__tests__RET501_RET501.py.snap

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,23 @@ RET501.py:14:9: RET501 [*] Do not explicitly `return None` in function if it is
4040
15 15 |
4141
16 16 | @property
4242
17 17 | def prop(self) -> None:
43+
44+
RET501.py:59:9: RET501 [*] Do not explicitly `return None` in function if it is the only possible return value
45+
|
46+
57 | if not bar:
47+
58 | return
48+
59 | / return (
49+
60 | | None # comment
50+
61 | | )
51+
| |_________^ RET501
52+
|
53+
= help: Remove explicit `return None`
54+
55+
ℹ Unsafe fix
56+
56 56 | def foo(bar):
57+
57 57 | if not bar:
58+
58 58 | return
59+
59 |- return (
60+
60 |- None # comment
61+
61 |- )
62+
59 |+ return

0 commit comments

Comments
 (0)