Avoid adding return types to stub methods (#9277)

We should avoid adding `-> None` to stubs in `.pyi` files, along with a
few other cases. (We already ignore abstract methods.)

Closes https://github.com/astral-sh/ruff/issues/9270.
This commit is contained in:
Charlie Marsh 2023-12-25 09:03:24 -05:00 committed by GitHub
parent 8b70240fa2
commit fa78d2d97c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 85 additions and 55 deletions

View file

@ -212,3 +212,20 @@ def func(x: int):
raise ValueError raise ValueError
else: else:
return 1 return 1
from typing import overload
@overload
def overloaded(i: int) -> "int":
...
@overload
def overloaded(i: "str") -> "str":
...
def overloaded(i):
return i

View file

@ -482,7 +482,6 @@ impl Violation for AnyType {
format!("Dynamically typed expressions (typing.Any) are disallowed in `{name}`") format!("Dynamically typed expressions (typing.Any) are disallowed in `{name}`")
} }
} }
fn is_none_returning(body: &[Stmt]) -> bool { fn is_none_returning(body: &[Stmt]) -> bool {
let mut visitor = ReturnStatementVisitor::default(); let mut visitor = ReturnStatementVisitor::default();
visitor.visit_body(body); visitor.visit_body(body);
@ -537,8 +536,11 @@ fn check_dynamically_typed<F>(
} }
} }
fn is_empty_body(body: &[Stmt]) -> bool { /// Return `true` if a function appears to be a stub.
body.iter().all(|stmt| match stmt { fn is_stub_function(function_def: &ast::StmtFunctionDef, checker: &Checker) -> bool {
/// Returns `true` if a function has an empty body.
fn is_empty_body(function_def: &ast::StmtFunctionDef) -> bool {
function_def.body.iter().all(|stmt| match stmt {
Stmt::Pass(_) => true, Stmt::Pass(_) => true,
Stmt::Expr(ast::StmtExpr { value, range: _ }) => { Stmt::Expr(ast::StmtExpr { value, range: _ }) => {
matches!( matches!(
@ -548,6 +550,27 @@ fn is_empty_body(body: &[Stmt]) -> bool {
} }
_ => false, _ => false,
}) })
}
// Ignore functions with empty bodies in...
if is_empty_body(function_def) {
// Stub definitions (.pyi files)...
if checker.source_type.is_stub() {
return true;
}
// Abstract methods...
if visibility::is_abstract(&function_def.decorator_list, checker.semantic()) {
return true;
}
// Overload definitions...
if visibility::is_overload(&function_def.decorator_list, checker.semantic()) {
return true;
}
}
false
} }
/// Generate flake8-annotation checks for a given `Definition`. /// Generate flake8-annotation checks for a given `Definition`.
@ -738,9 +761,7 @@ pub(crate) fn definition(
) { ) {
if is_method && visibility::is_classmethod(decorator_list, checker.semantic()) { if is_method && visibility::is_classmethod(decorator_list, checker.semantic()) {
if checker.enabled(Rule::MissingReturnTypeClassMethod) { if checker.enabled(Rule::MissingReturnTypeClassMethod) {
let return_type = if visibility::is_abstract(decorator_list, checker.semantic()) let return_type = if is_stub_function(function, checker) {
&& is_empty_body(body)
{
None None
} else { } else {
auto_return_type(function) auto_return_type(function)
@ -771,9 +792,7 @@ pub(crate) fn definition(
} }
} else if is_method && visibility::is_staticmethod(decorator_list, checker.semantic()) { } else if is_method && visibility::is_staticmethod(decorator_list, checker.semantic()) {
if checker.enabled(Rule::MissingReturnTypeStaticMethod) { if checker.enabled(Rule::MissingReturnTypeStaticMethod) {
let return_type = if visibility::is_abstract(decorator_list, checker.semantic()) let return_type = if is_stub_function(function, checker) {
&& is_empty_body(body)
{
None None
} else { } else {
auto_return_type(function) auto_return_type(function)
@ -843,10 +862,7 @@ pub(crate) fn definition(
match visibility { match visibility {
visibility::Visibility::Public => { visibility::Visibility::Public => {
if checker.enabled(Rule::MissingReturnTypeUndocumentedPublicFunction) { if checker.enabled(Rule::MissingReturnTypeUndocumentedPublicFunction) {
let return_type = let return_type = if is_stub_function(function, checker) {
if visibility::is_abstract(decorator_list, checker.semantic())
&& is_empty_body(body)
{
None None
} else { } else {
auto_return_type(function) auto_return_type(function)
@ -885,10 +901,7 @@ pub(crate) fn definition(
} }
visibility::Visibility::Private => { visibility::Visibility::Private => {
if checker.enabled(Rule::MissingReturnTypePrivateFunction) { if checker.enabled(Rule::MissingReturnTypePrivateFunction) {
let return_type = let return_type = if is_stub_function(function, checker) {
if visibility::is_abstract(decorator_list, checker.semantic())
&& is_empty_body(body)
{
None None
} else { } else {
auto_return_type(function) auto_return_type(function)