Skip to content

Commit

Permalink
Warn about incompatible formatter options (#8088)
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaReiser authored Oct 23, 2023
1 parent c704674 commit 08519e2
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 3 deletions.
77 changes: 75 additions & 2 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::time::Instant;
use anyhow::Result;
use colored::Colorize;
use itertools::Itertools;
use log::error;
use log::{error, warn};
use rayon::iter::Either::{Left, Right};
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
use similar::TextDiff;
Expand All @@ -18,12 +18,17 @@ use tracing::debug;
use ruff_diagnostics::SourceMap;
use ruff_linter::fs;
use ruff_linter::logging::LogLevel;
use ruff_linter::registry::Rule;
use ruff_linter::rules::isort;
use ruff_linter::settings::rule_table::RuleTable;
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_text_size::{TextLen, TextRange, TextSize};
use ruff_workspace::resolver::{match_exclusion, python_files_in_path, ResolvedFile};
use ruff_workspace::resolver::{
match_exclusion, python_files_in_path, PyprojectConfig, ResolvedFile, Resolver,
};
use ruff_workspace::FormatterSettings;

use crate::args::{CliOverrides, FormatArguments};
Expand Down Expand Up @@ -74,6 +79,8 @@ pub(crate) fn format(
return Ok(ExitStatus::Success);
}

warn_incompatible_formatter_settings(&pyproject_config, Some(&resolver));

// Discover the package root for each Python file.
let package_roots = resolver.package_roots(
&paths
Expand Down Expand Up @@ -638,3 +645,69 @@ impl Display for FormatCommandError {
}
}
}

pub(super) fn warn_incompatible_formatter_settings(
pyproject_config: &PyprojectConfig,
resolver: Option<&Resolver>,
) {
for setting in std::iter::once(&pyproject_config.settings)
.chain(resolver.iter().flat_map(|resolver| resolver.settings()))
{
let mut incompatible_rules = Vec::new();

for incompatible_rule in RuleTable::from_iter([
Rule::LineTooLong,
Rule::TabIndentation,
Rule::IndentationWithInvalidMultiple,
Rule::IndentationWithInvalidMultipleComment,
Rule::OverIndented,
Rule::IndentWithSpaces,
Rule::SingleLineImplicitStringConcatenation,
Rule::MissingTrailingComma,
Rule::ProhibitedTrailingComma,
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 !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 incompatible_options = Vec::new();

let isort_defaults = isort::settings::Settings::default();

if setting.linter.isort.force_single_line != isort_defaults.force_single_line {
incompatible_options.push("'isort.force-single-line'");
}

if setting.linter.isort.force_wrap_aliases != isort_defaults.force_wrap_aliases {
incompatible_options.push("'isort.force-wrap-aliases'");
}

if setting.linter.isort.lines_after_imports != isort_defaults.lines_after_imports {
incompatible_options.push("'isort.lines-after-imports'");
}

if setting.linter.isort.lines_between_types != isort_defaults.lines_between_types {
incompatible_options.push("'isort.lines_between_types'");
}

if setting.linter.isort.split_on_trailing_comma != isort_defaults.split_on_trailing_comma {
incompatible_options.push("'isort.split_on_trailing_comma'");
}

if !incompatible_options.is_empty() {
warn!("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 the configuration.", incompatible_options.join(", "));
}
}
}
6 changes: 5 additions & 1 deletion crates/ruff_cli/src/commands/format_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ use ruff_workspace::FormatterSettings;

use crate::args::{CliOverrides, FormatArguments};
use crate::commands::format::{
format_source, FormatCommandError, FormatMode, FormatResult, FormattedSource,
format_source, warn_incompatible_formatter_settings, FormatCommandError, FormatMode,
FormatResult, FormattedSource,
};
use crate::resolve::resolve;
use crate::stdin::read_from_stdin;
Expand All @@ -27,6 +28,9 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R
overrides,
cli.stdin_filename.as_deref(),
)?;

warn_incompatible_formatter_settings(&pyproject_config, None);

let mode = FormatMode::from_cli(cli);

if let Some(filename) = cli.stdin_filename.as_deref() {
Expand Down
86 changes: 86 additions & 0 deletions crates/ruff_cli/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ fn exclude_stdin() -> Result<()> {
&ruff_toml,
r#"
extend-select = ["B", "Q"]
ignore = ["Q000", "Q001", "Q002", "Q003"]
[format]
exclude = ["generated.py"]
Expand Down Expand Up @@ -234,6 +235,7 @@ fn format_option_inheritance() -> Result<()> {
&ruff_toml,
r#"
extend = "base.toml"
extend-select = ["Q000"]
[format]
quote-style = "single"
Expand Down Expand Up @@ -277,6 +279,7 @@ if condition:
----- stderr -----
warning: `ruff format` is not yet stable, and subject to change in future versions.
warning: The following rules may cause conflicts when used with the formatter: 'Q000'. 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.
"###);
Ok(())
}
Expand Down Expand Up @@ -320,6 +323,89 @@ format = "json"
Ok(())
}

#[test]
fn conflicting_options() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
select = ["ALL"]
ignore = ["D203", "D212"]
[isort]
force-single-line = true
force-wrap-aliases = true
lines-after-imports = 0
lines-between-types = 2
split-on-trailing-comma = true
"#,
)?;

let test_path = tempdir.path().join("test.py");
fs::write(
&test_path,
r#"
def say_hy(name: str):
print(f"Hy {name}")"#,
)?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["format", "--no-cache", "--config"])
.arg(&ruff_toml)
.arg(test_path), @r###"
success: true
exit_code: 0
----- stdout -----
1 file reformatted
----- stderr -----
warning: `ruff format` is not yet stable, and subject to change in future versions.
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'COM819', 'D206', 'E501', 'ISC001', 'Q000', 'Q001', 'Q002', 'Q003', 'W191'. 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.
warning: The following isort options may cause conflicts when used with the formatter: 'isort.force-single-line', 'isort.force-wrap-aliases', 'isort.lines-after-imports', 'isort.lines_between_types'. To avoid unexpected behavior, we recommend disabling these options by removing them from the configuration.
"###);
Ok(())
}

#[test]
fn conflicting_options_stdin() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
select = ["ALL"]
ignore = ["D203", "D212"]
[isort]
force-single-line = true
force-wrap-aliases = true
lines-after-imports = 0
lines-between-types = 2
split-on-trailing-comma = true
"#,
)?;

assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["format", "--config"])
.arg(&ruff_toml)
.arg("-")
.pass_stdin(r#"
def say_hy(name: str):
print(f"Hy {name}")"#), @r###"
success: true
exit_code: 0
----- stdout -----
def say_hy(name: str):
print(f"Hy {name}")
----- stderr -----
warning: `ruff format` is not yet stable, and subject to change in future versions.
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'COM819', 'D206', 'E501', 'ISC001', 'Q000', 'Q001', 'Q002', 'Q003', 'W191'. 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.
warning: The following isort options may cause conflicts when used with the formatter: 'isort.force-single-line', 'isort.force-wrap-aliases', 'isort.lines-after-imports', 'isort.lines_between_types'. To avoid unexpected behavior, we recommend disabling these options by removing them from the configuration.
"###);
Ok(())
}
#[test]
fn test_diff() {
let args = ["format", "--no-cache", "--isolated", "--diff"];
Expand Down

0 comments on commit 08519e2

Please sign in to comment.