From 9d698acc0553769d5bb51bf6c31943a677c147a8 Mon Sep 17 00:00:00 2001 From: Watercycle Date: Fri, 11 Oct 2024 02:08:42 -0500 Subject: [PATCH 01/10] Make ARG002 compatible with EM101 --- .../fixtures/flake8_unused_arguments/ARG.py | 4 ++ .../rules/unused_arguments.rs | 71 ++++++++++++++++++- ...nused_arguments__tests__ARG002_ARG.py.snap | 12 ++-- 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py index 9a8fd4c83585c..45f9a0199754d 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py @@ -107,6 +107,10 @@ def f(self, x): def f(self, x): raise NotImplemented("...") + def f(self, x): + msg = "..." + raise NotImplementedError(msg) + ### # Unused functions attached to abstract methods (OK). ### diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index beabe2e33917f..45c62e7ed0e72 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -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,71 @@ 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 = +/// 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) -> bool { + // Ignore doc-strings. + let statements = match function_def.body.as_slice() { + [Stmt::Expr(StmtExpr { value, .. }), rest @ ..] + if matches!(value.as_ref(), ast::Expr::StringLiteral(_)) => + { + rest + } + _ => &function_def.body, + }; + + let [first_stmt, second_stmt] = statements else { + return false; + }; + + let Stmt::Assign(ast::StmtAssign { value, .. }) = first_stmt else { + return false; + }; + + if !matches!( + value.as_ref(), + ast::Expr::StringLiteral(_) | ast::Expr::FString(_) + ) { + return false; + } + + let Stmt::Raise(StmtRaise { + exc: Some(exception), + .. + }) = second_stmt + else { + return false; + }; + + let ast::Expr::Call(ast::ExprCall { + func, arguments, .. + }) = exception.as_ref() + else { + return false; + }; + + if !matches!(func.as_ref(), ast::Expr::Name(name) if name.id == "NotImplementedError") { + return false; + } + + if arguments.len() != 1 { + return false; + } + + true +} + /// ARG001, ARG002, ARG003, ARG004, ARG005 pub(crate) fn unused_arguments( checker: &Checker, @@ -345,6 +410,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) && !visibility::is_overload(decorator_list, checker.semantic()) { function( @@ -364,6 +430,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) && (!visibility::is_magic(name) || visibility::is_init(name) || visibility::is_new(name) @@ -389,6 +456,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) && (!visibility::is_magic(name) || visibility::is_init(name) || visibility::is_new(name) @@ -414,6 +482,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) && (!visibility::is_magic(name) || visibility::is_init(name) || visibility::is_new(name) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap index e235a37a5394c..ee204934ff3e4 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap @@ -28,13 +28,11 @@ ARG.py:43:16: ARG002 Unused method argument: `x` 44 | print("Hello, world!") | -ARG.py:192:24: ARG002 Unused method argument: `x` +ARG.py:196:24: ARG002 Unused method argument: `x` | -190 | ### -191 | class C: -192 | def __init__(self, x) -> None: +194 | ### +195 | class C: +196 | def __init__(self, x) -> None: | ^ ARG002 -193 | print("Hello, world!") +197 | print("Hello, world!") | - - From 37c23591dfd232d41b38460879b13201c91a633c Mon Sep 17 00:00:00 2001 From: Watercycle Date: Tue, 15 Oct 2024 20:48:29 -0500 Subject: [PATCH 02/10] Prefer manual dereferencing --- .../flake8_unused_arguments/rules/unused_arguments.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 45c62e7ed0e72..01db9b6cf6273 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -328,7 +328,7 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool // Ignore doc-strings. let statements = match function_def.body.as_slice() { [Stmt::Expr(StmtExpr { value, .. }), rest @ ..] - if matches!(value.as_ref(), ast::Expr::StringLiteral(_)) => + if matches!(**value, ast::Expr::StringLiteral(_)) => { rest } @@ -343,10 +343,7 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool return false; }; - if !matches!( - value.as_ref(), - ast::Expr::StringLiteral(_) | ast::Expr::FString(_) - ) { + if !matches!(**value, ast::Expr::StringLiteral(_) | ast::Expr::FString(_)) { return false; } @@ -360,12 +357,12 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool let ast::Expr::Call(ast::ExprCall { func, arguments, .. - }) = exception.as_ref() + }) = &**exception else { return false; }; - if !matches!(func.as_ref(), ast::Expr::Name(name) if name.id == "NotImplementedError") { + if !matches!(&**func, ast::Expr::Name(name) if name.id == "NotImplementedError") { return false; } From a926d88bec7f3e87105cfa9898e8bc743f563a6e Mon Sep 17 00:00:00 2001 From: Watercycle Date: Tue, 15 Oct 2024 20:51:20 -0500 Subject: [PATCH 03/10] Refactor away intermediate variables --- .../rules/unused_arguments.rs | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 01db9b6cf6273..a025b16f20813 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -335,26 +335,21 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool _ => &function_def.body, }; - let [first_stmt, second_stmt] = statements else { - return false; - }; - - let Stmt::Assign(ast::StmtAssign { value, .. }) = first_stmt else { - return false; - }; - - if !matches!(**value, ast::Expr::StringLiteral(_) | ast::Expr::FString(_)) { - return false; - } - - let Stmt::Raise(StmtRaise { + let [Stmt::Assign(ast::StmtAssign { value, .. }), Stmt::Raise(StmtRaise { exc: Some(exception), .. - }) = second_stmt + })] = statements else { return false; }; + if !matches!( + value.as_ref(), + ast::Expr::StringLiteral(_) | ast::Expr::FString(_) + ) { + return false; + } + let ast::Expr::Call(ast::ExprCall { func, arguments, .. }) = &**exception From b9295743d28ea3f6efb5089376e85f04fb133f3c Mon Sep 17 00:00:00 2001 From: Watercycle Date: Tue, 15 Oct 2024 21:01:00 -0500 Subject: [PATCH 04/10] Add f-string variable test to ARG.py --- .../test/fixtures/flake8_unused_arguments/ARG.py | 5 +++++ ..._flake8_unused_arguments__tests__ARG002_ARG.py.snap | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py index 45f9a0199754d..a00d4a67ff037 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py @@ -111,6 +111,11 @@ 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). ### diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap index ee204934ff3e4..f8326fed02579 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap @@ -28,11 +28,11 @@ ARG.py:43:16: ARG002 Unused method argument: `x` 44 | print("Hello, world!") | -ARG.py:196:24: ARG002 Unused method argument: `x` +ARG.py:201:24: ARG002 Unused method argument: `x` | -194 | ### -195 | class C: -196 | def __init__(self, x) -> None: +199 | ### +200 | class C: +201 | def __init__(self, x) -> None: | ^ ARG002 -197 | print("Hello, world!") +202 | print("Hello, world!") | From 0f9ee125ffadb382c8d6e05ea9f56b2bda5eb2b9 Mon Sep 17 00:00:00 2001 From: Watercycle Date: Thu, 17 Oct 2024 20:48:00 -0500 Subject: [PATCH 05/10] Simplify match case --- .../rules/flake8_unused_arguments/rules/unused_arguments.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index a025b16f20813..80c3cc855dbaa 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -327,11 +327,7 @@ fn call<'a>( fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool { // Ignore doc-strings. let statements = match function_def.body.as_slice() { - [Stmt::Expr(StmtExpr { value, .. }), rest @ ..] - if matches!(**value, ast::Expr::StringLiteral(_)) => - { - rest - } + [Stmt::Expr(StmtExpr { value, .. }), rest @ ..] if value.is_string_literal_expr() => rest, _ => &function_def.body, }; From c5e1b4a8c46ef2360021a4ce39c3bc19da1b3e20 Mon Sep 17 00:00:00 2001 From: Watercycle Date: Thu, 17 Oct 2024 20:50:15 -0500 Subject: [PATCH 06/10] Require the message variable to be used --- .../test/fixtures/flake8_unused_arguments/ARG.py | 4 ++++ .../flake8_unused_arguments/rules/unused_arguments.rs | 8 +++++++- ..._flake8_unused_arguments__tests__ARG002_ARG.py.snap | 10 +++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py index a00d4a67ff037..38b513c8d2c73 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py @@ -111,6 +111,10 @@ def f(self, x): msg = "..." raise NotImplementedError(msg) + def f(self, x): + msg = "..." + raise NotImplementedError("must use msg") + def f(self, x, y): """Docstring.""" msg = f"{x}..." diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 80c3cc855dbaa..289d7575c81f2 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -331,7 +331,7 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool _ => &function_def.body, }; - let [Stmt::Assign(ast::StmtAssign { value, .. }), Stmt::Raise(StmtRaise { + let [Stmt::Assign(ast::StmtAssign { targets, value, .. }), Stmt::Raise(StmtRaise { exc: Some(exception), .. })] = statements @@ -361,6 +361,12 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool return false; } + if let ast::Expr::Name(exception_arg_name) = &arguments.args[0] { + if let ast::Expr::Name(assign_var_name) = &targets[0] { + return assign_var_name.id == exception_arg_name.id; + } + } + true } diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap index f8326fed02579..f177ec5787484 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap @@ -28,11 +28,11 @@ ARG.py:43:16: ARG002 Unused method argument: `x` 44 | print("Hello, world!") | -ARG.py:201:24: ARG002 Unused method argument: `x` +ARG.py:205:24: ARG002 Unused method argument: `x` | -199 | ### -200 | class C: -201 | def __init__(self, x) -> None: +203 | ### +204 | class C: +205 | def __init__(self, x) -> None: | ^ ARG002 -202 | print("Hello, world!") +206 | print("Hello, world!") | From 62650d87f0e161800eb3a1f4944a8122c9001b34 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 18 Oct 2024 11:29:55 +0530 Subject: [PATCH 07/10] False is not a name node; additional test cases --- .../fixtures/flake8_unused_arguments/ARG.py | 16 ++++++-- .../rules/unused_arguments.rs | 14 +++---- ...nused_arguments__tests__ARG002_ARG.py.snap | 40 ++++++++++++++++--- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py index 38b513c8d2c73..fbebdff4b2b61 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py @@ -55,6 +55,18 @@ def f(cls, x): 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). ### @@ -111,10 +123,6 @@ def f(self, x): msg = "..." raise NotImplementedError(msg) - def f(self, x): - msg = "..." - raise NotImplementedError("must use msg") - def f(self, x, y): """Docstring.""" msg = f"{x}..." diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 289d7575c81f2..065de816b0b4c 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -357,17 +357,15 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool return false; } - if arguments.len() != 1 { + let [argument] = &*arguments.args else { return false; - } + }; - if let ast::Expr::Name(exception_arg_name) = &arguments.args[0] { - if let ast::Expr::Name(assign_var_name) = &targets[0] { - return assign_var_name.id == exception_arg_name.id; - } - } + let [target] = targets.as_slice() else { + return false; + }; - true + argument.as_name_expr().map(|name| name.id()) == target.as_name_expr().map(|name| name.id()) } /// ARG001, ARG002, ARG003, ARG004, ARG005 diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap index f177ec5787484..d04530343580f 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/snapshots/ruff_linter__rules__flake8_unused_arguments__tests__ARG002_ARG.py.snap @@ -28,11 +28,41 @@ ARG.py:43:16: ARG002 Unused method argument: `x` 44 | print("Hello, world!") | -ARG.py:205: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` | -203 | ### -204 | class C: -205 | def __init__(self, x) -> None: +211 | ### +212 | class C: +213 | def __init__(self, x) -> None: | ^ ARG002 -206 | print("Hello, world!") +214 | print("Hello, world!") | From 168e631ec613afa779446e570e40ebcef6086893 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 18 Oct 2024 11:33:52 +0530 Subject: [PATCH 08/10] Use `**` instead of `as_ref` --- .../rules/flake8_unused_arguments/rules/unused_arguments.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 065de816b0b4c..6c0289d8a05ba 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -339,10 +339,7 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool return false; }; - if !matches!( - value.as_ref(), - ast::Expr::StringLiteral(_) | ast::Expr::FString(_) - ) { + if !matches!(**value, ast::Expr::StringLiteral(_) | ast::Expr::FString(_)) { return false; } From 488a407328390806b341c4af01ef6adeb5459985 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 18 Oct 2024 11:34:52 +0530 Subject: [PATCH 09/10] Fix clippy --- .../src/rules/flake8_unused_arguments/rules/unused_arguments.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 6c0289d8a05ba..2889d07b1212d 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -362,7 +362,7 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool return false; }; - argument.as_name_expr().map(|name| name.id()) == target.as_name_expr().map(|name| name.id()) + argument.as_name_expr().map(ast::ExprName::id) == target.as_name_expr().map(ast::ExprName::id) } /// ARG001, ARG002, ARG003, ARG004, ARG005 From e92ccf4b0f71a1b7022a902484cfc36d45db1b18 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Fri, 18 Oct 2024 12:08:38 +0530 Subject: [PATCH 10/10] Make sure `NotImplementedError` is a builtin --- .../rules/unused_arguments.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 2889d07b1212d..6361f2d785644 100644 --- a/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs +++ b/crates/ruff_linter/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -324,7 +324,10 @@ fn call<'a>( /// /// [`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) -> bool { +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, @@ -350,7 +353,7 @@ fn is_not_implemented_stub_with_variable(function_def: &StmtFunctionDef) -> bool return false; }; - if !matches!(&**func, ast::Expr::Name(name) if name.id == "NotImplementedError") { + if !semantic.match_builtin_expr(func, "NotImplementedError") { return false; } @@ -399,7 +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) + && !is_not_implemented_stub_with_variable(function_def, checker.semantic()) && !visibility::is_overload(decorator_list, checker.semantic()) { function( @@ -419,7 +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) + && !is_not_implemented_stub_with_variable(function_def, checker.semantic()) && (!visibility::is_magic(name) || visibility::is_init(name) || visibility::is_new(name) @@ -445,7 +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) + && !is_not_implemented_stub_with_variable(function_def, checker.semantic()) && (!visibility::is_magic(name) || visibility::is_init(name) || visibility::is_new(name) @@ -471,7 +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) + && !is_not_implemented_stub_with_variable(function_def, checker.semantic()) && (!visibility::is_magic(name) || visibility::is_init(name) || visibility::is_new(name)