Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions crates/ruff_linter/src/preview.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,6 @@ pub(crate) const fn is_bad_version_info_in_non_stub_enabled(settings: &LinterSet
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/11074
pub(crate) const fn is_only_add_return_none_at_end_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
}

// https://github.com/astral-sh/ruff/pull/16719
pub(crate) const fn is_fix_manual_dict_comprehension_enabled(settings: &LinterSettings) -> bool {
settings.preview.is_enabled()
Expand Down
21 changes: 1 addition & 20 deletions crates/ruff_linter/src/rules/flake8_return/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ mod tests {
use anyhow::Result;
use test_case::test_case;

use crate::assert_messages;
use crate::registry::Rule;
use crate::settings::LinterSettings;
use crate::settings::types::PreviewMode;
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::UnnecessaryReturnNone, Path::new("RET501.py"))]
#[test_case(Rule::ImplicitReturnValue, Path::new("RET502.py"))]
Expand All @@ -34,22 +33,4 @@ mod tests {
assert_messages!(snapshot, diagnostics);
Ok(())
}

#[test_case(Rule::ImplicitReturn, Path::new("RET503.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
rule_code.noqa_code(),
path.to_string_lossy()
);
let diagnostics = test_path(
Path::new("flake8_return").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(snapshot, diagnostics);
Ok(())
}
}
87 changes: 30 additions & 57 deletions crates/ruff_linter/src/rules/flake8_return/rules/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use ruff_text_size::{Ranged, TextRange, TextSize};
use crate::checkers::ast::Checker;
use crate::fix::edits;
use crate::fix::edits::adjust_indentation;
use crate::preview::is_only_add_return_none_at_end_enabled;
use crate::registry::Rule;
use crate::rules::flake8_return::helpers::end_of_last_statement;
use crate::{AlwaysFixableViolation, FixAvailability, Violation};
Expand Down Expand Up @@ -463,96 +462,70 @@ fn add_return_none(checker: &Checker, stmt: &Stmt, range: TextRange) {
}
}

/// Returns a list of all implicit returns in the given statement.
///
/// Note: The function should be refactored to `has_implicit_return` with an early return (when seeing the first implicit return)
/// when removing the preview gating.
fn implicit_returns<'a>(checker: &Checker, stmt: &'a Stmt) -> Vec<&'a Stmt> {
fn has_implicit_return(checker: &Checker, stmt: &Stmt) -> bool {
match stmt {
Stmt::If(ast::StmtIf {
body,
elif_else_clauses,
..
}) => {
let mut implicit_stmts = body
if body
.last()
.map(|last| implicit_returns(checker, last))
.unwrap_or_default();

for clause in elif_else_clauses {
implicit_stmts.extend(
clause
.body
.last()
.iter()
.flat_map(|last| implicit_returns(checker, last)),
);
.is_some_and(|last| has_implicit_return(checker, last))
{
return true;
}

if elif_else_clauses.iter().any(|clause| {
clause
.body
.last()
.is_some_and(|last| has_implicit_return(checker, last))
}) {
return true;
}

// Check if we don't have an else clause
if matches!(
matches!(
elif_else_clauses.last(),
None | Some(ast::ElifElseClause { test: Some(_), .. })
) {
implicit_stmts.push(stmt);
}
implicit_stmts
)
}
Stmt::Assert(ast::StmtAssert { test, .. }) if is_const_false(test) => vec![],
Stmt::While(ast::StmtWhile { test, .. }) if is_const_true(test) => vec![],
Stmt::Assert(ast::StmtAssert { test, .. }) if is_const_false(test) => false,
Stmt::While(ast::StmtWhile { test, .. }) if is_const_true(test) => false,
Stmt::For(ast::StmtFor { orelse, .. }) | Stmt::While(ast::StmtWhile { orelse, .. }) => {
if let Some(last_stmt) = orelse.last() {
implicit_returns(checker, last_stmt)
has_implicit_return(checker, last_stmt)
} else {
vec![stmt]
}
}
Stmt::Match(ast::StmtMatch { cases, .. }) => {
let mut implicit_stmts = vec![];
for case in cases {
implicit_stmts.extend(
case.body
.last()
.into_iter()
.flat_map(|last_stmt| implicit_returns(checker, last_stmt)),
);
true
}
implicit_stmts
}
Stmt::Match(ast::StmtMatch { cases, .. }) => cases.iter().any(|case| {
case.body
.last()
.is_some_and(|last| has_implicit_return(checker, last))
}),
Stmt::With(ast::StmtWith { body, .. }) => body
.last()
.map(|last_stmt| implicit_returns(checker, last_stmt))
.unwrap_or_default(),
Stmt::Return(_) | Stmt::Raise(_) | Stmt::Try(_) => vec![],
.is_some_and(|last_stmt| has_implicit_return(checker, last_stmt)),
Stmt::Return(_) | Stmt::Raise(_) | Stmt::Try(_) => false,
Stmt::Expr(ast::StmtExpr { value, .. })
if matches!(
value.as_ref(),
Expr::Call(ast::ExprCall { func, .. })
if is_noreturn_func(func, checker.semantic())
) =>
{
vec![]
}
_ => {
vec![stmt]
false
}
_ => true,
}
}

/// RET503
fn implicit_return(checker: &Checker, function_def: &ast::StmtFunctionDef, stmt: &Stmt) {
let implicit_stmts = implicit_returns(checker, stmt);

if implicit_stmts.is_empty() {
return;
}

if is_only_add_return_none_at_end_enabled(checker.settings) {
if has_implicit_return(checker, stmt) {
add_return_none(checker, stmt, function_def.range());
} else {
for implicit_stmt in implicit_stmts {
add_return_none(checker, implicit_stmt, implicit_stmt.range());
}
}
}

Expand Down
Loading
Loading