Skip to content

Commit

Permalink
Refine the warnings about incompatible linter options
Browse files Browse the repository at this point in the history
  • Loading branch information
@MichaReiser
MichaReiser committed Oct 25, 2023
1 parent 27257e6 commit 08adb86
Show file tree
Hide file tree
Showing 9 changed files with 197 additions and 34 deletions.
90 changes: 69 additions & 21 deletions crates/ruff_cli/src/commands/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,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 @@ -685,31 +685,79 @@ 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,
Rule::OverIndented,
Rule::IndentWithSpaces,
for rule in [
// The formatter might collapse implicit string concatenation on a single line.
Rule::SingleLineImplicitStringConcatenation,
// 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,
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])
&& matches!(
(
setting.formatter.quote_style,
setting.linter.flake8_quotes.inline_quotes
),
(QuoteStyle::Single, Quote::Double) | (QuoteStyle::Double, Quote::Single)
)
{
warn!("The `flake8-quotes.inline-quotes` option is incompatible with the formatter's `format.quote-style` option. We recommend disabling 'Q000' and 'Q003' when using the formatter because they're redundant. If you do need the rules, change the configuration and set both options either to `\"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 because it enforces double quotes for multiline strings, making this rule redundant. If you do need the rule, change 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.docstring-quotes=\"single\"` option is incompatible with the formatter. We recommend disabling 'Q002' when using the formatter because it enforce double quotes for docstrings, making this rule redundant. If you do need the rule, change the `flake8-quotes.docstring-quotes` option to `\"double\"`.");
}

