mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-03 18:28:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Detect implicit returns in auto-return-types (#8952)

Browse source 
## Summary

Adds detection for branches without a `return` or `raise`, so that we
can properly `Optional` the return types. I'd like to remove this and
replace it with our code graph analysis from the `unreachable.rs` rule,
but it at least fixes the worst offenders.

Closes #8942.
This commit is contained in:
Charlie Marsh 2023-12-01 12:35:01 -05:00 • committed by GitHub
parent d66063bb33
commit e5db72459e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 860 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

174
crates/ruff_python_ast/src/helpers.rs
View file

@ -911,6 +911,180 @@ where
}
}
/// Returns `true` if the function has an implicit return.
pub fn implicit_return(function: &ast::StmtFunctionDef) -> bool {
/// Returns `true` if the body may break via a `break` statement.
fn sometimes_breaks(stmts: &[Stmt]) -> bool {
for stmt in stmts {
match stmt {
Stmt::For(ast::StmtFor { body, orelse, .. }) => {
if returns(body) {
return false;
}
if sometimes_breaks(orelse) {
return true;
}
}
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
if returns(body) {
return false;
}
if sometimes_breaks(orelse) {
return true;
}
}
Stmt::If(ast::StmtIf {
body,
elif_else_clauses,
..
}) => {
if std::iter::once(body)
.chain(elif_else_clauses.iter().map(|clause| &clause.body))
.any(|body| sometimes_breaks(body))
{
return true;
}
}
Stmt::Match(ast::StmtMatch { cases, .. }) => {
if cases.iter().any(|case| sometimes_breaks(&case.body)) {
return true;
}
}
Stmt::Try(ast::StmtTry {
body,
handlers,
orelse,
finalbody,
..
}) => {
if sometimes_breaks(body)
|| handlers.iter().any(|handler| {
let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
body,
..
}) = handler;
sometimes_breaks(body)
})
|| sometimes_breaks(orelse)
|| sometimes_breaks(finalbody)
{
return true;
}
}
Stmt::With(ast::StmtWith { body, .. }) => {
if sometimes_breaks(body) {
return true;
}
}
Stmt::Break(_) => return true,
Stmt::Return(_) => return false,
Stmt::Raise(_) => return false,
_ => {}
}
}
false
}
/// Returns `true` if the body may break via a `break` statement.
fn always_breaks(stmts: &[Stmt]) -> bool {
for stmt in stmts {
match stmt {
Stmt::Break(_) => return true,
Stmt::Return(_) => return false,
Stmt::Raise(_) => return false,
_ => {}
}
}
false
}
/// Returns `true` if the body contains a branch that ends without an explicit `return` or
/// `raise` statement.
fn returns(stmts: &[Stmt]) -> bool {
for stmt in stmts.iter().rev() {
match stmt {
Stmt::For(ast::StmtFor { body, orelse, .. }) => {
if always_breaks(body) {
return false;
}
if returns(body) {
return true;
}
if returns(orelse) && !sometimes_breaks(body) {
return true;
}
}
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
if always_breaks(body) {
return false;
}
if returns(body) {
return true;
}
if returns(orelse) && !sometimes_breaks(body) {
return true;
}
}
Stmt::If(ast::StmtIf {
body,
elif_else_clauses,
..
}) => {
if elif_else_clauses.iter().any(|clause| clause.test.is_none())
&& std::iter::once(body)
.chain(elif_else_clauses.iter().map(|clause| &clause.body))
.all(|body| returns(body))
{
return true;
}
}
Stmt::Match(ast::StmtMatch { cases, .. }) => {
// Note: we assume the `match` is exhaustive.
if cases.iter().all(|case| returns(&case.body)) {
return true;
}
}
Stmt::Try(ast::StmtTry {
body,
handlers,
orelse,
finalbody,
..
}) => {
// If the `finally` block returns, the `try` block must also return.
if returns(finalbody) {
return true;
}
// If the `body` or the `else` block returns, the `try` block must also return.
if (returns(body) || returns(orelse))
&& handlers.iter().all(|handler| {
let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler {
body,
..
}) = handler;
returns(body)
})
{
return true;
}
}
Stmt::With(ast::StmtWith { body, .. }) => {
if returns(body) {
return true;
}
}
Stmt::Return(_) => return true,
Stmt::Raise(_) => return true,
_ => {}
}
}
false
}
!returns(&function.body)
}
/// A [`StatementVisitor`] that collects all `raise` statements in a function or method.
#[derive(Default)]
pub struct RaiseStatementVisitor<'a> {