Merge pull request #631 from vmware-tanzu/build-ann-positions

plumb annotation positions through compilation

pkg/filepos/position.go

@@ -5,7 +5,6 @@ package filepos
 
 import (
 	"fmt"
-	"strings"
 )
 
 type Position struct {
@@ -123,32 +122,3 @@ func (p *Position) IsNextTo(otherPostion *Position) bool {
 	}
 	return false
 }
-
-// PopulateAnnotationPositionFromNode takes an annotation's position, the position of the node it annotates,
-// and the comments from the same node, and creates an approximation of the annotation file and line string.
-func PopulateAnnotationPositionFromNode(annPos *Position, nodePosition *Position, nodeComments []Meta) *Position {
-	leftPadding := 0
-	if nodePosition.IsKnown() {
-		nodeLine := nodePosition.GetLine()
-		leftPadding = len(nodeLine) - len(strings.TrimLeft(nodeLine, " "))
-	}
-
-	lineString := ""
-	for _, c := range nodeComments {
-		if c.Position.IsKnown() && c.Position.AsIntString() == fmt.Sprintf("%d", annPos.LineNum()) {
-			lineString = fmt.Sprintf("%v#%s", strings.Repeat(" ", leftPadding), c.Data)
-		}
-	}
-
-	annPos.SetFile(nodePosition.GetFile())
-	annPos.SetLine(lineString)
-
-	return annPos
-}
-
-// Meta is a representation of comment's source, contains comment as a string, and original position.
-// Used to populate
-type Meta struct {
-	Data     string
-	Position *Position
-}

pkg/schema/annotations.go

