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.

Sign up for GitHub

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

Jump to bottom

PERF102 - False Positive with Poorly Named Variables #5692

Closed
Skylion007 opened this issue Jul 11, 2023 · 4 comments · Fixed by #5763
Closed

PERF102 - False Positive with Poorly Named Variables #5692

Skylion007 opened this issue Jul 11, 2023 · 4 comments · Fixed by #5763
Assignees
@charliermarsh
Labels
bug Something isn't working

Comments

@Skylion007
Copy link
Contributor

Skylion007 commented Jul 11, 2023 •
edited
Loading 

-                    for (i, j), _ in smat.items():
+                    for (i, j) in smat.keys():
                         diag_entries[(i + rmax, j + cmax)] = _

This code snippit is from sympy/matrices/common.py, this uses the latest stable ruff version and just has PERF102 manually selected using the command ruff --select PERF --fix .

I know that the _ prefix is often used to look for variables that should be unused in a loop, but ruff should check if the variable is actually unused before autofixing it to prevent bugprone autofixes. It should at least handle this trivial case.
@Skylion007 Skylion007 changed the title PERF102 - False Positive + Bad Fix PERF102 - False Positive with Poorly Named Variables Jul 11, 2023
@charliermarsh
Copy link
Member

Yeah we can support this.

@charliermarsh charliermarsh added the bug Something isn't working label Jul 11, 2023
@zanieb
Copy link
Member

zanieb commented Jul 11, 2023

Wow that's madness :) We should add a rule that checks for usage of _ outside of unpacking!

@dhruvmanila
Copy link
Member

Yeah, I've seen this kind of usage in the wild especially in comprehensions for one off variables.

@charliermarsh
Copy link
Member

crates/ruff/src/rules/flake8_bugbear/rules/unused_loop_control_variable.rs is a similar rule (it detects unused loop control variables more generally) that we can use as a reference.

@charliermarsh charliermarsh self-assigned this Jul 14, 2023
@charliermarsh charliermarsh mentioned this issue Jul 14, 2023
Merged
charliermarsh added a commit that referenced this issue Jul 14, 2023
@charliermarsh
Use unused variable detection to power incorrect-dict-iterator (#5763)
7c32e98 
## Summary

`PERF102` looks for unused keys or values in `dict.items()` calls, and
suggests instead using `dict.keys()` or `dict.values()`. Previously,
this check determined usage by looking for underscore-prefixed
variables. However, we can use the semantic model to actually detect
whether a variable is used. This has two nice effects:

1. We avoid odd false-positives whereby underscore-prefixed variables
are actually used.
2. We can catch more cases (fewer false-negatives) by detecting unused
loop variables that _aren't_ underscore-prefixed.

Closes #5692.
evanrittenhouse pushed a commit to evanrittenhouse/ruff that referenced this issue Jul 19, 2023
@charliermarsh @evanrittenhouse
Use unused variable detection to power incorrect-dict-iterator (ast…
2391f26 
…ral-sh#5763)

## Summary

`PERF102` looks for unused keys or values in `dict.items()` calls, and
suggests instead using `dict.keys()` or `dict.values()`. Previously,
this check determined usage by looking for underscore-prefixed
variables. However, we can use the semantic model to actually detect
whether a variable is used. This has two nice effects:

1. We avoid odd false-positives whereby underscore-prefixed variables
are actually used.
2. We can catch more cases (fewer false-negatives) by detecting unused
loop variables that _aren't_ underscore-prefixed.

Closes astral-sh#5692.
konstin pushed a commit that referenced this issue Jul 19, 2023
@charliermarsh @konstin
Use unused variable detection to power incorrect-dict-iterator (#5763)
fe54aa2 
## Summary

`PERF102` looks for unused keys or values in `dict.items()` calls, and
suggests instead using `dict.keys()` or `dict.values()`. Previously,
this check determined usage by looking for underscore-prefixed
variables. However, we can use the semantic model to actually detect
whether a variable is used. This has two nice effects:

1. We avoid odd false-positives whereby underscore-prefixed variables
are actually used.
2. We can catch more cases (fewer false-negatives) by detecting unused
loop variables that _aren't_ underscore-prefixed.

Closes #5692.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@charliermarsh charliermarsh

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use unused variable detection to power incorrect-dict-iterator astral-sh/ruff
4 participants
@charliermarsh @Skylion007 @zanieb @dhruvmanila