fix(derive): name the mistake when settings has nothing to collect

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<proc_macro2::Span>,
     /// From the struct's doc comment: first paragraph, and the whole thing.
     pub about: Option<String>,
     pub long_about: Option<String>,
@@ -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",
@@ -390,14 +413,35 @@ 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(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 \
+                 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 [
             (self.mount.is_some(), "mount"),
             (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 \
@@ -414,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",
@@ -2309,6 +2353,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::<syn::DeriveInput>(
+            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<String> }"#,
+            r#"struct Ex { #[usage(flatten)] group: Group }"#,
+            r#"struct Ex { #[usage(subcommand)] command: Option<Commands> }"#,
+        ] {
+            let body = format!("#[usage(settings)]\n{body}");
+            let input = syn::parse_str::<syn::DeriveInput>(&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