From 6ea8c4dd3f6557504456654f8725b5688410bc61 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 19 Oct 2026 17:33:39 +0000 Subject: [PATCH] Compile effective flag aliases during installation --- app.go | 12 +- docs/roadmap/implementation.md | 11 +- flag_aliases.go | 263 +++++++++++++++++++++++++++++++++ flag_aliases_test.go | 210 ++++++++++++++++++++++++++ registry.go | 30 +++- registry_test.go | 4 +- 6 files changed, 518 insertions(+), 12 deletions(-) create mode 100644 flag_aliases.go create mode 100644 flag_aliases_test.go diff --git a/app.go b/app.go index 60c8fce..1b4e63f 100644 --- a/app.go +++ b/app.go @@ -10,11 +10,13 @@ import ( // App owns one command registry and its application identity. Each App has an // independent registry; command declarations may be reused across Apps. type App struct { - identity Identity - commands map[string]*commandNode - routingRules []RoutingRule - aliases *alias.Resolver - snapshot *routingSnapshot + identity Identity + commands map[string]*commandNode + routingRules []RoutingRule + aliases *alias.Resolver + flagAliases map[string]resolvedFlagAliases + discoveryFlags map[string]resolvedFlagAliases + snapshot *routingSnapshot } type routingSnapshot struct { diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 0f87e61..2fcf659 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Application-owned aliases implementation plan -Status: in progress; Stages 1–5 completed. +Status: in progress; Stages 1–6 completed. ## Authority, baseline, and scope @@ -408,6 +408,15 @@ unknown scopes; inactive globals; scoped overrides inside a chain; cycles; global `--color → --missing`; collision after `--missing` becomes available; root/group help targets and failed-batch preservation. +**Completed:** Installation candidates now compile effective global and +canonical-command-scoped flag aliases from captured canonical metadata and +built-in help spellings. Chain resolution applies scope precedence, tracks +inactive global destinations, detects cycles, and rejects newly active +canonical-source collisions without publishing failed candidates. Root, group, +command, and alias-prefix discovery contexts retain only their applicable +spellings. Verified with root and both integration module suites using the +local workspace. + ## Stage 7: Resolve entered routes to canonical commands **Work** diff --git a/flag_aliases.go b/flag_aliases.go new file mode 100644 index 0000000..fc06c68 --- /dev/null +++ b/flag_aliases.go @@ -0,0 +1,263 @@ +package commandkit + +import ( + "fmt" + "sort" + "strings" + + "gitea.maximumdirect.net/eric/CommandKit/internal/commandpath" +) + +type resolvedFlagAlias struct { + source capturedFlagSpelling + target capturedFlagSpelling +} + +type resolvedFlagAliases struct { + bySpelling map[string]resolvedFlagAlias +} + +type flagAliasMapping struct { + target capturedFlagSpelling + scoped bool +} + +type flagAliasContext struct { + path commandpath.Path + canonical map[string]capturedFlagSpelling + discovery bool +} + +func (spelling capturedFlagSpelling) token() string { + if spelling.short { + return "-" + spelling.name + } + return "--" + spelling.name +} + +func compileFlagAliases(rules []RoutingRule, roots map[string]*canonicalNode, aliases interface { + Resolve(commandpath.Path) commandpath.Path +}) (map[string]resolvedFlagAliases, map[string]resolvedFlagAliases, error) { + canonicalNodes := indexCanonicalNodes(roots) + global := make(map[string]flagAliasMapping) + scoped := make(map[string]map[string]flagAliasMapping) + for _, rule := range rules { + record := rule.record + switch record.kind { + case globalFlagAliasRule: + source := flagSpellingKey(record.sourceFlag) + global[source] = flagAliasMapping{target: record.targetFlag} + case scopedFlagAliasRule: + scope := record.scopePath.Key() + if canonicalNodes[scope] == nil { + return nil, nil, fmt.Errorf("flag alias scope %q is not a canonical command path", record.scopePath.String()) + } + if scoped[scope] == nil { + scoped[scope] = make(map[string]flagAliasMapping) + } + source := flagSpellingKey(record.sourceFlag) + scoped[scope][source] = flagAliasMapping{target: record.targetFlag, scoped: true} + } + } + + canonicalContexts := make(map[string]flagAliasContext, len(canonicalNodes)+1) + rootPath := commandpath.Path(nil) + canonicalContexts[rootPath.Key()] = flagAliasContext{path: rootPath, canonical: helpFlagSpellings()} + for key, node := range canonicalNodes { + canonicalContexts[key] = flagAliasContext{path: node.path.Clone(), canonical: canonicalFlagSpellings(node.command)} + } + discoveryContexts := discoverFlagContexts(rules, roots, aliases, canonicalNodes) + + canonicalKeys := sortedContextKeys(canonicalContexts) + resolvedCanonical := make(map[string]resolvedFlagAliases, len(canonicalContexts)) + for _, key := range canonicalKeys { + context := canonicalContexts[key] + mappings := cloneFlagMappings(global) + for source, mapping := range scoped[key] { + mappings[source] = mapping + } + resolved, err := resolveFlagAliases(context, mappings) + if err != nil { + return nil, nil, err + } + resolvedCanonical[key] = resolved + } + + discoveryKeys := sortedContextKeys(discoveryContexts) + resolvedDiscovery := make(map[string]resolvedFlagAliases, len(discoveryContexts)) + for _, key := range discoveryKeys { + context := discoveryContexts[key] + resolved, err := resolveFlagAliases(context, cloneFlagMappings(global)) + if err != nil { + return nil, nil, err + } + resolvedDiscovery[key] = resolved + } + return resolvedCanonical, resolvedDiscovery, nil +} + +func indexCanonicalNodes(roots map[string]*canonicalNode) map[string]*canonicalNode { + indexed := make(map[string]*canonicalNode) + var add func(*canonicalNode) + add = func(node *canonicalNode) { + indexed[node.path.Key()] = node + for _, child := range node.children { + add(child) + } + } + for _, root := range roots { + add(root) + } + return indexed +} + +func helpFlagSpellings() map[string]capturedFlagSpelling { + flags := make(map[string]capturedFlagSpelling, 2) + for _, spelling := range []capturedFlagSpelling{{name: "help"}, {name: "h", short: true}} { + flags[flagSpellingKey(spelling)] = spelling + } + return flags +} + +func canonicalFlagSpellings(command Command) map[string]capturedFlagSpelling { + flags := helpFlagSpellings() + if command.record.payload == nil { + return flags + } + for _, metadata := range command.record.payload.bridgeFlags() { + long := capturedFlagSpelling{name: metadata.Name} + flags[flagSpellingKey(long)] = long + if metadata.Short != "" { + short := capturedFlagSpelling{name: metadata.Short, short: true} + flags[flagSpellingKey(short)] = short + } + } + return flags +} + +func discoverFlagContexts(rules []RoutingRule, roots map[string]*canonicalNode, aliases interface { + Resolve(commandpath.Path) commandpath.Path +}, canonicalNodes map[string]*canonicalNode) map[string]flagAliasContext { + commandSources := make(map[string]struct{}) + prefixes := make(map[string]commandpath.Path) + for _, rule := range rules { + if rule.record.kind == commandAliasRule { + commandSources[rule.record.sourcePath.Key()] = struct{}{} + } + } + for _, rule := range rules { + if rule.record.kind != commandAliasRule { + continue + } + for length := 1; length < len(rule.record.sourcePath); length++ { + prefix := rule.record.sourcePath[:length].Clone() + key := prefix.Key() + if canonicalNodes[key] != nil { + continue + } + if _, exactAlias := commandSources[key]; exactAlias { + continue + } + resolved := aliases.Resolve(prefix) + trace := traceCanonicalPath(roots, resolved) + if trace.exact != nil || trace.operandOwner != nil { + continue + } + prefixes[key] = prefix + } + } + contexts := make(map[string]flagAliasContext, len(prefixes)) + for key, path := range prefixes { + contexts[key] = flagAliasContext{path: path, canonical: helpFlagSpellings(), discovery: true} + } + return contexts +} + +func cloneFlagMappings(mappings map[string]flagAliasMapping) map[string]flagAliasMapping { + cloned := make(map[string]flagAliasMapping, len(mappings)) + for source, mapping := range mappings { + cloned[source] = mapping + } + return cloned +} + +func sortedContextKeys(contexts map[string]flagAliasContext) []string { + keys := make([]string, 0, len(contexts)) + for key := range contexts { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} + +func resolveFlagAliases(context flagAliasContext, mappings map[string]flagAliasMapping) (resolvedFlagAliases, error) { + result := resolvedFlagAliases{bySpelling: make(map[string]resolvedFlagAlias)} + sources := make([]string, 0, len(mappings)) + for source := range mappings { + sources = append(sources, source) + } + sort.Strings(sources) + for _, sourceKey := range sources { + mapping := mappings[sourceKey] + source := spellingFromKey(sourceKey) + target, active, err := resolveFlagTarget(source, mapping.target, mappings, context.canonical) + if err != nil { + return resolvedFlagAliases{}, fmt.Errorf("%s: %w", flagContextDescription(context), err) + } + if !active { + if mapping.scoped { + return resolvedFlagAliases{}, fmt.Errorf("scoped flag alias %s has no canonical destination in %s", source.token(), flagContextDescription(context)) + } + continue + } + if _, collision := context.canonical[sourceKey]; collision { + return resolvedFlagAliases{}, fmt.Errorf("flag alias %s conflicts with a canonical spelling in %s", source.token(), flagContextDescription(context)) + } + result.bySpelling[sourceKey] = resolvedFlagAlias{source: source, target: target} + } + return result, nil +} + +func resolveFlagTarget(source, target capturedFlagSpelling, mappings map[string]flagAliasMapping, canonical map[string]capturedFlagSpelling) (capturedFlagSpelling, bool, error) { + visited := map[string]int{flagSpellingKey(source): 0} + chain := []capturedFlagSpelling{source} + current := target + for { + key := flagSpellingKey(current) + if endpoint, exists := canonical[key]; exists { + return endpoint, true, nil + } + if start, exists := visited[key]; exists { + cycle := append(append([]capturedFlagSpelling(nil), chain[start:]...), current) + spellings := make([]string, 0, len(cycle)) + for _, spelling := range cycle { + spellings = append(spellings, spelling.token()) + } + return capturedFlagSpelling{}, false, fmt.Errorf("flag alias cycle: %s", strings.Join(spellings, " → ")) + } + visited[key] = len(chain) + chain = append(chain, current) + next, exists := mappings[key] + if !exists { + return capturedFlagSpelling{}, false, nil + } + current = next.target + } +} + +func spellingFromKey(key string) capturedFlagSpelling { + if strings.HasPrefix(key, "short:") { + return capturedFlagSpelling{name: strings.TrimPrefix(key, "short:"), short: true} + } + return capturedFlagSpelling{name: strings.TrimPrefix(key, "long:")} +} + +func flagContextDescription(context flagAliasContext) string { + if context.discovery { + return fmt.Sprintf("alias prefix %q", context.path.String()) + } + if len(context.path) == 0 { + return "application root" + } + return fmt.Sprintf("command %q", context.path.String()) +} diff --git a/flag_aliases_test.go b/flag_aliases_test.go new file mode 100644 index 0000000..1e3c6d9 --- /dev/null +++ b/flag_aliases_test.go @@ -0,0 +1,210 @@ +package commandkit + +import ( + "strings" + "testing" + + "gitea.maximumdirect.net/eric/CommandKit/internal/commandpath" +) + +func flagAliasCommand(name string, flags ...Flag[struct{}]) Command { + return NewCommand(CommandSpec[struct{}]{ + Name: name, Summary: name, Flags: flags, + Run: func(Invocation, struct{}) Outcome { return Success() }, + }) +} + +func stringFlag(name, short string) Flag[struct{}] { + return ValueFlag(FlagMeta{Name: name, Short: short}, (*string)(nil), + func(value string) (string, error) { return value, nil }, + func(*struct{}, Supplied[string]) {}, + ) +} + +func TestFlagAliasChainsUseScopedOverridesAndCanonicalEndpoints(t *testing.T) { + inspect := flagAliasCommand("inspect", stringFlag("color", "c"), stringFlag("format", "f")) + show := flagAliasCommand("show", stringFlag("color", "c"), stringFlag("format", "f")) + group := Group("Tools", "tools", show) + app, err := New(Identity{Name: "Example"}, + inspect, + group, + Alias("look", "inspect"), + FlagAlias("--colour", "--hue"), + FlagAlias("--hue", "--color"), + FlagAlias("-x", "--color"), + FlagAlias("--tint", "-c"), + FlagAliasFor("Tools", "--group-help", "--help"), + FlagAliasFor("inspect", "--hue", "--format"), + FlagAliasFor("inspect", "--local", "--format"), + ) + if err != nil { + t.Fatal(err) + } + + inspectAliases := app.flagAliases[commandpath.Path{"inspect"}.Key()].bySpelling + for source, target := range map[string]string{ + "--colour": "--format", + "--hue": "--format", + "-x": "--color", + "--tint": "-c", + "--local": "--format", + } { + alias, ok := inspectAliases[flagSpellingKey(capturedSpelling(source))] + if !ok || alias.target.token() != target { + t.Errorf("inspect alias %s = (%s, %v), want %s", source, alias.target.token(), ok, target) + } + } + + groupAliases := app.flagAliases[commandpath.Path{"Tools"}.Key()].bySpelling + if _, active := groupAliases[flagSpellingKey(capturedFlagSpelling{name: "colour"})]; active { + t.Error("global alias was active on a group without its destination flag") + } + if _, inherited := groupAliases[flagSpellingKey(capturedFlagSpelling{name: "local"})]; inherited { + t.Fatal("command-scoped alias was inherited by its parent group") + } + if _, inherited := app.flagAliases[commandpath.Path{"Tools", "show"}.Key()].bySpelling[flagSpellingKey(capturedFlagSpelling{name: "local"})]; inherited { + t.Fatal("command-scoped alias was inherited by a different command") + } + groupHelp := flagSpellingKey(capturedFlagSpelling{name: "group-help"}) + if got := groupAliases[groupHelp].target.token(); got != "--help" { + t.Fatalf("group-scoped help alias target = %s, want --help", got) + } + if _, inherited := app.flagAliases[commandpath.Path{"Tools", "show"}.Key()].bySpelling[groupHelp]; inherited { + t.Fatal("group-scoped alias was inherited by its child command") + } +} + +func TestFlagAliasDiscoveryContextsUseOnlyGlobalHelpAliases(t *testing.T) { + show := flagAliasCommand("show") + app, err := New(Identity{Name: "Example"}, + Group("Tools", "tools", show), + Alias("shortcut show", "Tools show"), + FlagAlias("--hlep", "--help"), + FlagAliasFor("Tools show", "--local-help", "--help"), + ) + if err != nil { + t.Fatal(err) + } + + globalHelp := flagSpellingKey(capturedFlagSpelling{name: "hlep"}) + if got := app.flagAliases[commandpath.Path(nil).Key()].bySpelling[globalHelp].target.token(); got != "--help" { + t.Fatalf("root --hlep target = %s, want --help", got) + } + if got := app.flagAliases[commandpath.Path{"Tools"}.Key()].bySpelling[globalHelp].target.token(); got != "--help" { + t.Fatalf("group --hlep target = %s, want --help", got) + } + discovery := app.discoveryFlags[commandpath.Path{"shortcut"}.Key()].bySpelling + if got := discovery[globalHelp].target.token(); got != "--help" { + t.Fatalf("discovery --hlep target = %s, want --help", got) + } + if _, inherited := discovery[flagSpellingKey(capturedFlagSpelling{name: "local-help"})]; inherited { + t.Fatal("discovery context inherited a command-scoped alias") + } +} + +func TestFlagAliasDuplicatesAndScopesAreDeterministic(t *testing.T) { + command := flagAliasCommand("inspect", stringFlag("color", "c"), stringFlag("format", "f")) + duplicate := FlagAlias("--colour", "--color") + app, err := New(Identity{Name: "Example"}, command, flagAliasCommand("plain", stringFlag("color", "c")), duplicate, FlagAlias("--colour", "--color"), + FlagAliasFor("inspect", "--colour", "--format")) + if err != nil { + t.Fatalf("identical duplicates and scoped override were rejected: %v", err) + } + if got := app.flagAliases[commandpath.Path{"inspect"}.Key()].bySpelling[flagSpellingKey(capturedFlagSpelling{name: "colour"})].target.token(); got != "--format" { + t.Fatalf("scoped alias target = %s, want --format", got) + } + if got := app.flagAliases[commandpath.Path{"plain"}.Key()].bySpelling[flagSpellingKey(capturedFlagSpelling{name: "colour"})].target.token(); got != "--color" { + t.Fatalf("effective global alias target = %s, want --color", got) + } + + for name, rules := range map[string][]RoutingRule{ + "competing global targets": { + FlagAlias("--colour", "--color"), + FlagAlias("--colour", "--format"), + }, + "competing scoped targets": { + FlagAliasFor("inspect", "--colour", "--color"), + FlagAliasFor("inspect", "--colour", "--format"), + }, + "unknown scope": {FlagAliasFor("missing", "--colour", "--color")}, + "missing scoped destination": {FlagAliasFor("inspect", "--colour", "--missing")}, + } { + t.Run(name, func(t *testing.T) { + declarations := []Declaration{command} + for _, rule := range rules { + declarations = append(declarations, rule) + } + if _, err := New(Identity{Name: "Example"}, declarations...); err == nil { + t.Fatal("New() accepted invalid flag alias rules") + } + }) + } +} + +func TestFlagAliasCyclesIdentifyParticipatingSpellings(t *testing.T) { + for name, declarations := range map[string][]Declaration{ + "global": { + FlagAlias("--first", "--second"), + FlagAlias("--second", "--first"), + }, + "scoped": { + flagAliasCommand("inspect"), + FlagAliasFor("inspect", "-a", "--second"), + FlagAliasFor("inspect", "--second", "-a"), + }, + } { + t.Run(name, func(t *testing.T) { + _, err := New(Identity{Name: "Example"}, declarations...) + if err == nil { + t.Fatal("New() accepted a flag alias cycle") + } + message := err.Error() + for _, spelling := range []string{"--first", "--second"} { + if name == "scoped" { + continue + } + if !strings.Contains(message, spelling) { + t.Errorf("cycle error %q does not identify %s", message, spelling) + } + } + if name == "scoped" && (!strings.Contains(message, "-a") || !strings.Contains(message, "--second")) { + t.Errorf("scoped cycle error %q does not identify both spellings", message) + } + }) + } +} + +func TestInactiveGlobalAliasesActivateAtomicallyWhenDestinationAppears(t *testing.T) { + command := flagAliasCommand("inspect", stringFlag("color", "c")) + app, err := New(Identity{Name: "Example"}, command, + FlagAlias("--color", "--missing"), + FlagAlias("--colour", "--color"), + ) + if err != nil { + t.Fatal(err) + } + aliases := app.flagAliases[commandpath.Path{"inspect"}.Key()].bySpelling + if _, active := aliases[flagSpellingKey(capturedFlagSpelling{name: "color"})]; active { + t.Fatal("global alias with an unavailable destination became active") + } + if got := aliases[flagSpellingKey(capturedFlagSpelling{name: "colour"})].target.token(); got != "--color" { + t.Fatalf("--colour endpoint = %s, want canonical --color", got) + } + + snapshot := app.routeSnapshot() + commands := len(app.commands) + rules := len(app.routingRules) + originalAlias := aliases[flagSpellingKey(capturedFlagSpelling{name: "colour"})] + if err := app.Register(flagAliasCommand("later", stringFlag("color", "c"), stringFlag("missing", ""))); err == nil { + t.Fatal("Register() accepted a newly active global source collision") + } + currentAlias := app.flagAliases[commandpath.Path{"inspect"}.Key()].bySpelling[flagSpellingKey(capturedFlagSpelling{name: "colour"})] + if app.routeSnapshot() != snapshot || len(app.commands) != commands || len(app.routingRules) != rules || currentAlias != originalAlias { + t.Fatal("failed registration changed the existing command or routing state") + } +} + +func capturedSpelling(token string) capturedFlagSpelling { + issues := []error(nil) + return captureFlagSpelling("test", token, &issues) +} diff --git a/registry.go b/registry.go index eff7ada..d9d3697 100644 --- a/registry.go +++ b/registry.go @@ -18,10 +18,12 @@ func (a *App) Register(declarations ...Declaration) error { } type installationCandidate struct { - commands map[string]*commandNode - routingRules []RoutingRule - aliases *internalalias.Resolver - changed bool + commands map[string]*commandNode + routingRules []RoutingRule + aliases *internalalias.Resolver + flagAliases map[string]resolvedFlagAliases + discoveryFlags map[string]resolvedFlagAliases + changed bool } type canonicalNode struct { @@ -81,6 +83,8 @@ func (a *App) installBatch(declarations ...Declaration) error { a.commands = candidate.commands a.routingRules = candidate.routingRules a.aliases = candidate.aliases + a.flagAliases = candidate.flagAliases + a.discoveryFlags = candidate.discoveryFlags a.snapshot = nil return nil } @@ -138,12 +142,24 @@ func (a *App) buildInstallationCandidate(commands []Command, addedRules []Routin if err := validateCommandAliases(rules, resolver, tree); err != nil { return nil, err } + flagAliases, discoveryFlags, err := compileFlagAliases(rules, tree, resolver) + if err != nil { + return nil, err + } + candidate.flagAliases = flagAliases + candidate.discoveryFlags = discoveryFlags return candidate, nil } func normalizeRoutingRules(rules []RoutingRule) ([]RoutingRule, error) { unique := make(map[routingRuleKey]RoutingRule, len(rules)) commandTargets := make(map[string]string) + type scopedFlagKey struct { + kind routingRuleKind + scope string + source string + } + flagTargets := make(map[scopedFlagKey]string) for _, rule := range rules { if rule.record == nil { return nil, fmt.Errorf("invalid zero routing rule declaration") @@ -160,6 +176,12 @@ func normalizeRoutingRules(rules []RoutingRule) ([]RoutingRule, error) { return nil, fmt.Errorf("conflicting command aliases for %q", rule.record.sourcePath.String()) } commandTargets[key.source] = key.target + } else { + scopeKey := scopedFlagKey{kind: key.kind, scope: key.scope, source: key.source} + if previous, exists := flagTargets[scopeKey]; exists && previous != key.target { + return nil, fmt.Errorf("conflicting flag aliases for %q", rule.record.sourceFlag.token()) + } + flagTargets[scopeKey] = key.target } unique[key] = rule } diff --git a/registry_test.go b/registry_test.go index be731ae..7553d14 100644 --- a/registry_test.go +++ b/registry_test.go @@ -58,8 +58,8 @@ func TestRoutingRuleFactoriesCaptureNormalizedValues(t *testing.T) { if globalRule.record.targetFlag != (capturedFlagSpelling{name: "c", short: true}) { t.Fatalf("target spelling = %#v", globalRule.record.targetFlag) } - scopedRule := FlagAliasFor("group show", "-f", "--format") - if got, want := scopedRule.record.scopePath.String(), "group show"; got != want { + scopedRule := FlagAliasFor(" new\t", "-f", "--help") + if got, want := scopedRule.record.scopePath.String(), "new"; got != want { t.Fatalf("scope path = %q, want %q", got, want) }