Refine the warnings about incompatible linter options#8196
Refine the warnings about incompatible linter options#8196charliermarsh merged 5 commits intomainfrom
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
3a2dac1 to
142cc30
Compare
| Rule::IndentWithSpaces, | ||
| for rule in [ | ||
| // The formatter might collapse implicit string concatenation on a single line. | ||
| Rule::SingleLineImplicitStringConcatenation, |
There was a problem hiding this comment.
I'm undecided if we should warn about this rule. The formatter might collapse implicit string concatenated lines which then triggers this rule. However, it might be something users want to fix manually.
Same for MissingTrailingComma. The rule might require manual intervention (and magic-trailing-comma set to respect), but once fixed it remains fixed.
There was a problem hiding this comment.
If the formatter collapses the strings, I would love to know about it to fix it afterward.
55ecede to
27257e6
Compare
08adb86 to
2d34854
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
|
Looks great, most of my warnings are gone! |
|
2d34854 to
5aaa44d
Compare
| Rule::LineTooLong, | ||
| Rule::TabIndentation, | ||
| Rule::IndentationWithInvalidMultiple, | ||
| Rule::IndentationWithInvalidMultipleComment, |
There was a problem hiding this comment.
I these think these are redundant when using tabs in the formatter.
| // pass | ||
| // ``` | ||
| Rule::MissingTrailingComma, | ||
| Rule::ProhibitedTrailingComma, |
There was a problem hiding this comment.
This was removed because the formatter will never output code that triggers it? Is that right?
There was a problem hiding this comment.
Yes, I believe this is the case.
Summary
Avoid warning about incompatible rules except if their configuration directly conflicts with the formatter. This should reduce the noise and potentially the need for #8175 and #8185
I also extended the rule and option documentation to mention any potential formatter incompatibilities or whether they're redundant when using the formatter.
LineTooLong: This is a use case we explicitly want to support. Don't warn about itTabIndentation,IndentWithSpaces: Only warn ifindent-style="tab"IndentationWithInvalidMultiple,IndentationWithInvalidMultipleComment: Only warn ifindent-width != 4OverIndented: Don't warn, but mention that the rule is redundantBadQuotesInlineString: Warn if quote setting is different fromformat.quote-styleBadQuotesMultilineString,BadQuotesDocstring: Warn ifquote != "double"Test Plan
I added a new integration test for the default configuration with
ALL.ruff formatnow only shows two incompatible rules, which feels more reasonable.