Better error message when `--config` is given a table key and a non-inline-table value by InSyncWithFoo · Pull Request #15266 · astral-sh/ruff

InSyncWithFoo · 2025-01-05T08:57:29Z

Summary

Resolves #13995.

Test Plan

cargo nextest run and cargo insta test.

…nline-table value

github-actions · 2025-01-05T09:03:27Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

AlexWaygood · 2025-01-05T12:49:59Z

This is great! I think we can do even better with a few tweaks, though. With your branch currently, this is the error message I get:

~/dev/ruff (cli-config)⚡ % cargo run -- check . --config='lint.flake8-pytest-style="csv"'
   Compiling ruff v0.8.6 (/Users/alexw/dev/ruff/crates/ruff)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.51s
     Running `target/debug/ruff check . '--config=lint.flake8-pytest-style="csv"'`
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.
Did you mean to use one of its subkeys instead? 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

Could not parse the supplied argument as a `ruff.toml` configuration option:

invalid type: string "csv", expected struct Flake8PytestStyleOptions
in `lint`

For more information, try '--help'.

Colorized screenshot

But if I apply this diff to your branch:

--- a/crates/ruff/src/args.rs
+++ b/crates/ruff/src/args.rs
@@ -978,23 +978,25 @@ The path `{value}` does not point to a configuration file"
                     let prefixed_subkeys = format!("{set}")
                         .trim_ascii()
                         .split('\n')
-                        .map(|child| format!("{key}.{child}"))
+                        .map(|child| format!(" - `{key}.{child}`"))
                         .join("\n");
 
                     tip.push_str(&format!(
                         "
 
-`{key}` is a table.
-Did you mean to use one of its subkeys instead? Possible choices:
+`{key}` is a table of configuration options, but only a single option can be
+overridden with `--config`. Did you want to override one of the table's subkeys?
+
+Possible choices:
 {prefixed_subkeys}"
                     ));
                 }
-            };
-
-            tip.push_str(&format!(
-                "\n\n{}:\n\n{underlying_error}",
-                config_parse_error.description()
-            ));
+            } else {
+                tip.push_str(&format!(
+                    "\n\n{}:\n\n{underlying_error}",
+                    config_parse_error.description()
+                ));
+            }
         }

Then it becomes:

~/dev/ruff (cli-config)⚡ [2] % cargo run -- check . --config='lint.flake8-pytest-style="csv"'
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.36s
     Running `target/debug/ruff check . '--config=lint.flake8-pytest-style="csv"'`
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, but only a single option can be
overridden with `--config`. 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'.

Colorized screenshot

which feels significantly more readable to me. WDYT?

InSyncWithFoo · 2025-01-05T13:40:19Z

@AlexWaygood I was actually aiming for copyability, but readability is fine too.

[...] but only a single option can be overridden with --config.

This is rather misleading, because you can pass an inline table as the value:

# Same as "lint.select = ['I001']"
$ ruff check --isolated --config="lint = { select = ['I001'] }"

AlexWaygood · 2025-01-05T13:43:03Z

This is rather misleading, because you can pass an inline table as the value:

Oh, great point! I forgot how my own feature worked :-) We can scrap that clause, then.

AlexWaygood

LGTM, thank you!

crates/ruff/src/args.rs

MichaReiser

This looks great. I'm a bit worried about the output parsing and I think we can improve the handling if set itself has sub-table entries.

crates/ruff/tests/lint.rs

