ruff: How to use COM812 in linter but avoid conflict with formatter?
I would like to achieve the following things:
- Use the ruff linter on my code
- Use the ruff formatter on my code
- Enable rule COM812 for the linter
I like the linter, I like the formatter, and I like rule COM812. However, when I have COM812 as an allowed rule I get a warning when I run the formatter that rule COM812 may cause conflicts and that it should be disabled to avoid unexpected behavior. Is there, then, a recommended way to achieve my three goals?
I see a number of issues discussing rule COM812 but I don’t see any addressing this exact question.
About this issue
- Original URL
- State: open
- Created 6 months ago
- Reactions: 3
- Comments: 31 (19 by maintainers)
I’m currently looking into what it takes to make
COM812
compatible with the formatter. It sounds like a simple change, but it’s a very expensive token-based rule. I need to figure out how to test if all arguments (or list elements etc) are on their own line without having to track too much state.I don’t intend to add a configuration option that gives users the old behavior because I don’t see the formatter’s behavior to be in contradiction to the rule’s goals.
Yeah, using the formatter and the linter together is great and very much intended. As you say, they solve different problems! I don’t have a solution here, but adding some more color from my perspective… The issue in this case is that this rule pre-dated the formatter – if the rule didn’t exist today, we likely wouldn’t add it to Ruff, since these kinds of linter-formatter conflicts create a lot of problems for users.
One answer would be to deprecate and remove the rule. But we’ve shied away from doing that, and instead we’ve:
Modifying the rule to allow Case (D) above would be a reasonable compromise, and would make it non-conflicting, but it’d probably also make it redundant, and my guess is that users of this rule actually like that it enforces that style.
(Just to be clear, I think the current situation where COM812 is not compatible with the formatter is worse than a situation where COM812 doesn’t enforce comma in this case, so I think #10111 is an improvement. I’m just thinking if somehow we can have both).
I’m refactoring large code bases, and the case you quote above is particularly common as an outcome of reformatting.
What the formatter does is indeed very natural and intuitive: When the line gets too long it first tries to put everything inside braces onto a line of its own. Shouldn’t COM812 accept this as a good case, in general? Kind of, “there are several arguments on a single line”, that’s a signal that it’s okay not to have a trailing comma.
The
trailing-comma = "force"
is probably just that. - Maybe"strict"
vs."forgiving"
would reflect the intention even better, with"forgiving"
as the default, probably.It would solve the conflict, but I don’t see the difference to disabling the lint and only use the formatter (except that the warning doesn’t show up by default when using
--select ALL
).@AlexWaygood’s currently figuring out how we want to categorize Ruff’s rules in the future. This work also touches on if Ruff should support rules that are incompatible with the formatter (probably?) and, if so, how the user experience looks like.
That’s why I want to hold off from making any changes to
COM812
.Yeah, we might have to write up the rules eventually. For now, Black’s style guide also applies for Ruff and it explains how it breaks lines here.
That makes sense to me. It may also be worth improving the warning message with a clearer recommendation to disable
COM812
when using the formatter because it is superfluous (except you disagree with the formatter’s style but want to use the formatter regardless, which is unlikely a case we want to support).Agree and thanks for the detailed explanation. Changing
COM812
to make the trailing comma optional makes it less useful for users who don’t use the formatter.The next steps are:
I believe the formatter does a sensible job.
To solve the contradiction,
COM812
should accept case “D” from above (for short enough lines, obviously). Isn’t that all there is to do?(Or don’t I get the point of all the numerous cases from above? Frankly, I’ve never seen the other cases caused by the formatter.)
I use both the linter and the formatter, because they serve different jobs. I have all linter rules enabled, I try to make exceptions rare and file-based. Ruff is great for this, giving actionable feedback collected from years of investment of a significant number of people in the Python community.
A note aside, I try to stay away from
--fix
and also review changes made by the formatter. It makes sense more often than expected to take your time and come up with “creative solutions” that are easier to read and more intuitive from a Clean Code point of view. When those also make the formatter and linter happy, you’re done.@MichaReiser ok, I think the suggestion to disable
COM812
and just use the formatter will indeed give me outputs that are satisfactory. I think the expected behaviors are a bit confusing though and the documentation wasn’t very clarifying for me. Actually I generally don’t see much documentation about what changes I should expect from the formatter? Maybe I should check documentation forblack
?In any case, it seems like the formatter will:
(1) If the line is too long to have params/args share with def then it will (2) try to put all parameters on a single line after
def(
and before):
on a third line. It does this WITHOUT a trailing comma (3) If the line is still too long then it breaks out each param/arg onto its own line WITH a trailing commaUnder my current workflow I will never get output formatted like (2) because of the lack of trailing comma. But if I were to disable the lint rule
COM812
I would sometimes get output like (2). That’s ok with me, I don’t have anything against (2).So for me I would be happy to disable
COM812
. The reason I didn’t want to do that was I thought it would allow output likewithout a trailing comma. That is, since
COM812
was disabled I didn’t expect anything to be there to enforce the presence of the trailing comma. But, it seems the formatter on its own already enforces the trailing comma in this case.I would be helped by more clear documentation pointing out that running the formatter will already enforce trailing commas in the single param/arg per line format. Though the gotcha edge case here, as @bluetech highlights, is that the formatter will NOT enforce the trailing comma in the multiple params on one line not shared with
def
case. This is ok from a VCS perspective since if parameters are added the signature would be totally reformatted to the single param per line style. This is ok if you’re totally relying on the formatter though because you’ll never be outside one of the three possible cases.@bluetech indicates that they want the trailing comma so that they can extend multi-params not shared with
def
onto more than one line. But in this case they are operating outside the rules of the formatter so they should not use the formatter. In this case I thinkCOM812
alone would suffice.To summarize, I think status quo is ok for me, but better documentation would be helpful. Especially in light of the fact that the multi-parameters per line not shared with
def
is kind of a funny edge case in my view.@MichaReiser really appreciate your detailed consideration of this issue.
Though I admit to being a bit confused at the outcome (and in my previous comments I was confused myself):
The current situation is that COM812 and formatter are officially incompatible, possibly this even becomes a hard error in the future.
This issue asks them to not be incompatible. If I understand the reasoning from @jagerber48 and @bittner (and myself) correctly, the reason why we want COM812 + formatter is to avoid the “SINGLE LINE, DON’T SHARE WITH DEF” case.
The PR makes COM812 + formatter compatible, by making COM812 not complain about the “SINGLE LINE, DON’T SHARE WITH DEF” case.
It therefore seems to that the PR wouldn’t actually satisfy what’s being requested in this issue, and actually make the situation worse than the current situation for people not using the formatter (because they want a comma there).
Really sorry if I’m misunderstanding something and just wasting your time…
Yes, though of course with tab -> 4 spaces, and a trailing comma on
iiii
, that isthough realistically when this happens I prefer to explode.
I don’t think it’s currently possible based on our definition of “incompatible”. In short, we want to avoid cases in which running the formatter introduces new lint rule violations. In the case of COM812, you could have a function like:
Which the formatter may reformat as:
Which would violate COM812. After adding the trailing comma to fix COM812, the formatter would then reformat again as:
We would need a new trailing-comma setting in the formatter, like
"force"
.\cc @MichaReiser