diff --git a/cue/testdata/definitions/issue4000.txtar b/cue/testdata/definitions/issue4000.txtar index 87a8eecbc..c384e7b80 100644 --- a/cue/testdata/definitions/issue4000.txtar +++ b/cue/testdata/definitions/issue4000.txtar @@ -24,11 +24,11 @@ items: list.Sort(#List.items, { _items: [#VMRuleList] -// TODO: this works, but make the in-place ones work as well for lists. -// @test(shareID=L0, at=items.0) -// @test(shareID=L1, at=items.1) -// @test(shareID=L0, at=#VMRuleList.0) -// @test(shareID=L1, at=#VMRuleList.1) +// This is an alternative way of writing sharing. +@test(shareID=L0, at=items.0) +@test(shareID=L1, at=items.1) +@test(shareID=L0, at=#VMRuleList.0) +@test(shareID=L1, at=#VMRuleList.1) #VMRuleList: [...null | #VMRule] & [{ spec: [{ diff --git a/cue/testdata/definitions/typocheck.txtar b/cue/testdata/definitions/typocheck.txtar index a0a7ed7b3..568b67f15 100644 --- a/cue/testdata/definitions/typocheck.txtar +++ b/cue/testdata/definitions/typocheck.txtar @@ -44,8 +44,8 @@ embed: andEmbed: { @test(err, at=d.err, code=eval, contains="field not allowed", pos=[4:11]) } and: transitive: ok: { - Z: {a: string} @test(shareID=Z) - #Y: Z @test(shareID=Z) + Z: {a: string} @test(shareID:todo=Z) + #Y: Z @test(shareID:todo=Z) #X: #Y & Z out: #X & { a: "foo" diff --git a/internal/cuetxtar/inline.go b/internal/cuetxtar/inline.go index 440370f7f..ca7d709da 100644 --- a/internal/cuetxtar/inline.go +++ b/internal/cuetxtar/inline.go @@ -895,6 +895,14 @@ func (r *inlineRunner) runKindAssertion(t testing.TB, path cue.Path, val cue.Val } expectedKind |= k } + if pa.isTodo { + if gotKind == expectedKind { + t.Logf("WARNING: path %s: TODO kind:todo now passes — consider upgrading to @test(kind=%s)", path, expectedStr) + } else { + t.Logf("path %s: TODO kind:todo still failing: got kind %v, want %v", path, gotKind, expectedKind) + } + return + } if gotKind != expectedKind { t.Errorf("path %s: @test(kind=%s): got kind %v, want %v", path, expectedStr, gotKind, expectedKind) logHint(t, pa.hint) diff --git a/internal/cuetxtar/inline_attr.go b/internal/cuetxtar/inline_attr.go index f56d02c96..e6cd5e4f5 100644 --- a/internal/cuetxtar/inline_attr.go +++ b/internal/cuetxtar/inline_attr.go @@ -497,18 +497,32 @@ func labelSelector(label ast.Label, hidPkg string) cue.Selector { // appendPath appends a selector for label to path. // hidPkg is forwarded to labelSelector; see its documentation. +// +// NOTE: a fresh slice is allocated intentionally so that multiple calls with +// the same base do not share the same backing array. cue.Path.Append reuses +// excess capacity, so callers that store the result and then append again from +// the same base would silently overwrite each other's stored paths. func appendPath(base cue.Path, label ast.Label, hidPkg string) cue.Path { - return base.Append(labelSelector(label, hidPkg)) + sels := base.Selectors() + fresh := make([]cue.Selector, len(sels)+1) + copy(fresh, sels) + fresh[len(sels)] = labelSelector(label, hidPkg) + return cue.MakePath(fresh...) } // parseAtPath parses an at= selector string into a cue.Path. -// Unlike cue.ParsePath, it handles hidden field names with a $pkg qualifier -// (e.g. "_foo$pkg" → cue.Hid("_foo", ":pkg"), matching the same syntax -// accepted inside @test(eq, {...}) bodies). Dotted paths are split on "." and -// each segment is processed independently, so "a._foo$pkg.b" works correctly. +// Unlike cue.ParsePath, it handles: +// - Hidden field names with a $pkg qualifier, e.g. "_foo$pkg" → +// cue.Hid("_foo", ":pkg"), matching the syntax used inside @test(eq, ...) +// bodies. +// - Integer segments as list-index selectors, e.g. "items.0" → +// [items, Index(0)]. +// +// Dotted paths are split on "." and each segment is processed independently, +// so "a._foo$pkg.0" works correctly. func parseAtPath(at string) (cue.Path, error) { - // Fast path: if there are no hidden-field indicators, delegate directly. - if !strings.Contains(at, "_") { + // Fast path: no hidden fields and no integers — delegate directly. + if !strings.Contains(at, "_") && !strings.ContainsAny(at, "0123456789") { p := cue.ParsePath(at) return p, p.Err() } @@ -522,6 +536,8 @@ func parseAtPath(at string) (cue.Path, error) { name = name[:i] } sels = append(sels, cue.Hid(name, pkg)) + } else if n, err := strconv.Atoi(seg); err == nil { + sels = append(sels, cue.Index(n)) } else { p := cue.ParsePath(seg) if err := p.Err(); err != nil { diff --git a/internal/cuetxtar/inline_shareid.go b/internal/cuetxtar/inline_shareid.go index 5a3dd1cc5..d1769a790 100644 --- a/internal/cuetxtar/inline_shareid.go +++ b/internal/cuetxtar/inline_shareid.go @@ -31,24 +31,27 @@ import ( // Section 8: shareID — vertex sharing assertions // ───────────────────────────────────────────────────────────────────────────── -// extractShareIDsFromEqExpr walks the struct literal of an @test(eq, STRUCT) -// body and collects all @test(shareID=name) annotations on fields. -// basePath is the CUE path of the @test(eq) attribute; field paths in the -// struct are appended to it. version is the active evaluator version name -// used for version-specific share groups (@test(shareID=name)). +// extractShareIDsFromEqExpr walks the expression of an @test(eq, EXPR) body +// and collects all @test(shareID=name) annotations. +// basePath is the CUE path of the @test(eq) attribute. +// version is the active evaluator version for version-specific share groups. +// +// Supported expression forms: +// - *ast.StructLit: @test(shareID=name) on fields → path = basePath.fieldLabel +// - *ast.ListLit: @test(shareID=name) as decl attrs inside struct elements +// → path = basePath.Index(i) +// // Returns a map from shareID name to the absolute paths of fields in that group. func extractShareIDsFromEqExpr(expr ast.Expr, basePath cue.Path, version string) map[string][]cue.Path { - s, ok := expr.(*ast.StructLit) - if !ok { - return nil - } var result map[string][]cue.Path - for _, d := range s.Elts { - f, ok := d.(*ast.Field) - if !ok { - continue + addResult := func(name string, p cue.Path) { + if result == nil { + result = make(map[string][]cue.Path) } - for _, a := range f.Attrs { + result[name] = append(result[name], p) + } + collectShareIDAttrs := func(attrs []*ast.Attribute, path cue.Path) { + for _, a := range attrs { if k, _ := a.Split(); k != "test" { continue } @@ -56,22 +59,47 @@ func extractShareIDsFromEqExpr(expr ast.Expr, basePath cue.Path, version string) if err != nil || pa.directive != "shareID" { continue } - // Version filter: skip if a non-matching version is specified. if pa.version != "" && pa.version != version { continue } if len(pa.raw.Fields) == 0 { continue } - shareIDName := pa.raw.Fields[0].Value() - if shareIDName == "" { + name := pa.raw.Fields[0].Value() + if name == "" { continue } - fieldPath := applyShareIDAt(basePath.Append(labelSelector(f.Label, "")), pa) - if result == nil { - result = make(map[string][]cue.Path) + addResult(name, applyShareIDAt(path, pa)) + } + } + + switch x := expr.(type) { + case *ast.StructLit: + // Struct body: look for @test(shareID=name) on fields. + for _, d := range x.Elts { + f, ok := d.(*ast.Field) + if !ok { + continue + } + collectShareIDAttrs(f.Attrs, basePath.Append(labelSelector(f.Label, ""))) + } + + case *ast.ListLit: + // List body: look for @test(shareID=name) as decl attrs inside + // struct elements. The path for element i is basePath.Index(i). + for i, elt := range x.Elts { + s, ok := elt.(*ast.StructLit) + if !ok { + continue + } + elemPath := basePath.Append(cue.Index(i)) + for _, d := range s.Elts { + a, ok := d.(*ast.Attribute) + if !ok { + continue + } + collectShareIDAttrs([]*ast.Attribute{a}, elemPath) } - result[shareIDName] = append(result[shareIDName], fieldPath) } } return result @@ -150,35 +178,59 @@ func (r *inlineRunner) collectShareIDsForRoot(records []attrRecord, rootPath cue } // collectDirectShareIDs builds a shareID group map from direct @test(shareID=name) -// field attributes across ALL records at any nesting depth (no root filtering). +// field attributes and in-place @test(shareID=name) inside @test(eq, ...) bodies, +// across ALL records at any nesting depth (no root filtering). // This is used for cross-root sharing assertions where fields from different -// roots share a vertex. Eq-body sharing is handled per-root by -// collectShareIDsForRoot. +// roots share a vertex. func (r *inlineRunner) collectDirectShareIDs(records []attrRecord, version string) map[string][]cue.Path { + type attrKey struct { + file string + offset int + } var shareGroups map[string][]cue.Path - for _, rec := range records { - if rec.fileLevel { - continue + seenEq := make(map[attrKey]bool) + add := func(id string, p cue.Path) { + if shareGroups == nil { + shareGroups = make(map[string][]cue.Path) } - + shareGroups[id] = append(shareGroups[id], p) + } + for _, rec := range records { pa := rec.parsed if pa.version != "" && pa.version != version { continue } - if pa.directive != "shareID" { - continue - } - if len(pa.raw.Fields) == 0 { - continue - } - shareIDName := pa.raw.Fields[0].Value() - if shareIDName == "" { - continue - } - if shareGroups == nil { - shareGroups = make(map[string][]cue.Path) + switch pa.directive { + case "shareID": + if len(pa.raw.Fields) == 0 { + continue + } + shareIDName := pa.raw.Fields[0].Value() + if shareIDName == "" { + continue + } + add(shareIDName, applyShareIDAt(rec.path, pa)) + + case "eq": + // Extract in-place @test(shareID=name) from fields in the eq body. + if len(pa.raw.Fields) < 2 { + continue + } + key := attrKey{file: pa.srcFileName, offset: pa.srcAttr.Pos().Offset()} + if seenEq[key] { + continue + } + seenEq[key] = true + eqExpr, err := parser.ParseExpr("shareID", pa.raw.Fields[1].Text()) + if err != nil { + continue + } + for id, paths := range extractShareIDsFromEqExpr(eqExpr, rec.path, version) { + for _, p := range paths { + add(id, p) + } + } } - shareGroups[shareIDName] = append(shareGroups[shareIDName], applyShareIDAt(rec.path, pa)) } return shareGroups } diff --git a/internal/cuetxtar/inline_test.go b/internal/cuetxtar/inline_test.go index d9e203254..c805ab398 100644 --- a/internal/cuetxtar/inline_test.go +++ b/internal/cuetxtar/inline_test.go @@ -516,6 +516,57 @@ func TestRunShareIDChecks_Negative(t *testing.T) { }) } +// TestShareIDPathAliasing is a regression test for a cue.Path backing-array +// aliasing bug in appendPath. When a parent path has excess backing capacity, +// cue.Path.Append reuses the same underlying array for all children, so a +// later sibling's append overwrites position len(parent) for all previously +// stored paths. The result: every @test(shareID=...) record for the same +// parent ends up with the path of the last child visited. +// +// The test verifies that collectShareIDsForRoot records the correct path for +// each annotated sibling — not the last one — by checking that the collected +// paths match the expected field names. +func TestShareIDPathAliasing(t *testing.T) { + // The aliasing requires the parent path to have excess backing capacity. + // Go doubles slice capacity on growth: len 2 → cap 4 at the third append. + // So a path of length 3 (e.g. "p.q.r") has cap 4, meaning all children + // of "p.q.r" share backing-array index 3 without the fix. + // + // Four siblings under "p.q.r". @test(shareID=AB) is on "a" and "b" (not + // the last field "d"), so without the fix both records end up with path + // "p.q.r.d" instead of "p.q.r.a" / "p.q.r.b". + src := `p: q: r: { + a: {x: 1} @test(shareID=AB) + b: a @test(shareID=AB) + c: {x: 2} + d: {x: 3} +}` + f, err := parser.ParseFile("test.cue", src, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + records := extractTestAttrs(f, "test.cue") + + r := &inlineRunner{} + rootPath := cue.MakePath(cue.Str("p"), cue.Str("q"), cue.Str("r")) + groups := r.collectShareIDsForRoot(records, rootPath, "v3") + + paths, ok := groups["AB"] + if !ok { + t.Fatal("shareID group 'AB' not found") + } + if len(paths) != 2 { + t.Fatalf("expected 2 paths in group AB, got %d: %v", len(paths), paths) + } + want := []string{"p.q.r.a", "p.q.r.b"} + got := []string{paths[0].String(), paths[1].String()} + if !slices.Equal(got, want) { + t.Errorf("path aliasing: got %v, want %v\n"+ + "(if both paths show 'root.d', appendPath is aliasing sibling paths)", + got, want) + } +} + // TestAtDirective verifies that @test(err, at=, ...) navigates to a // sub-path before checking the error. func TestAtDirective(t *testing.T) { diff --git a/internal/cuetxtar/inlinerunner_test.go b/internal/cuetxtar/inlinerunner_test.go index f8f94964c..496843f3c 100644 --- a/internal/cuetxtar/inlinerunner_test.go +++ b/internal/cuetxtar/inlinerunner_test.go @@ -185,6 +185,16 @@ x: 42 @test(eq, 99) @test(todo, p=1, why="known issue") runExpectPass(t, "-- test.cue --\nx: 42 @test(err:todo, p=1, code=eval)\n") }) + t.Run("kind:todo still failing does not fail test", func(t *testing.T) { + // @test(kind:todo=int) where kind is float: no test failure. + runExpectPass(t, "-- test.cue --\nx: 1.0 @test(kind:todo=int)\n") + }) + + t.Run("kind:todo passing does not fail test", func(t *testing.T) { + // @test(kind:todo=int) where kind matches: logs a warning but no failure. + runExpectPass(t, "-- test.cue --\nx: int @test(kind:todo=int)\n") + }) + t.Run("eq incorrect passing logs note", func(t *testing.T) { // @test(eq, X, incorrect) where value matches X: suppresses "this is wrong" // feeling by logging a NOTE, but does not fail.