From eebda685134d9fad770123bb709312596ab0e520 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Sun, 16 Aug 2026 17:53:55 +0000 Subject: [PATCH 1/2] fix(derive): name the mistake when `settings` has nothing to collect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `#[usage(settings)]` says this CLI resolves settings whose flags are declared elsewhere. With no elsewhere — nothing binding a setting, no flattened group, no subcommand — the layer was still emitted while the function it calls was not, so an adopter's build failed with error[E0425]: cannot find function `settings_given` in this scope --> src/main.rs:4:10 | 4 | #[derive(Cli)] an unresolved name inside generated code, pointing at the derive, naming neither the attribute that caused it nor what to do. Refused where it is written instead, beside the check for the same attribute in the wrong *place*. Any of the three is enough, since each is a way for a flag to be somewhere this struct does not declare it. Found by CodeRabbit while reviewing a different stack. Co-Authored-By: Claude Opus 5 --- derive/src/model.rs | 61 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/derive/src/model.rs b/derive/src/model.rs index 58ef78a91..e7300fbcc 100644 --- a/derive/src/model.rs +++ b/derive/src/model.rs @@ -390,6 +390,27 @@ impl Cli { return Ok(()); } + // `settings` says "this CLI resolves settings whose flags are declared elsewhere", so + // there has to be an elsewhere: a field that binds one, a flattened group, or a + // subcommand. With none of the three the attribute describes nothing, and the generated + // layer called a `settings_given` that was never emitted — so an adopter's build failed + // with `cannot find function settings_given` pointing at `#[derive(Cli)]`, which names + // neither the attribute nor the mistake. + if self.settings + && !self.fields.iter().any(|f| { + f.setting.is_some() + || matches!(f.kind, Kind::Flatten { .. } | Kind::Subcommand { .. }) + }) + { + return Err(syn::Error::new_spanned( + ident, + "`settings` says this CLI resolves settings whose flags are declared \ + elsewhere, and there is no elsewhere: nothing here binds a setting, and \ + there is no flattened group or subcommand that could. Add `setting = \"…\"` \ + to the flag that sets one, or drop the attribute", + )); + } + // `mount` and `restart_token` are written on a `cmd` node, and the root is not one. // Verified against usage-lib, which rejects a spec that puts either at the top. for (present, what) in [ @@ -2309,6 +2330,46 @@ mod tests { .to_string() } + #[test] + fn settings_needs_something_to_collect() { + // The attribute says the flags are declared *elsewhere*, so there has to be an + // elsewhere. With none, the generated layer called a `settings_given` that nothing + // emitted, and an adopter's build failed with `cannot find function settings_given` + // pointing at `#[derive(Cli)]` — naming neither the attribute nor the mistake. + let input = syn::parse_str::( + r#" + #[usage(settings)] + struct Ex { + #[usage(long)] + plain: bool, + } + "#, + ) + .expect("valid Rust"); + let cli = Cli::from_input(&input).expect("parses"); + let err = cli + .check_position(&input.ident, true) + .expect_err("should not have compiled") + .to_string(); + assert!(err.contains("no elsewhere"), "unhelpful message: {err}"); + + // Any of the three is enough, and each is a different way for a flag to be somewhere + // this struct does not declare it. + for body in [ + r#"struct Ex { #[usage(long, setting = "jobs")] jobs: Option }"#, + r#"struct Ex { #[usage(flatten)] group: Group }"#, + r#"struct Ex { #[usage(subcommand)] command: Option }"#, + ] { + let body = format!("#[usage(settings)]\n{body}"); + let input = syn::parse_str::(&body).expect("valid Rust"); + let cli = Cli::from_input(&input).expect("parses"); + assert!( + cli.check_position(&input.ident, true).is_ok(), + "should have been accepted: {body}" + ); + } + } + #[test] fn the_settings_attribute_belongs_on_the_root() { // It says "this CLI resolves settings whose flags are declared elsewhere", which only a From dde9e1d6ceb16665530ac16b3f3ea2e2760f9371 Mon Sep 17 00:00:00 2001 From: default <216188+jdx@users.noreply.github.com> Date: Sun, 16 Aug 2026 18:45:33 +0000 Subject: [PATCH 2/2] fix(derive): underline the attribute that is in the wrong place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every rule in `check_position` is about an attribute written somewhere it cannot mean what it says, and every one of them spanned the *struct's name* — so rustc underlined `struct Ex` while the mistake was on the line above it: error: `settings` says this CLI resolves settings whose flags are declared elsewhere… --> src/main.rs:5:3 | 5 | #[usage(bin = "ex", settings)] | ^^^^^ Done for all six rather than for the one that was reported. Spanning the attribute is the right answer for `completion`, `mount`, `restart_token` and `default_subcommand` for exactly the same reason, and fixing one would have made it the odd one out. No unit test pins the span: `Span::start()` needs `proc-macro2/span-locations`, which is off, and without it every span in a `syn::parse_str` fixture is call-site — so a test could not tell the two apart even while rustc can. Checked against a real compile for `settings` and for `mount`, which is where it is observable. Found by CodeRabbit. Co-Authored-By: Claude Opus 5 --- derive/src/model.rs | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/derive/src/model.rs b/derive/src/model.rs index e7300fbcc..8cca990d2 100644 --- a/derive/src/model.rs +++ b/derive/src/model.rs @@ -35,6 +35,12 @@ pub struct Cli { /// CLI with a subcommand depend on `usage-config`. A root that binds a setting of its own has /// already said it, and does not need this. pub settings: bool, + /// Where `#[usage(...)]` was written on the struct, when it was. + /// + /// Every position rule in [`Cli::check_position`] is about an attribute in the wrong place, + /// so every one of them should point at the attribute. Spanning the struct's *name* put the + /// underline one line below the thing that was wrong. + pub attr_span: Option, /// From the struct's doc comment: first paragraph, and the whole thing. pub about: Option, pub long_about: Option, @@ -245,6 +251,11 @@ impl Cli { bin: None, completion: false, settings: false, + attr_span: input + .attrs + .iter() + .find(|a| a.path().is_ident("usage")) + .map(|a| a.path().span()), version: None, about, long_about, @@ -341,6 +352,18 @@ impl Cli { /// /// A negation counts, since `--no-color` is another way to name the field `--color` /// declared — the two share one place to record whether they were given. + /// A position error, pointed at the attribute rather than at the struct's name. + /// + /// Every rule in [`check_position`](Self::check_position) is about an attribute written in + /// the wrong place, so the underline belongs on the attribute. Falls back to the name for a + /// struct that has none, which cannot reach these rules but keeps the helper total. + fn misplaced(&self, ident: &syn::Ident, message: impl std::fmt::Display) -> syn::Error { + match self.attr_span { + Some(span) => syn::Error::new(span, message), + None => syn::Error::new_spanned(ident, message), + } + } + /// Check the command-level properties against where this struct sits in the tree. /// /// Both derives share this parse, so these rules cannot live inside it: the same @@ -361,7 +384,7 @@ impl Cli { // whole CLI is. Accepted silently on an `Args`, it generated nothing and said // nothing, which reads as a CLI that has completions and does not. if self.completion { - return Err(syn::Error::new_spanned( + return Err(self.misplaced( ident, "`completion` belongs on the root, where `#[derive(Cli)]` is: the hidden \ command it adds answers for the whole program, not for one of its commands", @@ -372,7 +395,7 @@ impl Cli { // attribute has nothing left to say here, and saying it would read as the group // having asked for something. if self.settings { - return Err(syn::Error::new_spanned( + return Err(self.misplaced( ident, "`settings` belongs on the root, where `#[derive(Cli)]` is: it says that \ this CLI resolves settings whose flags are declared elsewhere, and a group \ @@ -381,7 +404,7 @@ impl Cli { } // A spec declares one `default_subcommand`, at the top. if self.default_subcommand.is_some() { - return Err(syn::Error::new_spanned( + return Err(self.misplaced( ident, "`default_subcommand` belongs on the root, where `#[derive(Cli)]` is: a \ spec declares one for the whole program, not one per command", @@ -402,7 +425,7 @@ impl Cli { || matches!(f.kind, Kind::Flatten { .. } | Kind::Subcommand { .. }) }) { - return Err(syn::Error::new_spanned( + return Err(self.misplaced( ident, "`settings` says this CLI resolves settings whose flags are declared \ elsewhere, and there is no elsewhere: nothing here binds a setting, and \ @@ -418,7 +441,7 @@ impl Cli { (self.restart_token.is_some(), "restart_token"), ] { if present { - return Err(syn::Error::new_spanned( + return Err(self.misplaced( ident, format!( "`{what}` belongs on a command, not on the root: the spec accepts it \ @@ -435,7 +458,7 @@ impl Cli { .iter() .any(|f| matches!(f.kind, Kind::Subcommand { .. })) { - return Err(syn::Error::new_spanned( + return Err(self.misplaced( ident, "`default_subcommand` names the command a bare invocation means, and this \ one has no subcommands to name",