fix(parse): enforce double_dash="required" for positional args

cli/src/cli/complete_word.rs

@@ -12,8 +12,7 @@ use std::sync::LazyLock;
 use xx::process::check_status;
 use xx::{regex, XXError, XXResult};
 
-use usage::parse::{ParseOutput, ParseValue};
-use usage::{Spec, SpecArg, SpecCommand, SpecComplete, SpecFlag};
+use usage::{Spec, SpecArg, SpecCommand, SpecComplete, SpecDoubleDashChoices, SpecFlag};
 
 use crate::cli::generate;
 
@@ -120,22 +119,33 @@ impl CompleteWord {
         // Not `available_flags`: inside a mounted command, the mounting CLI's flags stay
         // recognized for parsing but are not accepted there, so they must not be offered.
         let flags = parsed.completion_flags();
-        let mut choices = if ctoken == "-" {
+        // An explicit `--` stops the parser reading flags, so past one there is no such thing
+        // as a flag to complete — a dash-prefixed word is a positional value.
+        let flags_possible = !parsed.double_dash_seen;
+        let mut choices = if flags_possible && ctoken == "-" {
             let shorts = self.complete_short_flag_names(&flags, "");
             let longs = self.complete_long_flag_names(&flags, "");
             shorts.into_iter().chain(longs).collect::<Vec<_>>()
-        } else if ctoken.starts_with("--") {
+        } else if flags_possible && ctoken.starts_with("--") {
             self.complete_long_flag_names(&flags, &ctoken)
-        } else if ctoken.starts_with('-') {
+        } else if flags_possible && ctoken.starts_with('-') {
             self.complete_short_flag_names(&flags, &ctoken)
         } else if after_restart_token {
             // After a restart_token, complete from the first arg of the current command
             // This must be checked after flag checks (to allow --flag after :::)
             // but before flag_awaiting_value (since restart clears pending flag values)
             let mut choices = vec![];
             if let Some(arg) = parsed.cmd.args.first() {
-                has_explicit_choices = arg.choices.is_some();
-                choices.extend(self.complete_arg(&ctx, spec, &parsed.cmd, arg, &ctoken)?);
+                let (found, constrained) = self.complete_positional(
+                    &ctx,
+                    spec,
+                    &parsed.cmd,
+                    arg,
+                    &ctoken,
+                    parsed.double_dash_seen,
+                )?;
+                has_explicit_choices = constrained;
+                choices.extend(found);
             }
             choices
         } else if let Some(flag) = parsed.flag_awaiting_value.first() {
@@ -144,9 +154,17 @@ impl CompleteWord {
             self.complete_arg(&ctx, spec, &parsed.cmd, arg, &ctoken)?
         } else {
             let mut choices = vec![];
-            if let Some(arg) = next_arg_for_completion(&parsed) {
-                has_explicit_choices = arg.choices.is_some();
-                choices.extend(self.complete_arg(&ctx, spec, &parsed.cmd, arg, &ctoken)?);
+            if let Some(arg) = parsed.next_arg.as_deref() {
+                let (found, constrained) = self.complete_positional(
+                    &ctx,
+                    spec,
+                    &parsed.cmd,
+                    arg,
+                    &ctoken,
+                    parsed.double_dash_seen,
+                )?;
+                has_explicit_choices = constrained;
+                choices.extend(found);
             }
             if !parsed.cmd.subcommands.is_empty() {
                 choices.extend(self.complete_subcommands(&parsed.cmd, &ctoken));
@@ -155,23 +173,37 @@ impl CompleteWord {
             if parsed.cmd.name == spec.cmd.name {
                 if let Some(default_name) = &spec.default_subcommand {
                     if let Some(default_cmd) = spec.cmd.find_subcommand(default_name) {
-                        // Include completions from default subcommand's first arg
+                        // Include completions from default subcommand's first arg.
+                        //
+                        // The `constrained` half is dropped on purpose: unlike the two call
+                        // sites above, this arg belongs to a *different* command and is only
+                        // a guess that the user means to elide the subcommand name. Letting
+                        // its choices set `has_explicit_choices` would suppress the root
+                        // command's own file fallback whenever the token failed to match
+                        // them — see `complete_word_default_subcommand_choices_do_not_block_
+                        // root_file_fallback`. The `double_dash="required"` rule does apply,
+                        // which is why this goes through the helper at all.
                         if let Some(arg) = default_cmd.args.first() {
-                            choices.extend(self.complete_arg(
+                            let (found, _) = self.complete_positional(
                                 &ctx,
                                 spec,
                                 default_cmd,
                                 arg,
                                 &ctoken,
-                            )?);
+                                parsed.double_dash_seen,
+                            )?;
+                            choices.extend(found);
                         }
                     }
                 }
             }
             choices
         };
-        // Fallback to file completions if nothing is known about this argument and it's not a flag
-        if choices.is_empty() && !ctoken.starts_with('-') && !has_explicit_choices {
+        // Fallback to file completions if nothing is known about this argument and it's not a
+        // flag. Past a `--` a dash-prefixed word is not a flag but a value, so a path like
+        // `-input` still gets completed there.
+        let looks_like_a_flag = flags_possible && ctoken.starts_with('-');
+        if choices.is_empty() && !looks_like_a_flag && !has_explicit_choices {
             let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
             let files = self.complete_path(&cwd, &ctoken, |_| true);
             choices = files.into_iter().map(|n| (n, String::new())).collect();
@@ -259,6 +291,36 @@ impl CompleteWord {
         names.into_iter().map(|n| (n, String::new())).collect()
     }
 
+    /// Completions for a positional argument, under the rule the parser enforces for
+    /// `double_dash="required"`: nothing reaches such an argument until an explicit `--` has
+    /// been typed, so before that the separator is the only useful candidate.
+    ///
+    /// Every path that completes a positional goes through here — the one at the parser's
+    /// cursor, the first argument after a `restart_token`, and the default subcommand's — so
+    /// the rule cannot be honoured in one and forgotten in another.
+    ///
+    /// The second return value says whether the argument constrains what may go there, which
+    /// is what suppresses the file-path fallback.
+    fn complete_positional(
+        &self,
+        ctx: &tera::Context,
+        spec: &Spec,
+        cmd: &SpecCommand,
+        arg: &SpecArg,
+        ctoken: &str,
+        double_dash_seen: bool,
+    ) -> miette::Result<(Vec<(String, String)>, bool)> {
+        if arg.double_dash == SpecDoubleDashChoices::Required && !double_dash_seen {
+            // No filename is valid here either, so the fallback stays off.
+            let separator = ctoken.is_empty().then(|| ("--".to_string(), String::new()));
+            return Ok((separator.into_iter().collect(), true));
+        }
+        Ok((
+            self.complete_arg(ctx, spec, cmd, arg, ctoken)?,
+            arg.choices.is_some(),
+        ))
+    }
+
     fn complete_arg(
         &self,
         ctx: &tera::Context,
@@ -375,23 +437,6 @@ impl CompleteWord {
     }
 }
 
-fn next_arg_for_completion(parsed: &ParseOutput) -> Option<&SpecArg> {
-    parsed.cmd.args.iter().find(|arg| {
-        let parsed_value = parsed
-            .args
-            .iter()
-            .find_map(|(parsed_arg, value)| (parsed_arg.name == arg.name).then_some(value));
-
-        match parsed_value {
-            Some(ParseValue::MultiString(values)) if arg.var => {
-                values.len() < arg.var_max.unwrap_or(usize::MAX)
-            }
-            Some(_) => false,
-            None => true,
-        }
-    })
-}
-
 /// Wrap a completion value in single quotes if any character would otherwise
 /// be interpreted by the shell. The result is meant to be inserted by
 /// `compadd -Q` verbatim, so the user sees consistent single-quote quoting

cli/tests/complete_word.rs

@@ -1,6 +1,7 @@
 use assert_cmd::assert::Assert;
 use assert_cmd::cargo;
 use std::env;
+use std::fs;
 use std::process::Command;
 
 use assert_cmd::prelude::*;
@@ -25,6 +26,101 @@ fn complete_word_variadic_arg_reuses_completer() {
     .stdout("foo\nbar\n");
 }
 
+#[test]
+fn complete_word_double_dash_required_offers_the_separator() {
+    // The parser rejects anything given to a `double_dash="required"` arg before `--`, so
+    // there is exactly one useful thing to complete there.
+    assert_cmd("double-dash.usage.kdl", &["--", "separator", ""]).stdout("--\n");
+    // Once the separator is in, the arg completes normally.
+    assert_cmd("double-dash.usage.kdl", &["--", "separator", "--", ""]).stdout("alpha\nbeta\n");
+}
+
+#[test]
+fn complete_word_double_dash_stops_flag_completion() {
+    // Before the separator, a dash-prefixed token completes to flags as usual...
+    assert_cmd("double-dash.usage.kdl", &["--", "separator", "--v"]).stdout(contains("--verbose"));
+    // ...and after it there are no flags left to complete: the parser reads everything past
+    // `--` as a positional value, so offering `--verbose` would suggest something it would
+    // hand to <target> verbatim.
+    assert_cmd("double-dash.usage.kdl", &["--", "separator", "--", "--v"]).stdout("");
+}
+
+#[test]
+fn complete_word_double_dash_keeps_file_fallback_for_dash_prefixed_values() {
+    // A dash-prefixed word means a flag before the separator and a value after it, so path
+    // completion has to be withheld in the first case and offered in the second. Needs a file
+    // whose name starts with `-` to tell the two apart, hence the scratch directory.
+    let dir = std::env::temp_dir().join(format!("usage_dash_file_{}", std::process::id()));
+    let _ = fs::remove_dir_all(&dir);
+    fs::create_dir_all(&dir).unwrap();
+    fs::write(dir.join("-dashed.txt"), "").unwrap();
+
+    let spec = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+        .parent()
+        .unwrap()
+        .join("examples")
+        .join("double-dash.usage.kdl");
+    let run = |args: &[&str]| {
+        let mut c = Command::new(cargo::cargo_bin!("usage"));
+        c.current_dir(&dir)
+            .args(["cw", "--shell", "fish", "-f"])
+            .arg(&spec)
+            .arg("mycli")
+            .args(args);
+        String::from_utf8(c.output().unwrap().stdout).unwrap()
+    };
+
+    // The leading `--` is clap's own escape, so the words start after it.
+    assert!(
+        !run(&["--", "paths", "-d"]).contains("-dashed.txt"),
+        "before `--`, `-d` is a flag prefix"
+    );
+    assert!(
+        run(&["--", "paths", "--", "-d"]).contains("-dashed.txt"),
+        "after `--`, `-d` is the start of a value"
+    );
+
+    fs::remove_dir_all(&dir).unwrap();
+}
+
+#[test]
+fn complete_word_double_dash_applies_after_a_restart_token() {
+    // A restart_token starts a fresh invocation, so the separator has to be typed again.
+    assert_cmd(
+        "double-dash.usage.kdl",
+        &["--", "restarted", "--", "alpha", ":::", ""],
+    )
+    .stdout("--\n");
+    assert_cmd(
+        "double-dash.usage.kdl",
+        &["--", "restarted", "--", "alpha", ":::", "--", ""],
+    )
+    .stdout("alpha\nbeta\n");
+}
+
+#[test]
+fn complete_word_double_dash_applies_to_a_default_subcommand() {
+    // Root-level completion reaches the default subcommand's first arg by its own path.
+    assert_cmd("double-dash-default-subcommand.usage.kdl", &["--", ""])
+        .stdout(contains("--\n"))
+        .stdout(contains("alpha").not());
+    assert_cmd(
+        "double-dash-default-subcommand.usage.kdl",
+        &["--", "--", ""],
+    )
+    .stdout(contains("alpha"))
+    .stdout(contains("beta"));
+}
+
+#[test]
+fn complete_word_double_dash_routes_past_greedy_variadic() {
+    // Before the separator the greedy variadic is still the target...
+    assert_cmd("double-dash.usage.kdl", &["--", "routed", ""]).stdout("one\ntwo\n");
+    assert_cmd("double-dash.usage.kdl", &["--", "routed", "one", ""]).stdout("one\ntwo\n");
+    // ...and after it the completion follows the parser onto the arg that required it.
+    assert_cmd("double-dash.usage.kdl", &["--", "routed", "one", "--", ""]).stdout("alpha\nbeta\n");
+}
+
 #[test]
 fn complete_word_variadic_arg_respects_var_max() {
     assert_cmd("variadic-completion.usage.kdl", &["--", "bounded", ""]).stdout("foo\nbar\n");

docs/spec/reference/arg.md

@@ -50,8 +50,42 @@ arg "<env>" {
 arg "<file>" long_help="longer help for --help (as oppoosed to -h)"
 
 // double-dash behavior
-arg "<file>" double_dash="required" // arg must be passed after a double dash (e.g. mycli -- file.txt)
-arg "<file>" double_dash="optional" // arg may be passed after a double dash (e.g. mycli -- file.txt or mycli file.txt)
-arg "<file>..." double_dash="automatic" // once arg is passed, behave as if a double dash was passed (e.g. mycli file.txt --filewithdash)
+arg "<file>" double_dash="required" // arg only accepts values after a double dash; `mycli file.txt` is an error, `mycli -- file.txt` is not
+arg "<-- file>"                     // shorthand for double_dash="required" (also `arg "[-- file]"`, `arg "<-- files>..."`)
+arg "<file>" double_dash="optional" // arg may be passed after a double dash (e.g. mycli -- file.txt or mycli file.txt) — the default
+arg "<file>..." double_dash="automatic" // once arg is passed, behave as if a double dash was passed (e.g. mycli file.txt --filewithdash) — not yet enforced by the parser
 arg "<args>..." double_dash="preserve" // preserve double dashes as args (e.g. mycli arg1 -- arg2 -- arg3)
 ```
+
+## Double-Dash Behavior
+
+`double_dash="required"` is enforced while parsing, so the three points below are
+what a caller of your CLI actually sees.
+
+**The arg is unreachable until a `--` is typed.** A word offered to it beforehand is
+rejected with `Argument <file> can only be set after a "--" separator`, and the value
+is not assigned. A variadic arg reports this once, not once per word, and an arg that
+is both `required` and `double_dash="required"` reports only this — not also
+`Missing required arg`.
+
+**Everything after the `--` is routed to that arg**, jumping past earlier args —
+including a greedy variadic that would otherwise swallow the rest. This matches
+clap's [`Arg::last(true)`](https://docs.rs/clap/latest/clap/struct.Arg.html#method.last),
+which is what specs generated from clap map to `double_dash="required"`.
+
+```kdl
+arg "[tool]..."
+arg "[-- command]..."
+// mycli node@20 -- node app.js  =>  tool=[node@20], command=[node, app.js]
+// mycli -- ls                   =>  tool=[],        command=[ls]
+```
+
+**Flag parsing stops at the `--`**, as usual, so `mycli -- --verbose` gives the arg the
+literal string `--verbose` rather than setting a `--verbose` flag.
+
+Two interactions are worth calling out:
+
+- A `--` that `double_dash="preserve"` keeps as a value is a _value_, not a separator.
+  It does not unlock a `double_dash="required"` arg — one token cannot be both.
+- A command's `restart_token` starts a fresh invocation, which resets the separator.
+  Each invocation after the token needs its own `--`.

examples/double-dash-default-subcommand.usage.kdl

@@ -0,0 +1,14 @@
+// A `default_subcommand` whose first arg requires `--`.
+//
+// Root-level completion also offers that arg, on a separate code path from the one the
+// parser's cursor uses, so the `double_dash="required"` rule has to hold there too.
+// Kept apart from `double-dash.usage.kdl` because `default_subcommand` is spec-level and
+// would change what every command in that fixture completes to.
+
+default_subcommand "run"
+
+cmd "run" {
+    arg "[-- target]" required=#false {
+        choices "alpha" "beta"
+    }
+}

examples/double-dash.usage.kdl

@@ -0,0 +1,37 @@
+// `double_dash="required"` args, spelled with the `[-- name]` shorthand.
+//
+// Choices rather than a `complete` runner so the completions are resolved in-process
+// and the fixture works on every platform.
+
+// A greedy variadic sits in front of the arg that requires `--`, so reaching the
+// latter depends on the separator moving the parser's cursor past the former.
+cmd routed {
+    arg "[args]…" var=#true required=#false {
+        choices "one" "two"
+    }
+    arg "[-- last]…" var=#true required=#false {
+        choices "alpha" "beta"
+    }
+}
+
+// Nothing can be given to <target> until a `--` shows up. The name avoids `file`/`dir`,
+// which are completed as paths regardless of their choices.
+cmd separator {
+    flag --verbose help="Not a flag once `--` has been typed"
+    arg "[-- target]" required=#false {
+        choices "alpha" "beta"
+    }
+}
+
+// No choices, so completion falls back to paths — including for a dash-prefixed word,
+// which past the separator is a value rather than a flag.
+cmd paths {
+    arg "[-- target]" required=#false
+}
+
+// A `restart_token` begins a fresh invocation, which needs its own `--`.
+cmd restarted restart_token=":::" {
+    arg "[-- target]" required=#false {
+        choices "alpha" "beta"
+    }
+}

lib/src/error.rs

@@ -26,6 +26,9 @@ pub enum UsageErr {
     #[error("Missing required arg: <{0}>")]
     MissingArg(String),
 
+    #[error("Argument <{0}> can only be set after a `--` separator")]
+    ArgRequiresDoubleDash(String),
+
     #[error("{0}")]
     Help(String),