internal/core/adt: error message improvements

- add position to cycle errors - stop resolving selectors for failed nodes Issue #602 Change-Id: I813abe8db9d63c1fdb0913370bb942b8657b5f8d Reviewed-on: https://cue-review.googlesource.com/c/cue/+/7885 Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
cue-lang · Dec 4, 2020 · cbcb701 · cbcb701
1 parent f62bfed
commit cbcb701
Show file tree

Hide file tree

Showing 11 changed files with 166 additions and 22 deletions.
diff --git a/cue/testdata/basicrewrite/018_self-reference_cycles.txtar b/cue/testdata/basicrewrite/018_self-reference_cycles.txtar
@@ -26,10 +26,12 @@ c: [c[1], c[0]]
 -- out/eval --
 (struct){
   a: (_|_){
-    // [cycle] cycle error
+    // [cycle] cycle error:
+    //     ./in.cue:2:4
   }
   b: (_|_){
-    // [cycle] cycle error
+    // [cycle] cycle error:
+    //     ./in.cue:2:4
   }
   c: (#list){
     0: (_){ _ }

diff --git a/cue/testdata/cycle/023_reentrance.txtar b/cue/testdata/cycle/023_reentrance.txtar
@@ -89,9 +89,7 @@ Result:
   fibRec: (struct){
     nn: (int){ int }
     out: (_|_){
-      // [incomplete] fibRec.out: undefined field out:
-      //     ./in.cue:3:40
-      // non-concrete value int in operand to >=:
+      // [incomplete] non-concrete value int in operand to >=:
       //     ./in.cue:7:5
       // non-concrete value int in operand to <:
       //     ./in.cue:10:5

diff --git a/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar b/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar
@@ -177,7 +177,8 @@ Result:
   }
   xb3: (int){ |((int){ 6 }, (int){ 9 }) }
   xb4: (_|_){
-    // [cycle] cycle error
+    // [cycle] cycle error:
+    //     ./in.cue:20:6
   }
   xc1: (int){ |((int){ 8 }, (int){ 9 }) }
   xc2: (int){ 8 }
@@ -219,7 +220,8 @@ Result:
   }
   xf3: (int){ |((int){ 6 }, (int){ 9 }) }
   xf4: (_|_){
-    // [cycle] cycle error
+    // [cycle] cycle error:
+    //     ./in.cue:54:6
   }
   z1: (int){ |((int){ 11 }, (int){ 13 }) }
   z2: (int){ 10 }

diff --git a/cue/testdata/cycle/052_resolved_self-reference_cycles_with_disjunction_with_defaults.txtar b/cue/testdata/cycle/052_resolved_self-reference_cycles_with_disjunction_with_defaults.txtar
@@ -144,7 +144,8 @@ Result:
   xb2: (int){ 8 }
   xb3: (int){ 6 }
   xb4: (_|_){
-    // [cycle] cycle error
+    // [cycle] cycle error:
+    //     ./in.cue:14:6
   }
   xc1: (int){ |(*(int){ 8 }, (int){ 9 }) }
   xc2: (int){ 8 }

diff --git a/cue/testdata/eval/incomplete.txtar b/cue/testdata/eval/incomplete.txtar
@@ -42,17 +42,27 @@ Result:
     //     ./in.cue:3:5
   }
   e5: (_|_){
-    // [cycle] cycle error
+    // [cycle] cycle error:
+    //     ./in.cue:8:6
   }
   E: (struct){
     a: (_|_){
-      // [cycle] cycle error
+      // [cycle] cycle error:
+      //     ./in.cue:12:6
+      // cycle error:
+      //     ./in.cue:13:6
     }
     b: (_|_){
-      // [cycle] cycle error
+      // [cycle] cycle error:
+      //     ./in.cue:12:6
+      // cycle error:
+      //     ./in.cue:13:6
     }
     c: (_|_){
-      // [cycle] cycle error
+      // [cycle] cycle error:
+      //     ./in.cue:12:6
+      // cycle error:
+      //     ./in.cue:13:6
     }
   }
   permanentlyIncompleteOperands: (_|_){

diff --git a/cue/testdata/fulleval/026_dont_convert_incomplete_errors_to_non-incomplete.txtar b/cue/testdata/fulleval/026_dont_convert_incomplete_errors_to_non-incomplete.txtar
@@ -48,10 +48,12 @@ s3:  strings.ContainsAny(str, "dd")
 (struct){
   n1: (struct){
     min: (_|_){
-      // [cycle] cycle error
+      // [cycle] cycle error:
+      //     ./in.cue:3:23
     }
     max: (_|_){
-      // [cycle] cycle error
+      // [cycle] cycle error:
+      //     ./in.cue:3:23
     }
   }
   n2: (_|_){

diff --git a/cue/testdata/fulleval/046_non-structural_direct_cycles.txtar b/cue/testdata/fulleval/046_non-structural_direct_cycles.txtar
@@ -29,15 +29,15 @@ c2: _|_ // conflicting values {bar: 1} and 1 (mismatched types struct and int)
 -- out/eval --
 (struct){
   c1: (_|_){
-    // [incomplete] c1: undefined field bar:
-    //     ./in.cue:1:24
+    // [cycle] cycle error:
+    //     ./in.cue:1:21
     bar: (struct){
       baz: (int){ 2 }
     }
   }
   c2: (_|_){
-    // [incomplete] c2: undefined field bar:
-    //     ./in.cue:2:24
+    // [cycle] cycle error:
+    //     ./in.cue:2:21
     bar: (int){ 1 }
   }
 }
diff --git a/cue/testdata/references/errors.txtar b/cue/testdata/references/errors.txtar
@@ -0,0 +1,114 @@
+-- references.cue --
+
+missingField: {
+    a: {}
+    r: a.b
+}
+
+missingFieldClosed: {
+    #a: {}
+    r: #a.b
+}
+
+missingFieldNested: {
+    a: {}
+    // Must refer to `b` in error
+    r: a.b.c
+}
+
+missingFieldNestedClosed: {
+    #a: {}
+    // Must refer to `d` in error
+    r: #a.d.c
+}
+
+missingFieldNestedInInterpolation: {
+    a: {}
+    // Must refer to `b` in error
+    r1: "\(a.b.c)"
+    // Must refer to `d` in error: in case only one error is shown for a
+    // a location, ensure it doesn't alphabetically sort and pick `c` instead.
+    r2: "\(a.d.c)"
+}
+-- out/eval --
+Errors:
+missingFieldClosed.r: undefined field b:
+    ./references.cue:9:11
+missingFieldNestedClosed.r: undefined field d:
+    ./references.cue:21:11
+
+Result:
+(_|_){
+  // [eval]
+  missingField: (struct){
+    a: (struct){
+    }
+    r: (_|_){
+      // [incomplete] missingField.r: undefined field b:
+      //     ./references.cue:4:10
+    }
+  }
+  missingFieldClosed: (_|_){
+    // [eval]
+    #a: (#struct){
+    }
+    r: (_|_){
+      // [eval] missingFieldClosed.r: undefined field b:
+      //     ./references.cue:9:11
+    }
+  }
+  missingFieldNested: (struct){
+    a: (struct){
+    }
+    r: (_|_){
+      // [incomplete] missingFieldNested.r: undefined field b:
+      //     ./references.cue:15:10
+    }
+  }
+  missingFieldNestedClosed: (_|_){
+    // [eval]
+    #a: (#struct){
+    }
+    r: (_|_){
+      // [eval] missingFieldNestedClosed.r: undefined field d:
+      //     ./references.cue:21:11
+    }
+  }
+  missingFieldNestedInInterpolation: (struct){
+    a: (struct){
+    }
+    r1: (_|_){
+      // [incomplete] missingFieldNestedInInterpolation.r1: invalid interpolation: undefined field b:
+      //     ./references.cue:27:9
+    }
+    r2: (_|_){
+      // [incomplete] missingFieldNestedInInterpolation.r2: invalid interpolation: undefined field d:
+      //     ./references.cue:30:9
+    }
+  }
+}
+-- out/compile --
+--- references.cue
+{
+  missingField: {
+    a: {}
+    r: 〈0;a〉.b
+  }
+  missingFieldClosed: {
+    #a: {}
+    r: 〈0;#a〉.b
+  }
+  missingFieldNested: {
+    a: {}
+    r: 〈0;a〉.b.c
+  }
+  missingFieldNestedClosed: {
+    #a: {}
+    r: 〈0;#a〉.d.c
+  }
+  missingFieldNestedInInterpolation: {
+    a: {}
+    r1: "\(〈0;a〉.b.c)"
+    r2: "\(〈0;a〉.d.c)"
+  }
+}
diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go
@@ -269,7 +269,7 @@ func (c *OpContext) addErrf(code ErrorCode, pos token.Pos, msg string, args ...i
 }
 
 func (c *OpContext) addErr(code ErrorCode, err errors.Error) {
-	c.errs = CombineErrors(c.src, c.errs, &Bottom{Code: code, Err: err})
+	c.AddBottom(&Bottom{Code: code, Err: err})
 }
 
 // AddBottom records an error in OpContext.
@@ -280,7 +280,7 @@ func (c *OpContext) AddBottom(b *Bottom) {
 // AddErr records an error in OpContext. It returns errors collected so far.
 func (c *OpContext) AddErr(err errors.Error) *Bottom {
 	if err != nil {
-		c.errs = CombineErrors(c.src, c.errs, &Bottom{Err: err})
+		c.AddBottom(&Bottom{Err: err})
 	}
 	return c.errs
 }
@@ -529,6 +529,15 @@ func (c *OpContext) evalState(v Expr, state VertexStatus) (result Value) {
 
 	defer func() {
 		c.errs = CombineErrors(c.src, c.errs, err)
+		// TODO: remove this when we handle errors more principally.
+		if b, ok := result.(*Bottom); ok && c.src != nil &&
+			b.Code == CycleError &&
+			b.Err.Position() == token.NoPos &&
+			len(b.Err.InputPositions()) == 0 {
+			bb := *b
+			bb.Err = errors.Wrapf(b.Err, c.src.Pos(), "")
+			result = &bb
+		}
 		c.errs = CombineErrors(c.src, c.errs, result)
 		if c.errs != nil {
 			result = c.errs

diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go
@@ -586,6 +586,9 @@ func (x *SelectorExpr) Source() ast.Node {
 
 func (x *SelectorExpr) resolve(c *OpContext) *Vertex {
 	n := c.node(x, x.X, x.Sel.IsRegular())
+	if n == emptyNode {
+		return n
+	}
 	return c.lookup(n, x.Src.Sel.Pos(), x.Sel)
 }
 
@@ -610,6 +613,9 @@ func (x *IndexExpr) resolve(ctx *OpContext) *Vertex {
 	// TODO: support byte index.
 	n := ctx.node(x, x.X, true)
 	i := ctx.value(x.Index)
+	if n == emptyNode {
+		return n
+	}
 	f := ctx.Label(x.Index, i)
 	return ctx.lookup(n, x.Src.Index.Pos(), f)
 }

diff --git a/internal/core/validate/validate_test.go b/internal/core/validate/validate_test.go
@@ -121,15 +121,15 @@ y: conflicting values 4 and 2:
 		y: x + 1
 		x: y - 1
 		`,
-		out: "cycle\ncycle error",
+		out: "cycle\ncycle error:\n    test:3:6",
 	}, {
 		desc: "disallow cycle",
 		cfg:  &Config{DisallowCycles: true},
 		in: `
 		a: b - 100
 		b: a + 100
 		c: [c[1], c[0]]		`,
-		out: "cycle\ncycle error",
+		out: "cycle\ncycle error:\n    test:3:6",
 	}, {
 		desc: "treat cycles as incomplete when not disallowing",
 		cfg:  &Config{},