From 08519e22e4982f8b8b1300a27c47e89ae7eee100 Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Mon, 23 Oct 2023 19:04:20 +0900 Subject: [PATCH] Warn about incompatible formatter options (#8088) --- crates/ruff_cli/src/commands/format.rs | 77 +++++++++++++++++- crates/ruff_cli/src/commands/format_stdin.rs | 6 +- crates/ruff_cli/tests/format.rs | 86 ++++++++++++++++++++ 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs index e9ff23ee4f45b..693137f076a39 100644 --- a/crates/ruff_cli/src/commands/format.rs +++ b/crates/ruff_cli/src/commands/format.rs @@ -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; @@ -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}; @@ -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 @@ -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(", ")); + } + } +} diff --git a/crates/ruff_cli/src/commands/format_stdin.rs b/crates/ruff_cli/src/commands/format_stdin.rs index c483a4a8e96ed..0e55551451359 100644 --- a/crates/ruff_cli/src/commands/format_stdin.rs +++ b/crates/ruff_cli/src/commands/format_stdin.rs @@ -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; @@ -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() { diff --git a/crates/ruff_cli/tests/format.rs b/crates/ruff_cli/tests/format.rs index 6ec13b3435681..e9a1c9109169f 100644 --- a/crates/ruff_cli/tests/format.rs +++ b/crates/ruff_cli/tests/format.rs @@ -200,6 +200,7 @@ fn exclude_stdin() -> Result<()> { &ruff_toml, r#" extend-select = ["B", "Q"] +ignore = ["Q000", "Q001", "Q002", "Q003"] [format] exclude = ["generated.py"] @@ -234,6 +235,7 @@ fn format_option_inheritance() -> Result<()> { &ruff_toml, r#" extend = "base.toml" +extend-select = ["Q000"] [format] quote-style = "single" @@ -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(()) } @@ -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"];