Ref nodes by full path in violation messages

... because line numbers can sometimes be not present (e.g. data values
    being calculated) or shifted.

pkg/cmd/template/schema_consumer_test.go

@@ -1537,7 +1537,7 @@ foo: 0
 		valuesYAML := `foo: 1`
 
 		expectedErrMsg := `One or more data values were invalid:
-- document (schema.yaml:3) requires "foo > 2" (by schema.yaml:2)
+-  (schema.yaml:3) requires "foo > 2" (by schema.yaml:2)
 `
 		assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 	})
@@ -1550,7 +1550,7 @@ foo: 0
 		valuesYAML := `foo: 1`
 
 		expectedErrMsg := `One or more data values were invalid:
-- "foo" (values.yaml:1) requires "foo > 2" (by schema.yaml:3)
+- .foo (values.yaml:1) requires "foo > 2" (by schema.yaml:3)
 `
 		assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 	})
@@ -1567,8 +1567,8 @@ foo:
 `
 
 		expectedErrMsg := `One or more data values were invalid:
-- array item (values.yaml:2) requires "foo > 2" (by schema.yaml:4)
-- array item (values.yaml:3) requires "foo > 2" (by schema.yaml:4)
+- .foo[0] (values.yaml:2) requires "foo > 2" (by schema.yaml:4)
+- .foo[1] (values.yaml:3) requires "foo > 2" (by schema.yaml:4)
 `
 		assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 	})
@@ -1586,7 +1586,7 @@ bar: 0
 		valuesYAML := ``
 
 		expectedErrMsg := `One or more data values were invalid:
-- "bar" (schema.yaml:8) requires "not null"; fail: value is null (by schema.yaml:7)
+- .bar (schema.yaml:8) requires "not null"; fail: value is null (by schema.yaml:7)
 `
 		assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 	})
@@ -1601,7 +1601,7 @@ foo: 0
 		valuesYAML := `foo: 1`
 
 		expectedErrMsg := `One or more data values were invalid:
-- "foo" (values.yaml:1) requires "foo > 2" (by schema.yaml:4)
+- .foo (values.yaml:1) requires "foo > 2" (by schema.yaml:4)
 `
 		assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 	})
@@ -1623,7 +1623,7 @@ new: ""
 			valuesYAML := `existing: foo`
 
 			expectedErrMsg := `One or more data values were invalid:
-- "new" (schema.yaml:11) requires "non-empty" (by schema.yaml:10)
+- .new (schema.yaml:11) requires "non-empty" (by schema.yaml:10)
 `
 			assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 		})
@@ -1644,7 +1644,7 @@ existing: foo
 `
 
 			expectedErrMsg := `One or more data values were invalid:
-- "existing" (values.yaml:2) requires "a long string" (by schema.yaml:4)
+- .existing (values.yaml:2) requires "a long string" (by schema.yaml:4)
 `
 			assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
 		})
@@ -1674,9 +1674,9 @@ bar:
 
 		// none of the rules are from values.yml:
 		expectedErr := `One or more data values were invalid:
-- document (schema.yml:3) requires "has 3 map items" (by schema.yml:2)
-- "foo" (values.yml:5) requires "non-zero" (by schema.yml:4)
-- "bar" (schema.yml:7) requires "has 2 items" (by schema.yml:6)
+-  (schema.yml:3) requires "has 3 map items" (by schema.yml:2)
+- .foo (values.yml:5) requires "non-zero" (by schema.yml:4)
+- .bar (schema.yml:7) requires "has 2 items" (by schema.yml:6)
 `
 
 		opts := &cmdtpl.Options{}
@@ -1703,10 +1703,10 @@ foo:
 `
 
 		expectedErr := `One or more data values were invalid:
