[ruff] Classes with mixed type variable style (RUF053)

[InSyncWithFoo]

Summary

Resolves #15620.

RUF053 reports the first Generic[...] base of any class with a PEP 695 type parameter list. It reuses much of the logic added in #15565, #15659 and other PRs.

Since the original code is fundamentally flawed, the fix takes some liberty in its rewrite. As such, it is always marked as unsafe.

Test Plan

cargo nextest run and cargo insta test.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[AlexWaygood]

Thanks! Some comments below. I haven't looked in detail at the implementation yet, just at docs and the behaviour of the rule as indicated by the snapshots.

I don't think this should be a pyupgrade rule, since all our existing pyupgrade rules are modernisation/stylistic rules. This rule is different, since it detects (and fixes) behaviour that would cause the code to raise an exception at runtime. I understand that you want to share the code being used in the existing pyugprade PEP-695 rules, but we could always move those utilities to somewhere else in the ruff_linter crate, where it could be accessed by both the pyupgrade rules and this rule

[ntBre]

I haven't reviewed the code in detail yet either, but it looks a little longer than I expected. I might be oversimplifying things, but can we pretty much reuse the logic from the UP046 class rule and just perform an Edit::insertion at the end of type parameter list instead of writing a full list? That would save you the effort of duplicating all the checks from UP046 (like the default kwarg that Alex pointed out) and converting existing TypeParams to TypeVars only to write them back into TypeParams. This would also lose you any deduplication it looks like you're doing with is_existing_param_of_same_class, but the incoming code was a real mess if it had mixed type variables with duplicates 😆

[InSyncWithFoo]

[...] can we [...] just perform an Edit::insertion at the end of type parameter list instead of writing a full list?

I added a testcase for this:

# Original
class C[
	T  # Comment about `T`
](Generic[_A]): ...

# Wrong fix
class C[
	T  # Comment about `T`, _A
]: ...

# Also wrong fix (slightly)
class C[
	T, _A  # Comment about `T`
]: ...

# Correct fix, but looks irritating
class C[
	T  # Comment about `T`
, _A]: ...

I wasn't sure if relying on the formatter is the way to go.

[ntBre]

Ah interesting, I did not catch that about comments, thanks! I'll try to give this a fuller review today.

[AlexWaygood]

Could you possibly retitle the PR and edit the description to make clear that this is no longer proposed as a pyupgrade rule?

[ntBre]

Thanks for this! It looks good to me overall besides a couple of nits and a bigger question about reusing the TypeVarReferenceVisitor.

[ntBre]

Thanks for your work on this. I especially appreciate the thorough test fixtures. I think I'll need to steal some of those for UP046 and UP047, if you don't get to them first 🙂

I'd like to see my latest comments addressed, but they're mostly nits so I'm still happy to approve.

[AlexWaygood]

Thank you -- this overall looks great now! And I'll second @ntBre that your tests are excellent. I just have a few coding nits:

requested changes were made

[AlexWaygood]

Nice!