Fix RUF100 to detect unused file-level noqa directives with specific codes (#17042) by maxmynter · Pull Request #17061 · astral-sh/ruff

maxmynter · 2025-03-30T09:20:57Z

Summary

This PR fixes the issue outlined in #17042 where RUF100 (unused-noqa) fails to detect unused file-level noqa directives (# ruff: noqa or # ruff: noqa: {code}).

The issue stems from two underlying causes:

For blanket file-level directives (# ruff: noqa), there's a circular dependency: the directive exempts all rules including RUF100 itself, which prevents checking for usage. This isn't changed by this PR. I would argue it is intendend behavior - a blanket # ruff: noqa directive should exempt all rules including RUF100 itself.
For code-specific file-level directives (e.g. # ruff: noqa: F841), the handling was missing in the check_noqa function. This is added in this PR.

Notes

For file-level directives, the matches array is pre-populated with the specified codes during parsing, unlike line-level directives which only populate their matches array when actually suppressing diagnostics. This difference requires the somewhat clunky handling of both cases. I would appreciate guidance on a cleaner design :)
A more fundamental solution would be to change how file-level directives initialize the matches array in FileNoqaDirectives::extract(), but that requires more substantial changes as it breaks existing functionality. I suspect discussions in Make noqa parsing consistent and more robust #16483 are relevant for this.

Test Plan

Local verification
Added a test case and fixture

by implementing a check on them. There is different handling necessary as the matches array is prepopulated for file level directives.

github-actions · 2025-03-30T14:59:52Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+17 -0 violations, +0 -0 fixes in 5 projects; 50 projects unchanged)

ibis-project/ibis (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ ibis/backends/tests/signature/typecheck.py:8:1: RUF100 Unused `noqa` directive (unused: `D205`, `D415`, `D400`)

python/typeshed (+2 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/asyncio/__init__.pyi:1:1: RUF100 [*] Unused `noqa` directive (non-enabled: `PLR5501`)
+ stdlib/typing.pyi:2:1: RUF100 [*] Unused `noqa` directive (unused: `F811`)

scikit-build/scikit-build-core (+6 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ tests/packages/importlib_editable/pkg/pmod_a.py:1:1: RUF100 [*] Unused `noqa` directive (unused: `I001`)
+ tests/packages/importlib_editable/pkg/sub_a/pmod_b.py:1:1: RUF100 [*] Unused `noqa` directive (unused: `I001`)
+ tests/packages/importlib_editable/pkg/sub_b/pmod_c.py:1:1: RUF100 [*] Unused `noqa` directive (unused: `I001`)
+ tests/packages/importlib_editable/pkg/sub_b/sub_c/pmod_d.py:1:1: RUF100 [*] Unused `noqa` directive (unused: `I001`)
+ tests/packages/importlib_editable/pkg/sub_b/sub_d/pmod_e.py:1:1: RUF100 [*] Unused `noqa` directive (unused: `I001`)
+ tests/packages/importlib_editable/pmod.py:1:1: RUF100 [*] Unused `noqa` directive (unused: `I001`)

indico/indico (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ indico/legacy/pdfinterface/conference.py:8:1: RUF100 [*] Unused `noqa` directive (unused: `PLR1702`)

astropy/astropy (+7 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

+ astropy/cosmology/_src/core.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `RUF009`)
+ astropy/cosmology/_src/flrw/base.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `RUF009`)
+ astropy/cosmology/_src/flrw/w0cdm.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `RUF009`)
+ astropy/cosmology/_src/flrw/w0wacdm.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `RUF009`)
+ astropy/cosmology/_src/flrw/w0wzcdm.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `RUF009`)
+ astropy/cosmology/_src/flrw/wpwazpcdm.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `RUF009`)
+ astropy/units/tests/test_quantity_annotations.py:2:1: RUF100 [*] Unused `noqa` directive (unused: `FA100`, `FA102`)

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
RUF100	17	17	0	0	0

and satifsy clippy

MichaReiser · 2025-03-31T08:43:35Z

I'll leave this to @dylwil3 to review. What's important is that we gate these changes behind preview because it is a "significant" increase of a stable rule's scope.

ddeepwell · 2025-03-31T17:56:16Z

I'm curious how this PR handles the case with # ruff: noqa: {code1}, {code2}, where code2 is a code for a used directive while code1 is for an unused directive which should be removed? This case isn't in the original issue (#17042), but it's basically the same concern, except that the valid code should remain after a fix.

dylwil3 · 2025-03-31T18:02:23Z

@maxmynter could you add some test cases along the lines of what @ddeepwell mentioned? and also gate the change behind preview when you get a chance?

I'm gonna try to check out some of the ecosystem reports to see if we're getting any false positives before diving into the implementation

Thank you for your help with this!

maxmynter · 2025-03-31T23:15:03Z

@dylwil3 is the implementation of the feature guard in ecbe562 sufficient?

I am a unsure since I haven't worked with feature guards -- let me know if something is missing.

maxmynter · 2025-03-31T23:47:20Z

Thanks for flagging @ddeepwell, ~~the current implementation deletes the whole file level ruff: noqa: directive if one code is wrong.~~

Edit: This is not the case. I had misremembered rule F841 which does not apply at the file root; deletion in the example that prompted this comment was wanted.

The proposed changes handle (and test) the cases when one code is unused and the other one is used.

maxmynter · 2025-04-02T21:01:49Z

@dylwil3 ping :)

No worries if you don't have the bandwidth at the moment; i just wanted to let you know that I'm unsure about the next steps here.

dylwil3 · 2025-04-02T21:15:23Z

Thanks for the ping! Hoping to get to this tomorrow morning - sorry for the delay!

dylwil3

