From 26e820d8293dc6374a6fc589cdb4ecdefd28972e Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 10 May 2018 15:59:47 -0700
Subject: [PATCH] addrs: Don't return zero-length slice in Reference.Remaining.

Although this rarely matters, making it always be nil when empty makes
deep assertions simpler in tests.

This also includes a minor update to the test in the core package that
first encountered this problem, to improve the quality of its output
on failure.
---
 addrs/parse_ref.go                     | 72 +++++++++++++++-----------
 terraform/node_module_variable_test.go | 17 +++---
 2 files changed, 51 insertions(+), 38 deletions(-)

diff --git a/addrs/parse_ref.go b/addrs/parse_ref.go
index 0798d03cb6..84fe8a0d05 100644
--- a/addrs/parse_ref.go
+++ b/addrs/parse_ref.go
@@ -28,6 +28,48 @@ type Reference struct {
 // If error diagnostics are returned then the Reference value is invalid and
 // must not be used.
 func ParseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
+	ref, diags := parseRef(traversal)
+
+	// Normalize a little to make life easier for callers.
+	if ref != nil {
+		if len(ref.Remaining) == 0 {
+			ref.Remaining = nil
+		}
+	}
+
+	return ref, diags
+}
+
+// ParseRefStr is a helper wrapper around ParseRef that takes a string
+// and parses it with the HCL native syntax traversal parser before
+// interpreting it.
+//
+// This should be used only in specialized situations since it will cause the
+// created references to not have any meaningful source location information.
+// If a reference string is coming from a source that should be identified in
+// error messages then the caller should instead parse it directly using a
+// suitable function from the HCL API and pass the traversal itself to
+// ParseRef.
+//
+// Error diagnostics are returned if either the parsing fails or the analysis
+// of the traversal fails. There is no way for the caller to distinguish the
+// two kinds of diagnostics programmatically. If error diagnostics are returned
+// the returned reference may be nil or incomplete.
+func ParseRefStr(str string) (*Reference, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+
+	traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1})
+	diags = diags.Append(parseDiags)
+	if parseDiags.HasErrors() {
+		return nil, diags
+	}
+
+	ref, targetDiags := ParseRef(traversal)
+	diags = diags.Append(targetDiags)
+	return ref, diags
+}
+
+func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	root := traversal.RootName()
@@ -174,35 +216,6 @@ func ParseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 	}
 }
 
-// ParseRefStr is a helper wrapper around ParseRef that takes a string
-// and parses it with the HCL native syntax traversal parser before
-// interpreting it.
-//
-// This should be used only in specialized situations since it will cause the
-// created references to not have any meaningful source location information.
-// If a reference string is coming from a source that should be identified in
-// error messages then the caller should instead parse it directly using a
-// suitable function from the HCL API and pass the traversal itself to
-// ParseRef.
-//
-// Error diagnostics are returned if either the parsing fails or the analysis
-// of the traversal fails. There is no way for the caller to distinguish the
-// two kinds of diagnostics programmatically. If error diagnostics are returned
-// the returned reference may be nil or incomplete.
-func ParseRefStr(str string) (*Reference, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
-
-	traversal, parseDiags := hclsyntax.ParseTraversalAbs([]byte(str), "", hcl.Pos{Line: 1, Column: 1})
-	diags = diags.Append(parseDiags)
-	if parseDiags.HasErrors() {
-		return nil, diags
-	}
-
-	ref, targetDiags := ParseRef(traversal)
-	diags = diags.Append(targetDiags)
-	return ref, diags
-}
-
 func parseResourceRef(mode ResourceMode, startRange hcl.Range, traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
@@ -271,7 +284,6 @@ func parseResourceRef(mode ResourceMode, startRange hcl.Range, traversal hcl.Tra
 		return &Reference{
 			Subject:     resourceInstAddr,
 			SourceRange: tfdiags.SourceRangeFromHCL(rng),
-			Remaining:   remain,
 		}, diags
 	}
 
diff --git a/terraform/node_module_variable_test.go b/terraform/node_module_variable_test.go
index 6e08a00c16..0046dfe1b5 100644
--- a/terraform/node_module_variable_test.go
+++ b/terraform/node_module_variable_test.go
@@ -4,6 +4,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/go-test/deep"
 	"github.com/hashicorp/hcl2/hcl"
 	"github.com/hashicorp/hcl2/hcl/hclsyntax"
 
@@ -72,14 +73,14 @@ func TestNodeApplyableModuleVariableReference(t *testing.T) {
 		},
 	}
 
-	expected := []*addrs.Reference{
+	want := []*addrs.Reference{
 		{
 			Subject: addrs.InputVariable{Name: "foo"},
 		},
 	}
-	actual := n.References()
-	if !reflect.DeepEqual(actual, expected) {
-		t.Fatalf("%#v != %#v", actual, expected)
+	got := n.References()
+	for _, problem := range deep.Equal(got, want) {
+		t.Error(problem)
 	}
 }
 
@@ -100,13 +101,13 @@ func TestNodeApplyableModuleVariableReference_grandchild(t *testing.T) {
 		},
 	}
 
-	expected := []*addrs.Reference{
+	want := []*addrs.Reference{
 		{
 			Subject: addrs.InputVariable{Name: "foo"},
 		},
 	}
-	actual := n.References()
-	if !reflect.DeepEqual(actual, expected) {
-		t.Fatalf("%#v != %#v", actual, expected)
+	got := n.References()
+	for _, problem := range deep.Equal(got, want) {
+		t.Error(problem)
 	}
 }