this repo has no description
internal/cuetxtar: detect misplaced @test
Detect @test attributes silently ignored when
placed inside struct literals that are operands
of a binary expression (e.g. X & {f: v @test(...)}).
Such attributes are never visited by the inline
test runner and would pass without checking
anything; they are now reported as parse errors.
Also lower eqCompactThreshold from 60 to 40 to
produce indented output for moderately-sized
struct values.
Fix existing tests that used the now-illegal form
by moving @test to after the enclosing expression.
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: I7a63656a7de18261d1f4126f5a30ef10fc840660
Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1235357
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
+138
-18
+1
-1
cue/testdata/builtins/closed.txtar
+1
-1
cue/testdata/builtins/closed.txtar
+2
-10
cue/testdata/eval/issue1640.txtar
+2
-10
cue/testdata/eval/issue1640.txtar
···
16
16
-- x.cue --
17
17
package bug
18
18
19
-
fact1: #MCS & {
20
-
@test(eq, {lc: "foo", uc: "foo"})
21
-
@test(closed)
22
-
lc: fact2.lc
23
-
}
24
-
fact2: #MCS & {
25
-
@test(eq, {lc: "foo", uc: "foo"})
26
-
@test(closed)
27
-
lc: "foo"
28
-
}
19
+
fact1: #MCS & {lc: fact2.lc} @test(eq, {lc: "foo", uc: "foo"}) @test(closed)
20
+
fact2: #MCS & {lc: "foo"} @test(eq, {lc: "foo", uc: "foo"}) @test(closed)
29
21
30
22
#MCS: #MultiCaseString @test(eq, {lc: string, uc: string}) @test(closed) @test(shareID=MCS)
31
23
#MultiCaseString: {
+65
-7
internal/cuetxtar/inline.go
+65
-7
internal/cuetxtar/inline.go
···
373
373
// walkStruct records @test decl attrs and recurses into all sub-fields.
374
374
var walkStruct func(sl *ast.StructLit, path cue.Path)
375
375
376
+
// scanUnreachableTests scans expr for @test attributes that appear inside
377
+
// struct literals which are binary-expression operands. Such attributes are
378
+
// never visited by walkField/walkStruct and would be silently ignored;
379
+
// they are reported here as parse errors so the test suite fails loudly.
380
+
//
381
+
// inOperand must be true when expr is already inside a binary-expression
382
+
// operand (meaning all @test within it are unreachable regardless of depth).
383
+
var scanUnreachableTests func(expr ast.Expr, path cue.Path, inOperand bool)
384
+
376
385
walkField = func(field *ast.Field, path cue.Path) {
377
386
fieldBaseLine := field.Pos().Line()
378
387
···
396
405
397
406
if sl, ok := field.Value.(*ast.StructLit); ok {
398
407
walkStruct(sl, path)
408
+
} else {
409
+
scanUnreachableTests(field.Value, path, false)
399
410
}
400
411
}
401
412
···
463
474
})
464
475
}
465
476
}
477
+
478
+
case *ast.EmbedDecl:
479
+
// Bare embeddings (e.g. `A & {f: v @test(...)}`) are not
480
+
// walked by walkField/walkStruct, so any @test inside them
481
+
// would be silently ignored. Scan and report them as errors.
482
+
scanUnreachableTests(e.Expr, path, false)
483
+
}
484
+
}
485
+
}
486
+
487
+
scanUnreachableTests = func(expr ast.Expr, path cue.Path, inOperand bool) {
488
+
switch e := expr.(type) {
489
+
case *ast.BinaryExpr:
490
+
scanUnreachableTests(e.X, path, true)
491
+
scanUnreachableTests(e.Y, path, true)
492
+
case *ast.ParenExpr:
493
+
scanUnreachableTests(e.X, path, inOperand)
494
+
case *ast.StructLit:
495
+
if !inOperand {
496
+
return // reachable; handled by walkField/walkStruct
497
+
}
498
+
for _, elt := range e.Elts {
499
+
switch elt := elt.(type) {
500
+
case *ast.Attribute:
501
+
k, _ := elt.Split()
502
+
if k == "test" {
503
+
appendErrRecord(elt, path, true, false, fmt.Errorf(
504
+
"@test inside a struct literal that is a binary-expression operand "+
505
+
"(e.g. X & {@test(...)}) is not reachable by the test runner; "+
506
+
"place @test after the field value (e.g. f: X & {...} @test(...)) "+
507
+
"or as a decl attribute in the enclosing struct"))
508
+
}
509
+
case *ast.Field:
510
+
for _, a := range elt.Attrs {
511
+
k, _ := a.Split()
512
+
if k == "test" {
513
+
appendErrRecord(a, path, false, false, fmt.Errorf(
514
+
"@test on a field inside a struct literal that is a binary-expression operand "+
515
+
"(e.g. X & {f: v @test(...)}) is not reachable by the test runner; "+
516
+
"place @test after the enclosing field value (e.g. f: X & {...} @test(...))"))
517
+
}
518
+
}
519
+
scanUnreachableTests(elt.Value, path, true)
520
+
}
466
521
}
467
522
}
468
523
}
···
489
544
}
490
545
491
546
for _, decl := range f.Decls {
492
-
field, ok := decl.(*ast.Field)
493
-
if !ok {
494
-
continue
495
-
}
496
-
fieldPath := appendPath(cue.Path{}, field.Label, hidPkg)
497
-
if fieldPath.Err() == nil {
498
-
walkField(field, fieldPath)
547
+
switch d := decl.(type) {
548
+
case *ast.Field:
549
+
fieldPath := appendPath(cue.Path{}, d.Label, hidPkg)
550
+
if fieldPath.Err() == nil {
551
+
walkField(d, fieldPath)
552
+
}
553
+
case *ast.EmbedDecl:
554
+
// Top-level bare embeddings are also not walked by walkField, so
555
+
// scan them for unreachable @test attributes.
556
+
scanUnreachableTests(d.Expr, cue.Path{}, false)
499
557
}
500
558
}
501
559
+70
internal/cuetxtar/inline_test.go
+70
internal/cuetxtar/inline_test.go
···
745
745
}
746
746
})
747
747
}
748
+
749
+
// TestUnreachableTestAttr verifies that @test directives placed inside struct
750
+
// literals that are binary-expression operands (e.g. X & {@test(...)}) are
751
+
// detected and reported as errors rather than silently ignored.
752
+
func TestUnreachableTestAttr(t *testing.T) {
753
+
tests := []struct {
754
+
name string
755
+
src string
756
+
wantErrFrag string // non-empty means a parseErr record with this substring is expected
757
+
}{
758
+
{
759
+
name: "decl attr in conjunction operand",
760
+
src: "f: X & {\n\t@test(eq, 1)\n}\n",
761
+
wantErrFrag: "not reachable by the test runner",
762
+
},
763
+
{
764
+
name: "field attr in conjunction operand",
765
+
src: "f: X & {\n\tv: 1 @test(eq, 1)\n}\n",
766
+
wantErrFrag: "not reachable by the test runner",
767
+
},
768
+
{
769
+
name: "decl attr in bare embedding conjunction",
770
+
src: "f: 1\nX & {\n\t@test(eq, 1)\n}\n",
771
+
wantErrFrag: "not reachable by the test runner",
772
+
},
773
+
{
774
+
// @test as a field attr after the full expression must NOT be flagged.
775
+
name: "field attr after conjunction is valid",
776
+
src: "f: X & {} @test(eq, 1)\n",
777
+
},
778
+
{
779
+
// Decl @test inside a plain struct (not a binary operand) is valid.
780
+
name: "decl attr in plain struct is valid",
781
+
src: "f: {\n\t@test(eq, {v: 1})\n\tv: 1\n}\n",
782
+
},
783
+
}
784
+
for _, tt := range tests {
785
+
t.Run(tt.name, func(t *testing.T) {
786
+
f, err := parser.ParseFile("test.cue", tt.src)
787
+
if err != nil {
788
+
t.Fatalf("parse error: %v", err)
789
+
}
790
+
records := extractTestAttrs(f, "test.cue")
791
+
792
+
var errMsgs []string
793
+
for _, r := range records {
794
+
if r.parseErr != nil {
795
+
errMsgs = append(errMsgs, r.parseErr.Error())
796
+
}
797
+
}
798
+
799
+
if tt.wantErrFrag == "" {
800
+
if len(errMsgs) > 0 {
801
+
t.Errorf("expected no parse errors, got: %v", errMsgs)
802
+
}
803
+
return
804
+
}
805
+
if len(errMsgs) == 0 {
806
+
t.Errorf("expected a parse error containing %q, got none", tt.wantErrFrag)
807
+
return
808
+
}
809
+
for _, msg := range errMsgs {
810
+
if strings.Contains(msg, tt.wantErrFrag) {
811
+
return
812
+
}
813
+
}
814
+
t.Errorf("no error message contains %q; got: %v", tt.wantErrFrag, errMsgs)
815
+
})
816
+
}
817
+
}