-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[flake8-unused-arguments] Mark **kwargs in TypeVar as used (ARG001)
#22214
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
[flake8-unused-arguments] Mark **kwargs in TypeVar as used (ARG001)
#22214
Conversation
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| RUF100 | 1 | 1 | 0 | 0 | 0 |
Linter (preview)
ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 1 projects; 54 projects unchanged)
scikit-build/scikit-build-core (+1 -0 violations, +0 -0 fixes)
ruff check --no-cache --exit-zero --no-fix --output-format concise --preview
+ src/scikit_build_core/_compat/typing.py:36:32: RUF100 [*] Unused `noqa` directive (unused: `ARG001`)
Changes by rule (1 rules affected)
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| RUF100 | 1 | 1 | 0 | 0 | 0 |
ntBre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! This is exactly the fix I had in mind.
Could you add your manual test case from the PR summary as a new ARG001 case just to prevent future regressions?
You could add it at the bottom of this file:
ruff/crates/ruff_linter/resources/test/fixtures/flake8_unused_arguments/ARG.py
Lines 254 to 258 in fbb5c8a
| class C: | |
| def f(self, x, y): | |
| """Docstring.""" | |
| msg = t"{x}..." | |
| raise NotImplementedError(msg) |
| // The only case when a keyword argument is None is when using **kwargs, since there is no explicit 'arg' name for it. | ||
| // Ex: typing.TypeVar(*args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need the first part of this comment, but I like the example.
| // The only case when a keyword argument is None is when using **kwargs, since there is no explicit 'arg' name for it. | |
| // Ex: typing.TypeVar(*args, **kwargs) | |
| // Ex: typing.TypeVar(**kwargs) |
I just removed the *args to make it even more clear which part is relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ntBre Added the required test case & removed the first part of the comment.
Thank you!
ntBre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
flake8-unused-arguments] Mark **kwargs in TypeVar as used (ARG001)
Summary
Fixes false positive in ARG001 when
**kwargsis passed totyping.TypeVarCloses #22178
When
**kwargsis used in atyping.TypeVarcall, the checker was not recognizing it as a usage, leading to false positive "unused function argument" warnings.Root Cause
In the AST, keyword arguments are represented by the
Keywordstruct with anargfield of typeOption<Identifier>:bound=inthavearg = Some("bound")**kwargshasarg = NoneThe existing code only handled the
Some(id)case, never visiting the expression whenargwasNone, so**kwargswas never marked as used.Changes
Added an
elsebranch to handle**kwargsunpacking by callingvisit_non_type_definition(value)whenargisNone. This ensures thekwargsvariable is properly visited and marked as used by the semantic model.Test Plan
Tested with the following code:
Before :
ARG001 Unused function argument: kwargsAfter :
All checks passed!Run the example with the following command(from the root of ruff and please change the path to the module that contains the code example):
cargo run -p ruff -- check /path/to/file.py --isolated --select=ARG --no-cache