* main: (60 commits) [`ruff`] Dataclass enums (`RUF049`) (#15299) Better error message when `--config` is given a table key and a non-inline-table value (#15266) Update pre-commit dependencies (#15289) Don't fix in ecosystem check (#15267) Update Rust crate itertools to 0.14.0 (#15287) Remove accidental empty block at the bottom of `split-static-string (SIM905)` doc (#15290) Update Rust crate clearscreen to v4 (#15288) Update Rust crate insta to v1.42.0 (#15286) Update NPM Development dependencies (#15285) Update dependency uuid to v11.0.4 (#15284) Update dependency ruff to v0.8.6 (#15283) Update Rust crate syn to v2.0.95 (#15282) Update Rust crate matchit to v0.8.6 (#15281) Update Rust crate bstr to v1.11.3 (#15280) [red-knot] Future-proof `Type::is_disjoint_from()` (#15262) [red-knot] Improve `Type::is_disjoint_from()` for `KnownInstanceType`s (#15261) [red-knot] Minor simplifications and improvements to constraint narrowing logic (#15270) Allow assigning ellipsis literal as parameter default value (#14982) [red-knot] fix control flow for assignment expressions in elif tests (#15274) [`refurb`] Mark fix as unsafe when the right-hand side is a string (`FURB171`) (#15273) ...

* main: (29 commits) [`ruff`] Dataclass enums (`RUF049`) (#15299) Better error message when `--config` is given a table key and a non-inline-table value (#15266) Update pre-commit dependencies (#15289) Don't fix in ecosystem check (#15267) Update Rust crate itertools to 0.14.0 (#15287) Remove accidental empty block at the bottom of `split-static-string (SIM905)` doc (#15290) Update Rust crate clearscreen to v4 (#15288) Update Rust crate insta to v1.42.0 (#15286) Update NPM Development dependencies (#15285) Update dependency uuid to v11.0.4 (#15284) Update dependency ruff to v0.8.6 (#15283) Update Rust crate syn to v2.0.95 (#15282) Update Rust crate matchit to v0.8.6 (#15281) Update Rust crate bstr to v1.11.3 (#15280) [red-knot] Future-proof `Type::is_disjoint_from()` (#15262) [red-knot] Improve `Type::is_disjoint_from()` for `KnownInstanceType`s (#15261) [red-knot] Minor simplifications and improvements to constraint narrowing logic (#15270) Allow assigning ellipsis literal as parameter default value (#14982) [red-knot] fix control flow for assignment expressions in elif tests (#15274) [`refurb`] Mark fix as unsafe when the right-hand side is a string (`FURB171`) (#15273) ...

Better error message when --config is given a table key and a non-i…

9ab8641

…nline-table value

AlexWaygood self-requested a review January 5, 2025 11:15

Per review

b524d39

AlexWaygood approved these changes Jan 5, 2025

View reviewed changes

AlexWaygood requested a review from MichaReiser January 5, 2025 14:03

InSyncWithFoo added 2 commits January 5, 2025 14:08

Fix

f918d3a

Fix

d73bb45

MichaReiser added the cli Related to the command-line interface label Jan 6, 2025

MichaReiser reviewed Jan 6, 2025

View reviewed changes

crates/ruff/src/args.rs Outdated Show resolved Hide resolved

MichaReiser reviewed Jan 6, 2025

View reviewed changes

Per review

e7d3a68

InSyncWithFoo force-pushed the cli-config branch from d6f4210 to e7d3a68 Compare January 6, 2025 12:54

Use collect_fields, limit visiblity

f16eff8

MichaReiser approved these changes Jan 6, 2025

View reviewed changes

MichaReiser enabled auto-merge (squash) January 6, 2025 13:11

AlexWaygood reviewed Jan 6, 2025

View reviewed changes

crates/ruff/tests/lint.rs Outdated Show resolved Hide resolved

crates/ruff/tests/lint.rs Outdated Show resolved Hide resolved

AlexWaygood disabled auto-merge January 6, 2025 13:14

remove nonsensical comments

0fdb4f9

AlexWaygood enabled auto-merge (squash) January 6, 2025 13:15

AlexWaygood merged commit 832c0fa into astral-sh:main Jan 6, 2025
20 checks passed

InSyncWithFoo deleted the cli-config branch January 6, 2025 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

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

Better error message when `--config` is given a table key and a non-inline-table value#15266
AlexWaygood merged 7 commits intoastral-sh:mainfrom
InSyncWithFoo:cli-config

InSyncWithFoo commented Jan 5, 2025

Uh oh!

github-actions bot commented Jan 5, 2025 •

edited

Loading

Uh oh!

AlexWaygood commented Jan 5, 2025 •

edited

Loading

Uh oh!

InSyncWithFoo commented Jan 5, 2025 •

edited

Loading

Uh oh!

AlexWaygood commented Jan 5, 2025

Uh oh!

AlexWaygood left a comment

Uh oh!

Uh oh!

MichaReiser left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Comments

Conversation

InSyncWithFoo commented Jan 5, 2025

Summary

Test Plan

Uh oh!

github-actions bot commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

AlexWaygood commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

InSyncWithFoo commented Jan 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AlexWaygood commented Jan 5, 2025

Uh oh!

AlexWaygood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jan 5, 2025 •

edited

Loading

`ruff-ecosystem` results

AlexWaygood commented Jan 5, 2025 •

edited

Loading

InSyncWithFoo commented Jan 5, 2025 •

edited

Loading