hcl2template: detect duplicate locals during parse

Previously duplicate detection for local variables happened during
`Initialise`, through a call to `checkForDuplicateLocalDefinition`.

This works in a majority of cases, but for commands like `console`, this
was not detected as the return diagnostics for `Initialise` are ignored.

That check can be done as early as during parsing however, as the names
of blocks are not dynamic in the slightest (no interpolation possible),
so we move that detection logic into `Parse`, so that the behaviour is
coherent between all commands.

hcl2template/parser.go

@@ -190,6 +190,8 @@ func (p *Parser) Parse(filename string, varFiles []string, argVars map[string]st
 			diags = append(diags, morediags...)
 			cfg.LocalBlocks = append(cfg.LocalBlocks, moreLocals...)
 		}
+
+		diags = diags.Extend(cfg.checkForDuplicateLocalDefinition())
 	}
 
 	// parse var files
@@ -296,7 +298,6 @@ func filterVarsFromLogs(inputOrLocal Variables) {
 func (cfg *PackerConfig) Initialize(opts packer.InitializeOptions) hcl.Diagnostics {
 	diags := cfg.InputVariables.ValidateValues()
 	diags = append(diags, cfg.evaluateDatasources(opts.SkipDatasourcesExecution)...)
-	diags = append(diags, checkForDuplicateLocalDefinition(cfg.LocalBlocks)...)
 	diags = append(diags, cfg.evaluateLocalVariables(cfg.LocalBlocks)...)
 
 	filterVarsFromLogs(cfg.InputVariables)

hcl2template/types.packer_config.go

@@ -280,24 +280,29 @@ func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnost
 	return diags
 }
 
-func checkForDuplicateLocalDefinition(locals []*LocalBlock) hcl.Diagnostics {
+// checkForDuplicateLocalDefinition walks through the list of defined variables
+// in order to detect duplicate locals definitions.
+func (c *PackerConfig) checkForDuplicateLocalDefinition() hcl.Diagnostics {
 	var diags hcl.Diagnostics
 
-	// we could sort by name and then check contiguous names to use less memory,
-	// but using a map sounds good enough.
-	names := map[string]struct{}{}
-	for _, local := range locals {
-		if _, found := names[local.Name]; found {
-			diags = append(diags, &hcl.Diagnostic{
+	localNames := map[string]*LocalBlock{}
+
+	for _, block := range c.LocalBlocks {
+		loc, ok := localNames[block.Name]
+		if ok {
+			diags = diags.Append(&hcl.Diagnostic{
 				Severity: hcl.DiagError,
 				Summary:  "Duplicate local definition",
-				Detail:   "Duplicate " + local.Name + " definition found.",
-				Subject:  local.Expr.Range().Ptr(),
+				Detail: fmt.Sprintf("Local variable %q is defined twice in your templates. Other definition found at %q",
+					block.Name, loc.Expr.Range()),
+				Subject: block.Expr.Range().Ptr(),
 			})
 			continue
 		}
-		names[local.Name] = struct{}{}
+
+		localNames[block.Name] = block
 	}
+
 	return diags
 }
 

packer_test/local_eval_test.go

@@ -1,5 +1,7 @@
 package packer_test
 
+import "fmt"
+
 func (ts *PackerTestSuite) TestEvalLocalsOrder() {
 	ts.SkipNoAcc()
 
@@ -12,3 +14,18 @@ func (ts *PackerTestSuite) TestEvalLocalsOrder() {
 		SetArgs("console", "./templates/locals_no_order.pkr.hcl").
 		Assert(MustSucceed(), Grep("\\[\\]", grepStdout, grepInvert))
 }
+
+func (ts *PackerTestSuite) TestLocalDuplicates() {
+	pluginDir, cleanup := ts.MakePluginDir()
+	defer cleanup()
+
+	for _, cmd := range []string{"console", "validate", "build"} {
+		ts.Run(fmt.Sprintf("duplicate local detection with %s command - expect error", cmd), func() {
+			ts.PackerCommand().UsePluginDir(pluginDir).
+				SetArgs(cmd, "./templates/locals_duplicate.pkr.hcl").
+				Assert(MustFail(),
+					Grep("Duplicate local definition"),
+					Grep("Local variable \"test\" is defined twice"))
+		})
+	}
+}

packer_test/templates/locals_duplicate.pkr.hcl

@@ -0,0 +1,18 @@
+local "test" {
+    expression = "two"
+    sensitive = true
+}
+
+locals {
+    test = local.test
+}
+
+variable "test" {
+    type = string
+    default = "home"
+}
+source "null" "example" {}
+
+build {
+    sources = ["source.null.example"]
+}