Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore ANN401 for overridden methods #4409

Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from typing import Any, Type
from typing_extensions import override

# Error
def foo(a, b):
Expand Down Expand Up @@ -94,6 +95,31 @@ def foo(self: "Foo", a: int, *params: Any, **options: str) -> int:
def foo(self: "Foo", a: int, *params: str, **options: Any) -> int:
pass

# ANN401
@override
def foo(self: "Foo", a: Any, *params: str, **options: str) -> int:
pass

# ANN401
@override
def foo(self: "Foo", a: int, *params: str, **options: str) -> Any:
pass

# ANN401
@override
def foo(self: "Foo", a: int, *params: Any, **options: Any) -> int:
pass

# ANN401
@override
def foo(self: "Foo", a: int, *params: Any, **options: str) -> int:
pass

# ANN401
@override
def foo(self: "Foo", a: int, *params: str, **options: Any) -> int:
pass

# OK
@classmethod
def foo(cls: Type["Foo"], a: int, b: int) -> int:
Expand Down
14 changes: 12 additions & 2 deletions crates/ruff/src/rules/flake8_annotations/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,32 @@ use ruff_python_semantic::definition::{Definition, Member, MemberKind};

use crate::checkers::ast::Checker;

pub(super) fn match_function_def(stmt: &Stmt) -> (&str, &Arguments, Option<&Expr>, &Vec<Stmt>) {
pub(super) fn match_function_def(
stmt: &Stmt,
) -> (&str, &Arguments, Option<&Expr>, &Vec<Stmt>, &Vec<Expr>) {
Copy link
Member

@MichaReiser MichaReiser May 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4359 introduced named types for each StmtKind variant. That means, we can now define custom unions:

enum MatchFunctionDef<'a> {
	FunctionDef(&'a ast::StmtFunctionDef),
	AsyncFunctionDef(&'a ast::StmtAsyncFunctionDef)
}

impl<'a> MatchFunctionDef<'a> {
	fn cast(stmt: &'a Stmt) -> Option<Self> {
		match &stmt.node {
			StmtKind::FunctionDef(def) => Some(Self::FunctionDef(def)),
			StmtKind::AsyncFunctionDef(def) => Some(Self::AsyncFunctionDef(def)),
			_ => None
		}
	},

	fn name(&self) -> &str {
		match self {
			Self::FunctionDef(def) => def.name(),
			Self::AsyncFunctionDef(def) => def.name(),
		}
	}

	...
}

I agree, it's definitely more code but it allows you to retrieve the underlying node when necessary and is more meaningful than a tuple with 5 fields.

match &stmt.node {
StmtKind::FunctionDef(ast::StmtFunctionDef {
name,
args,
returns,
body,
decorator_list,
..
})
| StmtKind::AsyncFunctionDef(ast::StmtAsyncFunctionDef {
name,
args,
returns,
body,
decorator_list,
..
}) => (name, args, returns.as_ref().map(|expr| &**expr), body),
}) => (
name,
args,
returns.as_ref().map(|expr| &**expr),
body,
decorator_list,
),
_ => panic!("Found non-FunctionDef in match_name"),
}
}
Expand Down
25 changes: 21 additions & 4 deletions crates/ruff/src/rules/flake8_annotations/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,10 +434,11 @@ fn check_dynamically_typed<F>(
annotation: &Expr,
func: F,
diagnostics: &mut Vec<Diagnostic>,
is_overridden: bool,
) where
F: FnOnce() -> String,
{
if checker.ctx.match_typing_expr(annotation, "Any") {
if !is_overridden && checker.ctx.match_typing_expr(annotation, "Any") {
diagnostics.push(Diagnostic::new(
AnyType { name: func() },
annotation.range(),
Expand Down Expand Up @@ -468,7 +469,7 @@ pub(crate) fn definition(
_ => return vec![],
};

let (name, args, returns, body) = match_function_def(stmt);
let (name, args, returns, body, decorator_list) = match_function_def(stmt);
// Keep track of whether we've seen any typed arguments or return values.
let mut has_any_typed_arg = false; // Any argument has been typed?
let mut has_typed_return = false; // Return value has been typed?
Expand All @@ -478,6 +479,8 @@ pub(crate) fn definition(
// unless configured to suppress ANN* for declarations that are fully untyped.
let mut diagnostics = Vec::new();

let is_overridden = visibility::is_override(&checker.ctx, decorator_list);

// ANN001, ANN401
for arg in args
.posonlyargs
Expand All @@ -500,6 +503,7 @@ pub(crate) fn definition(
annotation,
|| arg.node.arg.to_string(),
&mut diagnostics,
is_overridden,
);
}
} else {
Expand Down Expand Up @@ -529,7 +533,13 @@ pub(crate) fn definition(
if !checker.settings.flake8_annotations.allow_star_arg_any {
if checker.settings.rules.enabled(Rule::AnyType) {
let name = &arg.node.arg;
check_dynamically_typed(checker, expr, || format!("*{name}"), &mut diagnostics);
check_dynamically_typed(
checker,
expr,
|| format!("*{name}"),
&mut diagnostics,
is_overridden,
);
}
}
} else {
Expand Down Expand Up @@ -560,6 +570,7 @@ pub(crate) fn definition(
expr,
|| format!("**{name}"),
&mut diagnostics,
is_overridden,
);
}
}
Expand Down Expand Up @@ -612,7 +623,13 @@ pub(crate) fn definition(
if let Some(expr) = &returns {
has_typed_return = true;
if checker.settings.rules.enabled(Rule::AnyType) {
check_dynamically_typed(checker, expr, || name.to_string(), &mut diagnostics);
check_dynamically_typed(
checker,
expr,
|| name.to_string(),
&mut diagnostics,
is_overridden,
);
}
} else if !(
// Allow omission of return annotation if the function only returns `None`
Expand Down
Loading