From e1cf0ac80172909a069f9ee0cf4ad72458c14cee Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Thu, 8 Oct 2020 11:11:24 -0700
Subject: [PATCH] internal/depsfile: Control how the "hashes" value is
 formatted

Previously we were just letting hclwrite do its default formatting
behavior here. The current behavior there isn't ideal anyway -- it puts
big data structures all on one line -- but even ignoring that our goal
for this file format is to keep things in a highly-normalized shape so
that diffs against the file are clear and easy to read.

With that in mind, here we directly control how we write that value into
the file, which means that later changes to hclwrite's list/set
presentation won't affect it, regardless of what form they take.
---
 command/init_test.go                 |  4 ++-
 internal/depsfile/locks_file.go      | 53 +++++++++++++++++++++++-----
 internal/depsfile/locks_file_test.go |  6 +++-
 3 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/command/init_test.go b/command/init_test.go
index 985aad232..23f076980 100644
--- a/command/init_test.go
+++ b/command/init_test.go
@@ -1591,7 +1591,9 @@ func TestInit_providerLockFile(t *testing.T) {
 provider "registry.terraform.io/hashicorp/test" {
   version     = "1.2.3"
   constraints = "1.2.3"
-  hashes      = ["h1:wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno="]
+  hashes = [
+    "h1:wlbEC2mChQZ2hhgUhl6SeVLPP7fMqOFUZAQhQ9GIIno=",
+  ]
 }
 `)
 	if diff := cmp.Diff(wantLockFile, string(buf)); diff != "" {
diff --git a/internal/depsfile/locks_file.go b/internal/depsfile/locks_file.go
index dfce807ed..e11bbcc2f 100644
--- a/internal/depsfile/locks_file.go
+++ b/internal/depsfile/locks_file.go
@@ -101,15 +101,8 @@ func SaveLocksToFile(locks *Locks, filename string) tfdiags.Diagnostics {
 			body.SetAttributeValue("constraints", cty.StringVal(constraintsStr))
 		}
 		if len(lock.hashes) != 0 {
-			hashVals := make([]cty.Value, 0, len(lock.hashes))
-			for _, hash := range lock.hashes {
-				hashVals = append(hashVals, cty.StringVal(hash.String()))
-			}
-			// We're using a set rather than a list here because the order
-			// isn't significant and SetAttributeValue will automatically
-			// write the set elements in a consistent lexical order.
-			hashSet := cty.SetVal(hashVals)
-			body.SetAttributeValue("hashes", hashSet)
+			hashToks := encodeHashSetTokens(lock.hashes)
+			body.SetAttributeRaw("hashes", hashToks)
 		}
 	}
 
@@ -421,3 +414,45 @@ func decodeProviderHashesArgument(provider addrs.Provider, attr *hcl.Attribute)
 
 	return ret, diags
 }
+
+func encodeHashSetTokens(hashes []getproviders.Hash) hclwrite.Tokens {
+	// We'll generate the source code in a low-level way here (direct
+	// token manipulation) because it's desirable to maintain exactly
+	// the layout implemented here so that diffs against the locks
+	// file are easy to read; we don't want potential future changes to
+	// hclwrite to inadvertently introduce whitespace changes here.
+	ret := hclwrite.Tokens{
+		{
+			Type:  hclsyntax.TokenOBrack,
+			Bytes: []byte{'['},
+		},
+		{
+			Type:  hclsyntax.TokenNewline,
+			Bytes: []byte{'\n'},
+		},
+	}
+
+	// Although lock.hashes is a slice, we de-dupe and sort it on
+	// initialization so it's normalized for interpretation as a logical
+	// set, and so we can just trust it's already in a good order here.
+	for _, hash := range hashes {
+		hashVal := cty.StringVal(hash.String())
+		ret = append(ret, hclwrite.TokensForValue(hashVal)...)
+		ret = append(ret, hclwrite.Tokens{
+			{
+				Type:  hclsyntax.TokenComma,
+				Bytes: []byte{','},
+			},
+			{
+				Type:  hclsyntax.TokenNewline,
+				Bytes: []byte{'\n'},
+			},
+		}...)
+	}
+	ret = append(ret, &hclwrite.Token{
+		Type:  hclsyntax.TokenCBrack,
+		Bytes: []byte{']'},
+	})
+
+	return ret
+}
diff --git a/internal/depsfile/locks_file_test.go b/internal/depsfile/locks_file_test.go
index 86b0a7058..cdd6e6a03 100644
--- a/internal/depsfile/locks_file_test.go
+++ b/internal/depsfile/locks_file_test.go
@@ -210,7 +210,11 @@ provider "registry.terraform.io/test/baz" {
 provider "registry.terraform.io/test/foo" {
   version     = "1.0.0"
   constraints = ">= 1.0.0"
-  hashes      = ["test:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", "test:cccccccccccccccccccccccccccccccccccccccccccccccc"]
+  hashes = [
+    "test:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+    "test:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+    "test:cccccccccccccccccccccccccccccccccccccccccccccccc",
+  ]
 }
 `
 	if diff := cmp.Diff(wantContent, gotContent); diff != "" {