Handle functions that never return in RET503 (#2701)

This commit is contained in:
Peter Pentchev 2023-02-10 16:28:34 +02:00 committed by GitHub
parent ec63658250
commit cda2ff0b18
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 155 additions and 16 deletions

View file

@ -1,3 +1,11 @@
import os
import posix
from posix import abort
import sys as std_sys
import pytest
from pytest import xfail as py_xfail
### ###
# Errors # Errors
### ###
@ -39,6 +47,20 @@ def x(y):
print() # error print() # error
# A nonexistent function
def func_unknown(x):
if x > 0:
return False
no_such_function() # error
# A function that does return the control
def func_no_noreturn(x):
if x > 0:
return False
print("", end="") # error
### ###
# Non-errors # Non-errors
### ###
@ -123,3 +145,52 @@ def prompts(self, foo):
for x in foo: for x in foo:
yield x yield x
yield x + 1 yield x + 1
# Functions that never return
def noreturn__exit(x):
if x > 0:
return 1
os._exit(0)
def noreturn_abort(x):
if x > 0:
return 1
os.abort()
def noreturn_abort_2():
if x > 0:
return 1
abort()
def noreturn_exit():
if x > 0:
return 1
std_sys.exit(0)
def noreturn_pytest_exit():
if x > 0:
return 1
pytest.exit("oof")
def noreturn_pytest_fail():
if x > 0:
return 1
pytest.fail("oof")
def noreturn_pytest_skip():
if x > 0:
return 1
pytest.skip("oof")
def noreturn_pytest_xfail():
if x > 0:
return 1
py_xfail("oof")

View file

@ -165,6 +165,30 @@ fn implicit_return_value(checker: &mut Checker, stack: &Stack) {
} }
} }
const NORETURN_FUNCS: &[&[&str]] = &[
// builtins
&["", "exit"],
// stdlib
&["os", "_exit"],
&["os", "abort"],
&["posix", "abort"],
&["sys", "exit"],
// third-party modules
&["pytest", "exit"],
&["pytest", "fail"],
&["pytest", "skip"],
&["pytest", "xfail"],
];
/// Return `true` if the `func` is a known function that never returns.
fn is_noreturn_func(checker: &Checker, func: &Expr) -> bool {
checker.resolve_call_path(func).map_or(false, |call_path| {
NORETURN_FUNCS
.iter()
.any(|target| call_path.as_slice() == *target)
})
}
/// RET503 /// RET503
fn implicit_return(checker: &mut Checker, last_stmt: &Stmt) { fn implicit_return(checker: &mut Checker, last_stmt: &Stmt) {
match &last_stmt.node { match &last_stmt.node {
@ -208,6 +232,12 @@ fn implicit_return(checker: &mut Checker, last_stmt: &Stmt) {
| StmtKind::While { .. } | StmtKind::While { .. }
| StmtKind::Raise { .. } | StmtKind::Raise { .. }
| StmtKind::Try { .. } => {} | StmtKind::Try { .. } => {}
StmtKind::Expr { value, .. }
if matches!(
&value.node,
ExprKind::Call { func, .. }
if is_noreturn_func(checker, func)
) => {}
_ => { _ => {
let mut diagnostic = Diagnostic::new(ImplicitReturn, Range::from_located(last_stmt)); let mut diagnostic = Diagnostic::new(ImplicitReturn, Range::from_located(last_stmt));
if checker.patch(diagnostic.kind.rule()) { if checker.patch(diagnostic.kind.rule()) {

View file

@ -5,78 +5,116 @@ expression: diagnostics
- kind: - kind:
ImplicitReturn: ~ ImplicitReturn: ~
location: location:
row: 7 row: 15
column: 4 column: 4
end_location: end_location:
row: 8 row: 16
column: 16 column: 16
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
ImplicitReturn: ~ ImplicitReturn: ~
location: location:
row: 14 row: 22
column: 8 column: 8
end_location: end_location:
row: 14 row: 22
column: 15 column: 15
fix: fix:
content: content:
- " return None" - " return None"
- "" - ""
location: location:
row: 15 row: 23
column: 0 column: 0
end_location: end_location:
row: 15 row: 23
column: 0 column: 0
parent: ~ parent: ~
- kind: - kind:
ImplicitReturn: ~ ImplicitReturn: ~
location: location:
row: 23 row: 31
column: 4 column: 4
end_location: end_location:
row: 23 row: 31
column: 11 column: 11
fix: fix:
content: content:
- " return None" - " return None"
- "" - ""
location: location:
row: 24 row: 32
column: 0 column: 0
end_location: end_location:
row: 24 row: 32
column: 0 column: 0
parent: ~ parent: ~
- kind: - kind:
ImplicitReturn: ~ ImplicitReturn: ~
location: location:
row: 29 row: 37
column: 8 column: 8
end_location: end_location:
row: 30 row: 38
column: 20 column: 20
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
ImplicitReturn: ~ ImplicitReturn: ~
location: location:
row: 39 row: 47
column: 8 column: 8
end_location: end_location:
row: 39 row: 47
column: 15 column: 15
fix: fix:
content: content:
- " return None" - " return None"
- "" - ""
location: location:
row: 40 row: 48
column: 0 column: 0
end_location: end_location:
row: 40 row: 48
column: 0
parent: ~
- kind:
ImplicitReturn: ~
location:
row: 54
column: 4
end_location:
row: 54
column: 22
fix:
content:
- " return None"
- ""
location:
row: 55
column: 0
end_location:
row: 55
column: 0
parent: ~
- kind:
ImplicitReturn: ~
location:
row: 61
column: 4
end_location:
row: 61
column: 21
fix:
content:
- " return None"
- ""
location:
row: 62
column: 0
end_location:
row: 62
column: 0 column: 0
parent: ~ parent: ~