ruff: How to use COM812 in linter but avoid conflict with formatter?

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:

• Documented the rules that conflict, and warned against them.
• Held back from adding new rules that cause linter-formatter conflicts.
• In some cases, modified rules to make them non-conflicting.

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.

def args(
    aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh
):
    ...

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.

My take is, the formatter does a decent job, also for people who really don’t want to compromise with COM812. If we had a configuration switch for the linter that would allow a less strict interpretation of COM812 (for accepting “the less compact” variant) wouldn’t we fix today’s situation, which would allow us to remove the conflict warning?

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.

Actually I generally don’t see much documentation about what changes I should expect from the formatter? Maybe I should check documentation for black?

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.

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.

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).

But I think there’s a case where someone who DOESN’T use the formatter might want something like G) above.

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:

• Improve deprecation warning
• Improve the rule’s documentation related to formatter compatibility

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 for black?

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 comma

Under 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 like

def args(
    aaaaaaaa, 
    bbbbbbbbb, 
    cccccccccc, 
    ddddddddd, 
    eeeeeeee, 
    ffffff, 
    gggggggggggg, 
    hhhh, 
    iiii
):
    pass

without 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 think COM812 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…

Like, would you write this as:

Yes, though of course with tab -> 4 spaces, and a trailing comma on iiii, that is

def args(
    aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh, 
    iiii,
):
    pass

though 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:

def args(aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh):
    pass

Which the formatter may reformat as:

def args(
    aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh
):
    pass

Which would violate COM812. After adding the trailing comma to fix COM812, the formatter would then reformat again as:

def args(
    aaaaaaaa,
    bbbbbbbbb,
    cccccccccc,
    ddddddddd,
    eeeeeeee,
    ffffff,
    gggggggggggg,
    hhhh,
):
    pass

We would need a new trailing-comma setting in the formatter, like "force".

\cc @MichaReiser