mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-03 18:28:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Make ARG002 compatible with EM101 when raising NotImplementedError (#13714)

Browse source 
## Summary

This pull request resolves some rule thrashing identified in #12427 by
allowing for unused arguments when using `NotImplementedError` with a
variable per [this
comment](https://github.com/astral-sh/ruff/issues/12427#issuecomment-2384727468).

**Note**

This feels a little heavy-handed / edge-case-prone. So, to be clear, I'm
happy to scrap this code and just update the docs to communicate that
`abstractmethod` and friends should be used in this scenario (or
similar). Just let me know what you'd like done!

fixes: #12427 

## Test Plan

I added a test-case to the existing `ARG.py` file and ran...

```sh
cargo run -p ruff -- check crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py --no-cache --preview --select ARG002
```

---------

Co-authored-by: Dhruv Manilawala <dhruvmanila@gmail.com>
This commit is contained in:
Matthew Spero 2024-10-18 01:44:22 -05:00 • committed by GitHub
parent 040a591cad
commit f80528fbf2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 118 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

21
crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py vendored
View file

@ -55,6 +55,18 @@ class C:
def f(x):
print("Hello, world!")
def f(self, x):
msg[0] = "..."
raise NotImplementedError(msg)
def f(self, x):
msg = "..."
raise NotImplementedError(foo)
def f(self, x):
msg = "..."
raise NotImplementedError("must use msg")
###
# Unused arguments attached to empty functions (OK).
###
@ -107,6 +119,15 @@ class C:
def f(self, x):
raise NotImplemented("...")
def f(self, x):
msg = "..."
raise NotImplementedError(msg)
def f(self, x, y):
"""Docstring."""
msg = f"{x}..."
raise NotImplementedError(msg)
###
# Unused functions attached to abstract methods (OK).
###

63
crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs
View file

@ -1,6 +1,6 @@
use regex::Regex;
use ruff_python_ast as ast;
use ruff_python_ast::{Parameter, Parameters};
use ruff_python_ast::{Parameter, Parameters, Stmt, StmtExpr, StmtFunctionDef, StmtRaise};
use ruff_diagnostics::DiagnosticKind;
use ruff_diagnostics::{Diagnostic, Violation};
@ -311,6 +311,63 @@ fn call<'a>(
}));
}
/// Returns `true` if a function appears to be a base class stub. In other
/// words, if it matches the following syntax:
///
/// ```text
/// variable = <string | f-string>
/// raise NotImplementedError(variable)
/// ```
///
/// See also [`is_stub`]. We're a bit more generous in what is considered a
/// stub in this rule to avoid clashing with [`EM101`].
///
/// [`is_stub`]: function_type::is_stub
/// [`EM101`]: https://docs.astral.sh/ruff/rules/raw-string-in-exception/
fn is_not_implemented_stub_with_variable(
function_def: &StmtFunctionDef,
semantic: &SemanticModel,
) -> bool {
// Ignore doc-strings.
let statements = match function_def.body.as_slice() {
[Stmt::Expr(StmtExpr { value, .. }), rest @ ..] if value.is_string_literal_expr() => rest,
_ => &function_def.body,
};
let [Stmt::Assign(ast::StmtAssign { targets, value, .. }), Stmt::Raise(StmtRaise {
exc: Some(exception),
..
})] = statements
else {
return false;
};
if !matches!(**value, ast::Expr::StringLiteral(_) | ast::Expr::FString(_)) {
return false;
}
let ast::Expr::Call(ast::ExprCall {
func, arguments, ..
}) = &**exception
else {
return false;
};
if !semantic.match_builtin_expr(func, "NotImplementedError") {
return false;
}
let [argument] = &*arguments.args else {
return false;
};
let [target] = targets.as_slice() else {
return false;
};
argument.as_name_expr().map(ast::ExprName::id) == target.as_name_expr().map(ast::ExprName::id)
}
/// ARG001, ARG002, ARG003, ARG004, ARG005
pub(crate) fn unused_arguments(
checker: &Checker,
@ -345,6 +402,7 @@ pub(crate) fn unused_arguments(
function_type::FunctionType::Function => {
if checker.enabled(Argumentable::Function.rule_code())
&& !function_type::is_stub(function_def, checker.semantic())
&& !is_not_implemented_stub_with_variable(function_def, checker.semantic())
&& !visibility::is_overload(decorator_list, checker.semantic())
{
function(
@ -364,6 +422,7 @@ pub(crate) fn unused_arguments(
function_type::FunctionType::Method => {
if checker.enabled(Argumentable::Method.rule_code())
&& !function_type::is_stub(function_def, checker.semantic())
&& !is_not_implemented_stub_with_variable(function_def, checker.semantic())
&& (!visibility::is_magic(name)
|| visibility::is_init(name)
|| visibility::is_new(name)
@ -389,6 +448,7 @@ pub(crate) fn unused_arguments(
function_type::FunctionType::ClassMethod => {
if checker.enabled(Argumentable::ClassMethod.rule_code())
&& !function_type::is_stub(function_def, checker.semantic())
&& !is_not_implemented_stub_with_variable(function_def, checker.semantic())
&& (!visibility::is_magic(name)
|| visibility::is_init(name)
|| visibility::is_new(name)
@ -414,6 +474,7 @@ pub(crate) fn unused_arguments(
function_type::FunctionType::StaticMethod => {
if checker.enabled(Argumentable::StaticMethod.rule_code())
&& !function_type::is_stub(function_def, checker.semantic())
&& !is_not_implemented_stub_with_variable(function_def, checker.semantic())
&& (!visibility::is_magic(name)
|| visibility::is_init(name)
|| visibility::is_new(name)

42
crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap
View file

@ -28,13 +28,41 @@ ARG.py:43:16: ARG002 Unused method argument: `x`
44 | print("Hello, world!")
|
ARG.py:192:24: ARG002 Unused method argument: `x`
ARG.py:58:17: ARG002 Unused method argument: `x`
|
56 | print("Hello, world!")
57 |
58 | def f(self, x):
| ^ ARG002
59 | msg[0] = "..."
60 | raise NotImplementedError(msg)
|
ARG.py:62:17: ARG002 Unused method argument: `x`
|
60 | raise NotImplementedError(msg)
61 |
62 | def f(self, x):
| ^ ARG002
63 | msg = "..."
64 | raise NotImplementedError(foo)
|
ARG.py:66:17: ARG002 Unused method argument: `x`
|
64 | raise NotImplementedError(foo)
65 |
66 | def f(self, x):
| ^ ARG002
67 | msg = "..."
68 | raise NotImplementedError("must use msg")
|
ARG.py:213:24: ARG002 Unused method argument: `x`
|
190 | ###
191 | class C:
192 | def __init__(self, x) -> None:
211 | ###
212 | class C:
213 | def __init__(self, x) -> None:
| ^ ARG002
193 | print("Hello, world!")
214 | print("Hello, world!")
|