Avoid ANN2xx autofix for abstract methods with empty body (#9034)

## Summary

This PR updates the `ANN201`, `ANN202`, `ANN205`, and `ANN206` rules to
not create a fix for the return type when it's an abstract method and
the function body is empty i.e., it only contains either a pass
statement, docstring or an ellipsis literal.

fixes: #9004

## Test Plan

Add the following test cases:
- Abstract method with pass statement
- Abstract method with docstring
- Abstract method with ellipsis literal
- Abstract method with possible return type
This commit is contained in:
Dhruv Manilawala 2023-12-06 20:47:36 -06:00 committed by GitHub
parent f484df5470
commit 9361e22fe9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 254 additions and 44 deletions

View file

@ -147,3 +147,38 @@ def func(x: int):
while x > 0: while x > 0:
break break
return 1 return 1
import abc
from abc import abstractmethod
class Foo(abc.ABC):
@abstractmethod
def method(self):
pass
@abc.abstractmethod
def method(self):
"""Docstring."""
@abc.abstractmethod
def method(self):
...
@staticmethod
@abstractmethod
def method():
pass
@classmethod
@abstractmethod
def method(cls):
pass
@abstractmethod
def method(self):
if self.x > 0:
return 1
else:
return 1.5

View file

@ -537,6 +537,19 @@ fn check_dynamically_typed<F>(
} }
} }
fn is_empty_body(body: &[Stmt]) -> bool {
body.iter().all(|stmt| match stmt {
Stmt::Pass(_) => true,
Stmt::Expr(ast::StmtExpr { value, range: _ }) => {
matches!(
value.as_ref(),
Expr::StringLiteral(_) | Expr::EllipsisLiteral(_)
)
}
_ => false,
})
}
/// Generate flake8-annotation checks for a given `Definition`. /// Generate flake8-annotation checks for a given `Definition`.
pub(crate) fn definition( pub(crate) fn definition(
checker: &Checker, checker: &Checker,
@ -725,7 +738,12 @@ 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 = auto_return_type(function) let return_type = if visibility::is_abstract(decorator_list, checker.semantic())
&& is_empty_body(body)
{
None
} else {
auto_return_type(function)
.and_then(|return_type| { .and_then(|return_type| {
return_type.into_expression( return_type.into_expression(
checker.importer(), checker.importer(),
@ -734,7 +752,8 @@ pub(crate) fn definition(
checker.settings.target_version, checker.settings.target_version,
) )
}) })
.map(|(return_type, edits)| (checker.generator().expr(&return_type), edits)); .map(|(return_type, edits)| (checker.generator().expr(&return_type), edits))
};
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
MissingReturnTypeClassMethod { MissingReturnTypeClassMethod {
name: name.to_string(), name: name.to_string(),
@ -752,7 +771,12 @@ 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 = auto_return_type(function) let return_type = if visibility::is_abstract(decorator_list, checker.semantic())
&& is_empty_body(body)
{
None
} else {
auto_return_type(function)
.and_then(|return_type| { .and_then(|return_type| {
return_type.into_expression( return_type.into_expression(
checker.importer(), checker.importer(),
@ -761,7 +785,8 @@ pub(crate) fn definition(
checker.settings.target_version, checker.settings.target_version,
) )
}) })
.map(|(return_type, edits)| (checker.generator().expr(&return_type), edits)); .map(|(return_type, edits)| (checker.generator().expr(&return_type), edits))
};
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
MissingReturnTypeStaticMethod { MissingReturnTypeStaticMethod {
name: name.to_string(), name: name.to_string(),
@ -818,7 +843,13 @@ 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 = auto_return_type(function) let return_type =
if visibility::is_abstract(decorator_list, checker.semantic())
&& is_empty_body(body)
{
None
} else {
auto_return_type(function)
.and_then(|return_type| { .and_then(|return_type| {
return_type.into_expression( return_type.into_expression(
checker.importer(), checker.importer(),
@ -829,7 +860,8 @@ pub(crate) fn definition(
}) })
.map(|(return_type, edits)| { .map(|(return_type, edits)| {
(checker.generator().expr(&return_type), edits) (checker.generator().expr(&return_type), edits)
}); })
};
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
MissingReturnTypeUndocumentedPublicFunction { MissingReturnTypeUndocumentedPublicFunction {
name: name.to_string(), name: name.to_string(),
@ -853,7 +885,13 @@ pub(crate) fn definition(
} }
visibility::Visibility::Private => { visibility::Visibility::Private => {
if checker.enabled(Rule::MissingReturnTypePrivateFunction) { if checker.enabled(Rule::MissingReturnTypePrivateFunction) {
let return_type = auto_return_type(function) let return_type =
if visibility::is_abstract(decorator_list, checker.semantic())
&& is_empty_body(body)
{
None
} else {
auto_return_type(function)
.and_then(|return_type| { .and_then(|return_type| {
return_type.into_expression( return_type.into_expression(
checker.importer(), checker.importer(),
@ -864,7 +902,8 @@ pub(crate) fn definition(
}) })
.map(|(return_type, edits)| { .map(|(return_type, edits)| {
(checker.generator().expr(&return_type), edits) (checker.generator().expr(&return_type), edits)
}); })
};
let mut diagnostic = Diagnostic::new( let mut diagnostic = Diagnostic::new(
MissingReturnTypePrivateFunction { MissingReturnTypePrivateFunction {
name: name.to_string(), name: name.to_string(),

View file

@ -427,4 +427,72 @@ auto_return_type.py:146:5: ANN201 [*] Missing return type annotation for public
148 148 | break 148 148 | break
149 149 | return 1 149 149 | return 1
auto_return_type.py:158:9: ANN201 Missing return type annotation for public function `method`
|
156 | class Foo(abc.ABC):
157 | @abstractmethod
158 | def method(self):
| ^^^^^^ ANN201
159 | pass
|
= help: Add return type annotation
auto_return_type.py:162:9: ANN201 Missing return type annotation for public function `method`
|
161 | @abc.abstractmethod
162 | def method(self):
| ^^^^^^ ANN201
163 | """Docstring."""
|
= help: Add return type annotation
auto_return_type.py:166:9: ANN201 Missing return type annotation for public function `method`
|
165 | @abc.abstractmethod
166 | def method(self):
| ^^^^^^ ANN201
167 | ...
|
= help: Add return type annotation
auto_return_type.py:171:9: ANN205 Missing return type annotation for staticmethod `method`
|
169 | @staticmethod
170 | @abstractmethod
171 | def method():
| ^^^^^^ ANN205
172 | pass
|
= help: Add return type annotation
auto_return_type.py:176:9: ANN206 Missing return type annotation for classmethod `method`
|
174 | @classmethod
175 | @abstractmethod
176 | def method(cls):
| ^^^^^^ ANN206
177 | pass
|
= help: Add return type annotation
auto_return_type.py:180:9: ANN201 [*] Missing return type annotation for public function `method`
|
179 | @abstractmethod
180 | def method(self):
| ^^^^^^ ANN201
181 | if self.x > 0:
182 | return 1
|
= help: Add return type annotation: `float`
Unsafe fix
177 177 | pass
178 178 |
179 179 | @abstractmethod
180 |- def method(self):
180 |+ def method(self) -> float:
181 181 | if self.x > 0:
182 182 | return 1
183 183 | else:

View file

@ -482,4 +482,72 @@ auto_return_type.py:146:5: ANN201 [*] Missing return type annotation for public
148 149 | break 148 149 | break
149 150 | return 1 149 150 | return 1
auto_return_type.py:158:9: ANN201 Missing return type annotation for public function `method`
|
156 | class Foo(abc.ABC):
157 | @abstractmethod
158 | def method(self):
| ^^^^^^ ANN201
159 | pass
|
= help: Add return type annotation
auto_return_type.py:162:9: ANN201 Missing return type annotation for public function `method`
|
161 | @abc.abstractmethod
162 | def method(self):
| ^^^^^^ ANN201
163 | """Docstring."""
|
= help: Add return type annotation
auto_return_type.py:166:9: ANN201 Missing return type annotation for public function `method`
|
165 | @abc.abstractmethod
166 | def method(self):
| ^^^^^^ ANN201
167 | ...
|
= help: Add return type annotation
auto_return_type.py:171:9: ANN205 Missing return type annotation for staticmethod `method`
|
169 | @staticmethod
170 | @abstractmethod
171 | def method():
| ^^^^^^ ANN205
172 | pass
|
= help: Add return type annotation
auto_return_type.py:176:9: ANN206 Missing return type annotation for classmethod `method`
|
174 | @classmethod
175 | @abstractmethod
176 | def method(cls):
| ^^^^^^ ANN206
177 | pass
|
= help: Add return type annotation
auto_return_type.py:180:9: ANN201 [*] Missing return type annotation for public function `method`
|
179 | @abstractmethod
180 | def method(self):
| ^^^^^^ ANN201
181 | if self.x > 0:
182 | return 1
|
= help: Add return type annotation: `float`
Unsafe fix
177 177 | pass
178 178 |
179 179 | @abstractmethod
180 |- def method(self):
180 |+ def method(self) -> float:
181 181 | if self.x > 0:
182 182 | return 1
183 183 | else: