diff --git a/README.md b/README.md index f3c20ff..2a10eb2 100644 --- a/README.md +++ b/README.md @@ -833,8 +833,8 @@ hclexp locate -dump prod/eu \ # Ad-hoc layer dirs or files, before a manifest exists (no placement info) hclexp locate -layer roles/shared,roles/ops/prod events -# CI guard: every object defined at more than one site, even -# across layers that never co-compose into one stack +# Resolve every manifest composition and find repeated definitions that +# produce the same object, including copies in mutually exclusive layers hclexp locate -manifest manifest.hcl -layer-root ./schema -duplicates ``` @@ -889,21 +889,31 @@ identity. A valid query with no matches succeeds and JSON includes `database.table`; column patterns match bare, `table.column`, or `database.table.column`. - `-format` — `text` (default) or `json` (a `{"patterns": [...], - "objects": [...]}` / `{"duplicates": [...]}` document with per-site + "objects": [...]}` or duplicate-classification document with `duplicates` + `variants`, and `collisions`, plus per-site file/line/layer/markers/placements, or a column-selector document containing `table_patterns`, `column_patterns`, and `columns`; every column has its matching `models`) - `-duplicates` — takes no name argument and requires `-manifest` or - `-layer` (mutually exclusive with `-dump` and column selectors); lists every object defined - at more than one site and exits non-zero when any is found. Patch sites, - `override = true` redeclarations, and declarations carrying `extend` are - refinements rather than definitions and do not count. Abstract declarations - do count: copying the same abstract schema is still duplication. Reported - groups retain every site, including refinements. + `-layer` (mutually exclusive with `-dump` and column selectors). It finds + names with two or more full-object declarations, resolves every manifest + `(role, env)` in parallel, and compares the resulting objects with the same + semantic diff used elsewhere. `duplicates` contains equal resolved shapes; + `variants` contains repeated names whose shapes differ. Each + `resolved_variants` group lists the models producing that shape and the + effective source definitions, so one shared declaration used by many models + is not mistaken for duplication. A same-stack undeclared replacement cannot + be resolved for comparison, so it is reported separately under `collisions`, + with the affected stacks in `collision_in`. `override = true` counts as a + full declaration and therefore cannot hide an identical copy in another + composition. Patch and `extend` sites remain refinements. The comma-separated + `-layer` list is one composition. Interactive terminal runs print parallel + loading progress to stderr; redirected stderr stays clean. Exit codes: `0` on success (including an empty column query), `1` when any -positional object pattern matches nothing, `-duplicates` finds any, or an -input cannot be read/parsed; `2` on usage errors. +positional object pattern matches nothing, `-duplicates` finds a real +duplicate or declaration collision, or an input cannot be read/parsed; +genuinely different variants do not fail the guard. Usage errors exit `2`. ## Browse the schema in a web UI diff --git a/cmd/hclexp/locate.go b/cmd/hclexp/locate.go index 4b7da13..f537e0a 100644 --- a/cmd/hclexp/locate.go +++ b/cmd/hclexp/locate.go @@ -10,9 +10,9 @@ import ( "path/filepath" "sort" "strings" - "sync" hclload "github.com/posthog/chschema/internal/loader/hcl" + "golang.org/x/term" ) // locateStack is one (role, env) deployment from the manifest and its @@ -28,6 +28,27 @@ type locatePlacement struct { Env string `json:"env"` } +type locateCompositionRef struct { + Source string `json:"source"` + Role string `json:"role,omitempty"` + Env string `json:"env,omitempty"` + Layers []string `json:"layers"` +} + +type locateDefinitionRef struct { + File string `json:"file"` + Line int `json:"line"` +} + +// locateResolvedVariant is one semantic object shape and every resolved +// composition that produces it. Definitions names the effective full-object +// declaration behind those models, deduplicated when one shared declaration +// is reused by several environments. +type locateResolvedVariant struct { + Models []locateModelRef `json:"models"` + Definitions []locateDefinitionRef `json:"definitions"` +} + // locateDecl is one declaration site plus its derived placements: the // (role, env) stacks whose layer lists include the declaring layer. type locateDecl struct { @@ -82,21 +103,27 @@ type locateColumnDoc struct { // whose extend attribute names this one, whether or not they matched the // query themselves. type locateObject struct { - Database string `json:"database,omitempty"` - Name string `json:"name"` - Types []string `json:"types"` - ExtendedBy []string `json:"extended_by,omitempty"` - Declarations []locateDecl `json:"declarations"` - Dumps []locateDump `json:"dumps,omitempty"` + Database string `json:"database,omitempty"` + Name string `json:"name"` + Types []string `json:"types"` + ExtendedBy []string `json:"extended_by,omitempty"` + Declarations []locateDecl `json:"declarations"` + Dumps []locateDump `json:"dumps,omitempty"` + CollisionIn []locateCompositionRef `json:"collision_in,omitempty"` + ResolvedVariants []locateResolvedVariant `json:"resolved_variants,omitempty"` } // locateDoc is the `locate -format json` document. Objects carries the -// pattern query's results; Duplicates carries -duplicates mode's. Exactly -// one of the two is populated (non-nil, so JSON emits [] rather than null). +// pattern query's results. In -duplicates mode, Duplicates contains repeated +// definitions that produce an equal resolved object, Variants contains repeated +// names whose resolved shapes remain distinct, and Collisions contains invalid +// same-composition redeclarations that cannot be resolved for comparison. type locateDoc struct { Patterns []string `json:"patterns,omitempty"` Objects []locateObject `json:"objects,omitempty"` Duplicates []locateObject `json:"duplicates,omitempty"` + Variants []locateObject `json:"variants,omitempty"` + Collisions []locateObject `json:"collisions,omitempty"` } // locateFlagsError reports the usage error in a locate invocation, if any. @@ -199,6 +226,18 @@ func parseManifestAllEnvs(path string) ([]locateStack, error) { // duplicates = true the patterns are ignored and the doc's Duplicates side // is populated instead. func buildLocateDoc(stacks []locateStack, layerRoot string, extraLayers []string, dumpDir string, patterns []string, duplicates bool) (locateDoc, []string, error) { + return buildLocateDocWithProgress(stacks, layerRoot, extraLayers, dumpDir, patterns, duplicates, nil) +} + +func buildLocateDocWithProgress( + stacks []locateStack, + layerRoot string, + extraLayers []string, + dumpDir string, + patterns []string, + duplicates bool, + status io.Writer, +) (locateDoc, []string, error) { // Index which (role, env) stacks include each resolved layer, keeping // first-seen layer order so output is stable. stacksByLayer, layerOrder := indexLocateLayers(stacks, layerRoot, extraLayers) @@ -208,11 +247,13 @@ func buildLocateDoc(stacks []locateStack, layerRoot string, extraLayers []string // attribution. var decls []hclload.Declaration layerByFile := map[string]string{} + filesByLayer := map[string][]string{} for _, layer := range layerOrder { files, err := hclload.LayerFiles(layer) if err != nil { return locateDoc{}, nil, err } + filesByLayer[layer] = append([]string(nil), files...) for _, file := range files { if _, ok := layerByFile[file]; ok { continue @@ -232,14 +273,48 @@ func buildLocateDoc(stacks []locateStack, layerRoot string, extraLayers []string extendedBy := extendedByIndex(decls) if duplicates { - doc := locateDoc{Duplicates: []locateObject{}} - for _, g := range hclload.FindDuplicates(decls) { - obj := locateObject{Database: g.Database, Name: g.Name, ExtendedBy: extendedBy[[2]string{g.Database, g.Name}]} - for _, d := range g.Declarations { + doc := locateDoc{Duplicates: []locateObject{}, Variants: []locateObject{}, Collisions: []locateObject{}} + scopes := locateDuplicateScopes(stacks, layerRoot, extraLayers) + candidates := hclload.FindDuplicateCandidates(decls) + blockedScopes := locateCollisionScopes(candidates, scopes, filesByLayer) + models := map[int]locateModel{} + if len(candidates) > 0 { + var err error + models, err = loadLocateDuplicateModels( + scopes, blockedScopes, locateCandidatesHaveAbstracts(candidates), + parallelLoadProgress{Writer: status, Prefix: "locate", Label: "composition models"}, + ) + if err != nil { + return locateDoc{}, nil, err + } + } + for _, candidate := range candidates { + collisionIn := collisionCompositions(candidate, scopes, filesByLayer) + resolvedVariants := resolvedLocateVariants(candidate, scopes, models, filesByLayer) + if len(collisionIn) == 0 && resolvedVariantDefinitionCount(resolvedVariants) < 2 { + // A base and its valid replacement may occur in one stack, leaving + // only the replacement as an effective definition. There is no pair + // of independently composed objects to classify in that case. + continue + } + isDuplicate := variantsContainDuplicate(resolvedVariants) + obj := locateObject{ + Database: candidate.Database, Name: candidate.Name, + ExtendedBy: extendedBy[[2]string{candidate.Database, candidate.Name}], + CollisionIn: collisionIn, + ResolvedVariants: resolvedVariants, + } + for _, d := range candidate.Declarations { obj.Types = appendUniqueString(obj.Types, d.ObjectType) obj.Declarations = append(obj.Declarations, toLocateDecl(d, layerByFile, stacksByLayer)) } - doc.Duplicates = append(doc.Duplicates, obj) + if isDuplicate { + doc.Duplicates = append(doc.Duplicates, obj) + } else if len(collisionIn) > 0 { + doc.Collisions = append(doc.Collisions, obj) + } else { + doc.Variants = append(doc.Variants, obj) + } } return doc, nil, nil } @@ -314,6 +389,332 @@ func buildLocateDoc(stacks []locateStack, layerRoot string, extraLayers []string return doc, unmatched, nil } +type locateDuplicateScope struct { + Ref locateCompositionRef + Resolved []string +} + +func locateDuplicateScopes( + stacks []locateStack, + layerRoot string, + extraLayers []string, +) []locateDuplicateScope { + scopes := make([]locateDuplicateScope, 0, len(stacks)+1) + for _, stack := range stacks { + resolved := make([]string, len(stack.Layers)) + for i, layer := range stack.Layers { + resolved[i] = filepath.Join(layerRoot, layer) + } + scopes = append(scopes, locateDuplicateScope{ + Ref: locateCompositionRef{ + Source: "manifest", Role: stack.Role, Env: stack.Env, + Layers: append([]string(nil), stack.Layers...), + }, + Resolved: resolved, + }) + } + if len(extraLayers) > 0 { + resolved := make([]string, len(extraLayers)) + for i, layer := range extraLayers { + resolved[i] = filepath.Clean(layer) + } + scopes = append(scopes, locateDuplicateScope{ + Ref: locateCompositionRef{Source: "layer", Layers: append([]string(nil), extraLayers...)}, + Resolved: resolved, + }) + } + return scopes +} + +func collisionCompositions( + candidate hclload.DuplicateGroup, + scopes []locateDuplicateScope, + filesByLayer map[string][]string, +) []locateCompositionRef { + var collisions []locateCompositionRef + for _, scope := range scopes { + if locateScopeCollides(candidate, scope, filesByLayer) { + collisions = append(collisions, scope.Ref) + } + } + return collisions +} + +func locateCollisionScopes( + candidates []hclload.DuplicateGroup, + scopes []locateDuplicateScope, + filesByLayer map[string][]string, +) map[int]bool { + blocked := map[int]bool{} + for i, scope := range scopes { + for _, candidate := range candidates { + if locateScopeCollides(candidate, scope, filesByLayer) { + blocked[i] = true + break + } + } + } + return blocked +} + +func locateScopeCollides( + candidate hclload.DuplicateGroup, + scope locateDuplicateScope, + filesByLayer map[string][]string, +) bool { + defined := false + for _, declaration := range orderedLocateDeclarations(candidate, scope, filesByLayer) { + if !locateFullDefinition(declaration) { + continue + } + if defined && !declaration.Override { + return true + } + defined = true + } + return false +} + +func locateFullDefinition(declaration hclload.Declaration) bool { + return !declaration.Patch && declaration.Extends == "" +} + +func orderedLocateDeclarations( + candidate hclload.DuplicateGroup, + scope locateDuplicateScope, + filesByLayer map[string][]string, +) []hclload.Declaration { + var ordered []hclload.Declaration + seenFiles := map[string]bool{} + for _, layer := range scope.Resolved { + for _, file := range filesByLayer[layer] { + if seenFiles[file] { + continue + } + seenFiles[file] = true + for _, declaration := range candidate.Declarations { + if declaration.File == file { + ordered = append(ordered, declaration) + } + } + } + } + return ordered +} + +func effectiveLocateDefinition( + candidate hclload.DuplicateGroup, + scope locateDuplicateScope, + filesByLayer map[string][]string, +) (hclload.Declaration, bool) { + var effective hclload.Declaration + found := false + for _, declaration := range orderedLocateDeclarations(candidate, scope, filesByLayer) { + if !locateFullDefinition(declaration) { + continue + } + effective = declaration + found = true + } + return effective, found +} + +func locateCandidatesHaveAbstracts(candidates []hclload.DuplicateGroup) bool { + for _, candidate := range candidates { + for _, declaration := range candidate.Declarations { + if locateFullDefinition(declaration) && declaration.Abstract { + return true + } + } + } + return false +} + +func loadLocateDuplicateModels( + scopes []locateDuplicateScope, + blocked map[int]bool, + preserveRaw bool, + progress parallelLoadProgress, +) (map[int]locateModel, error) { + var tasks []locateModelTask + var scopeIndexes []int + for i, scope := range scopes { + if blocked[i] { + continue + } + tasks = append(tasks, locateModelTask{ + Ref: locateModelRef{ + Source: scope.Ref.Source, Role: scope.Ref.Role, Env: scope.Ref.Env, + Layers: append([]string(nil), scope.Ref.Layers...), + }, + Resolved: append([]string(nil), scope.Resolved...), + }) + scopeIndexes = append(scopeIndexes, i) + } + loader := loadLocateModel + if preserveRaw { + loader = loadLocateDuplicateModel + } + loaded, err := loadLocateModelTasksWithProgress(tasks, locateLoadParallelism, progress, loader) + if err != nil { + return nil, err + } + models := make(map[int]locateModel, len(loaded)) + for i, model := range loaded { + models[scopeIndexes[i]] = model + } + return models, nil +} + +func loadLocateDuplicateModel(task locateModelTask) (locateModel, error) { + raw, err := hclload.LoadLayers(task.Resolved) + if err != nil { + return locateModel{}, fmt.Errorf("load %s: %w", locateModelLabel(task.Ref), err) + } + model, err := loadLocateModel(task) + if err != nil { + return locateModel{}, err + } + model.RawSchema = raw + return model, nil +} + +func resolvedLocateVariants( + candidate hclload.DuplicateGroup, + scopes []locateDuplicateScope, + models map[int]locateModel, + filesByLayer map[string][]string, +) []locateResolvedVariant { + type variant struct { + locateResolvedVariant + schema *hclload.Schema + } + var groups []variant + for i, scope := range scopes { + model, ok := models[i] + if !ok { + continue + } + definition, ok := effectiveLocateDefinition(candidate, scope, filesByLayer) + if !ok { + continue + } + objectSchema, ok := scopeLocateObject(model.Schema, candidate.Database, candidate.Name) + if !ok && model.RawSchema != nil { + // Abstract definitions intentionally disappear from the final model. + // Compare their merged pre-resolution shape so copied templates remain + // detectable without weakening the semantic classifier. + objectSchema, ok = scopeLocateObject(model.RawSchema, candidate.Database, candidate.Name) + } + if !ok { + continue + } + definitionRef := locateDefinitionRef{File: definition.File, Line: definition.Line} + matched := false + for gi := range groups { + if !hclload.Diff(groups[gi].schema, objectSchema).IsEmpty() { + continue + } + groups[gi].Models = append(groups[gi].Models, model.Ref) + groups[gi].Definitions = appendUniqueDefinitionRef(groups[gi].Definitions, definitionRef) + matched = true + break + } + if !matched { + groups = append(groups, variant{ + locateResolvedVariant: locateResolvedVariant{ + Models: []locateModelRef{model.Ref}, + Definitions: []locateDefinitionRef{definitionRef}, + }, + schema: objectSchema, + }) + } + } + out := make([]locateResolvedVariant, len(groups)) + for i := range groups { + out[i] = groups[i].locateResolvedVariant + } + return out +} + +func scopeLocateObject(schema *hclload.Schema, database, name string) (*hclload.Schema, bool) { + if database == "" { + out := &hclload.Schema{} + for _, collection := range schema.NamedCollections { + if collection.Name == name { + out.NamedCollections = append(out.NamedCollections, collection) + } + } + return out, len(out.NamedCollections) > 0 + } + for _, databaseSpec := range schema.Databases { + if databaseSpec.Name != database { + continue + } + objectDB := hclload.DatabaseSpec{Name: database} + for _, table := range databaseSpec.Tables { + if table.Name == name { + objectDB.Tables = append(objectDB.Tables, table) + } + } + for _, view := range databaseSpec.MaterializedViews { + if view.Name == name { + objectDB.MaterializedViews = append(objectDB.MaterializedViews, view) + } + } + for _, view := range databaseSpec.Views { + if view.Name == name { + objectDB.Views = append(objectDB.Views, view) + } + } + for _, dictionary := range databaseSpec.Dictionaries { + if dictionary.Name == name { + objectDB.Dictionaries = append(objectDB.Dictionaries, dictionary) + } + } + for _, raw := range databaseSpec.Raws { + if raw.Name == name { + objectDB.Raws = append(objectDB.Raws, raw) + } + } + present := len(objectDB.Tables)+len(objectDB.MaterializedViews)+len(objectDB.Views)+ + len(objectDB.Dictionaries)+len(objectDB.Raws) > 0 + if present { + return &hclload.Schema{Databases: []hclload.DatabaseSpec{objectDB}}, true + } + break + } + return &hclload.Schema{}, false +} + +func appendUniqueDefinitionRef(refs []locateDefinitionRef, ref locateDefinitionRef) []locateDefinitionRef { + for _, existing := range refs { + if existing == ref { + return refs + } + } + return append(refs, ref) +} + +func variantsContainDuplicate(variants []locateResolvedVariant) bool { + for _, variant := range variants { + if len(variant.Definitions) >= 2 { + return true + } + } + return false +} + +func resolvedVariantDefinitionCount(variants []locateResolvedVariant) int { + seen := map[locateDefinitionRef]bool{} + for _, variant := range variants { + for _, definition := range variant.Definitions { + seen[definition] = true + } + } + return len(seen) +} + func indexLocateLayers(stacks []locateStack, layerRoot string, extraLayers []string) (map[string][]locatePlacement, []string) { stacksByLayer := map[string][]locatePlacement{} var layerOrder []string @@ -340,8 +741,9 @@ func indexLocateLayers(stacks []locateStack, layerRoot string, extraLayers []str } type locateModel struct { - Ref locateModelRef - Schema *hclload.Schema + Ref locateModelRef + Schema *hclload.Schema + RawSchema *hclload.Schema } type locateModelTask struct { @@ -488,41 +890,16 @@ type locateModelLoader func(locateModelTask) (locateModel, error) // the result. Thirty dump files therefore load concurrently, but arbitrarily // large directories cannot create an unbounded number of goroutines. func loadLocateModelTasks(tasks []locateModelTask, parallelism int, loader locateModelLoader) ([]locateModel, error) { - if len(tasks) == 0 { - return []locateModel{}, nil - } - if parallelism < 1 { - parallelism = 1 - } - if parallelism > len(tasks) { - parallelism = len(tasks) - } - - models := make([]locateModel, len(tasks)) - errs := make([]error, len(tasks)) - jobs := make(chan int, len(tasks)) - for i := range tasks { - jobs <- i - } - close(jobs) + return loadLocateModelTasksWithProgress(tasks, parallelism, parallelLoadProgress{}, loader) +} - var wg sync.WaitGroup - wg.Add(parallelism) - for range parallelism { - go func() { - defer wg.Done() - for i := range jobs { - models[i], errs[i] = loader(tasks[i]) - } - }() - } - wg.Wait() - for _, err := range errs { - if err != nil { - return nil, err - } - } - return models, nil +func loadLocateModelTasksWithProgress( + tasks []locateModelTask, + parallelism int, + progress parallelLoadProgress, + loader locateModelLoader, +) ([]locateModel, error) { + return loadInParallel(tasks, parallelism, progress, loader) } func matchesTablePatterns(patterns []string, database, table string) bool { @@ -683,23 +1060,71 @@ func renderLocateColumnText(w io.Writer, doc locateColumnDoc) { } func renderDuplicatesText(w io.Writer, doc locateDoc) { - if len(doc.Duplicates) == 0 { - fmt.Fprintln(w, "no duplicate declarations") + if len(doc.Duplicates) == 0 && len(doc.Variants) == 0 && len(doc.Collisions) == 0 { + fmt.Fprintln(w, "no repeated object definitions") return } for _, o := range doc.Duplicates { fmt.Fprintf(w, "duplicate %s %s (%d sites)\n", strings.Join(o.Types, "|"), qualifiedName(o.Database, o.Name), len(o.Declarations)) + renderCollisionCompositions(w, o.CollisionIn) + renderResolvedVariants(w, o.ResolvedVariants) + renderLocateSites(w, o) + } + for _, o := range doc.Collisions { + fmt.Fprintf(w, "declaration collision %s %s (%d sites)\n", strings.Join(o.Types, "|"), qualifiedName(o.Database, o.Name), len(o.Declarations)) + renderCollisionCompositions(w, o.CollisionIn) + renderResolvedVariants(w, o.ResolvedVariants) + renderLocateSites(w, o) + } + for _, o := range doc.Variants { + fmt.Fprintf(w, "distinct variants %s %s (%d sites)\n", strings.Join(o.Types, "|"), qualifiedName(o.Database, o.Name), len(o.Declarations)) + renderResolvedVariants(w, o.ResolvedVariants) renderLocateSites(w, o) } } +func renderCollisionCompositions(w io.Writer, compositions []locateCompositionRef) { + if len(compositions) == 0 { + return + } + labels := make([]string, 0, len(compositions)) + for _, composition := range compositions { + if composition.Source == "manifest" { + labels = append(labels, fmt.Sprintf("(%s, %s)", composition.Role, composition.Env)) + continue + } + labels = append(labels, "layer stack "+strings.Join(composition.Layers, ",")) + } + fmt.Fprintf(w, " declaration collision in: %s\n", strings.Join(labels, ", ")) +} + +func renderResolvedVariants(w io.Writer, variants []locateResolvedVariant) { + for i, variant := range variants { + label := fmt.Sprintf("resolved variant %d", i+1) + if len(variant.Definitions) >= 2 { + label += fmt.Sprintf(" (duplicate across %d definitions)", len(variant.Definitions)) + } + fmt.Fprintf(w, " %s:\n", label) + for _, definition := range variant.Definitions { + fmt.Fprintf(w, " definition: %s:%d\n", definition.File, definition.Line) + } + for _, model := range variant.Models { + if model.Source == "manifest" { + fmt.Fprintf(w, " (%s, %s) layers: %s\n", model.Role, model.Env, strings.Join(model.Layers, ",")) + continue + } + fmt.Fprintf(w, " layer stack: %s\n", strings.Join(model.Layers, ",")) + } + } +} + // runLocate answers "where is object X declared?" across a manifest's layer // tree, ad-hoc -layer entries, and/or a dump directory, or (with // -duplicates) audits the layer tree for objects defined at more than one // site. Column selector mode loads resolved models before searching and // returns success with an empty result. Object pattern mode exits 1 when any -// pattern matches nothing; duplicate mode exits 1 when duplicates exist. -// Usage errors exit 2. +// pattern matches nothing; duplicate mode exits 1 when semantic duplicates or +// invalid declaration collisions exist. Usage errors exit 2. func runLocate(args []string) { fs := flag.NewFlagSet("hclexp locate", flag.ExitOnError) manifestFlag := fs.String("manifest", "", "HCL manifest: object mode scans its layers; column mode resolves every (role, env) stack") @@ -707,7 +1132,7 @@ func runLocate(args []string) { layersFlag := fs.String("layer", "", "comma-separated ad-hoc layer dirs or .hcl files; column mode resolves them in order as one model") dumpFlag := fs.String("dump", "", "directory of per-node .hcl dumps; column mode resolves node models concurrently") formatFlag := fs.String("format", "text", "output format: text (default) or json") - duplicatesFlag := fs.Bool("duplicates", false, "list every object defined at more than one site (patch/override/extend sites refine; abstracts define); takes no name argument") + duplicatesFlag := fs.Bool("duplicates", false, "compare repeated full-object definitions across resolved compositions; takes no name argument") tablesFlag := fs.String("tables", "", "comma-separated table names or globs for column lookup; requires -columns and no name argument") columnsFlag := fs.String("columns", "", "comma-separated column names or globs to locate within -tables") _ = fs.Parse(args) @@ -753,7 +1178,13 @@ func runLocate(args []string) { return } - doc, unmatched, err := buildLocateDoc(stacks, *layerRootFlag, splitList(*layersFlag), *dumpFlag, patterns, *duplicatesFlag) + var status io.Writer + if *duplicatesFlag && term.IsTerminal(int(os.Stderr.Fd())) { + status = os.Stderr + } + doc, unmatched, err := buildLocateDocWithProgress( + stacks, *layerRootFlag, splitList(*layersFlag), *dumpFlag, patterns, *duplicatesFlag, status, + ) if err != nil { slog.Error("locate failed", "err", err) os.Exit(1) @@ -773,7 +1204,7 @@ func runLocate(args []string) { } if *duplicatesFlag { - if len(doc.Duplicates) > 0 { + if len(doc.Duplicates) > 0 || len(doc.Collisions) > 0 { os.Exit(1) } return diff --git a/cmd/hclexp/locate_test.go b/cmd/hclexp/locate_test.go index f76f3af..66c0968 100644 --- a/cmd/hclexp/locate_test.go +++ b/cmd/hclexp/locate_test.go @@ -344,15 +344,20 @@ func TestBuildLocateDocExtraLayers(t *testing.T) { // works before any manifest exists. func TestBuildLocateDocDuplicatesFromExtraLayers(t *testing.T) { root := locateTree(t) + shared := filepath.Join(root, "shared") + aux := filepath.Join(root, "aux") doc, _, err := buildLocateDoc(nil, "", []string{ - filepath.Join(root, "shared"), - filepath.Join(root, "aux"), + shared, + aux, }, "", nil, true) require.NoError(t, err) - require.Len(t, doc.Duplicates, 1) - assert.Equal(t, "person", doc.Duplicates[0].Name) + assert.Empty(t, doc.Duplicates) + assert.Empty(t, doc.Variants) + require.Len(t, doc.Collisions, 1) + assert.Equal(t, "person", doc.Collisions[0].Name) + assert.Equal(t, []locateCompositionRef{{Source: "layer", Layers: []string{shared, aux}}}, doc.Collisions[0].CollisionIn) } // An extended object cross-links its children even when the pattern matches @@ -381,13 +386,209 @@ func TestBuildLocateDocDuplicates(t *testing.T) { // person is declared plainly in shared and aux; events_base (abstract) + // events (extend child) must not be flagged. + assert.Empty(t, doc.Duplicates) + assert.Empty(t, doc.Variants) + require.Len(t, doc.Collisions, 1) + collision := doc.Collisions[0] + assert.Equal(t, "posthog", collision.Database) + assert.Equal(t, "person", collision.Name) + require.Len(t, collision.Declarations, 2) + assert.Equal(t, filepath.Join(root, "aux", "dup.hcl"), collision.Declarations[0].File) + assert.Equal(t, filepath.Join(root, "shared", "base.hcl"), collision.Declarations[1].File) + assert.Equal(t, []locateCompositionRef{{ + Source: "manifest", Role: "aux", Env: "prod-us", Layers: []string{"shared", "aux"}, + }}, collision.CollisionIn) + require.Len(t, collision.ResolvedVariants, 1) + assert.Len(t, collision.ResolvedVariants[0].Models, 2, "the two valid ingestion compositions still resolve") + assert.Len(t, collision.ResolvedVariants[0].Definitions, 1, "both valid compositions reuse the shared declaration") +} + +func TestBuildLocateDocClassifiesDifferentResolvedObjectsAsVariants(t *testing.T) { + root := t.TempDir() + writeFileT(t, filepath.Join(root, "manifest.hcl"), ` +role "data" { + env "dev" { layers = ["dev"] } + env "local" { layers = ["local"] } +} +`) + definition := func(kind string) string { + return `database "posthog" { + table "events" { + column "source" { + type = "String" + default = "'` + kind + `'" + } + engine "log" {} + } +} +` + } + writeFileT(t, filepath.Join(root, "dev", "events.hcl"), definition("dev")) + writeFileT(t, filepath.Join(root, "local", "events.hcl"), definition("local")) + stacks, err := parseManifestAllEnvs(filepath.Join(root, "manifest.hcl")) + require.NoError(t, err) + + doc, _, err := buildLocateDoc(stacks, root, nil, "", nil, true) + require.NoError(t, err) + assert.Empty(t, doc.Duplicates) + require.Len(t, doc.Variants, 1) + variant := doc.Variants[0] + assert.Equal(t, "events", variant.Name) + assert.Empty(t, variant.CollisionIn) + require.Len(t, variant.Declarations, 2) + require.Len(t, variant.ResolvedVariants, 2) + assert.Len(t, variant.ResolvedVariants[0].Definitions, 1) + assert.Len(t, variant.ResolvedVariants[1].Definitions, 1) + + var text bytes.Buffer + renderDuplicatesText(&text, doc) + assert.Contains(t, text.String(), "distinct variants table posthog.events (2 sites)") +} + +func TestBuildLocateDocSemanticDuplicatesCoverEveryManagedObjectKind(t *testing.T) { + root := t.TempDir() + writeFileT(t, filepath.Join(root, "manifest.hcl"), ` +role "data" { + env "left" { layers = ["left"] } + env "right" { layers = ["right"] } +} +`) + writeFileT(t, filepath.Join(root, "left", "schema.hcl"), ` +named_collection "warehouse" { + param "host" { value = "warehouse.internal" } +} +database "analytics" { + table "events" { + column "id" { type = "UInt64" } + engine "log" {} + } + materialized_view "events_mv" { + to_table = "analytics.events" + query = "SELECT id FROM analytics.events" + column "id" { type = "UInt64" } + } + view "environment" { + query = "SELECT 'production' AS name" + } + dictionary "labels" { + primary_key = ["id"] + attribute "id" { type = "UInt64" } + attribute "label" { type = "String" } + source "null" {} + layout "flat" {} + } + raw "view" "legacy_environment" { + sql = "CREATE VIEW analytics.legacy_environment AS SELECT 'production'" + } +} +`) + writeFileT(t, filepath.Join(root, "right", "schema.hcl"), ` +named_collection "warehouse" { + override = true + param "host" { value = "warehouse.internal" } +} +database "analytics" { + table "events" { + override = true + column "id" { type = "UInt64" } + engine "log" {} + } + materialized_view "events_mv" { + override = true + to_table = "analytics.events" + query = "SELECT id FROM analytics.events" + column "id" { type = "UInt64" } + } + view "environment" { + override = true + query = "SELECT 'production' AS name" + } + dictionary "labels" { + override = true + primary_key = ["id"] + attribute "id" { type = "UInt64" } + attribute "label" { type = "String" } + source "null" {} + layout "flat" {} + } + raw "view" "legacy_environment" { + override = true + sql = "CREATE VIEW analytics.legacy_environment AS SELECT 'production'" + } +} +`) + stacks, err := parseManifestAllEnvs(filepath.Join(root, "manifest.hcl")) + require.NoError(t, err) + + var status bytes.Buffer + doc, _, err := buildLocateDocWithProgress(stacks, root, nil, "", nil, true, &status) + require.NoError(t, err) + assert.Contains(t, status.String(), "locate: loading 2 composition models in parallel (2 workers)") + assert.Contains(t, status.String(), "locate: loaded 2 composition models") + assert.Empty(t, doc.Variants) + require.Len(t, doc.Duplicates, 6) + for _, duplicate := range doc.Duplicates { + require.Len(t, duplicate.ResolvedVariants, 1, duplicate.Name) + assert.Len(t, duplicate.ResolvedVariants[0].Models, 2, duplicate.Name) + assert.Len(t, duplicate.ResolvedVariants[0].Definitions, 2, duplicate.Name) + } +} + +func TestBuildLocateDocComparesAbstractDefinitionsBeforeTheyAreDropped(t *testing.T) { + root := t.TempDir() + writeFileT(t, filepath.Join(root, "manifest.hcl"), ` +role "data" { + env "left" { layers = ["left"] } + env "right" { layers = ["right"] } +} +`) + definition := `database "analytics" { + table "event_base" { + abstract = true + column "id" { type = "UInt64" } + } +} +` + writeFileT(t, filepath.Join(root, "left", "base.hcl"), definition) + writeFileT(t, filepath.Join(root, "right", "base.hcl"), definition) + stacks, err := parseManifestAllEnvs(filepath.Join(root, "manifest.hcl")) + require.NoError(t, err) + + doc, _, err := buildLocateDoc(stacks, root, nil, "", nil, true) + require.NoError(t, err) require.Len(t, doc.Duplicates, 1) - dup := doc.Duplicates[0] - assert.Equal(t, "posthog", dup.Database) - assert.Equal(t, "person", dup.Name) - require.Len(t, dup.Declarations, 2) - assert.Equal(t, filepath.Join(root, "aux", "dup.hcl"), dup.Declarations[0].File) - assert.Equal(t, filepath.Join(root, "shared", "base.hcl"), dup.Declarations[1].File) + assert.Equal(t, "event_base", doc.Duplicates[0].Name) + require.Len(t, doc.Duplicates[0].ResolvedVariants, 1) + assert.Len(t, doc.Duplicates[0].ResolvedVariants[0].Definitions, 2) +} + +func TestBuildLocateDocDoesNotClassifyAReplacedDefinitionFromOneModel(t *testing.T) { + root := t.TempDir() + writeFileT(t, filepath.Join(root, "base", "events.hcl"), ` +database "analytics" { + table "events" { + column "id" { type = "UInt64" } + engine "log" {} + } +} +`) + writeFileT(t, filepath.Join(root, "env", "events.hcl"), ` +database "analytics" { + table "events" { + override = true + column "id" { type = "String" } + engine "log" {} + } +} +`) + + doc, _, err := buildLocateDoc(nil, "", []string{ + filepath.Join(root, "base"), filepath.Join(root, "env"), + }, "", nil, true) + require.NoError(t, err) + assert.Empty(t, doc.Duplicates) + assert.Empty(t, doc.Variants) + assert.Empty(t, doc.Collisions) } func TestRenderLocateText(t *testing.T) { @@ -411,7 +612,7 @@ func TestRenderLocateText(t *testing.T) { dupDoc, _, err := buildLocateDoc(stacks, root, nil, "", nil, true) require.NoError(t, err) renderDuplicatesText(&dupBuf, dupDoc) - assert.Contains(t, dupBuf.String(), "duplicate table posthog.person") + assert.Contains(t, dupBuf.String(), "declaration collision table posthog.person") assert.Contains(t, dupBuf.String(), "base.hcl:7") } @@ -601,6 +802,96 @@ database "posthog" { } } +func TestLocateDuplicatesSemanticClassificationEndToEnd(t *testing.T) { + root := t.TempDir() + manifest := filepath.Join(root, "manifest.hcl") + writeFileT(t, filepath.Join(root, "dev", "events.hcl"), ` +database "posthog" { + table "events" { + column "id" { type = "UInt64" } + engine "log" {} + } +} +`) + writeFileT(t, filepath.Join(root, "local", "events.hcl"), ` +database "posthog" { + table "events" { + override = true + column "id" { type = "String" } + engine "log" {} + } +} +`) + writeFileT(t, manifest, ` +role "data" { + env "dev" { layers = ["dev"] } + env "local" { layers = ["local"] } +} +`) + + output, err := runLocateColumnsCLI(t, "-manifest", manifest, "-layer-root", root, "-duplicates", "-format", "json") + require.NoError(t, err, string(output)) + var disjoint locateDoc + require.NoError(t, json.Unmarshal(output, &disjoint), string(output)) + assert.Empty(t, disjoint.Duplicates) + require.Len(t, disjoint.Variants, 1) + assert.Equal(t, "events", disjoint.Variants[0].Name) + assert.Len(t, disjoint.Variants[0].ResolvedVariants, 2) + + // An override in a mutually exclusive layer must not hide an identical + // full definition. Once both resolved objects are equal, the command + // classifies them as a duplicate even though they never co-compose. + writeFileT(t, filepath.Join(root, "local", "events.hcl"), ` +database "posthog" { + table "events" { + override = true + column "id" { type = "UInt64" } + engine "log" {} + } +} +`) + output, err = runLocateColumnsCLI(t, "-manifest", manifest, "-layer-root", root, "-duplicates", "-format", "json") + require.Error(t, err, string(output)) + exitErr, ok := err.(*exec.ExitError) + require.True(t, ok) + assert.Equal(t, 1, exitErr.ExitCode()) + var composed locateDoc + require.NoError(t, json.Unmarshal(output, &composed), string(output)) + require.Len(t, composed.Duplicates, 1) + assert.Empty(t, composed.Variants) + assert.Empty(t, composed.Duplicates[0].CollisionIn) + require.Len(t, composed.Duplicates[0].ResolvedVariants, 1) + assert.Len(t, composed.Duplicates[0].ResolvedVariants[0].Models, 2) + assert.Len(t, composed.Duplicates[0].ResolvedVariants[0].Definitions, 2) + + writeFileT(t, filepath.Join(root, "local", "events.hcl"), ` +database "posthog" { + table "events" { + column "id" { type = "String" } + engine "log" {} + } +} +`) + writeFileT(t, manifest, ` +role "data" { + env "dev" { layers = ["dev", "local"] } +} +`) + output, err = runLocateColumnsCLI(t, "-manifest", manifest, "-layer-root", root, "-duplicates", "-format", "json") + require.Error(t, err, string(output)) + exitErr, ok = err.(*exec.ExitError) + require.True(t, ok) + assert.Equal(t, 1, exitErr.ExitCode()) + var collision locateDoc + require.NoError(t, json.Unmarshal(output, &collision), string(output)) + assert.Empty(t, collision.Duplicates) + assert.Empty(t, collision.Variants) + require.Len(t, collision.Collisions, 1) + assert.Equal(t, []locateCompositionRef{{ + Source: "manifest", Role: "data", Env: "dev", Layers: []string{"dev", "local"}, + }}, collision.Collisions[0].CollisionIn) +} + func runLocateColumnsCLI(t *testing.T, args ...string) ([]byte, error) { t.Helper() commandArgs := append([]string{"-test.run=^TestLocateColumnsCLIProcess$", "--"}, args...) diff --git a/docs/FAQ.md b/docs/FAQ.md index 55af33a..bb370fc 100644 --- a/docs/FAQ.md +++ b/docs/FAQ.md @@ -260,6 +260,12 @@ Without `override = true`, the cross-layer collision is an error. With it, the dev-layer definition wins. Raw objects collide by `(kind, name)`; the other database objects and named collections collide by name. +`hclexp locate -duplicates` still includes overrides as full-object +definitions when it audits a manifest. It resolves every composition and +compares the resulting objects: a genuinely different override is reported as +a variant, while an identical copy is a duplicate and exits 1. This keeps +`override` from becoming a way to silence the once-only guard. + ## What's the difference between `extend` and `patch_table`? They answer different questions. `extend` says *"these are **different diff --git a/docs/README.hcl.md b/docs/README.hcl.md index 1e5d7d4..e11cc92 100644 --- a/docs/README.hcl.md +++ b/docs/README.hcl.md @@ -671,12 +671,12 @@ which question you are asking: follows the same rule for every managed object kind. The declaration count is not cosmetic: `hclexp locate -duplicates` (the -once-only CI guard) counts definition sites, while treating patch, -`override`, and `extend` sites as sanctioned refinements. This permits a -same-name extend child per disjoint environment stack: the shared column set -stays on the abstract parent, and each child carries only its local engine, -ordering, or additive columns. Two copied abstracts still count as two -definitions and are flagged. +once-only CI guard) finds repeated full-object declarations and then compares +their resolved objects. `override` is included in that audit: it is honest when +the resulting object is genuinely different, but it cannot hide an identical +copy in another composition. Patch and `extend` sites are refinements rather +than full definitions. Two mutually exclusive layers can therefore still hold +a real duplicate when their composed objects are equal. The other asymmetry worth internalizing: **`extend`'s `settings` replace wholesale, `patch_table`'s `settings` merge (patch wins per key)**. An @@ -1123,7 +1123,7 @@ hclexp locate -dump ./prod-us \ # Ad-hoc layer dirs or .hcl files, no manifest required (no placements) hclexp locate -layer ./schema/shared,./schema/ingestion events -# CI guard: any object defined at more than one site exits 1 +# Classify repeated definitions by their resolved semantic shape hclexp locate -manifest manifest.hcl -layer-root ./schema -duplicates ``` @@ -1175,19 +1175,40 @@ Objects are grouped by `(database, name)` — the namespace ClickHouse object types share — so a stray `view "events"` next to a `table "events"` shows up as one entry with both types. -`-duplicates` (no name argument; requires `-manifest` or `-layer`) audits -the once-only discipline: `load`/compose only reject a redeclaration when -the two layers meet in one stack, so two layers that never co-compose can -silently hold divergent copies of the same object. A definition site is a -declaration that is not a patch or override and does not carry `extend`. -Objects with two or more definition sites are reported and the command exits -1; their output still includes every refinement site. Abstract declarations -count as definitions, because copying a template copies its schema even though -the abstract itself is dropped during resolution. An in-stack same-name -redeclaration remains a compose error regardless of this cross-stack audit. - -Exit codes: 0 found / no duplicates; 1 any pattern without a match, -duplicates found, or a load error; 2 usage. +`-duplicates` (no name argument; requires `-manifest` or `-layer`) audits the +once-only discipline semantically. It first finds every name with two or more +full-object declarations across the scanned source tree. Patch declarations +and declarations carrying `extend` are refinements and do not create a +candidate. `override = true` does count: it is a complete replacement and must +not be able to suppress an otherwise identical copy. + +For a manifest, hclexp loads and resolves every `(role, env)` layer stack in +parallel. It scopes the normal schema diff to each candidate object and groups +the models by equal resolved shape. Equality across two distinct effective +source declarations is emitted under `duplicates`, even when the layers never +co-compose. Repeated names whose resolved shapes differ are emitted under +`variants`; that is the case where an override expresses a real environment +difference. A shared definition deployed by many environments still counts as +one source definition, not many duplicates. Abstract definitions, which are +removed from final models, are compared from the merged pre-resolution model. +When stderr is a terminal, locate reports parallel loading progress; redirected +stderr receives no status chatter. + +Each JSON object retains every declaration site and adds +`resolved_variants`, whose entries list the models producing one shape and the +effective source `definitions`. An invalid same-stack plain redeclaration has +no resolved model, so it is classified separately under `collisions`, with the +affected models listed in `collision_in`. For `-layer`, the complete +comma-separated list is one ad-hoc composition; manifest and ad-hoc +compositions are evaluated independently when both are supplied. + +Text output labels the same groups `duplicate`, `distinct variants`, and +`declaration collision`. The command exits 1 when `duplicates` or `collisions` +is non-empty; genuinely different variants remain visible but do not fail the +guard. + +Exit codes: 0 found / no semantic duplicates; 1 any pattern without a match, +semantic duplicates or declaration collisions found, or a load error; 2 usage. ## Structured comparison output diff --git a/internal/loader/hcl/locate.go b/internal/loader/hcl/locate.go index 4eca7b2..dd57e5e 100644 --- a/internal/loader/hcl/locate.go +++ b/internal/loader/hcl/locate.go @@ -204,12 +204,29 @@ type DuplicateGroup struct { Declarations []Declaration // every site for the name, legitimate ones included } +// FindDuplicateCandidates returns names authored as a complete object at two +// or more sites. Unlike FindDuplicates, an override declaration counts as a +// complete site: callers that load the resolved compositions can determine +// whether it is a genuine replacement or an identical copy. Patch and extend +// declarations remain refinements and do not create candidates by themselves. +func FindDuplicateCandidates(decls []Declaration) []DuplicateGroup { + return findDuplicateGroups(decls, func(d Declaration) bool { + return !d.Patch && d.Extends == "" + }) +} + // FindDuplicates groups declarations by (database, name) — the ClickHouse // namespace, which object types share — and returns groups holding two or more // definition sites: declarations that are not patches or overrides and do not // carry extend. Every declaration is retained in a reported group's output. // Results are sorted by database then name. func FindDuplicates(decls []Declaration) []DuplicateGroup { + return findDuplicateGroups(decls, func(d Declaration) bool { + return !d.Patch && !d.Override && d.Extends == "" + }) +} + +func findDuplicateGroups(decls []Declaration, definition func(Declaration) bool) []DuplicateGroup { type key struct{ db, name string } byKey := map[key][]Declaration{} for _, d := range decls { @@ -220,7 +237,7 @@ func FindDuplicates(decls []Declaration) []DuplicateGroup { for k, group := range byKey { definitions := 0 for _, d := range group { - if !d.Patch && !d.Override && d.Extends == "" { + if definition(d) { definitions++ } } diff --git a/internal/loader/hcl/locate_test.go b/internal/loader/hcl/locate_test.go index 6ac043e..3f75354 100644 --- a/internal/loader/hcl/locate_test.go +++ b/internal/loader/hcl/locate_test.go @@ -215,6 +215,15 @@ func TestFindDuplicates(t *testing.T) { } got := FindDuplicates(decls) + candidates := FindDuplicateCandidates(decls) + require.Len(t, candidates, 5) + assert.Equal(t, "events", candidates[2].Name) + assert.Equal(t, []Declaration{ + {ObjectType: KindTable, Database: "posthog", Name: "events", File: "a/shared.hcl", Line: 20}, + {ObjectType: KindTable, Database: "posthog", Name: "events", File: "b/data.hcl", Line: 6, Extends: "events_base"}, + {ObjectType: KindTable, Database: "posthog", Name: "events", File: "c/prod.hcl", Line: 3, Override: true}, + {ObjectType: KindTable, Database: "posthog", Name: "events", File: "c/satellite.hcl", Line: 12, Extends: "events_base"}, + }, candidates[2].Declarations, "override is a full-object candidate while extend remains a reported refinement") want := []DuplicateGroup{ {Name: "kafka_creds", Declarations: []Declaration{