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

@@ -296,7 +296,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

@@ -214,6 +214,25 @@ func parseLocalVariableBlocks(f *hcl.File) ([]*LocalBlock, hcl.Diagnostics) {
 		}
 	}
 
+	// duplicate detection
+	localNames := map[string]*LocalBlock{}
+
+	for _, block := range locals {
+		loc, ok := localNames[block.Name]
+		if ok {
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Duplicate local definition",
+				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
+		}
+
+		localNames[block.Name] = block
+	}
+
 	return locals, diags
 }
 
@@ -280,27 +299,6 @@ func (c *PackerConfig) evaluateLocalVariables(locals []*LocalBlock) hcl.Diagnost
 	return diags
 }
 
-func checkForDuplicateLocalDefinition(locals []*LocalBlock) 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{
-				Severity: hcl.DiagError,
-				Summary:  "Duplicate local definition",
-				Detail:   "Duplicate " + local.Name + " definition found.",
-				Subject:  local.Expr.Range().Ptr(),
-			})
-			continue
-		}
-		names[local.Name] = struct{}{}
-	}
-	return diags
-}
-
 func (c *PackerConfig) evaluateLocalVariable(local *LocalBlock, depth int) hcl.Diagnostics {
 	// If the variable already was evaluated, we can return immediately
 	if local.evaluated {

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"]
+}