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

Eradicate (ERA001) is triggered on well formated TODO #7031

Closed
Cjkjvfnby opened this issue Aug 31, 2023 · 2 comments · Fixed by #7523
Closed

Eradicate (ERA001) is triggered on well formated TODO #7031

Cjkjvfnby opened this issue Aug 31, 2023 · 2 comments · Fixed by #7523
Labels
bug Something isn't working help wanted Contributions especially welcome

Comments

@Cjkjvfnby
Copy link
Contributor

ruff 0.0.286

When my TODO is bad (according to the TD rules), it's not reported as unused code
But when I fix TD warnings the code becomes a subject of ERA

Broken TODO

def foo():
    # TODO: implement
    pass
ruff "foo.py"  --select TD002,ERA
TD002 Missing author in TODO; try: `# TODO(<author_name>): ...` or `# TODO @<author_name>: ...`

When I fix this warning

def foo():
    # TODO(CJ): implement
    pass
ruff foo.py --select TD002,ERA
ERA001 [*] Found commented-out code
Found 1 error.
[*] 1 potentially fixable with the --fix option.

I expect that the TODO comment should not be marked as unused code. And TD requirements were aligned with other rules.

@zanieb zanieb added the bug Something isn't working label Aug 31, 2023
@zanieb
Copy link
Member

zanieb commented Aug 31, 2023

I think it makes sense to add an exception to ERA001 if feasible — the pattern used for matching violations is brittle and has a lot of false positives.

@zanieb zanieb added the help wanted Contributions especially welcome label Aug 31, 2023
@charliermarsh
Copy link
Member

I'm not sure why it triggers in one case but not the other, but yeah I definitely would've expected TODO to be in the allowlist already. Oops!

charliermarsh pushed a commit that referenced this issue Sep 28, 2023
## Summary

Extend the `task-tags` checking logic to ignore TODO tags (with or
without parentheses). For example,

```python
# TODO(tjkuson): Rewrite in Rust
```

is no longer flagged as commented-out code.

Closes #7031.

I also updated the documentation to inform users that the rule is prone
to false positives like this!

EDIT: Accidentally linked to the wrong issue when first opening this PR,
now corrected.

## Test Plan

`cargo test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Contributions especially welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants