Reuse alias tree for discovery and help
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Consolidated audit catalog
|
||||
|
||||
Status: **11 distinct open findings: 11 P3**, consolidated from 30
|
||||
Status: **9 distinct open findings: 9 P3**, consolidated from 30
|
||||
numbered entries in seven audit documents. Reviewed audit state:
|
||||
`db732e3a1de55c844f77870e1c6db683c2685d5d`.
|
||||
|
||||
@@ -296,6 +296,10 @@ selected routing and discovery semantics.
|
||||
|
||||
## F14 — P3: Discovery-context preparation repeatedly rebuilds shared prefixes
|
||||
|
||||
**Complete.** Distinct trie prefixes are enumerated and classified once;
|
||||
focused discovery and resolver tests pass, and traversal was reviewed for
|
||||
elimination of repeated per-declaration prefix construction.
|
||||
|
||||
R alias sources sharing a depth-D proper prefix cause Θ(RD²) copying and key
|
||||
construction before result deduplication, with repeated classification as well.
|
||||
The source declarations contain only Θ(RD) tokens.
|
||||
@@ -313,12 +317,16 @@ Reuse the tree's routing-snapshot lifetime; do not introduce a parallel index or
|
||||
a cross-invocation result cache. The repeated preparation and help-time scan
|
||||
remain distinct findings. The [joint analysis](remediation-analysis.md#f14f15--joint-source-prefix-design)
|
||||
records the accepted design. The performance-testing decision under F13 applies
|
||||
to both; implementation remains pending.
|
||||
to both.
|
||||
|
||||
**Original:** [02-efficiency E3](02-efficiency.md#e3--p3-discovery-context-preparation-rebuilds-shared-prefixes-repeatedly).
|
||||
|
||||
## F15 — P3: Alias help scans the entire source catalog for each intermediate route
|
||||
|
||||
**Complete.** Help queries strict descendants from each resolution path instead
|
||||
of scanning the complete catalog. Focused alias/help tests pass, and the tree
|
||||
traversal was reviewed for the removal of unrelated source scans.
|
||||
|
||||
Help discovery examines all A alias sources at each of P rewrite paths, doing PA
|
||||
examinations and repeated copies even when there are no descendants. A valid
|
||||
single-token alias chain produces Θ(A²) work to discover an empty alias section
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Audit remediation implementation plan
|
||||
|
||||
Status: in progress; Stages 1–15 complete.
|
||||
Status: in progress; Stages 1–16 complete.
|
||||
|
||||
Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint,
|
||||
intervening changes recorded decisions and retired the completed older
|
||||
@@ -527,6 +527,12 @@ Complete when repeated suffix traversal is removed without routing changes.
|
||||
|
||||
## Stage 16 — Reuse the resolver tree for prefix discovery and help
|
||||
|
||||
**Complete.** Discovery classifies each eligible trie prefix once (F14), and
|
||||
help visits only strict descendants for each resolution path (F15). Source
|
||||
review confirms both changes use the existing resolver tree; focused alias,
|
||||
discovery, help and resolver tests pass offline with Go 1.27.0 and
|
||||
`-mod=readonly`, including returned-path ownership and exact aliases with children.
|
||||
|
||||
Findings: F14 and F15, one coordinated change. Sources:
|
||||
`internal/alias/resolver.go`, `flag_aliases.go`, and `help_view.go`.
|
||||
|
||||
|
||||
@@ -66,6 +66,7 @@ func (spelling capturedFlagSpelling) token() string {
|
||||
|
||||
func compileFlagAliases(rules []RoutingRule, roots map[string]*canonicalNode, aliases interface {
|
||||
Resolve(commandpath.Path) commandpath.Path
|
||||
ProperSourcePrefixes() []commandpath.Path
|
||||
}) (map[string]resolvedFlagAliases, map[string]resolvedFlagAliases, error) {
|
||||
canonicalNodes := indexCanonicalNodes(roots)
|
||||
global := make(map[string]flagAliasMapping)
|
||||
@@ -170,6 +171,7 @@ func canonicalFlagSpellings(command Command) map[string]capturedFlagSpelling {
|
||||
|
||||
func discoverFlagContexts(rules []RoutingRule, roots map[string]*canonicalNode, aliases interface {
|
||||
Resolve(commandpath.Path) commandpath.Path
|
||||
ProperSourcePrefixes() []commandpath.Path
|
||||
}, canonicalNodes map[string]*canonicalNode) map[string]flagAliasContext {
|
||||
commandSources := make(map[string]struct{})
|
||||
prefixes := make(map[string]commandpath.Path)
|
||||
@@ -178,26 +180,20 @@ func discoverFlagContexts(rules []RoutingRule, roots map[string]*canonicalNode,
|
||||
commandSources[rule.record.sourcePath.Key()] = struct{}{}
|
||||
}
|
||||
}
|
||||
for _, rule := range rules {
|
||||
if rule.record.kind != commandAliasRule {
|
||||
for _, prefix := range aliases.ProperSourcePrefixes() {
|
||||
key := prefix.Key()
|
||||
if canonicalNodes[key] != nil {
|
||||
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 && acceptsOperandPrefix(trace.operandOwner.command, len(resolved)-len(trace.operandOwner.path))) {
|
||||
continue
|
||||
}
|
||||
prefixes[key] = prefix
|
||||
if _, exactAlias := commandSources[key]; exactAlias {
|
||||
continue
|
||||
}
|
||||
resolved := aliases.Resolve(prefix)
|
||||
trace := traceCanonicalPath(roots, resolved)
|
||||
if trace.exact != nil || (trace.operandOwner != nil && acceptsOperandPrefix(trace.operandOwner.command, len(resolved)-len(trace.operandOwner.path))) {
|
||||
continue
|
||||
}
|
||||
prefixes[key] = prefix
|
||||
}
|
||||
contexts := make(map[string]flagAliasContext, len(prefixes))
|
||||
for key, path := range prefixes {
|
||||
|
||||
@@ -36,11 +36,8 @@ func routeHelpAliases(snapshot *routingSnapshot, route routeResult) []commandmet
|
||||
}
|
||||
bySource := make(map[string]commandmeta.CommandAlias)
|
||||
for _, prefix := range snapshot.aliases.ResolutionPaths(route.enteredPath) {
|
||||
for _, alias := range snapshot.commandAliases {
|
||||
source := commandpath.FromTokens(alias.Source)
|
||||
if len(source) <= len(prefix) || !source.HasPrefix(prefix) {
|
||||
continue
|
||||
}
|
||||
for _, alias := range snapshot.aliases.Descendants(prefix) {
|
||||
source := alias.Source
|
||||
suffix := source[len(prefix):]
|
||||
entered := append(route.enteredPath.Clone(), suffix...)
|
||||
target := snapshot.aliases.Resolve(entered)
|
||||
|
||||
@@ -182,3 +182,50 @@ func (resolver *Resolver) ResolveWithProvenance(path commandpath.Path) (commandp
|
||||
origins = nextOrigins
|
||||
}
|
||||
}
|
||||
|
||||
// ProperSourcePrefixes returns distinct nonempty proper prefixes of source paths.
|
||||
// Returned paths are owned by the caller.
|
||||
func (resolver *Resolver) ProperSourcePrefixes() []commandpath.Path {
|
||||
if resolver == nil {
|
||||
return nil
|
||||
}
|
||||
var paths []commandpath.Path
|
||||
for index := 1; index < len(resolver.trie); index++ {
|
||||
node := &resolver.trie[index]
|
||||
if len(node.children) != 0 {
|
||||
paths = append(paths, node.path.Clone())
|
||||
}
|
||||
}
|
||||
return paths
|
||||
}
|
||||
|
||||
// Descendants returns owned mappings whose sources are strictly below prefix.
|
||||
func (resolver *Resolver) Descendants(prefix commandpath.Path) []Mapping {
|
||||
if resolver == nil {
|
||||
return nil
|
||||
}
|
||||
index := 0
|
||||
for _, component := range prefix {
|
||||
next, ok := resolver.trie[index].children[component]
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
index = next
|
||||
}
|
||||
var mappings []Mapping
|
||||
var visit func(int)
|
||||
visit = func(index int) {
|
||||
node := &resolver.trie[index]
|
||||
if node.mapping >= 0 {
|
||||
mapping := resolver.mappings[node.mapping]
|
||||
mappings = append(mappings, Mapping{Source: mapping.Source.Clone(), Target: mapping.Target.Clone()})
|
||||
}
|
||||
for _, child := range node.children {
|
||||
visit(child)
|
||||
}
|
||||
}
|
||||
for _, child := range resolver.trie[index].children {
|
||||
visit(child)
|
||||
}
|
||||
return mappings
|
||||
}
|
||||
|
||||
@@ -314,3 +314,34 @@ func (machine *Machine) classify(token string) symbol {
|
||||
}
|
||||
return symbol{kind: otherSymbol}
|
||||
}
|
||||
|
||||
func TestResolverSourcePrefixesAndDescendantsAreOwned(t *testing.T) {
|
||||
resolver, err := NewResolver([]Mapping{
|
||||
{Source: commandpath.Path{"a"}, Target: commandpath.Path{"root"}},
|
||||
{Source: commandpath.Path{"a", "b", "one"}, Target: commandpath.Path{"root", "one"}},
|
||||
{Source: commandpath.Path{"a", "b", "two"}, Target: commandpath.Path{"root", "two"}},
|
||||
{Source: commandpath.Path{"other"}, Target: commandpath.Path{"root"}},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
prefixes := resolver.ProperSourcePrefixes()
|
||||
if len(prefixes) != 2 || !prefixes[0].Equal(commandpath.Path{"a"}) || !prefixes[1].Equal(commandpath.Path{"a", "b"}) {
|
||||
t.Fatalf("prefixes = %#v", prefixes)
|
||||
}
|
||||
prefixes[0][0] = "mutated"
|
||||
descendants := resolver.Descendants(commandpath.Path{"a"})
|
||||
if len(descendants) != 2 {
|
||||
t.Fatalf("descendants = %#v", descendants)
|
||||
}
|
||||
for _, mapping := range descendants {
|
||||
if len(mapping.Source) != 3 || mapping.Source[0] != "a" {
|
||||
t.Fatalf("mapping = %#v", mapping)
|
||||
}
|
||||
}
|
||||
descendants[0].Source[0] = "mutated"
|
||||
descendants[0].Target[0] = "mutated"
|
||||
if !resolver.Resolve(commandpath.Path{"a", "b", "one"}).Equal(commandpath.Path{"root", "one"}) || len(resolver.Descendants(commandpath.Path{"missing"})) != 0 {
|
||||
t.Fatal("resolver was mutated or missing branch returned mappings")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user