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

Ignoring commented-out code with noqa: ERA001 may still trigger RUF100 #6821

Closed
yigor opened this issue Aug 23, 2023 · 2 comments · Fixed by #6822
Closed

Ignoring commented-out code with noqa: ERA001 may still trigger RUF100 #6821

yigor opened this issue Aug 23, 2023 · 2 comments · Fixed by #6822
Labels
accepted Ready for implementation bug Something isn't working

Comments

@yigor
Copy link

yigor commented Aug 23, 2023

Looks like #1148 wasn't completely fixed:

# commented.py

dictionary = {
    # "key1": 123,  # noqa: ERA001
    # "key2": 456,
}

executing ruff against this file results in the following:

$ ruff commented.py                                                                                                                                                              master
commented.py:2:21: RUF100 [*] Unused `noqa` directive (unused: `ERA001`)
commented.py:3:5: ERA001 [*] Found commented-out code
Found 2 errors.
[*] 2 potentially fixable with the --fix option.

$ ruff --version
ruff 0.0.285
@zanieb
Copy link
Member

zanieb commented Aug 23, 2023

Thanks for raising! I've reproduced this in #6822 although I'm not sure what the fix is yet. I'm not sure how #1157 was intended to address this.

@zanieb zanieb added bug Something isn't working accepted Ready for implementation labels Aug 23, 2023
zanieb added a commit that referenced this issue Aug 25, 2023
…mments (#6822)

Closes #6821

ERA100 was not raising on commented parts of dictionaries if it included
another comment (such as a noqa clause). In cases where this comment was
a noqa clause, RUF100 to be emitted since the noqa would have no effect.
Here, we update ERA100 to raise even when there are trailing comments.
This resolves the linked issue _and_ increases the scope of ERA100. We
could narrow the regular expression to only apply to noqa comments if we
do not want to expand ERA100 however I think this change is within the
spirit of the rule.
@yigor
Copy link
Author

yigor commented Aug 25, 2023

Awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants