diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 13e371c..bca51a0 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Invocation reporting audit remediation -Status: remediation in progress; 4 of 7 findings complete. +Status: remediation in progress; 5 of 7 findings complete. ## Review scope @@ -318,7 +318,13 @@ caller-owned writes that have already started. ## 5. Apply reporting-flag aliases in unresolved-route parse contexts -Priority: P2. Status: open. +Priority: P2. Status: complete. + +- [x] Completed: unresolved routes obtain aliases from the context selected by + normal parsing, through neutral internal metadata. Root and nested-group + canonical/alias comparisons, quiet failure visibility, scoped precedence, + unchanged unresolved identity, and factory non-execution pass. + Validation: `GOTOOLCHAIN=local go test ./...` at the repository root. ### Finding and evidence diff --git a/execute.go b/execute.go index 983bc81..97bc313 100644 --- a/execute.go +++ b/execute.go @@ -10,6 +10,7 @@ import ( internalcobra "gitea.maximumdirect.net/eric/CommandKit/internal/cobra" commandmeta "gitea.maximumdirect.net/eric/CommandKit/internal/commandmeta" + "gitea.maximumdirect.net/eric/CommandKit/internal/commandpath" ) // Execute parses and dispatches one invocation. args excludes the executable @@ -149,6 +150,11 @@ func (current *execution) prepareSession() { options.FlagAliases = current.snapshot.flagAliases[current.route.canonicalPath.Key()].neutralRecords() case routeDiscovery: options.FlagAliases = current.snapshot.discoveryFlags[current.route.discoveryPath.Key()].neutralRecords() + default: + snapshot := current.snapshot + options.FlagAliasesForPath = func(path []string) []commandmeta.FlagAlias { + return snapshot.flagAliases[commandpath.FromTokens(path).Key()].neutralRecords() + } } current.session = internalcobra.Prepare(current.snapshot.metadata, options) } diff --git a/execute_reporting_test.go b/execute_reporting_test.go index 5a0c99d..4deb87e 100644 --- a/execute_reporting_test.go +++ b/execute_reporting_test.go @@ -92,6 +92,53 @@ func TestExecuteQuietUsageKeepsAliasGuidanceVisible(t *testing.T) { } } +func TestUnresolvedRoutesUseReportingAliasesForTheirParseContext(t *testing.T) { + calls := 0 + command := NewCommand(CommandSpec[struct{}]{ + Name: "run", + NewInput: func() struct{} { calls++; return struct{}{} }, + Run: func(Invocation, struct{}) Outcome { calls++; return Success() }, + }) + app, err := New(Identity{Name: "Example"}, Group("group", "", Group("nested", "", command)), + FlagAlias("-d", "--debug"), FlagAlias("-q", "--quiet"), + FlagAlias("-x", "--quiet"), FlagAliasFor("group nested", "-x", "--debug")) + if err != nil { + t.Fatal(err) + } + for _, path := range [][]string{{"missing"}, {"group", "missing"}, {"group", "nested", "missing"}} { + t.Run(strings.Join(path, "/"), func(t *testing.T) { + var canonicalDiagnostics, aliasDiagnostics bytes.Buffer + canonical := app.Execute(context.Background(), append(append([]string(nil), path...), "--debug"), Streams{Err: &canonicalDiagnostics}) + alias := app.Execute(context.Background(), append(append([]string(nil), path...), "-d"), Streams{Err: &aliasDiagnostics}) + if canonical.Kind != OutcomeUsage || alias.Kind != OutcomeUsage || canonical.Status != alias.Status || canonical.Explanation.Message != alias.Explanation.Message { + t.Fatalf("canonical=%#v alias=%#v", canonical, alias) + } + for _, text := range []string{canonicalDiagnostics.String(), aliasDiagnostics.String()} { + if !strings.Contains(text, "DEBUG:") || !strings.Contains(text, "ERROR:") || strings.Contains(text, "unknown shorthand") || strings.Contains(text, "Alias for:") { + t.Fatalf("diagnostics=%q", text) + } + } + route := resolveEnteredRoute(path, app.routeSnapshot()) + if route.kind != routeUnknown || len(route.canonicalPath) != 0 { + t.Fatalf("invalid route acquired canonical identity: %#v", route) + } + var quiet bytes.Buffer + got := app.Execute(context.Background(), append(append([]string(nil), path...), "-q", "--unknown"), Streams{Err: &quiet}) + if got.Kind != OutcomeUsage || !strings.Contains(quiet.String(), "ERROR: unknown flag") || strings.Contains(quiet.String(), "DEBUG:") { + t.Fatalf("quiet alias outcome=%#v diagnostics=%q", got, quiet.String()) + } + }) + } + var scoped bytes.Buffer + got := app.Execute(context.Background(), []string{"group", "nested", "missing", "-x"}, Streams{Err: &scoped}) + if got.Kind != OutcomeUsage || !strings.Contains(scoped.String(), "DEBUG:") { + t.Fatalf("scoped precedence outcome=%#v diagnostics=%q", got, scoped.String()) + } + if calls != 0 { + t.Fatalf("invalid routes ran factories or handlers %d times", calls) + } +} + type executeReportingFailureWriter struct{ err error } func (w executeReportingFailureWriter) Write([]byte) (int, error) { return 0, w.err } diff --git a/internal/cobra/parse.go b/internal/cobra/parse.go index 0b28c47..7bc4787 100644 --- a/internal/cobra/parse.go +++ b/internal/cobra/parse.go @@ -19,10 +19,11 @@ import ( // Session owns one mutable backend tree for a single command invocation. type Session struct { - metadata *commandmeta.Command - root *spfcobra.Command - declarations map[*spfcobra.Command]*commandmeta.Command - flagAliases []commandmeta.FlagAlias + metadata *commandmeta.Command + root *spfcobra.Command + declarations map[*spfcobra.Command]*commandmeta.Command + flagAliases []commandmeta.FlagAlias + flagAliasesForPath func([]string) []commandmeta.FlagAlias } // Prepare constructs a fresh backend tree for one invocation. @@ -33,6 +34,7 @@ func Prepare(root *commandmeta.Command, options commandmeta.ParseOptions) *Sessi } session.metadata = root session.flagAliases = append([]commandmeta.FlagAlias(nil), options.FlagAliases...) + session.flagAliasesForPath = options.FlagAliasesForPath options = normalizeOptions(options) session.root, session.declarations = buildTree(root, options) return session @@ -80,7 +82,11 @@ func (session *Session) Parse(argv []string) commandmeta.Result { localFlags := make([]commandmeta.Flag, 0, len(selectedNode.Flags)+len(selectedNode.CoreFlags)) localFlags = append(localFlags, selectedNode.Flags...) localFlags = append(localFlags, selectedNode.CoreFlags...) - interpreted, spellings, occurrences, interpretationFailure := interpretFlagTokens(remaining, localFlags, session.flagAliases) + aliases := session.flagAliases + if session.flagAliasesForPath != nil { + aliases = session.flagAliasesForPath(result.Path) + } + interpreted, spellings, occurrences, interpretationFailure := interpretFlagTokens(remaining, localFlags, aliases) for _, flag := range localFlags { if value := selected.Flags().Lookup(flag.Name); value != nil { value.Value.(*rawValue).queueSpellings(spellings[flag.Name]) diff --git a/internal/commandmeta/metadata.go b/internal/commandmeta/metadata.go index f71b49b..399f5cf 100644 --- a/internal/commandmeta/metadata.go +++ b/internal/commandmeta/metadata.go @@ -130,9 +130,12 @@ type Result struct { // ParseOptions makes parser context and streams explicit. type ParseOptions struct { - Context context.Context - In io.Reader - Out io.Writer - Err io.Writer - FlagAliases []FlagAlias + // FlagAliasesForPath selects aliases for the actual parser context when + // routing has not already selected one. It overrides FlagAliases when set. + FlagAliasesForPath func([]string) []FlagAlias + Context context.Context + In io.Reader + Out io.Writer + Err io.Writer + FlagAliases []FlagAlias }