Skip unused argument checks for magic methods (#1801)
We still check `__init__`, `__call__`, and `__new__`. Closes #1796.
This commit is contained in:
@@ -181,3 +181,17 @@ def f(a: int, b: int) -> str:
|
||||
|
||||
def f(a, b):
|
||||
return f"{a}{b}"
|
||||
|
||||
|
||||
###
|
||||
# Unused arguments on magic methods.
|
||||
###
|
||||
class C:
|
||||
def __init__(self, x) -> None:
|
||||
print("Hello, world!")
|
||||
|
||||
def __str__(self) -> str:
|
||||
return "Hello, world!"
|
||||
|
||||
def __exit__(self, exc_type, exc_value, traceback) -> None:
|
||||
print("Hello, world!")
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
use rustpython_ast::{Expr, Stmt, StmtKind};
|
||||
|
||||
pub fn name(stmt: &Stmt) -> &str {
|
||||
match &stmt.node {
|
||||
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => name,
|
||||
_ => panic!("Expected StmtKind::FunctionDef | StmtKind::AsyncFunctionDef"),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn decorator_list(stmt: &Stmt) -> &Vec<Expr> {
|
||||
match &stmt.node {
|
||||
StmtKind::FunctionDef { decorator_list, .. }
|
||||
|
||||
@@ -319,7 +319,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V
|
||||
helpers::identifier_range(stmt, checker.locator),
|
||||
));
|
||||
}
|
||||
} else if visibility::is_init(stmt) {
|
||||
} else if visibility::is_init(cast::name(stmt)) {
|
||||
// Allow omission of return annotation in `__init__` functions, as long as at
|
||||
// least one argument is typed.
|
||||
if checker.settings.enabled.contains(&RuleCode::ANN204) {
|
||||
@@ -341,7 +341,7 @@ pub fn definition(checker: &mut Checker, definition: &Definition, visibility: &V
|
||||
checker.diagnostics.push(diagnostic);
|
||||
}
|
||||
}
|
||||
} else if visibility::is_magic(stmt) {
|
||||
} else if visibility::is_magic(cast::name(stmt)) {
|
||||
if checker.settings.enabled.contains(&RuleCode::ANN204) {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
violations::MissingReturnTypeSpecialMethod(name.to_string()),
|
||||
|
||||
@@ -153,6 +153,10 @@ pub fn unused_arguments(
|
||||
.enabled
|
||||
.contains(Argumentable::Method.rule_code())
|
||||
&& !helpers::is_empty(body)
|
||||
&& (!visibility::is_magic(name)
|
||||
|| visibility::is_init(name)
|
||||
|| visibility::is_new(name)
|
||||
|| visibility::is_call(name))
|
||||
&& !visibility::is_abstract(checker, decorator_list)
|
||||
&& !visibility::is_override(checker, decorator_list)
|
||||
&& !visibility::is_overload(checker, decorator_list)
|
||||
@@ -178,6 +182,10 @@ pub fn unused_arguments(
|
||||
.enabled
|
||||
.contains(Argumentable::ClassMethod.rule_code())
|
||||
&& !helpers::is_empty(body)
|
||||
&& (!visibility::is_magic(name)
|
||||
|| visibility::is_init(name)
|
||||
|| visibility::is_new(name)
|
||||
|| visibility::is_call(name))
|
||||
&& !visibility::is_abstract(checker, decorator_list)
|
||||
&& !visibility::is_override(checker, decorator_list)
|
||||
&& !visibility::is_overload(checker, decorator_list)
|
||||
@@ -203,6 +211,10 @@ pub fn unused_arguments(
|
||||
.enabled
|
||||
.contains(Argumentable::StaticMethod.rule_code())
|
||||
&& !helpers::is_empty(body)
|
||||
&& (!visibility::is_magic(name)
|
||||
|| visibility::is_init(name)
|
||||
|| visibility::is_new(name)
|
||||
|| visibility::is_call(name))
|
||||
&& !visibility::is_abstract(checker, decorator_list)
|
||||
&& !visibility::is_override(checker, decorator_list)
|
||||
&& !visibility::is_overload(checker, decorator_list)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
source: src/flake8_unused_arguments/mod.rs
|
||||
expression: checks
|
||||
expression: diagnostics
|
||||
---
|
||||
- kind:
|
||||
UnusedMethodArgument: x
|
||||
@@ -32,4 +32,14 @@ expression: checks
|
||||
column: 16
|
||||
fix: ~
|
||||
parent: ~
|
||||
- kind:
|
||||
UnusedMethodArgument: x
|
||||
location:
|
||||
row: 190
|
||||
column: 23
|
||||
end_location:
|
||||
row: 190
|
||||
column: 24
|
||||
fix: ~
|
||||
parent: ~
|
||||
|
||||
|
||||
@@ -18,7 +18,9 @@ use crate::pydocstyle::helpers::{leading_quote, logical_line};
|
||||
use crate::pydocstyle::settings::Convention;
|
||||
use crate::registry::{Diagnostic, RuleCode};
|
||||
use crate::violations;
|
||||
use crate::visibility::{is_init, is_magic, is_overload, is_override, is_staticmethod, Visibility};
|
||||
use crate::visibility::{
|
||||
is_call, is_init, is_magic, is_new, is_overload, is_override, is_staticmethod, Visibility,
|
||||
};
|
||||
|
||||
/// D100, D101, D102, D103, D104, D105, D106, D107
|
||||
pub fn not_missing(
|
||||
@@ -85,18 +87,26 @@ pub fn not_missing(
|
||||
|| is_override(checker, cast::decorator_list(stmt))
|
||||
{
|
||||
true
|
||||
} else if is_magic(stmt) {
|
||||
if checker.settings.enabled.contains(&RuleCode::D105) {
|
||||
} else if is_init(cast::name(stmt)) {
|
||||
if checker.settings.enabled.contains(&RuleCode::D107) {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
violations::MagicMethod,
|
||||
violations::PublicInit,
|
||||
identifier_range(stmt, checker.locator),
|
||||
));
|
||||
}
|
||||
true
|
||||
} else if is_init(stmt) {
|
||||
if checker.settings.enabled.contains(&RuleCode::D107) {
|
||||
} else if is_new(cast::name(stmt)) || is_call(cast::name(stmt)) {
|
||||
if checker.settings.enabled.contains(&RuleCode::D102) {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
violations::PublicInit,
|
||||
violations::PublicMethod,
|
||||
identifier_range(stmt, checker.locator),
|
||||
));
|
||||
}
|
||||
true
|
||||
} else if is_magic(cast::name(stmt)) {
|
||||
if checker.settings.enabled.contains(&RuleCode::D105) {
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
violations::MagicMethod,
|
||||
identifier_range(stmt, checker.locator),
|
||||
));
|
||||
}
|
||||
|
||||
@@ -82,27 +82,23 @@ pub fn is_abstract(checker: &Checker, decorator_list: &[Expr]) -> bool {
|
||||
}
|
||||
|
||||
/// Returns `true` if a function is a "magic method".
|
||||
pub fn is_magic(stmt: &Stmt) -> bool {
|
||||
match &stmt.node {
|
||||
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
|
||||
name.starts_with("__")
|
||||
&& name.ends_with("__")
|
||||
&& name != "__init__"
|
||||
&& name != "__call__"
|
||||
&& name != "__new__"
|
||||
}
|
||||
_ => panic!("Found non-FunctionDef in is_magic"),
|
||||
}
|
||||
pub fn is_magic(name: &str) -> bool {
|
||||
name.starts_with("__") && name.ends_with("__")
|
||||
}
|
||||
|
||||
/// Returns `true` if a function is an `__init__`.
|
||||
pub fn is_init(stmt: &Stmt) -> bool {
|
||||
match &stmt.node {
|
||||
StmtKind::FunctionDef { name, .. } | StmtKind::AsyncFunctionDef { name, .. } => {
|
||||
name == "__init__"
|
||||
}
|
||||
_ => panic!("Found non-FunctionDef in is_init"),
|
||||
}
|
||||
pub fn is_init(name: &str) -> bool {
|
||||
name == "__init__"
|
||||
}
|
||||
|
||||
/// Returns `true` if a function is an `__new__`.
|
||||
pub fn is_new(name: &str) -> bool {
|
||||
name == "__new__"
|
||||
}
|
||||
|
||||
/// Returns `true` if a function is an `__call__`.
|
||||
pub fn is_call(name: &str) -> bool {
|
||||
name == "__call__"
|
||||
}
|
||||
|
||||
/// Returns `true` if a module name indicates public visibility.
|
||||
|
||||
Reference in New Issue
Block a user