fix(skilldoc): remove false positional-as-flag validation rule

The validator's positional-as-flag check asserted that a flag folded
into a required positional always fails the binary's Args check when
passed alone, and flagged any doc example using such a flag. That
premise doesn't hold: every generated command with this kind of fold
uses requireBodyFieldOrExactArg or requireBodyFieldOrArgs
(internal/cli/args.go), both of which explicitly accept the flag as a
standalone alternative to the positional. cligen never emits the bare
requireExactArg/requireArgs form that would make a flag-only call
fail, so the failure mode this rule targeted cannot occur for any
currently generated command — a truthful version of the check would
never fire. Removed it rather than keep an inert rule; a folded flag
now validates through the same path as any other registered flag, and
an actually unregistered flag is still caught as unknown-flag.

Adjusted the corresponding test to assert the real behavior: a folded
flag used alone or alongside its positional validates clean, while a
genuinely unregistered flag on the same command still trips
unknown-flag.

Author: ysyneu

internal/skilldoc/validate.go

@@ -16,7 +16,7 @@ type Doc struct {
 type Issue struct {
 	Doc    string
 	Line   int
-	Kind   string // "unknown-command" | "unknown-flag" | "positional-as-flag" | "stale-fence"
+	Kind   string // "unknown-command" | "unknown-flag" | "stale-fence"
 	Detail string
 }
 
@@ -104,27 +104,23 @@ func lineOf(body string, off int) int {
 	return strings.Count(body[:off], "\n") + 1
 }
 
-// commandIndex maps a command path to its set of declared flag names and to the
-// set of flags cligen folded into required positionals, and carries the sorted
-// list of paths for longest-prefix resolution.
+// commandIndex maps a command path to its set of declared flag names, and
+// carries the sorted list of paths for longest-prefix resolution.
 type commandIndex struct {
-	flags  map[string]map[string]bool
-	folded map[string]map[string]bool
-	paths  []string
+	flags map[string]map[string]bool
+	paths []string
 }
 
 func indexDump(d Dump) commandIndex {
 	idx := commandIndex{
-		flags:  make(map[string]map[string]bool),
-		folded: make(map[string]map[string]bool),
+		flags: make(map[string]map[string]bool),
 	}
 	for _, c := range d.Commands {
 		set := make(map[string]bool, len(c.Flags))
 		for _, f := range c.Flags {
 			set[f.Name] = true
 		}
 		idx.flags[c.Path] = set
-		idx.folded[c.Path] = foldedFlagNames(positionalsOf(c.Use))
 		idx.paths = append(idx.paths, c.Path)
 	}
 	// Longest paths first so resolveCommand prefers the most specific match.
@@ -156,26 +152,19 @@ func validateExample(idx commandIndex, docPath string, ex Example) []Issue {
 		}}
 	}
 
-	folded := idx.folded[path]
 	var issues []Issue
 	for _, tok := range ex.Tokens {
 		name, isFlag := flagName(tok)
 		if !isFlag || HasPlaceholder(name) {
 			continue
 		}
-		// cligen folded this field into a required positional: the flag is still
-		// registered (so it is in flagSet) but passing it as a flag fails the
-		// binary's Args check. Catch it before the flagSet pass would wave it
-		// through — this is the exact misuse only a live run surfaced before.
-		if folded[name] {
-			issues = append(issues, Issue{
-				Doc:    docPath,
-				Line:   ex.Line,
-				Kind:   "positional-as-flag",
-				Detail: "--" + name + " is folded into a required positional of `" + path + "` — pass it as a bare argument, not a flag",
-			})
-			continue
-		}
+		// A field cligen folds into a required positional keeps a same-named flag
+		// registered as a genuine alternative source: every generated command
+		// with such a fold uses requireBodyFieldOrExactArg/requireBodyFieldOrArgs
+		// (internal/cli/args.go), which explicitly accepts the flag alone — cligen
+		// never emits the bare requireExactArg/requireArgs form that would make
+		// passing the flag fail. So a folded name is just a flag like any other
+		// here: fall through to the flagSet check below.
 		if globalFlags[name] || flagSet[name] {
 			continue
 		}

internal/skilldoc/validate_test.go

@@ -45,13 +45,14 @@ func TestValidate_UnknownCommandAndFlag(t *testing.T) {
 	}
 }
 
