Lint: Use Ruff to format Tools/jit
#133123
Conversation
| case ( | ||
| Hole( | ||
| offset=offset, | ||
| kind="IMAGE_REL_AMD64_REL32", | ||
| value=HoleValue.GOT, | ||
| symbol="_JIT_CONTINUE", | ||
| addend=-4, | ||
| ) as hole | ||
| ): |
There was a problem hiding this comment.
These case rewrites are... odd. I don't see how it's better in any way. Is there a setting of some sort that's causing this? Maybe because one case has a guard... but that doesn't seem like a good enough reason to indent every case and wrap them in parens.
There was a problem hiding this comment.
this is https://astral.sh/blog/ruff-v0.9.0#automatically-parenthesized-if-guards-in-match-case-statements (which Black doesn't do)
There was a problem hiding this comment.
This case (237-245) doesn't have an if-guard, though. Are all cases put in brackets if any have a guard?
There was a problem hiding this comment.
in that case it might just be due to the line length being shorter than it was previously? Addressing #133123 (review) might fix it
There was a problem hiding this comment.
I’m not sure that’s the issue. This uses more horizontal space than not wrapping in parens, due to the additional indent.
I suspect it is because one of the cases has a guard, which feels like a bug.
There was a problem hiding this comment.
I kept the wrap-to-80 commit separate (42ce014 (#133123)), and the change was before that.
There was a problem hiding this comment.
Or maybe it has to do with the name assignment to hole?
There was a problem hiding this comment.
Yep, it seems to be as hole that triggers the brackets. That does seem like a bug?
There was a problem hiding this comment.
Yes. It's not just class patterns, outer grouping symbols in the pattern like (), [], and {} aren't being used effectively. See this playground, where I have my expected formatting going in and the buggy formatting coming out: https://play.ruff.rs/81f62f2d-6858-43eb-8a9b-67dcebdc56f6?secondary=Format
|
I'd like to wait on this change until the Ruff issue is fixed. |
Happy to, I'll split out the non-JIT changes from this PR. A |
Tools/jit
# Conflicts: # .pre-commit-config.yaml
It doesn't make that much sense to use two linters in our source tree. This switches
Tools/jitandTools/build/check-warnings.pyto use Ruff for their formatting, which has the side benefit of slightly speeding uppre-commitand the lint CI job.A