-- array item (values.yml:5) requires "non-zero" (by schema.yml:4)
-- array item (values.yml:5) requires "be odd" (by values.yml:4)
-- array item (values.yml:7) requires "non-zero" (by schema.yml:4)
-- array item (values.yml:7) requires "be even" (by values.yml:6)
+- .foo[0] (values.yml:5) requires "non-zero" (by schema.yml:4)
+- .foo[0] (values.yml:5) requires "be odd" (by values.yml:4)
+- .foo[1] (values.yml:7) requires "non-zero" (by schema.yml:4)
+- .foo[1] (values.yml:7) requires "be even" (by values.yml:6)
 `
 
 		opts := &cmdtpl.Options{}

pkg/validations/filetests/all-rules-are-run.tpltest

@@ -6,8 +6,8 @@ config: [1,1]
 +++
 
 ERR:
-- "config" (stdin:4) requires "custom rule"; fail: fails (by stdin:3)
-- "config" (stdin:4) requires "length greater or equal to 10"; fail: length of 2 is less than 10 (by stdin:3)
-- "config" (stdin:4) requires "length less than or equal to 1"; fail: length of 2 is more than 1 (by stdin:3)
-- "config" (stdin:4) requires "a value greater or equal to [2, 2]"; fail: value is less than [2, 2] (by stdin:3)
-- "config" (stdin:4) requires "a value less than or equal to [0, 0]"; fail: value is more than [0, 0] (by stdin:3)
+- .config (stdin:4) requires "custom rule"; fail: fails (by stdin:3)
+- .config (stdin:4) requires "length greater or equal to 10"; fail: length of 2 is less than 10 (by stdin:3)
+- .config (stdin:4) requires "length less than or equal to 1"; fail: length of 2 is more than 1 (by stdin:3)
+- .config (stdin:4) requires "a value greater or equal to [2, 2]"; fail: value is less than [2, 2] (by stdin:3)
+- .config (stdin:4) requires "a value less than or equal to [0, 0]"; fail: value is more than [0, 0] (by stdin:3)

pkg/validations/filetests/max=/invalid-when-value-greater-than-max.tpltest

@@ -4,4 +4,4 @@ foo: 11
 +++
 
 ERR:
-- "foo" (stdin:2) requires "a value less than or equal to 10"; fail: value is more than 10 (by stdin:1)
+- .foo (stdin:2) requires "a value less than or equal to 10"; fail: value is more than 10 (by stdin:1)

pkg/validations/filetests/max=/rejects-non-comparable-constraint.tpltest

@@ -5,4 +5,4 @@ value:
 +++
 
 ERR:
-- "value" (stdin:2) requires "a value less than or equal to {\"foo\": True}"; dict <= dict not implemented (by stdin:1)
+- .value (stdin:2) requires "a value less than or equal to {\"foo\": True}"; dict <= dict not implemented (by stdin:1)

pkg/validations/filetests/max=/rejects-non-comparable-value.tpltest

@@ -6,4 +6,4 @@ value:
 +++
 
 ERR:
-- "value" (stdin:2) requires "a value less than or equal to 4"; dict <= int not implemented (by stdin:1)
+- .value (stdin:2) requires "a value less than or equal to 4"; dict <= int not implemented (by stdin:1)

pkg/validations/filetests/max_len=/invalid-when-length-greater-than-max.tpltest

@@ -4,4 +4,4 @@ value: "123456"
 +++
 
 ERR:
-- "value" (stdin:2) requires "length less than or equal to 5"; fail: length of 6 is more than 5 (by stdin:1)
+- .value (stdin:2) requires "length less than or equal to 5"; fail: length of 6 is more than 5 (by stdin:1)

pkg/validations/filetests/max_len=/works-with-int-and-float

@@ -6,5 +6,5 @@ bar: "longer than 10"
 +++
 
 ERR:
-- "foo" (stdin:2) requires "length less than or equal to 10"; fail: length of 14 is more than 10 (by stdin:1)
-- "bar" (stdin:4) requires "length less than or equal to 10"; fail: length of 14 is more than 10 (by stdin:3)
+- .foo (stdin:2) requires "length less than or equal to 10"; fail: length of 14 is more than 10 (by stdin:1)
+- .bar (stdin:4) requires "length less than or equal to 10"; fail: length of 14 is more than 10 (by stdin:3)

pkg/validations/filetests/min=/invalid-when-value-less-than-min.tpltest

@@ -4,4 +4,4 @@ value: 9
 +++
 
 ERR:
-- "value" (stdin:2) requires "a value greater or equal to 10"; fail: value is less than 10 (by stdin:1)
+- .value (stdin:2) requires "a value greater or equal to 10"; fail: value is less than 10 (by stdin:1)

pkg/validations/filetests/min=/rejects-non-comparable-constraint.tpltest

@@ -5,4 +5,4 @@ value:
 +++
 
 ERR:
-- "value" (stdin:2) requires "a value greater or equal to {\"foo\": True}"; dict >= dict not implemented (by stdin:1)
+- .value (stdin:2) requires "a value greater or equal to {\"foo\": True}"; dict >= dict not implemented (by stdin:1)

pkg/validations/filetests/min=/rejects-non-comparable-value.tpltest

@@ -6,4 +6,4 @@ value:
 +++
 
 ERR:
-- "value" (stdin:2) requires "a value greater or equal to 4"; dict >= int not implemented (by stdin:1)
+- .value (stdin:2) requires "a value greater or equal to 4"; dict >= int not implemented (by stdin:1)

pkg/validations/filetests/min_len=/invalid-when-length-is-less-than-min.tpltest

@@ -4,4 +4,4 @@ value: "1234"
 +++
 
 ERR:
-- "value" (stdin:2) requires "length greater or equal to 5"; fail: length of 4 is less than 5 (by stdin:1)
+- .value (stdin:2) requires "length greater or equal to 5"; fail: length of 4 is less than 5 (by stdin:1)

pkg/validations/filetests/min_len=/works-with-int-and-float.tpltest

@@ -6,5 +6,5 @@ bar: "shorter than 20"
 +++
 
 ERR:
-- "foo" (stdin:2) requires "length greater or equal to 20"; fail: length of 15 is less than 20 (by stdin:1)
-- "bar" (stdin:4) requires "length greater or equal to 20"; fail: length of 15 is less than 20 (by stdin:3)
+- .foo (stdin:2) requires "length greater or equal to 20"; fail: length of 15 is less than 20 (by stdin:1)
+- .bar (stdin:4) requires "length greater or equal to 20"; fail: length of 15 is less than 20 (by stdin:3)

pkg/validations/filetests/not_null=/invalid-when-value-is-null.tpltest

@@ -4,4 +4,4 @@ value: null
 +++
 
 ERR:
-- "value" (stdin:2) requires "not null"; fail: value is null (by stdin:1)
+- .value (stdin:2) requires "not null"; fail: value is null (by stdin:1)

pkg/validations/filetests/not_null=/when-value-is-null-short-circuits-other-rules.tpltest

@@ -4,4 +4,4 @@ value: null
 +++
 
 ERR:
-- "value" (stdin:2) requires "not null"; fail: value is null (by stdin:1)
+- .value (stdin:2) requires "not null"; fail: value is null (by stdin:1)

pkg/validations/filetests/one_not_null=/invalid-when-more-than-one-is-not-null.tpltest

@@ -6,4 +6,4 @@ config:
 +++
 
 ERR:
-- "config" (stdin:2) requires "exactly one child not null"; check: multiple values are not null ["foo" "bar"] (by stdin:1)
+- .config (stdin:2) requires "exactly one child not null"; check: multiple values are not null ["foo" "bar"] (by stdin:1)

pkg/validations/filetests/one_of=/invalid-when-value-not-in-enum.tpltest

@@ -4,4 +4,4 @@ foo: critical
 +++
 
 ERR:
-- "foo" (stdin:2) requires "one of"; fail: value not in ["debug", "info", "warning", "error", "fatal"] (by stdin:1)
+- .foo (stdin:2) requires "one of"; fail: value not in ["debug", "info", "warning", "error", "fatal"] (by stdin:1)

pkg/validations/filetests/when=/can-use-parent-value.tpltest

@@ -8,4 +8,4 @@ counters:
 +++
 
 ERR:
-- "foo" (stdin:4) requires "fail"; fail: fails (by stdin:3)
+- .counters.foo (stdin:4) requires "fail"; fail: fails (by stdin:3)

pkg/validations/filetests/when=/can-use-root-value.tpltest

@@ -8,4 +8,4 @@ counters:
 +++
 
 ERR:
-- "bar" (stdin:6) requires "fail"; fail: fails (by stdin:5)
+- .counters.bar (stdin:6) requires "fail"; fail: fails (by stdin:5)

pkg/validations/filetests/when=/rejects-callable-arg-failing.tpltest

@@ -6,4 +6,4 @@ foo: bar
 +++
 
 ERR:
-Validating "foo": Failure evaluating when=: fail: error from within lambda
+Validating .foo: Failure evaluating when=: fail: error from within lambda

pkg/validations/filetests/when=/rejects-callable-arg-returns-non-bool.tpltest

@@ -4,4 +4,4 @@ foo: bar
 +++
 
 ERR:
-Validating "foo": want when= to be bool, got string
+Validating .foo: want when= to be bool, got string

pkg/validations/filetests/when=/true--runs-rules.tpltest

@@ -4,4 +4,4 @@ foo: ""
 +++
 
 ERR:
-- "foo" (stdin:2) requires "fail"; fail: fails (by stdin:1)
+- .foo (stdin:2) requires "fail"; fail: fails (by stdin:1)

pkg/validations/filetests/when_null_skip=/true--and-not_null--runs-rules.tpltest

@@ -4,4 +4,4 @@ foo: null
 +++
 
 ERR:
-- "foo" (stdin:2) requires "not null"; fail: value is null (by stdin:1)
+- .foo (stdin:2) requires "not null"; fail: value is null (by stdin:1)

pkg/validations/filetests/when_null_skip=/true--and-value-not-null--runs-rules.tpltest

@@ -4,4 +4,4 @@ foo: ""
 +++
 
 ERR:
-- "foo" (stdin:2) requires ""; fail: rules were run (by stdin:1)
+- .foo (stdin:2) requires ""; fail: rules were run (by stdin:1)

pkg/validations/validate.go

@@ -63,7 +63,7 @@ func Run(node yamlmeta.Node, threadName string) (Check, error) {
 	}
 
 	validation := newValidationRun(threadName, node)
-	err := yamlmeta.WalkWithParent(node, nil, validation)
+	err := yamlmeta.WalkWithParent(node, nil, "", validation)
 	if err != nil {
 		return Check{}, err
 	}
@@ -85,16 +85,16 @@ func newValidationRun(threadName string, root yamlmeta.Node) *validationRun {
 // Runs those validations, collecting any violations
 //
 // This visitor stores error(violations) in the validationRun and returns nil.
-func (a *validationRun) VisitWithParent(node yamlmeta.Node, parent yamlmeta.Node) error {
+func (a *validationRun) VisitWithParent(value yamlmeta.Node, parent yamlmeta.Node, path string) error {
 	// get rules in node's meta
-	validations := Get(node)
+	validations := Get(value)
 
 	if validations == nil {
 		return nil
 	}
 	for _, v := range validations {
 		// possible refactor to check validationRun kwargs prior to validating rules
-		violations, err := v.Validate(node, parent, a.root, a.thread)
+		violations, err := v.Validate(value, parent, a.root, path, a.thread)
 		if err != nil {
 			return err
 		}
@@ -111,14 +111,14 @@ func (a *validationRun) VisitWithParent(node yamlmeta.Node, parent yamlmeta.Node
 //
 // Returns an error if the assertion returns False (not-None), or assert.fail()s.
 // Otherwise, returns nil.
-func (v NodeValidation) Validate(node yamlmeta.Node, parent yamlmeta.Node, root yamlmeta.Node, thread *starlark.Thread) ([]error, error) {
-	nodeKey, nodeValue := v.newKeyAndStarlarkValue(node)
-	_, parentValue := v.newKeyAndStarlarkValue(parent)
-	_, rootValue := v.newKeyAndStarlarkValue(root)
+func (v NodeValidation) Validate(node yamlmeta.Node, parent yamlmeta.Node, root yamlmeta.Node, path string, thread *starlark.Thread) ([]error, error) {
+	nodeValue := v.newStarlarkValue(node)
+	parentValue := v.newStarlarkValue(parent)
+	rootValue := v.newStarlarkValue(root)
 
 	executeRules, err := v.kwargs.shouldValidate(nodeValue, parentValue, thread, rootValue)
 	if err != nil {
-		return nil, fmt.Errorf("Validating %s: %s", nodeKey, err)
+		return nil, fmt.Errorf("Validating %s: %s", path, err)
 	}
 	if !executeRules {
 		return nil, nil
@@ -128,13 +128,13 @@ func (v NodeValidation) Validate(node yamlmeta.Node, parent yamlmeta.Node, root
 	for _, rul := range byPriority(v.rules) {
 		result, err := starlark.Call(thread, rul.assertion, starlark.Tuple{nodeValue}, []starlark.Tuple{})
 		if err != nil {
-			violations = append(violations, fmt.Errorf("%s (%s) requires %q; %s (by %s)", nodeKey, node.GetPosition().AsCompactString(), rul.msg, err.Error(), v.position.AsCompactString()))
+			violations = append(violations, fmt.Errorf("%s (%s) requires %q; %s (by %s)", path, node.GetPosition().AsCompactString(), rul.msg, err.Error(), v.position.AsCompactString()))
 			if rul.isCritical {
 				break
 			}
 		} else {
 			if !(result == starlark.True) {
-				violations = append(violations, fmt.Errorf("%s (%s) requires %q (by %s)", nodeKey, node.GetPosition().AsCompactString(), rul.msg, v.position.AsCompactString()))
+				violations = append(violations, fmt.Errorf("%s (%s) requires %q (by %s)", path, node.GetPosition().AsCompactString(), rul.msg, v.position.AsCompactString()))
 				if rul.isCritical {
 					break
 				}
@@ -289,29 +289,22 @@ func (c *Check) HasViolations() bool {
 	return len(c.Violations) > 0
 }
 
-func (v NodeValidation) newKeyAndStarlarkValue(node yamlmeta.Node) (string, starlark.Value) {
-	var key string
+func (v NodeValidation) newStarlarkValue(node yamlmeta.Node) starlark.Value {
 	var nodeValue starlark.Value
 	switch typedNode := node.(type) {
 	case *yamlmeta.DocumentSet:
-		key = yamlmeta.TypeName(typedNode)
 		nodeValue = yamltemplate.NewGoValueWithYAML(typedNode).AsStarlarkValue()
 	case *yamlmeta.Array:
-		key = yamlmeta.TypeName(typedNode)
 		nodeValue = yamltemplate.NewGoValueWithYAML(typedNode).AsStarlarkValue()
 	case *yamlmeta.Map:
-		key = yamlmeta.TypeName(typedNode)
 		nodeValue = yamltemplate.NewGoValueWithYAML(typedNode).AsStarlarkValue()
 	case *yamlmeta.Document:
-		key = yamlmeta.TypeName(typedNode)
 		nodeValue = yamltemplate.NewGoValueWithYAML(typedNode.Value).AsStarlarkValue()
 	case *yamlmeta.MapItem:
-		key = fmt.Sprintf("%q", typedNode.Key)
 		nodeValue = yamltemplate.NewGoValueWithYAML(typedNode.Value).AsStarlarkValue()
 	case *yamlmeta.ArrayItem:
-		key = yamlmeta.TypeName(typedNode)
 		nodeValue = yamltemplate.NewGoValueWithYAML(typedNode.Value).AsStarlarkValue()
 	}
 
-	return key, nodeValue
+	return nodeValue
 }

pkg/yamlmeta/walk.go

@@ -3,6 +3,8 @@
 
 package yamlmeta
 
+import "fmt"
+
 // Visitor performs an operation on the given Node while traversing the AST.
 // Typically defines the action taken during a Walk().
 type Visitor interface {
@@ -33,25 +35,36 @@ func Walk(n Node, v Visitor) error {
 //
 // Typically defines the action taken during a WalkWithParent().
 type VisitorWithParent interface {
-	VisitWithParent(Node, Node) error
+	VisitWithParent(value Node, parent Node, path string) error
 }
 
 // WalkWithParent traverses the tree starting at `n`, recursively, depth-first, invoking `v` on each node and including
 //   a reference to "node"s parent node as well.
 // if `v` returns non-nil error, the traversal is aborted.
-func WalkWithParent(node Node, parent Node, v VisitorWithParent) error {
-	err := v.VisitWithParent(node, parent)
+func WalkWithParent(node Node, parent Node, path string, v VisitorWithParent) error {
+	err := v.VisitWithParent(node, parent, path)
 	if err != nil {
 		return err
 	}
 
-	for _, child := range node.GetValues() {
+	for idx, child := range node.GetValues() {
 		if childNode, ok := child.(Node); ok {
-			err = WalkWithParent(childNode, node, v)
+			err = WalkWithParent(childNode, node, path+keyOrIdxPart(childNode, idx), v)
 			if err != nil {
 				return err
 			}
 		}
 	}
 	return nil
 }
+
+func keyOrIdxPart(node Node, idx int) string {
+	switch typedNode := node.(type) {
+	case *MapItem:
+		return fmt.Sprintf(".%s", typedNode.Key)
+	case *ArrayItem:
+		return fmt.Sprintf("[%d]", idx)
+	default:
+		return ""
+	}
+}