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

Co-authored-by: Micha Reiser <[email protected]>
Co-authored-by: Alex Waygood <[email protected]>

Author: 3 people

crates/ruff/src/args.rs

@@ -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;
@@ -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;
@@ -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();
 

crates/ruff/tests/lint.rs

@@ -780,6 +780,90 @@ 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)
+        .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)
+        .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()?;

crates/ruff_workspace/src/options_base.rs

@@ -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.