@@ -539,7 +539,6 @@ func processOptionalAnnotation(node yamlmeta.Node, optionalAnnotation template.A
 
 	if nodeAnnotations.Has(optionalAnnotation) {
 		ann := nodeAnnotations[optionalAnnotation]
-		ann.Position = populateSchemaAnnotationPosition(ann.Position, node)
 
 		switch optionalAnnotation {
 		case AnnotationNullable:
@@ -725,11 +724,10 @@ func (c checkForAnnotations) Visit(n yamlmeta.Node) error {
 	var foundAnns []string
 	var foundAnnsPos []*filepos.Position
 	nodeAnnotations := template.NewAnnotations(n)
-	for _, annotation := range []template.AnnotationName{AnnotationNullable, AnnotationType, AnnotationDefault} {
-		if nodeAnnotations.Has(annotation) {
-			foundAnns = append(foundAnns, string(annotation))
-			annPos := populateSchemaAnnotationPosition(nodeAnnotations[annotation].Position, n)
-			foundAnnsPos = append(foundAnnsPos, annPos)
+	for _, annName := range []template.AnnotationName{AnnotationNullable, AnnotationType, AnnotationDefault} {
+		if nodeAnnotations.Has(annName) {
+			foundAnns = append(foundAnns, string(annName))
+			foundAnnsPos = append(foundAnnsPos, nodeAnnotations[annName].Position)
 		}
 	}
 	if len(foundAnnsPos) > 0 {
@@ -746,12 +744,3 @@ func (c checkForAnnotations) Visit(n yamlmeta.Node) error {
 
 	return nil
 }
-
-func populateSchemaAnnotationPosition(annPosition *filepos.Position, node yamlmeta.Node) *filepos.Position {
-	filePosComments := make([]filepos.Meta, 0, len(node.GetComments()))
-	for _, c := range node.GetComments() {
-		filePosComments = append(filePosComments, filepos.Meta{Data: c.Data, Position: c.Position.DeepCopy()})
-	}
-
-	return filepos.PopulateAnnotationPositionFromNode(annPosition, node.GetPosition(), filePosComments)
-}

pkg/template/annotations.go

@@ -10,12 +10,6 @@ import (
 	"github.com/vmware-tanzu/carvel-ytt/pkg/filepos"
 )
 
-const (
-	AnnotationComment AnnotationName = "comment"
-	AnnotationCode    AnnotationName = "template/code"
-	AnnotationValue   AnnotationName = "template/value"
-)
-
 type NodeAnnotations map[AnnotationName]NodeAnnotation
 
 type NodeAnnotation struct {

pkg/template/evaluation_ctx.go

@@ -5,10 +5,7 @@ package template
 
 import (
 	"fmt"
-	"strconv"
-
 	"github.com/k14s/starlark-go/starlark"
-	"github.com/vmware-tanzu/carvel-ytt/pkg/filepos"
 	"github.com/vmware-tanzu/carvel-ytt/pkg/template/core"
 	// Should not import template specific packages here (like yamlmeta)
 )
@@ -145,8 +142,8 @@ func (e *EvaluationCtx) TplStartNodeAnnotation(
 	thread *starlark.Thread, f *starlark.Builtin,
 	args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
 
-	if args.Len() != 4 {
-		return starlark.None, fmt.Errorf("expected exactly 4 arguments")
+	if args.Len() != 3 {
+		return starlark.None, fmt.Errorf("expected exactly 3 arguments")
 	}
 
 	nodeTag, err := NewNodeTagFromStarlarkValue(args.Index(0))
@@ -160,34 +157,25 @@ func (e *EvaluationCtx) TplStartNodeAnnotation(
 	}
 	annName := AnnotationName(annNameStr)
 
-	var position *filepos.Position
-	if _, ok := args.Index(2).(starlark.NoneType); ok {
-		position = filepos.NewUnknownPosition()
-	} else {
-		lineNum, err := strconv.Atoi(args.Index(2).String())
-		if err != nil {
-			panic(fmt.Sprintf("expected line num to be int, but found error: %v", err.Error()))
-		}
-		position = filepos.NewPosition(lineNum)
-	}
-
-	annVals := args.Index(3).(starlark.Tuple)
-
+	annVals := args.Index(2).(starlark.Tuple)
 	kwargs = []starlark.Tuple{}
 	for _, val := range annVals[1:] {
 		kwargs = append(kwargs, val.(starlark.Tuple))
 	}
 
+	ann, ok := e.nodes.FindAnnotation(nodeTag, annName)
+	if !ok {
+		return starlark.None, fmt.Errorf("expected to find %v on node %s", annName, nodeTag)
+	}
+	ann.Args = annVals[0].(starlark.Tuple)
+	ann.Kwargs = kwargs
+
 	if _, found := e.pendingAnnotations[nodeTag]; !found {
 		e.pendingAnnotations[nodeTag] = NodeAnnotations{}
 	}
 
 	// TODO overrides last set value
-	e.pendingAnnotations[nodeTag][annName] = NodeAnnotation{
-		Args:     annVals[0].(starlark.Tuple),
-		Kwargs:   kwargs,
-		Position: position,
-	}
+	e.pendingAnnotations[nodeTag][annName] = ann
 
 	return starlark.None, nil
 }

pkg/template/instructions.go

@@ -6,8 +6,6 @@ package template
 import (
 	"fmt"
 	"strings"
-
-	"github.com/k14s/starlark-go/starlark"
 )
 
 type InstructionSet struct {
@@ -60,13 +58,8 @@ func (is *InstructionSet) NewEndCtxNone() Instruction {
 
 func (is *InstructionSet) NewStartNodeAnnotation(nodeTag NodeTag, ann Annotation) Instruction {
 	collectedArgs := is.CollectNodeAnnotation.WithArgs(ann.Content).AsString()
-	var annLineNum string
-	if ann.Position.IsKnown() {
-		annLineNum = ann.Position.AsIntString()
-	} else {
-		annLineNum = starlark.None.String()
-	}
-	return is.StartNodeAnnotation.WithArgs(nodeTag.AsString(), `"`+string(ann.Name)+`"`, annLineNum, collectedArgs)
+
+	return is.StartNodeAnnotation.WithArgs(nodeTag.AsString(), `"`+string(ann.Name)+`"`, collectedArgs)
 }
 
 func (is *InstructionSet) NewStartNode(nodeTag NodeTag) Instruction {

pkg/template/meta.go

@@ -14,13 +14,14 @@ type AnnotationName string
 type AnnotationNs string
 
 const (
-	AnnotationNameComment AnnotationName = "comment"
+	// AnnotationComment contains user-facing documentation and should be ignored.
+	AnnotationComment AnnotationName = "comment"
+	// AnnotationCode contains Starlark code that should be inserted, verbatim, into the compiled template.
+	AnnotationCode AnnotationName = "template/code"
+	// AnnotationValue contains a Starlark expression, the result of which should be set as the value of the annotated node.
+	AnnotationValue AnnotationName = "template/value"
 )
 
-type Meta struct {
-	Annotations []*Annotation
-}
-
 type Annotation struct {
 	Name     AnnotationName // eg template/code
 	Content  string         // eg if True:
@@ -39,22 +40,25 @@ type MetaOpts struct {
 	IgnoreUnknown bool
 }
 
-// NewAnnotationFromString constructs an Annotation from a given string.
+// NewAnnotationFromComment constructs an Annotation from a string and position from a Comment.
 //
 // if opts.IgnoreUnknown is true and the annotation is unknown, it is returned as a comment.
 // if opts.IgnoreUnknown is false and the annotation is unknown, returns an error.
-func NewAnnotationFromString(data string, opts MetaOpts) (Annotation, error) {
+func NewAnnotationFromComment(data string, position *filepos.Position, opts MetaOpts) (Annotation, error) {
+	position = position.DeepCopy()
 	switch {
 	case len(data) > 0 && data[0] == '!':
 		return Annotation{
-			Name:    AnnotationNameComment,
-			Content: data[1:],
+			Name:     AnnotationComment,
+			Content:  data[1:],
+			Position: position,
 		}, nil
 
 	case len(data) > 0 && data[0] == '@':
 		nameAndContent := strings.SplitN(data[1:], " ", 2)
 		ann := Annotation{
-			Name: AnnotationName(nameAndContent[0]),
+			Name:     AnnotationName(nameAndContent[0]),
+			Position: position,
 		}
 		if len(nameAndContent) == 2 {
 			ann.Content = nameAndContent[1]
@@ -64,8 +68,9 @@ func NewAnnotationFromString(data string, opts MetaOpts) (Annotation, error) {
 	default:
 		if opts.IgnoreUnknown {
 			return Annotation{
-				Name:    AnnotationNameComment,
-				Content: data,
+				Name:     AnnotationComment,
+				Content:  data,
+				Position: position,
 			}, nil
 		} else {
 			return Annotation{}, fmt.Errorf("Expected ytt-formatted string (use '#@' for annotations or code, '#!' for comments)")

pkg/template/nodes.go

@@ -14,16 +14,25 @@ var (
 	NodeTagRoot = NodeTag{-100}
 )
 
+// Nodes contain source information that is used when building and compiling a template.
+//
+// Nodes track a source's structure by:
+//   - assigning a unique integer id, NodeTag, for each 'Node'.
+//   - storing breadcrumbs to find a Node's Parent.
+//   - keeping track each Node's non-template comments via annotations.
 type Nodes struct {
 	id               int
 	tagToNode        map[NodeTag]EvaluationNode
 	childToParentTag map[NodeTag]NodeTag
+	annotations      map[NodeTag]NodeAnnotations
 }
 
+// NewNodes initializes Nodes with empty maps to store info about the source template.
 func NewNodes() *Nodes {
 	return &Nodes{
 		tagToNode:        map[NodeTag]EvaluationNode{},
 		childToParentTag: map[NodeTag]NodeTag{},
+		annotations:      map[NodeTag]NodeAnnotations{},
 	}
 }
 
@@ -50,6 +59,22 @@ func (n *Nodes) FindNode(tag NodeTag) (EvaluationNode, bool) {
 	return node, ok
 }
 
+// AddAnnotation creates an entry in the annotations map of Nodes.
+// The entry is NodeAnnotation with only the annotation's position,
+// indexed by NodeTag and AnnotationName.
+func (n *Nodes) AddAnnotation(tag NodeTag, ann Annotation) {
+	if _, found := n.annotations[tag]; !found {
+		n.annotations[tag] = NodeAnnotations{}
+	}
+	n.annotations[tag][ann.Name] = NodeAnnotation{Position: ann.Position}
+}
+
+// FindAnnotation uses a NodeTag, and an AnnotationName to retrieve a NodeAnnotation from the annotations map in Nodes.
+func (n *Nodes) FindAnnotation(tag NodeTag, annName AnnotationName) (NodeAnnotation, bool) {
+	ann, ok := n.annotations[tag][annName]
+	return ann, ok
+}
+
 type NodeTag struct {
 	id int
 }

pkg/workspace/assert.go

@@ -82,7 +82,7 @@ func (a *assertChecker) Visit(node yamlmeta.Node) error {
 	case *yamlmeta.DocumentSet, *yamlmeta.Array, *yamlmeta.Map:
 		return fmt.Errorf("Invalid @%s annotation - not supported on %s at %s", AnnotationAssertValidate, yamlmeta.TypeName(node), node.GetPosition().AsCompactString())
 	default:
-		rules, syntaxErr := newRulesFromAssertValidateAnnotation(nodeAnnotations[AnnotationAssertValidate], node)
+		rules, syntaxErr := newRulesFromAssertValidateAnnotation(nodeAnnotations[AnnotationAssertValidate])
 		if syntaxErr != nil {
 			return syntaxErr
 		}
@@ -97,36 +97,35 @@ func (a *assertChecker) Visit(node yamlmeta.Node) error {
 	return nil
 }
 
-func newRulesFromAssertValidateAnnotation(annotation template.NodeAnnotation, n yamlmeta.Node) ([]Rule, error) {
+func newRulesFromAssertValidateAnnotation(annotation template.NodeAnnotation) ([]Rule, error) {
 	var rules []Rule
-	validationPosition := createAssertAnnotationPosition(annotation.Position, n)
 
 	if len(annotation.Kwargs) != 0 {
-		return nil, fmt.Errorf("Invalid @%s annotation - expected @%s to have 2-tuple as argument(s), but found keyword argument (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, validationPosition.AsCompactString())
+		return nil, fmt.Errorf("Invalid @%s annotation - expected @%s to have 2-tuple as argument(s), but found keyword argument (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, annotation.Position.AsCompactString())
 	}
 	if len(annotation.Args) == 0 {
-		return nil, fmt.Errorf("Invalid @%s annotation - expected @%s to have 2-tuple as argument(s), but found no arguments (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, validationPosition.AsCompactString())
+		return nil, fmt.Errorf("Invalid @%s annotation - expected @%s to have 2-tuple as argument(s), but found no arguments (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, annotation.Position.AsCompactString())
 	}
 	for _, arg := range annotation.Args {
 		ruleTuple, ok := arg.(starlark.Tuple)
 		if !ok {
-			return nil, fmt.Errorf("Invalid @%s annotation - expected @%s to have 2-tuple as argument(s), but found: %s (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, arg.String(), validationPosition.AsCompactString())
+			return nil, fmt.Errorf("Invalid @%s annotation - expected @%s to have 2-tuple as argument(s), but found: %s (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, arg.String(), annotation.Position.AsCompactString())
 		}
 		if len(ruleTuple) != 2 {
-			return nil, fmt.Errorf("Invalid @%s annotation - expected @%s 2-tuple, but found tuple with length %v (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, len(ruleTuple), validationPosition.AsCompactString())
+			return nil, fmt.Errorf("Invalid @%s annotation - expected @%s 2-tuple, but found tuple with length %v (by %s)", AnnotationAssertValidate, AnnotationAssertValidate, len(ruleTuple), annotation.Position.AsCompactString())
 		}
 		message, ok := ruleTuple[0].(starlark.String)
 		if !ok {
-			return nil, fmt.Errorf("Invalid @%s annotation - expected first item in the 2-tuple to be a string describing a valid value, but was %s (at %s)", AnnotationAssertValidate, ruleTuple[0].Type(), validationPosition.AsCompactString())
+			return nil, fmt.Errorf("Invalid @%s annotation - expected first item in the 2-tuple to be a string describing a valid value, but was %s (at %s)", AnnotationAssertValidate, ruleTuple[0].Type(), annotation.Position.AsCompactString())
 		}
 		lambda, ok := ruleTuple[1].(starlark.Callable)
 		if !ok {
-			return nil, fmt.Errorf("Invalid @%s annotation - expected second item in the 2-tuple to be an assertion function, but was %s (at %s)", AnnotationAssertValidate, ruleTuple[1].Type(), validationPosition.AsCompactString())
+			return nil, fmt.Errorf("Invalid @%s annotation - expected second item in the 2-tuple to be an assertion function, but was %s (at %s)", AnnotationAssertValidate, ruleTuple[1].Type(), annotation.Position.AsCompactString())
 		}
 		rules = append(rules, Rule{
 			msg:       message.GoString(),
 			assertion: lambda,
-			position:  validationPosition,
+			position:  annotation.Position,
 		})
 	}
 
@@ -178,13 +177,3 @@ func (r Rule) Validate(node yamlmeta.Node, thread *starlark.Thread) error {
 
 	return nil
 }
-
-func createAssertAnnotationPosition(annPosition *filepos.Position, node yamlmeta.Node) *filepos.Position {
-	filePosComments := make([]filepos.Meta, 0, len(node.GetComments()))
-	for _, c := range node.GetComments() {
-		filePosComments = append(filePosComments, filepos.Meta{Data: c.Data, Position: c.Position.DeepCopy()})
-	}
-
-	return filepos.PopulateAnnotationPositionFromNode(annPosition, node.GetPosition(), filePosComments)
-
-}

pkg/workspace/doc_extractor.go

@@ -43,7 +43,7 @@ func (v DocExtractor) extract(docSet *yamlmeta.DocumentSet,
 		for _, comment := range doc.GetComments() {
 			// TODO potentially use template.NewAnnotations(doc).Has(yttoverlay.AnnotationMatch)
 			// however if doc was not processed by the template, it wont have any annotations set
-			ann, err := yamltemplate.NewTemplateAnnotationFromYAMLComment(comment, doc.GetPosition(), yamltemplate.MetasOpts{IgnoreUnknown: true})
+			ann, err := yamltemplate.NewTemplateAnnotationFromYAMLComment(comment, doc.GetPosition(), template.MetaOpts{IgnoreUnknown: true})
 			if err != nil {
 				return nil, nil, err
 			}
@@ -72,7 +72,7 @@ func (v DocExtractor) checkNonDocs(val interface{}, annName template.AnnotationN
 	}
 
 	for _, comment := range node.GetComments() {
-		ann, err := yamltemplate.NewTemplateAnnotationFromYAMLComment(comment, node.GetPosition(), yamltemplate.MetasOpts{IgnoreUnknown: true})
+		ann, err := yamltemplate.NewTemplateAnnotationFromYAMLComment(comment, node.GetPosition(), template.MetaOpts{IgnoreUnknown: true})
 		if err != nil {
 			return err
 		}