Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine the warnings about incompatible linter options #8196

Merged
merged 5 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 77 additions & 25 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ use ruff_diagnostics::SourceMap;
use ruff_linter::fs;
use ruff_linter::logging::LogLevel;
use ruff_linter::registry::Rule;
use ruff_linter::settings::rule_table::RuleTable;
use ruff_linter::rules::flake8_quotes::settings::Quote;
use ruff_linter::source_kind::{SourceError, SourceKind};
use ruff_linter::warn_user_once;
use ruff_python_ast::{PySourceType, SourceType};
use ruff_python_formatter::{format_module_source, FormatModuleError};
use ruff_python_formatter::{format_module_source, FormatModuleError, QuoteStyle};
use ruff_text_size::{TextLen, TextRange, TextSize};
use ruff_workspace::resolver::{
match_exclusion, python_files_in_path, PyprojectConfig, ResolvedFile, Resolver,
Expand Down Expand Up @@ -700,53 +700,105 @@ pub(super) fn warn_incompatible_formatter_settings(
{
let mut incompatible_rules = Vec::new();

for incompatible_rule in RuleTable::from_iter([
Rule::LineTooLong,
Rule::TabIndentation,
Rule::IndentationWithInvalidMultiple,
Rule::IndentationWithInvalidMultipleComment,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I these think these are redundant when using tabs in the formatter.

Rule::OverIndented,
Rule::IndentWithSpaces,
for rule in [
// The formatter might collapse implicit string concatenation on a single line.
Rule::SingleLineImplicitStringConcatenation,
Copy link
Member Author

@MichaReiser MichaReiser Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the formatter collapses the strings, I would love to know about it to fix it afterward.

// Flags missing trailing commas when all arguments are on its own line:
// ```python
// def args(
// aaaaaaaa, bbbbbbbbb, cccccccccc, ddddddddd, eeeeeeee, ffffff, gggggggggggg, hhhh
// ):
// pass
// ```
Rule::MissingTrailingComma,
Rule::ProhibitedTrailingComma,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was removed because the formatter will never output code that triggers it? Is that right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe this is the case.

Rule::BadQuotesInlineString,
Rule::BadQuotesMultilineString,
Rule::BadQuotesDocstring,
Rule::AvoidableEscapedQuote,
])
.iter_enabled()
{
if setting.linter.rules.enabled(incompatible_rule) {
incompatible_rules.push(format!("'{}'", incompatible_rule.noqa_code()));
] {
if setting.linter.rules.enabled(rule) {
incompatible_rules.push(rule);
}
}

// Rules asserting for space indentation
if setting.formatter.indent_style.is_tab() {
for rule in [Rule::TabIndentation, Rule::IndentWithSpaces] {
if setting.linter.rules.enabled(rule) {
incompatible_rules.push(rule);
}
}
}

// Rules asserting for indent-width=4
if setting.formatter.indent_width.value() != 4 {
for rule in [
Rule::IndentationWithInvalidMultiple,
Rule::IndentationWithInvalidMultipleComment,
] {
if setting.linter.rules.enabled(rule) {
incompatible_rules.push(rule);
}
}
}

if !incompatible_rules.is_empty() {
incompatible_rules.sort();
warn!("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 the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.", incompatible_rules.join(", "));
let mut rule_names: Vec<_> = incompatible_rules
.into_iter()
.map(|rule| format!("`{}`", rule.noqa_code()))
.collect();
rule_names.sort();
warn!("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 the `select` or `extend-select` configuration, or adding then to the `ignore` configuration.", rule_names.join(", "));
}

// Rules with different quote styles.
if setting
.linter
.rules
.any_enabled(&[Rule::BadQuotesInlineString, Rule::AvoidableEscapedQuote])
{
match (
setting.linter.flake8_quotes.inline_quotes,
setting.formatter.quote_style,
) {
(Quote::Double, QuoteStyle::Single) => {
warn!("The `flake8-quotes.inline-quotes=\"double\"` option is incompatible with the formatter's `format.quote-style=\"single\"`. We recommend disabling `Q000` and `Q003` when using the formatter, which enforces a consistent quote style. Alternatively, set both options to either `\"single\"` or `\"double\"`.");
}
(Quote::Single, QuoteStyle::Double) => {
warn!("The `flake8-quotes.inline-quotes=\"single\"` option is incompatible with the formatter's `format.quote-style=\"double\"`. We recommend disabling `Q000` and `Q003` when using the formatter, which enforces a consistent quote style. Alternatively, set both options to either `\"single\"` or `\"double\"`.");
}
_ => {}
}
}

if setting.linter.rules.enabled(Rule::BadQuotesMultilineString)
&& setting.linter.flake8_quotes.multiline_quotes == Quote::Single
{
warn!("The `flake8-quotes.multiline-quotes=\"single\"` option is incompatible with the formatter. We recommend disabling `Q001` when using the formatter, which enforces double quotes for multiline strings. Alternatively, set the `flake8-quotes.multiline-quotes` option to `\"double\"`.`");
}

if setting.linter.rules.enabled(Rule::BadQuotesDocstring)
&& setting.linter.flake8_quotes.docstring_quotes == Quote::Single
{
warn!("The `flake8-quotes.multiline-quotes=\"single\"` option is incompatible with the formatter. We recommend disabling `Q002` when using the formatter, which enforces double quotes for docstrings. Alternatively, set the `flake8-quotes.docstring-quotes` option to `\"double\"`.`");
}

if setting.linter.rules.enabled(Rule::UnsortedImports) {
// The formatter removes empty lines if the value is larger than 2 but always inserts a empty line after imports.
// Two empty lines are okay because `isort` only uses this setting for top-level imports (not in nested blocks).
if !matches!(setting.linter.isort.lines_after_imports, 1 | 2 | -1) {
warn!("The isort option 'isort.lines-after-imports' with a value other than `-1`, `1` or `2` is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `2`, `1`, or `-1` (default).");
warn!("The isort option `isort.lines-after-imports` with a value other than `-1`, `1` or `2` is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `2`, `1`, or `-1` (default).");
}

// Values larger than two get reduced to one line by the formatter if the import is in a nested block.
if setting.linter.isort.lines_between_types > 1 {
warn!("The isort option 'isort.lines-between-types' with a value larger than 1 is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `1` or `0` (default).");
warn!("The isort option `isort.lines-between-types` with a value greater than 1 is incompatible with the formatter. To avoid unexpected behavior, we recommend setting the option to one of: `1` or `0` (default).");
}

// isort inserts a trailing comma which the formatter preserves, but only if `skip-magic-trailing-comma` isn't false.
if setting.formatter.magic_trailing_comma.is_ignore() {
if setting.linter.isort.force_wrap_aliases {
warn!("The isort option 'isort.force-wrap-aliases' is incompatible with the formatter 'format.skip-magic-trailing-comma=true' option. To avoid unexpected behavior, we recommend either setting 'isort.force-wrap-aliases=false' or 'format.skip-magic-trailing-comma=false'.");
warn!("The isort option `isort.force-wrap-aliases` is incompatible with the formatter `format.skip-magic-trailing-comma=true` option. To avoid unexpected behavior, we recommend either setting `isort.force-wrap-aliases=false` or `format.skip-magic-trailing-comma=false`.");
}

if setting.linter.isort.split_on_trailing_comma {
warn!("The isort option 'isort.split-on-trailing-comma' is incompatible with the formatter 'format.skip-magic-trailing-comma=true' option. To avoid unexpected behavior, we recommend either setting 'isort.split-on-trailing-comma=false' or 'format.skip-magic-trailing-comma=false'.");
warn!("The isort option `isort.split-on-trailing-comma` is incompatible with the formatter `format.skip-magic-trailing-comma=true` option. To avoid unexpected behavior, we recommend either setting `isort.split-on-trailing-comma=false` or `format.skip-magic-trailing-comma=false`.");
}
}
}
Expand Down
Loading
Loading