config: generate errors for unnamed blocks of various sources

We've been incorrectly validating (or not validating at all) the
requirement that certain blocks be followed by a name string, to prohibit
e.g. this:

    variable {}

and:

    variable = ""

Before this change we were catching this for most constructs only if
there were no _valid_ blocks of the same name in the same file. For
modules in particular, we were not catching this at all.

Now we detect this for all kinds of block (resources had a pre-existing
check, so aren't touched here) and produce a different error message
depending on which of the above incorrect forms are used.

This fixes #13575.

config/loader_hcl.go

@@ -327,6 +327,10 @@ func loadAtlasHcl(list *ast.ObjectList) (*AtlasConfig, error) {
 // represents exactly one module definition in the HCL configuration.
 // We leave it up to another pass to merge them together.
 func loadModulesHcl(list *ast.ObjectList) ([]*Module, error) {
+	if err := assertAllBlocksHaveNames("module", list); err != nil {
+		return nil, err
+	}
+
 	list = list.Children()
 	if len(list.Items) == 0 {
 		return nil, nil
@@ -391,12 +395,12 @@ func loadModulesHcl(list *ast.ObjectList) ([]*Module, error) {
 // LoadOutputsHcl recurses into the given HCL object and turns
 // it into a mapping of outputs.
 func loadOutputsHcl(list *ast.ObjectList) ([]*Output, error) {
-	list = list.Children()
-	if len(list.Items) == 0 {
-		return nil, fmt.Errorf(
-			"'output' must be followed by exactly one string: a name")
+	if err := assertAllBlocksHaveNames("output", list); err != nil {
+		return nil, err
 	}
 
+	list = list.Children()
+
 	// Go through each object and turn it into an actual result.
 	result := make([]*Output, 0, len(list.Items))
 	for _, item := range list.Items {
@@ -450,12 +454,12 @@ func loadOutputsHcl(list *ast.ObjectList) ([]*Output, error) {
 // LoadVariablesHcl recurses into the given HCL object and turns
 // it into a list of variables.
 func loadVariablesHcl(list *ast.ObjectList) ([]*Variable, error) {
-	list = list.Children()
-	if len(list.Items) == 0 {
-		return nil, fmt.Errorf(
-			"'variable' must be followed by exactly one strings: a name")
+	if err := assertAllBlocksHaveNames("variable", list); err != nil {
+		return nil, err
 	}
 
+	list = list.Children()
+
 	// hclVariable is the structure each variable is decoded into
 	type hclVariable struct {
 		DeclaredType string `hcl:"type"`
@@ -531,6 +535,10 @@ func loadVariablesHcl(list *ast.ObjectList) ([]*Variable, error) {
 // LoadProvidersHcl recurses into the given HCL object and turns
 // it into a mapping of provider configs.
 func loadProvidersHcl(list *ast.ObjectList) ([]*ProviderConfig, error) {
+	if err := assertAllBlocksHaveNames("provider", list); err != nil {
+		return nil, err
+	}
+
 	list = list.Children()
 	if len(list.Items) == 0 {
 		return nil, nil
@@ -592,6 +600,10 @@ func loadProvidersHcl(list *ast.ObjectList) ([]*ProviderConfig, error) {
 // represents exactly one data definition in the HCL configuration.
 // We leave it up to another pass to merge them together.
 func loadDataResourcesHcl(list *ast.ObjectList) ([]*Resource, error) {
+	if err := assertAllBlocksHaveNames("data", list); err != nil {
+		return nil, err
+	}
+
 	list = list.Children()
 	if len(list.Items) == 0 {
 		return nil, nil
@@ -901,6 +913,10 @@ func loadManagedResourcesHcl(list *ast.ObjectList) ([]*Resource, error) {
 }
 
 func loadProvisionersHcl(list *ast.ObjectList, connInfo map[string]interface{}) ([]*Provisioner, error) {
+	if err := assertAllBlocksHaveNames("provisioner", list); err != nil {
+		return nil, err
+	}
+
 	list = list.Children()
 	if len(list.Items) == 0 {
 		return nil, nil
@@ -1023,6 +1039,29 @@ func hclObjectMap(os *hclobj.Object) map[string]ast.ListNode {
 }
 */
 
+// assertAllBlocksHaveNames returns an error if any of the items in
+// the given object list are blocks without keys (like "module {}")
+// or simple assignments (like "module = 1"). It returns nil if
+// neither of these things are true.
+//
+// The given name is used in any generated error messages, and should
+// be the name of the block we're dealing with. The given list should
+// be the result of calling .Filter on an object list with that same
+// name.
+func assertAllBlocksHaveNames(name string, list *ast.ObjectList) error {
+	if elem := list.Elem(); len(elem.Items) != 0 {
+		switch et := elem.Items[0].Val.(type) {
+		case *ast.ObjectType:
+			pos := et.Lbrace
+			return fmt.Errorf("%s: %q must be followed by a name", pos, name)
+		default:
+			pos := elem.Items[0].Val.Pos()
+			return fmt.Errorf("%s: %q must be a configuration block", pos, name)
+		}
+	}
+	return nil
+}
+
 func checkHCLKeys(node ast.Node, valid []string) error {
 	var list *ast.ObjectList
 	switch n := node.(type) {

config/loader_test.go

@@ -314,6 +314,18 @@ func TestLoadFileBasic_modules(t *testing.T) {
 	}
 }
 
+func TestLoadFile_unnamedModule(t *testing.T) {
+	_, err := LoadFile(filepath.Join(fixtureDir, "module-unnamed.tf"))
+	if err == nil {
+		t.Fatalf("bad: expected error")
+	}
+
+	errorStr := err.Error()
+	if !strings.Contains(errorStr, `"module" must be followed`) {
+		t.Fatalf("bad: expected error has wrong text: %s", errorStr)
+	}
+}
+
 func TestLoadFile_outputDependsOn(t *testing.T) {
 	c, err := LoadFile(filepath.Join(fixtureDir, "output-depends-on.tf"))
 	if err != nil {
@@ -696,7 +708,7 @@ func TestLoadFile_variableNoName(t *testing.T) {
 	}
 
 	errorStr := err.Error()
-	if !strings.Contains(errorStr, "'variable' must be followed") {
+	if !strings.Contains(errorStr, `"variable" must be followed`) {
 		t.Fatalf("bad: expected error has wrong text: %s", errorStr)
 	}
 }
@@ -740,7 +752,7 @@ func TestLoadFile_unnamedOutput(t *testing.T) {
 	}
 
 	errorStr := err.Error()
-	if !strings.Contains(errorStr, "'output' must be followed") {
+	if !strings.Contains(errorStr, `"output" must be followed`) {
 		t.Fatalf("bad: expected error has wrong text: %s", errorStr)
 	}
 }

config/test-fixtures/module-unnamed.tf

@@ -0,0 +1,7 @@
+module "okay" {
+  source = "./okay"
+}
+
+module {
+  source = "./not-okay"
+}

config/test-fixtures/output-unnamed.tf

@@ -1,3 +1,7 @@
+output "okay" {
+  value = "bar"
+}
+
 output {
   value = "foo"
 }

config/test-fixtures/variable-no-name.tf

@@ -1,3 +1,6 @@
+variable "okay" {
+}
+
 variable {
   name    = "test"
   default = "test_value"