Warn about incompatible formatter options

[MichaReiser]

Summary

Warn users about options incompatible with the formatter when using ruff format

Closes #7646

Test Plan

Added integration test

[MichaReiser]

Current dependencies on/for this PR:

• main
  • PR Warn about incompatible formatter options #8088 👈

This comment was auto-generated by Graphite.

[charliermarsh]

One tricky thing here is that these don't necessarily conflict with the formatter. Some of them do overlap, but in a way that's at least consistent. Some of them are also arguably complimentary (like MissingTrailingComma, I think, is arguably complimentary, since it will insert trailing commas that the formatter will then respect).

[MichaReiser]

COM812 is conflicting because it disallows:

class PhotoMetadata:
    """Metadata about a photo."""

    def __init__(
        self, file: Path, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u
    ):
        ...

My goal is to only warn about rules that are incompatible. E.g. I excluded

• E501 because there are valid use cases to use it alongside the formatter, even though it's mostly redundant
• E701: while redundant, it doesn't conflict with the formatter.

Is your feedback mainly about the naming of the variables or the wording of the warning message? I'm trying to understand what I should change.

[charliermarsh]

I think I'm just trying to understand what we consider to be a conflict (and, based on that answer, perhaps looking to refine the list, or change the wording of the message).

For example, above, if you ran the linter, wouldn't COM812 just insert a trailing comma at the end, and then the formatter would in turn split the arguments over multiple lines? So it's not like the two tools would go back and forth endlessly disagreeing with one another, as they would if you set BadQuotes to divergent values. The COM812 case feels like it fits some definition of a conflict though, like: formatted code can introduce these violations.

[MichaReiser]

For example, above, if you ran the linter, wouldn't COM812 just insert a trailing comma at the end, and then the formatter would in turn split the arguments over multiple lines?

That's correct. But it requires that you need to run formatter, linter, and formatter again to get a stable result which doesn't seem ideal. I see incompatibilities as everything that leads to a bad experience.

[charliermarsh]

So, anything that might trigger or require some command to be repeated, even if that cycle isn't infinite. In that case, we probably want to include ISC001?

[MichaReiser]

Probably we should. This rule won't be conflicting with the preview style but the formatter could produce code today that conflicts with it

[charliermarsh]

Small nudges on the comment:

The following rules may cause conflicts when used with the formatter: {}.
To avoid unexpected behavior, we recommend disabling these rules, either
by removing them from your `select` or `extend-select`, or adding them to
your `ignore`.

[MichaReiser]

"your ignore" sounds a bit strange. I used your suggestion but changed it to "from the select or extend-select configuration.

[charliermarsh]

Small nudges on the comment:

The following isort options may cause conflicts when used with the formatter: {}.
To avoid unexpected behavior, we recommend disabling these options by removing
them from your configuration.

[MichaReiser]

Much better

[github-actions]

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

[smackesey]

warning: The following isort options may cause conflicts when used with the formatter: 'isort.force-wrap-aliases'. To avoid unexpected behavior, we recommend disabling these options by removing them from the configuration.

This rule is fairly important to us and it's not clear how it conflicts, as we never had a conflict using this together with black.

[MichaReiser]

@smackesey Thanks for your feedback. I'm sorry, I was over-cautious with the warnings. #8192 refines the warnings. You should no longer see it, as long as you don't set format.skip-magic-trailing-comma to true.

[charliermarsh]

@smackesey - We're planning to cut a release today that will include that fix.