This looks good, thank you! The gating behind preview looks correct. I'm not sure why it's not reflected in the ecosystem check... I can't reproduce it locally. I'll try to investigate it.

In the meantime, I've just left one more issue to address about preserving the prefix used by the user for file-level directives.

crates/ruff_linter/src/checkers/noqa.rs

dylwil3

Looks good! One small change and then looks like there are some merge conflicts to resolve as well

crates/ruff_linter/src/checkers/noqa.rs

maxmynter · 2025-04-04T22:08:42Z

Merge conflicts were with test cases that i added in #17138. They're resolved now.

dylwil3

Wonderful, thank you!

* main: (42 commits) [playground] New default program (#17277) [red-knot] Add `--python-platform` CLI option (#17284) [red-knot] Allow ellipsis default params in stub functions (#17243) [red-knot] Fix stale syntax errors in playground (#17280) Update Rust crate clap to v4.5.35 (#17273) Fix RUF100 to detect unused file-level noqa directives with specific codes (#17042) (#17061) [ci] Fix pattern for code changes (#17275) [`airflow`] Update oudated `AIR301`, `AIR302` rules (#17123) [docs] fix formatting of "See Style Guide" link (#17272) [red-knot] Support stub packages (#17204) ruff_annotate_snippets: address unused code warnings [red-knot] Add a couple more tests for `*` imports (#17270) [red-knot] Add 'Format document' to playground (#17217) Update actions/setup-node action to v4.3.0 (#17259) Update actions/upload-artifact action to v4.6.2 (#17261) Update actions/download-artifact action to v4.2.1 (#17258) Update actions/setup-python action to v5.5.0 (#17260) Update actions/cache action to v4.2.3 (#17256) Update Swatinem/rust-cache action to v2.7.8 (#17255) Update actions/checkout action to v4.2.2 (#17257) ...

* main: (222 commits) [playground] New default program (#17277) [red-knot] Add `--python-platform` CLI option (#17284) [red-knot] Allow ellipsis default params in stub functions (#17243) [red-knot] Fix stale syntax errors in playground (#17280) Update Rust crate clap to v4.5.35 (#17273) Fix RUF100 to detect unused file-level noqa directives with specific codes (#17042) (#17061) [ci] Fix pattern for code changes (#17275) [`airflow`] Update oudated `AIR301`, `AIR302` rules (#17123) [docs] fix formatting of "See Style Guide" link (#17272) [red-knot] Support stub packages (#17204) ruff_annotate_snippets: address unused code warnings [red-knot] Add a couple more tests for `*` imports (#17270) [red-knot] Add 'Format document' to playground (#17217) Update actions/setup-node action to v4.3.0 (#17259) Update actions/upload-artifact action to v4.6.2 (#17261) Update actions/download-artifact action to v4.2.1 (#17258) Update actions/setup-python action to v5.5.0 (#17260) Update actions/cache action to v4.2.3 (#17256) Update Swatinem/rust-cache action to v2.7.8 (#17255) Update actions/checkout action to v4.2.2 (#17257) ...

maxmynter added 2 commits March 30, 2025 05:03

(fix) RUF100 to detect unused file-level noqa directives

05d4958

by implementing a check on them. There is different handling necessary as the matches array is prepopulated for file level directives.

(tests) Test unused file level directives are flagged

cdb89eb

MichaReiser requested a review from dylwil3 March 30, 2025 12:42

MichaReiser assigned dylwil3 Mar 30, 2025

(fixup) Flip control flow to avoid unnecessary boolean not

7ab4d6c

and satifsy clippy

MichaReiser added the rule Implementing or modifying a lint rule label Mar 31, 2025

Preview guard changes to file-level RUF100

ecbe562

(test) Remove one unused in two file level noqa on a line

3e6a0a8

dylwil3 requested changes Apr 3, 2025

View reviewed changes

crates/ruff_linter/src/checkers/noqa.rs Outdated Show resolved Hide resolved

maxmynter added 2 commits April 4, 2025 00:17

Add add hoc support for flake8 noqa's

f8947c4

(tests) Adapt for flake8 support

685d85c

maxmynter requested a review from dylwil3 April 4, 2025 04:25

dylwil3 requested changes Apr 4, 2025

View reviewed changes

crates/ruff_linter/src/checkers/noqa.rs Outdated Show resolved Hide resolved

maxmynter and others added 3 commits April 4, 2025 15:17

(fixup) Remove colon from flake8 search string

7591bcc

Merge branch 'main' into noqa

d1447a4

Post merge cargo fmt / clippy

4a5608b

maxmynter requested a review from dylwil3 April 4, 2025 21:53

dylwil3 approved these changes Apr 7, 2025

View reviewed changes

dylwil3 merged commit f1d4ba3 into astral-sh:main Apr 7, 2025
22 checks passed

BrewTestBot mentioned this pull request Apr 10, 2025

ruff 0.11.5 Homebrew/homebrew-core#219299

Merged

Comments

Conversation

maxmynter commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Test Plan

Uh oh!

github-actions bot commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

MichaReiser commented Mar 31, 2025

Uh oh!

ddeepwell commented Mar 31, 2025

Uh oh!

dylwil3 commented Mar 31, 2025

Uh oh!

maxmynter commented Mar 31, 2025

Uh oh!

maxmynter commented Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxmynter commented Apr 2, 2025

Uh oh!

dylwil3 commented Apr 2, 2025

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

maxmynter commented Apr 4, 2025

Uh oh!

dylwil3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maxmynter commented Mar 30, 2025 •

edited

Loading

github-actions bot commented Mar 30, 2025 •

edited

Loading

`ruff-ecosystem` results

maxmynter commented Mar 31, 2025 •

edited

Loading