From 552dddfb4ccad40e67c8fb4f4f12d5c7d9364a63 Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Fri, 22 Feb 2019 17:51:48 -0800
Subject: [PATCH] command: Specialized error message for var decls in tfvars

A common new-user mistake is to place variable _declarations_ into .tfvars
files instead of variable _values_. To guide towards the correct approach
here, we add a specialized error message for that situation that includes
guidance on the distinction between declaring and setting values for
variables, and an example of what setting a value should look like.
---
 command/meta_vars.go | 31 +++++++++++++++++++++++++++++++
 command/plan_test.go | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/command/meta_vars.go b/command/meta_vars.go
index b11974cf7..30fe99765 100644
--- a/command/meta_vars.go
+++ b/command/meta_vars.go
@@ -165,6 +165,37 @@ func (m *Meta) addVarsFromFile(filename string, sourceType terraform.ValueSource
 		}
 	}
 
+	// Before we do our real decode, we'll probe to see if there are any blocks
+	// of type "variable" in this body, since it's a common mistake for new
+	// users to put variable declarations in tfvars rather than variable value
+	// definitions, and otherwise our error message for that case is not so
+	// helpful.
+	{
+		content, _, _ := f.Body.PartialContent(&hcl.BodySchema{
+			Blocks: []hcl.BlockHeaderSchema{
+				{
+					Type:       "variable",
+					LabelNames: []string{"name"},
+				},
+			},
+		})
+		for _, block := range content.Blocks {
+			name := block.Labels[0]
+			diags = diags.Append(&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "Variable declaration in .tfvars file",
+				Detail:   fmt.Sprintf("A .tfvars file is used to assign values to variables that have already been declared in .tf files, not to declare new variables. To declare variable %q, place this block in one of your .tf files, such as variables.tf.\n\nTo set a value for this variable in %s, use the definition syntax instead:\n    %s = <value>", name, block.TypeRange.Filename, name),
+				Subject:  &block.TypeRange,
+			})
+		}
+		if diags.HasErrors() {
+			// If we already found problems then JustAttributes below will find
+			// the same problems with less-helpful messages, so we'll bail for
+			// now to let the user focus on the immediate problem.
+			return diags
+		}
+	}
+
 	attrs, hclDiags := f.Body.JustAttributes()
 	diags = diags.Append(hclDiags)
 
diff --git a/command/plan_test.go b/command/plan_test.go
index 9e3791d84..6786ec81c 100644
--- a/command/plan_test.go
+++ b/command/plan_test.go
@@ -686,6 +686,38 @@ func TestPlan_varFileDefault(t *testing.T) {
 	}
 }
 
+func TestPlan_varFileWithDecls(t *testing.T) {
+	tmp, cwd := testCwd(t)
+	defer testFixCwd(t, tmp, cwd)
+
+	varFilePath := testTempFile(t)
+	if err := ioutil.WriteFile(varFilePath, []byte(planVarFileWithDecl), 0644); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	p := planVarsFixtureProvider()
+	ui := cli.NewMockUi()
+	c := &PlanCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			Ui:               ui,
+		},
+	}
+
+	args := []string{
+		"-var-file", varFilePath,
+		testFixturePath("plan-vars"),
+	}
+	if code := c.Run(args); code == 0 {
+		t.Fatalf("succeeded; want failure\n\n%s", ui.OutputWriter.String())
+	}
+
+	msg := ui.ErrorWriter.String()
+	if got, want := msg, "Variable declaration in .tfvars file"; !strings.Contains(got, want) {
+		t.Fatalf("missing expected error message\nwant message containing %q\ngot:\n%s", want, got)
+	}
+}
+
 func TestPlan_detailedExitcode(t *testing.T) {
 	cwd, err := os.Getwd()
 	if err != nil {
@@ -892,6 +924,13 @@ const planVarFile = `
 foo = "bar"
 `
 
+const planVarFileWithDecl = `
+foo = "bar"
+
+variable "nope" {
+}
+`
+
 const testPlanNoStateStr = `
 <not created>
 `