[flake8-commas] Add support for trailing comma checks in type parameter lists (COM812, COM819)

[danparizher]

Summary

Fixes #18844

I'm not too sure if the solution is as simple as the way I implemented it, but I'm curious to see if we are covering all cases correctly here.

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[MichaReiser]

We probably have to gate this behind preview. Did you have a chance to look through the ecosystem results. Do the changes look correct to you?

[danparizher]

Yes, the ecosystem changes look to be correct, from what I can tell. I also gave a shot at implementing the preview, but I'm not sure if I did it correctly. I would appreciate a review!

[ntBre]

Thanks! I haven't gone through all of the snapshots yet, but I had a couple of comments on the code in an initial pass.

This is kind of a note to myself for later, but we'll want to double check that the ecosystem check doesn't change on stable for a preview change. It looks like it didn't rerun after you added the preview gate, but hopefully it will run again after any additional commits.

[danparizher]

It looks like the ecosystem check didn't rerun. Maybe we can manually rerun it?

[MichaReiser]

The ecosystem results were updated 10h ago. That would suggest that they did run?

[ntBre]

Maybe there was a queue of ecosystem checks yesterday afternoon. It hadn't updated when I checked after the last commit here yesterday either. I see it now though!

My suggestion might have been slightly misleading about your old preview check. We'll need both the new match on two tokens and the preview check. Maybe something like this:

TokenType::OpeningSquareBracket if is_your_preview_enabled(settings) => match (prev.ty, prev_prev.ty) { // new code
    ...
}
TokenType::OpeningSquareBracket => match prev.ty { // the old code

Then hopefully all the ecosystem changes will show up in the preview section as expected.

[danparizher]

Appreciate the suggestions, I think I was confusing myself with this part 😆

[ntBre]

Thanks. I think you may have missed a couple of the key parts of my patch suggestion. There were still a lot of unexpected differences between the preview and non-preview snapshots, which I saw locally by diffing them. Hopefully we'll get #19351 at some point and this will be easier to see directly in PRs.

I made a couple of tweaks and then confirmed that

diff crates/ruff_linter/src/rules/flake8_commas/snapshots/ruff_linter__rules__flake8_commas__tests__COM81_syntax_error.py.snap crates/ruff_linter/src/rules/flake8_commas/snapshots/ruff_linter__rules__flake8_commas__tests__preview__COM81_syntax_error.py.snap

was totally empty and that

diff crates/ruff_linter/src/rules/flake8_commas/snapshots/ruff_linter__rules__flake8_commas__tests__COM81.py.snap crates/ruff_linter/src/rules/flake8_commas/snapshots/ruff_linter__rules__flake8_commas__tests__preview__COM81.py.snap

only showed modifications at the end of the file, where the new cases were added.

I think that will also cut down on the ecosystem changes. I'll double check that and then merge this.

[ntBre]

That actually entirely cleared the ecosystem changes 😅 I hadn't noticed before that they were all related to the slice change, not actual type parameters, but that's the case for all the ones I clicked through just now.