if setting.linter.rules.enabled(Rule::UnsortedImports) {
Expand Down
91 changes: 81 additions & 10 deletions crates/ruff_cli/tests/format.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ fn format_option_inheritance() -> Result<()> {
&ruff_toml,
r#"
extend = "base.toml"
extend-select = ["Q000"]
[lint]
extend-select = ["COM812"]
[format]
quote-style = "single"
Expand Down Expand Up @@ -273,7 +275,7 @@ if condition:
print('Should change quotes')
----- stderr -----
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.
warning: The following rules may cause conflicts when used with the formatter: 'COM812'. 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 @@ -359,18 +361,27 @@ fn conflicting_options() -> Result<()> {
fs::write(
&ruff_toml,
r#"
indent-width = 2
[lint]
select = ["ALL"]
ignore = ["D203", "D212"]
[isort]
[lint.isort]
lines-after-imports = 3
lines-between-types = 2
force-wrap-aliases = true
combine-as-imports = true
split-on-trailing-comma = true
[lint.flake8-quotes]
inline-quotes = "single"
docstring-quotes = "single"
multiline-quotes = "single"
[format]
skip-magic-trailing-comma = true
indent-style = "tab"
"#,
)?;

Expand All @@ -392,7 +403,10 @@ def say_hy(name: str):
1 file reformatted
----- stderr -----
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 rules may cause conflicts when used with the formatter: 'COM812', 'D206', 'ISC001', '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 `flake8-quotes.inline-quotes` option is incompatible with the formatter's `format.quote-style` option. We recommend disabling 'Q000' and 'Q003' when using the formatter because they're redundant. If you do need the rules, change the configuration and set both options either to `"single"` or `"double"`.
warning: The `flake8-quotes.multiline-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q001' when using the formatter because it enforces double quotes for multiline strings, making this rule redundant. If you do need the rule, change the `flake8-quotes.multiline-quotes` option to `"double"`.`
warning: The `flake8-quotes.docstring-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q002' when using the formatter because it enforce double quotes for docstrings, making this rule redundant. If you do need the rule, change the `flake8-quotes.docstring-quotes` option to `"double"`.
warning: 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).
warning: 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).
warning: 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'.
Expand All @@ -408,18 +422,27 @@ fn conflicting_options_stdin() -> Result<()> {
fs::write(
&ruff_toml,
r#"
indent-width = 2
[lint]
select = ["ALL"]
ignore = ["D203", "D212"]
[isort]
[lint.isort]
lines-after-imports = 3
lines-between-types = 2
force-wrap-aliases = true
combine-as-imports = true
split-on-trailing-comma = true
[lint.flake8-quotes]
inline-quotes = "single"
docstring-quotes = "single"
multiline-quotes = "single"
[format]
skip-magic-trailing-comma = true
indent-style = "tab"
"#,
)?;

Expand All @@ -434,10 +457,13 @@ def say_hy(name: str):
exit_code: 0
----- stdout -----
def say_hy(name: str):
print(f"Hy {name}")
print(f"Hy {name}")
----- stderr -----
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 rules may cause conflicts when used with the formatter: 'COM812', 'D206', 'ISC001', '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 `flake8-quotes.inline-quotes` option is incompatible with the formatter's `format.quote-style` option. We recommend disabling 'Q000' and 'Q003' when using the formatter because they're redundant. If you do need the rules, change the configuration and set both options either to `"single"` or `"double"`.
warning: The `flake8-quotes.multiline-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q001' when using the formatter because it enforces double quotes for multiline strings, making this rule redundant. If you do need the rule, change the `flake8-quotes.multiline-quotes` option to `"double"`.`
warning: The `flake8-quotes.docstring-quotes="single"` option is incompatible with the formatter. We recommend disabling 'Q002' when using the formatter because it enforce double quotes for docstrings, making this rule redundant. If you do need the rule, change the `flake8-quotes.docstring-quotes` option to `"double"`.
warning: 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).
warning: 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).
warning: 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'.
Expand All @@ -453,18 +479,60 @@ fn valid_linter_options() -> Result<()> {
fs::write(
&ruff_toml,
r#"
[lint]
select = ["ALL"]
ignore = ["D203", "D212"]
ignore = ["D203", "D212", "COM812", "ISC001"]
[isort]
[lint.isort]
lines-after-imports = 2
lines-between-types = 1
force-wrap-aliases = true
combine-as-imports = true
split-on-trailing-comma = true
[lint.flake8-quotes]
inline-quotes = "single"
docstring-quotes = "double"
multiline-quotes = "double"
[format]
skip-magic-trailing-comma = false
quote-style = "single"
"#,
)?;

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 -----
"###);
Ok(())
}

#[test]
fn all_rules_default_options() -> Result<()> {
let tempdir = TempDir::new()?;
let ruff_toml = tempdir.path().join("ruff.toml");

fs::write(
&ruff_toml,
r#"
[lint]
select = ["ALL"]
"#,
)?;

Expand All @@ -486,10 +554,13 @@ def say_hy(name: str):
1 file reformatted
----- stderr -----
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: `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `one-blank-line-before-class`.
warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
warning: The following rules may cause conflicts when used with the formatter: 'COM812', 'ISC001'. 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(())
}

#[test]
fn test_diff() {
let args = ["format", "--no-cache", "--isolated", "--diff"];
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/flake8_quotes/rules/avoidable_escaped_quote.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ use crate::settings::LinterSettings;
/// ```python
/// foo = "bar's"
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter automatically removes
/// unnecessary escapes, making this rule redundant.
#[violation]
pub struct AvoidableEscapedQuote;

Expand Down
12 changes: 12 additions & 0 deletions crates/ruff_linter/src/rules/flake8_quotes/rules/check_string_quotes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ use super::super::settings::Quote;
///
/// ## Options
/// - `flake8-quotes.inline-quotes`
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// quotes for inline strings automatically, making this rule redundant.
#[violation]
pub struct BadQuotesInlineString {
preferred_quote: Quote,
Expand Down Expand Up @@ -81,6 +85,10 @@ impl AlwaysFixableViolation for BadQuotesInlineString {
///
/// ## Options
/// - `flake8-quotes.multiline-quotes`
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces double quotes
/// for multiline strings automatically, making this rule redundant.
#[violation]
pub struct BadQuotesMultilineString {
preferred_quote: Quote,
Expand Down Expand Up @@ -129,6 +137,10 @@ impl AlwaysFixableViolation for BadQuotesMultilineString {
///
/// ## Options
/// - `flake8-quotes.docstring-quotes`
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces double
/// quotes for docstrings automatically, making this rule redundant.
#[violation]
pub struct BadQuotesDocstring {
preferred_quote: Quote,
Expand Down
7 changes: 7 additions & 0 deletions crates/ruff_linter/src/rules/pycodestyle/rules/logical_lines/indentation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ use super::LogicalLine;
/// a = 1
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `indent-width` with a value other than `4`.
///
/// [PEP 8]: https://peps.python.org/pep-0008/#indentation
#[violation]
pub struct IndentationWithInvalidMultiple {
Expand Down Expand Up @@ -54,6 +58,9 @@ impl Violation for IndentationWithInvalidMultiple {
/// if True:
/// # a = 1
/// ```
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `indent-width` with a value other than `4`.
///
/// [PEP 8]: https://peps.python.org/pep-0008/#indentation
#[violation]
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pycodestyle/rules/tab_indentation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ use ruff_text_size::{TextLen, TextRange, TextSize};
/// a = 1
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `format.indent-style="tab"`.
///
/// [PEP 8]: https://peps.python.org/pep-0008/#tabs-or-spaces
#[violation]
pub struct TabIndentation;
Expand Down
8 changes: 8 additions & 0 deletions crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ use crate::registry::Rule;
/// """
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant. The rule is incompatible with the formatter when using `format.indent-style="tab"`.
///
/// ## References
/// - [PEP 257 – Docstring Conventions](https://peps.python.org/pep-0257/)
/// - [NumPy Style Guide](https://numpydoc.readthedocs.io/en/latest/format.html)
Expand Down Expand Up @@ -126,6 +130,10 @@ impl AlwaysFixableViolation for UnderIndentation {
/// """
/// ```
///
/// ## Using with the formatter
/// We don't recommend using this rule when using the formatter because the formatter enforces consistent
/// indentations, making this rule redundant.
///
/// ## References
/// - [PEP 257 – Docstring Conventions](https://peps.python.org/pep-0257/)
/// - [NumPy Style Guide](https://numpydoc.readthedocs.io/en/latest/format.html)
Expand Down
Loading

0 comments on commit 08adb86

Please sign in to comment.