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

Better error message when --config is given a table key and a non-inline-table value #15266

Merged
merged 7 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 6 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
37 changes: 32 additions & 5 deletions crates/ruff/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use anyhow::bail;
use clap::builder::{TypedValueParser, ValueParserFactory};
use clap::{command, Parser, Subcommand};
use colored::Colorize;
use itertools::Itertools;
use path_absolutize::path_dedot;
use regex::Regex;
use ruff_graph::Direction;
Expand All @@ -24,6 +25,7 @@ use ruff_source_file::{LineIndex, OneIndexed};
use ruff_text_size::TextRange;
use ruff_workspace::configuration::{Configuration, RuleSelection};
use ruff_workspace::options::{Options, PycodestyleOptions};
use ruff_workspace::options_base::{OptionEntry, OptionsMetadata};
use ruff_workspace::resolver::ConfigurationTransformer;
use rustc_hash::FxHashMap;
use toml;
Expand Down Expand Up @@ -967,11 +969,36 @@ It looks like you were trying to pass a path to a configuration file.
The path `{value}` does not point to a configuration file"
));
}
} else if value.contains('=') {
tip.push_str(&format!(
"\n\n{}:\n\n{underlying_error}",
config_parse_error.description()
));
} else if let Some((key, value)) = value.split_once('=') {
let key = key.trim_ascii();
let value = value.trim_ascii_start();

match Options::metadata().find(key) {
Some(OptionEntry::Set(set)) if !value.starts_with('{') => {
let prefixed_subfields = set
.collect_fields()
.iter()
.map(|(name, _)| format!("- `{key}.{name}`"))
.join("\n");

tip.push_str(&format!(
"

`{key}` is a table of configuration options.
Did you want to override one of the table's subkeys?

Possible choices:

{prefixed_subfields}"
));
}
_ => {
tip.push_str(&format!(
"\n\n{}:\n\n{underlying_error}",
config_parse_error.description()
));
}
}
}
let tip = tip.trim_end().to_owned().into();

Expand Down
88 changes: 88 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,94 @@ fn each_toml_option_requires_a_new_flag_2() {
");
}

#[test]
fn value_given_to_table_key_is_not_inline_table_1() {
// https://github.com/astral-sh/ruff/issues/13995
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
// spaces *also* can't be used to delimit different config overrides;
// you need a new --config flag for each override
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
.args([".", "--config", r#"lint.flake8-pytest-style="csv""#]),
@r#"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: invalid value 'lint.flake8-pytest-style="csv"' for '--config <CONFIG_OPTION>'

tip: A `--config` flag must either be a path to a `.toml` configuration file
or a TOML `<KEY> = <VALUE>` pair overriding a specific configuration
option

`lint.flake8-pytest-style` is a table of configuration options.
Did you want to override one of the table's subkeys?

Possible choices:

- `lint.flake8-pytest-style.fixture-parentheses`
- `lint.flake8-pytest-style.parametrize-names-type`
- `lint.flake8-pytest-style.parametrize-values-type`
- `lint.flake8-pytest-style.parametrize-values-row-type`
- `lint.flake8-pytest-style.raises-require-match-for`
- `lint.flake8-pytest-style.raises-extend-require-match-for`
- `lint.flake8-pytest-style.mark-parentheses`

For more information, try '--help'.
"#);
}

#[test]
fn value_given_to_table_key_is_not_inline_table_2() {
// https://github.com/astral-sh/ruff/issues/13995
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
// spaces *also* can't be used to delimit different config overrides;
// you need a new --config flag for each override
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
.args([".", "--config", r#"lint=123"#]),
@r"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: invalid value 'lint=123' for '--config <CONFIG_OPTION>'

tip: A `--config` flag must either be a path to a `.toml` configuration file
or a TOML `<KEY> = <VALUE>` pair overriding a specific configuration
option

`lint` is a table of configuration options.
Did you want to override one of the table's subkeys?

Possible choices:

- `lint.allowed-confusables`
- `lint.dummy-variable-rgx`
- `lint.extend-ignore`
- `lint.extend-select`
- `lint.extend-fixable`
- `lint.external`
- `lint.fixable`
- `lint.ignore`
- `lint.extend-safe-fixes`
- `lint.extend-unsafe-fixes`
- `lint.ignore-init-module-imports`
- `lint.logger-objects`
- `lint.select`
- `lint.explicit-preview-rules`
- `lint.task-tags`
- `lint.typing-modules`
- `lint.unfixable`
- `lint.per-file-ignores`
- `lint.extend-per-file-ignores`
- `lint.exclude`
- `lint.preview`

For more information, try '--help'.
");
}

#[test]
fn config_doubly_overridden_via_cli() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down
16 changes: 16 additions & 0 deletions crates/ruff_workspace/src/options_base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,22 @@ impl OptionSet {
None
}
}

pub fn collect_fields(&self) -> Vec<(String, OptionField)> {
struct FieldsCollector(Vec<(String, OptionField)>);

impl Visit for FieldsCollector {
fn record_field(&mut self, name: &str, field: OptionField) {
self.0.push((name.to_string(), field));
}

fn record_set(&mut self, _name: &str, _group: OptionSet) {}
}

let mut visitor = FieldsCollector(vec![]);
self.record(&mut visitor);
visitor.0
}
}

/// Visitor that writes out the names of all fields and sets.
Expand Down
Loading