Merge pull request #22098 from hashicorp/jbardin/nested-module-diags

ensure we record diagnostics from nested modules

command/show_test.go

@@ -5,6 +5,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -153,6 +154,8 @@ func TestShow_json_output(t *testing.T) {
 			defer os.RemoveAll(td)
 			defer testChdir(t, td)()
 
+			expectError := strings.Contains(entry.Name(), "error")
+
 			p := showFixtureProvider()
 			ui := new(cli.MockUi)
 			m := Meta{
@@ -171,6 +174,10 @@ func TestShow_json_output(t *testing.T) {
 				},
 			}
 			if code := ic.Run([]string{}); code != 0 {
+				if expectError {
+					// this should error, but not panic.
+					return
+				}
 				t.Fatalf("init failed\n%s", ui.ErrorWriter)
 			}
 

command/testdata/show-json/nested-module-error/main.tf

@@ -0,0 +1,3 @@
+module "my_module" {
+  source = "./modules"
+}

command/testdata/show-json/nested-module-error/modules/main.tf

@@ -0,0 +1,3 @@
+module "more" {
+  source = "./more-modules"
+}

command/testdata/show-json/nested-module-error/modules/more-modules/main.tf

@@ -0,0 +1,4 @@
+variable "misspelled" {
+  default     = "ehllo"
+  descriptoni = "I am a misspelled attribute"
+}

command/testdata/show-json/nested-modules/modules/more-modules/main.tf

@@ -1,4 +1,4 @@
-variable "misspelled" {
-  default     = "ehllo"
-  descriptoni = "I am a misspelled attribute"
+variable "ok" {
+  default     = "something"
+  description = "description"
 }

command/testdata/show-json/nested-modules/output.json

@@ -1,23 +1,31 @@
 {
-    "format_version": "0.1",
-    "terraform_version": "0.12.1-dev",
-    "planned_values": {
-        "root_module": {}
-    },
-    "configuration": {
-        "root_module": {
+  "format_version": "0.1",
+  "terraform_version": "0.12.1-dev",
+  "planned_values": {
+    "root_module": {}
+  },
+  "configuration": {
+    "root_module": {
+      "module_calls": {
+        "my_module": {
+          "source": "./modules",
+          "module": {
             "module_calls": {
-                "my_module": {
-                    "source": "./modules",
-                    "module": {
-                        "module_calls": {
-                            "more": {
-                                "module": {}
-                            }
-                        }
+              "more": {
+                "module": {
+                  "variables": {
+                    "ok": {
+                      "default": "something",
+                      "description": "description"
                     }
-                }
+                  }
+                },
+                "source": "./more-modules"
+              }
             }
+          }
         }
+      }
     }
-}
+  }
+}

configs/config_build.go

@@ -76,6 +76,7 @@ func buildChildModules(parent *Config, walker ModuleWalker) (map[string]*Config,
 		}
 
 		child.Children, modDiags = buildChildModules(child, walker)
+		diags = append(diags, modDiags...)
 
 		ret[call.Name] = child
 	}

configs/config_build_test.go

@@ -69,3 +69,48 @@ func TestBuildConfig(t *testing.T) {
 		t.Fatalf("child_a.child_c is same object as child_b.child_c; should not be")
 	}
 }
+
+func TestBuildConfigDiags(t *testing.T) {
+	parser := NewParser(nil)
+	mod, diags := parser.LoadConfigDir("testdata/nested-errors")
+	assertNoDiagnostics(t, diags)
+	if mod == nil {
+		t.Fatal("got nil root module; want non-nil")
+	}
+
+	versionI := 0
+	cfg, diags := BuildConfig(mod, ModuleWalkerFunc(
+		func(req *ModuleRequest) (*Module, *version.Version, hcl.Diagnostics) {
+			// For the sake of this test we're going to just treat our
+			// SourceAddr as a path relative to our fixture directory.
+			// A "real" implementation of ModuleWalker should accept the
+			// various different source address syntaxes Terraform supports.
+			sourcePath := filepath.Join("testdata/nested-errors", req.SourceAddr)
+
+			mod, diags := parser.LoadConfigDir(sourcePath)
+			version, _ := version.NewVersion(fmt.Sprintf("1.0.%d", versionI))
+			versionI++
+			return mod, version, diags
+		},
+	))
+
+	wantDiag := `testdata/nested-errors/child_c/child_c.tf:5,1-8: ` +
+		`Unsupported block type; Blocks of type "invalid" are not expected here.`
+	assertExactDiagnostics(t, diags, []string{wantDiag})
+
+	// we should still have module structure loaded
+	var got []string
+	cfg.DeepEach(func(c *Config) {
+		got = append(got, fmt.Sprintf("%s %s", strings.Join(c.Path, "."), c.Version))
+	})
+	sort.Strings(got)
+	want := []string{
+		" <nil>",
+		"child_a 1.0.0",
+		"child_a.child_c 1.0.1",
+	}
+
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("wrong result\ngot: %swant: %s", spew.Sdump(got), spew.Sdump(want))
+	}
+}

configs/testdata/nested-errors/child_a/child_a.tf

@@ -0,0 +1,4 @@
+
+module "child_c" {
+  source = "child_c"
+}

configs/testdata/nested-errors/child_c/child_c.tf

@@ -0,0 +1,6 @@
+output "hello" {
+  value = "hello"
+}
+
+invalid "block" "type " {
+}

configs/testdata/nested-errors/root.tf

@@ -0,0 +1,3 @@
+module "child_a" {
+  source = "child_a"
+}