-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[flake8-type-checking
] Adds implementation for TC006
#14511
Conversation
|
code | total | + violation | - violation | + fix | - fix |
---|---|---|---|---|---|
TC006 | 431 | 431 | 0 | 0 | 0 |
TC001 | 149 | 149 | 0 | 0 | 0 |
TC003 | 27 | 27 | 0 | 0 | 0 |
SIM115 | 2 | 1 | 1 | 0 | 0 |
E501 | 1 | 1 | 0 | 0 | 0 |
TC002 | 1 | 1 | 0 | 0 | 0 |
The extra hits for the other rules make sense to me, since fixing In the case of |
crates/ruff_linter/src/rules/flake8_type_checking/rules/runtime_cast_value.rs
Outdated
Show resolved
Hide resolved
Thanks for contributing this rule. The code changes look good to me. I want to wait for updated ecosystem checks. I don't see why this PR should result in any new diagnostics other than TC006. |
crates/ruff_linter/resources/test/fixtures/flake8_type_checking/TC006.py
Outdated
Show resolved
Hide resolved
I'm a bit confused by the ecosystem checks. I understand that we run ruff exactly once and don't apply any fixes. That makes it unclear to me why there would be any changed diagnostics other than for TC006. |
The only way this output makes sense to me is if it does in fact repeatedly apply fixes until it runs out of fixes and uses the output from whatever the last iteration was for the ecosystem results. It's also a little sus that there's no +/- on the count of fixes, since these are definitely all auto-fixable. Maybe this was changed at some point without anyone noticing? Because the fixes count was always zero on my other PRs as well, even though they definitely included some new fixes. |
…e_cast_value.rs Co-authored-by: Micha Reiser <[email protected]>
Something's definitely broken. Running the two ruff binaries without the ecosystem wrapper script gives me zero changes for pandas. |
Indeed, I double checked, there's not even any type aliases defined in the first file in pandas. Maybe it's the result of broken caching on the CI side? I've noticed the ecosystem job has gotten significantly faster recently. |
I can reproduce the ecosystem differences locally.... I'm sure it's something obvious and very stupid :D |
Actually I confused myself, this is about So I still think the ecosystem script applies fixes repeatedly and uses either the state where it can no longer fix things or the state just before it fixed everything for the results. That also explains the poetry hit, since fixing TC006 causes the SIM115 violation to be moved two characters over, by the newly introduced quotes. |
Possibly. But the ecosystem check run
And that ultimately calls into Ohhhhh.... pandas sets |
I see, ecosystem should probably force that to |
Now that
TCH
has been renamed toTC
and the redirect fromTCH006
toTCH010
can no longer bite us, I've added an implementation for this very simple rule.Summary
TC006
checks for non-string literal arguments totyping.cast
which adds unnecessary runtime overhead, since the function doesn't do anything at runtime.This PR has a very tiny overlap with my other PR for
TCH007
/TCH008
inflake8_type_checking/helpers.rs
for adding thequote_type_expression
function, but the two changes don't really depend on one another, so they can be merged in any order.Test Plan
cargo nextest run