-// A field cligen folded into a required positional is still a registered flag,
-// but passing it as `--flag` fails the binary's Args check. The validator must
-// catch this misuse (kind "positional-as-flag") — the exact error that only a
-// live run surfaced before Use was threaded into the oracle. Passing the field
-// positionally must stay clean, and the same flag name on a command where it is
-// NOT folded (two required ids) must remain valid.
-func TestValidate_FoldedPositionalAsFlag(t *testing.T) {
+// A field cligen folds into a required positional keeps a same-named flag
+// registered as a genuine alternative source: every generated command with
+// such a fold uses requireBodyFieldOrExactArg/requireBodyFieldOrArgs
+// (internal/cli/args.go), which explicitly accepts the flag alone. So passing
+// that flag — with or without the positional also present — must validate
+// clean; only a flag that is not registered on the command at all is an
+// actual defect.
+func TestValidate_FoldedFlagIsValidAlternative(t *testing.T) {
 	d := Dump{Commands: []Command{
 		{ // single required id → cligen folds page-id into a positional
 			Path:  "status-page change-active-list",
@@ -67,23 +68,27 @@ func TestValidate_FoldedPositionalAsFlag(t *testing.T) {
 		},
 	}}
 	docs := []Doc{
-		{Path: "bad", Body: "```bash\nfduty status-page change-active-list --page-id 5\n```\n"},
-		{Path: "good", Body: "```bash\nfduty status-page change-active-list 5 --type incident\n```\n"},
+		{Path: "flag-alone", Body: "```bash\nfduty status-page change-active-list --page-id 5 --type incident\n```\n"},
+		{Path: "positional", Body: "```bash\nfduty status-page change-active-list 5 --type incident\n```\n"},
 		{Path: "twoid", Body: "```bash\nfduty status-page change-timeline-create --page-id 5 --change-id 9\n```\n"},
+		{Path: "unknown-on-folder", Body: "```bash\nfduty status-page change-active-list --page-id 5 --bogus x\n```\n"},
 	}
 	byDoc := map[string][]Issue{}
 	for _, is := range Validate(d, docs) {
 		byDoc[is.Doc] = append(byDoc[is.Doc], is)
 	}
-	if n := len(byDoc["bad"]); n != 1 || byDoc["bad"][0].Kind != "positional-as-flag" {
-		t.Errorf("bad: want 1 positional-as-flag, got %+v", byDoc["bad"])
+	if n := len(byDoc["flag-alone"]); n != 0 {
+		t.Errorf("flag-alone: folded flag used without the positional want 0 issues, got %+v", byDoc["flag-alone"])
 	}
-	if n := len(byDoc["good"]); n != 0 {
-		t.Errorf("good: positional usage want 0 issues, got %+v", byDoc["good"])
+	if n := len(byDoc["positional"]); n != 0 {
+		t.Errorf("positional: positional usage want 0 issues, got %+v", byDoc["positional"])
 	}
 	if n := len(byDoc["twoid"]); n != 0 {
 		t.Errorf("twoid: --page-id on non-folding command want 0 issues, got %+v", byDoc["twoid"])
 	}
+	if n := len(byDoc["unknown-on-folder"]); n != 1 || byDoc["unknown-on-folder"][0].Kind != "unknown-flag" {
+		t.Errorf("unknown-on-folder: want 1 unknown-flag, got %+v", byDoc["unknown-on-folder"])
+	}
 }
 
 func TestValidate_GlobalFlagsAllowed(t *testing.T) {