Skip to content
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

Warn on invalid # noqa rule codes #12811

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Warn on invalid # noqa rule codes #12811

wants to merge 2 commits into from

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Aug 11, 2024

Summary

Today, we already warn if you do # ruff: noqa: F401F841... But not inline (e.g., import os # noqa: F401F841).

This PR extends the warnings to the latter case.

It's not "breaking", but I'd suggest we ship it in a minor.

Fixes #15682

@charliermarsh charliermarsh added cli Related to the command-line interface suppression Related to supression of violations e.g. noqa labels Aug 11, 2024
@charliermarsh charliermarsh changed the title Warn on invalid # noqa rule codes Warn on invalid # noqa rule codes Aug 11, 2024
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not "breaking", but I'd suggest we ship it in a minor.

Seems reasonable to me.

I think it might be useful for #12831 to be fixed as well.

@MichaReiser MichaReiser added this to the v0.6 milestone Aug 12, 2024
@MichaReiser
Copy link
Member

There's now a ruff-0.6 feature branch into which you can merge this change.

@MichaReiser
Copy link
Member

@charliermarsh do you plan on merging this for the 0.6 release?

@charliermarsh
Copy link
Member Author

@MichaReiser - No, I should address Dhruv’s concern first but don’t have time immediately.

@MichaReiser MichaReiser modified the milestones: v0.6, v0.7 Aug 16, 2024
@AlexWaygood AlexWaygood modified the milestones: v0.7, v0.8 Oct 17, 2024
@charliermarsh charliermarsh force-pushed the charlie/parse branch 3 times, most recently from 5894aa0 to c2e9746 Compare November 2, 2024 20:15
Base automatically changed from charlie/parse to main November 2, 2024 20:25
@AlexWaygood AlexWaygood modified the milestones: v0.8, v0.9 Nov 18, 2024
@MichaReiser MichaReiser modified the milestones: v0.9, v.0.10 Jan 8, 2025
dylwil3 added a commit that referenced this pull request Feb 16, 2025
On `main` we warn the user if there is an invalid noqa comment[^1] and
at least one of the following holds:

- There is at least one diagnostic
- A lint rule related to `noqa`s is enabled (e.g. `RUF100`)

This is probably strange behavior from the point of view of the user, so
we now show invalid `noqa`s even when there are no diagnostics.

Closes #12831

[^1]: For the current definition of "invalid noqa comment", which may be
expanded in #12811 . This PR is independent of loc. cit. in the sense
that the CLI warnings should be consistent, regardless of which `noqa`
comments are considered invalid.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the command-line interface suppression Related to supression of violations e.g. noqa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unused-noqa (RUF100) - false negatives and strange behavior with multiple codes
4 participants