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

Confusing error message for C401 and C416 #10101

Closed
runfalk opened this issue Feb 23, 2024 · 1 comment
Closed

Confusing error message for C401 and C416 #10101

runfalk opened this issue Feb 23, 2024 · 1 comment
Labels
rule Implementing or modifying a lint rule

Comments

@runfalk
Copy link

runfalk commented Feb 23, 2024

I ran into this oddity. The suggested fix for C401 actually results in C416 because i isn't transformed in any way. Is there a better way to indicate this in the error message?

_a = set(i for i in range(10))
_b = {i for i in range(10)}
_c = set(range(10))
$ poetry run ruff check --output-format full test.py
test.py:1:6: C401 Unnecessary generator (rewrite as a `set` comprehension)
  |
1 | _a = set(i for i in range(10))
  |      ^^^^^^^^^^^^^^^^^^^^^^^^^ C401
2 | _b = {i for i in range(10)}
3 | _c = set(range(10))
  |
  = help: Rewrite as a `set` comprehension

test.py:2:6: C416 Unnecessary `set` comprehension (rewrite using `set()`)
  |
1 | _a = set(i for i in range(10))
2 | _b = {i for i in range(10)}
  |      ^^^^^^^^^^^^^^^^^^^^^^ C416
3 | _c = set(range(10))
  |
  = help: Rewrite using `set()`

Found 2 errors.
No fixes available (2 hidden fixes can be enabled with the `--unsafe-fixes` option).
$ poetry run ruff --version
ruff 0.2.2
@charliermarsh
Copy link
Member

Thanks for pointing this out -- fixed in #10596!

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Mar 26, 2024
charliermarsh pushed a commit that referenced this issue Mar 26, 2024
…matches `C416` (#10596)

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Similar to #10419, there was a case where there is a collision of C401
and C416 (as discussed in #10101).
Fixed this by implementing short-circuit for the comprehension of the
form `{x for x in foo}`.

## Test Plan

<!-- How was it tested? -->

Extended `C401.py` with the case where `set` is not builtin function,
and divided the case where the short-circuit should occur.
Removed the last testcase of `print(f"{ {set(a for a in 'abc')} }")`
test as this is invalid as a python code, but should I keep this?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

3 participants