From 1979cb4c568eaf51957e15200f88c4a66a916970 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Sun, 4 Jun 2017 08:30:54 -0700
Subject: [PATCH] zcldec: flesh out and test the Variables function

It no longer produces diagnostics, since that's redundant with the
diagnostics that Decode itself will produce, and it wasn't going to be
complete anyway due to our use of partial decoding and our inability
to thread through nested specs in child blocks.
---
 zcldec/spec.go           |  19 +++++++
 zcldec/variables.go      |  15 +++---
 zcldec/variables_test.go | 108 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 133 insertions(+), 9 deletions(-)
 create mode 100644 zcldec/variables_test.go

diff --git a/zcldec/spec.go b/zcldec/spec.go
index 55e92cf..83fa92a 100644
--- a/zcldec/spec.go
+++ b/zcldec/spec.go
@@ -242,6 +242,25 @@ func (s *BlockSpec) blockHeaderSchemata() []zcl.BlockHeaderSchema {
 	}
 }
 
+// specNeedingVariables implementation
+func (s *BlockSpec) variablesNeeded(content *zcl.BodyContent) []zcl.Traversal {
+	var childBlock *zcl.Block
+	for _, candidate := range content.Blocks {
+		if candidate.Type != s.TypeName {
+			continue
+		}
+
+		childBlock = candidate
+		break
+	}
+
+	if childBlock == nil {
+		return nil
+	}
+
+	return Variables(childBlock.Body, s.Nested)
+}
+
 func (s *BlockSpec) decode(content *zcl.BodyContent, block *zcl.Block, ctx *zcl.EvalContext) (cty.Value, zcl.Diagnostics) {
 	var diags zcl.Diagnostics
 
diff --git a/zcldec/variables.go b/zcldec/variables.go
index 19b915c..f5becbd 100644
--- a/zcldec/variables.go
+++ b/zcldec/variables.go
@@ -11,18 +11,15 @@ import (
 // This can be used to conditionally populate the variables in the EvalContext
 // passed to Decode, for applications where a static scope is insufficient.
 //
-// If the given body is not compliant with the given schema, diagnostics are
-// returned describing the problem, which could also serve as a pre-evaluation
-// partial validation step.
-func Variables(body zcl.Body, spec Spec) ([]zcl.Traversal, zcl.Diagnostics) {
+// If the given body is not compliant with the given schema, the result may
+// be incomplete, but that's assumed to be okay because the eventual call
+// to Decode will produce error diagnostics anyway.
+func Variables(body zcl.Body, spec Spec) []zcl.Traversal {
 	schema := ImpliedSchema(spec)
 
-	content, _, diags := body.PartialContent(schema)
+	content, _, _ := body.PartialContent(schema)
 
 	var vars []zcl.Traversal
-	if diags.HasErrors() {
-		return vars, diags
-	}
 
 	if vs, ok := spec.(specNeedingVariables); ok {
 		vars = append(vars, vs.variablesNeeded(content)...)
@@ -33,5 +30,5 @@ func Variables(body zcl.Body, spec Spec) ([]zcl.Traversal, zcl.Diagnostics) {
 		}
 	})
 
-	return vars, diags
+	return vars
 }
diff --git a/zcldec/variables_test.go b/zcldec/variables_test.go
new file mode 100644
index 0000000..e758da7
--- /dev/null
+++ b/zcldec/variables_test.go
@@ -0,0 +1,108 @@
+package zcldec
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+
+	"github.com/zclconf/go-zcl/zcl"
+	"github.com/zclconf/go-zcl/zcl/zclsyntax"
+)
+
+func TestVariables(t *testing.T) {
+	tests := []struct {
+		config string
+		spec   Spec
+		want   []zcl.Traversal
+	}{
+		{
+			``,
+			&ObjectSpec{},
+			nil,
+		},
+		{
+			`a = foo`,
+			&ObjectSpec{},
+			nil, // "a" is not actually used, so "foo" is not required
+		},
+		{
+			`a = foo`,
+			&AttrSpec{
+				Name: "a",
+			},
+			[]zcl.Traversal{
+				{
+					zcl.TraverseRoot{
+						Name: "foo",
+						SrcRange: zcl.Range{
+							Start: zcl.Pos{Line: 1, Column: 5, Byte: 4},
+							End:   zcl.Pos{Line: 1, Column: 8, Byte: 7},
+						},
+					},
+				},
+			},
+		},
+		{
+			`a = foo`,
+			&ObjectSpec{
+				"a": &AttrSpec{
+					Name: "a",
+				},
+			},
+			[]zcl.Traversal{
+				{
+					zcl.TraverseRoot{
+						Name: "foo",
+						SrcRange: zcl.Range{
+							Start: zcl.Pos{Line: 1, Column: 5, Byte: 4},
+							End:   zcl.Pos{Line: 1, Column: 8, Byte: 7},
+						},
+					},
+				},
+			},
+		},
+		{
+			`
+b {
+  a = foo
+}`,
+			&BlockSpec{
+				TypeName: "b",
+				Nested: &AttrSpec{
+					Name: "a",
+				},
+			},
+			[]zcl.Traversal{
+				{
+					zcl.TraverseRoot{
+						Name: "foo",
+						SrcRange: zcl.Range{
+							Start: zcl.Pos{Line: 3, Column: 7, Byte: 11},
+							End:   zcl.Pos{Line: 3, Column: 10, Byte: 14},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("%02d-%s", i, test.config), func(t *testing.T) {
+			file, diags := zclsyntax.ParseConfig([]byte(test.config), "", zcl.Pos{Line: 1, Column: 1, Byte: 0})
+			if len(diags) != 0 {
+				t.Errorf("wrong number of diagnostics from ParseConfig %d; want %d", len(diags), 0)
+				for _, diag := range diags {
+					t.Logf(" - %s", diag.Error())
+				}
+			}
+			body := file.Body
+
+			got := Variables(body, test.spec)
+
+			if !reflect.DeepEqual(got, test.want) {
+				t.Errorf("wrong result\ngot:  %#v\nwant: %#v", got, test.want)
+			}
+		})
+	}
+
+}