From 54523991a2147c5a7c4f1195dadeb3c6e9ab8e57 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 13 Jan 2021 17:51:05 -0500
Subject: [PATCH] test for invalid nil value during apply

---
 terraform/context_apply_test.go              | 35 ++++++++++++++++++++
 terraform/node_resource_abstract_instance.go | 18 +++++-----
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go
index c2e7a5878..f1da45c61 100644
--- a/terraform/context_apply_test.go
+++ b/terraform/context_apply_test.go
@@ -12522,3 +12522,38 @@ resource "test_object" "a" {
 		t.Fatalf("incorrect diagnostics, got %d values with %s", len(diags), diags.ErrWithWarnings())
 	}
 }
+
+func TestContext2Apply_nilResponse(t *testing.T) {
+	// empty config to remove our resource
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_object" "a" {
+}
+`,
+	})
+
+	p := simpleMockProvider()
+	p.ApplyResourceChangeResponse = &providers.ApplyResourceChangeResponse{}
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	_, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+
+	_, diags = ctx.Apply()
+	if !diags.HasErrors() {
+		t.Fatal("expected and error")
+	}
+
+	errString := diags.ErrWithWarnings().Error()
+	if !strings.Contains(errString, "invalid nil value") {
+		t.Fatalf("error missing expected info: %q", errString)
+	}
+}
diff --git a/terraform/node_resource_abstract_instance.go b/terraform/node_resource_abstract_instance.go
index c7d798807..e02b0b79c 100644
--- a/terraform/node_resource_abstract_instance.go
+++ b/terraform/node_resource_abstract_instance.go
@@ -1920,14 +1920,16 @@ func (n *NodeAbstractResourceInstance) apply(
 			newVal = cty.NullVal(schema.ImpliedType())
 		}
 
-		// Ideally we'd produce an error or warning here if newVal is nil and
-		// there are no errors in diags, because that indicates a buggy
-		// provider not properly reporting its result, but unfortunately many
-		// of our historical test mocks behave in this way and so producing
-		// a diagnostic here fails hundreds of tests. Instead, we must just
-		// silently retain the old value for now. Returning a nil value with
-		// no errors is still always considered a bug in the provider though,
-		// and should be fixed for any "real" providers that do it.
+		if !diags.HasErrors() {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Provider produced invalid object",
+				fmt.Sprintf(
+					"Provider %q produced an invalid nil value after apply for %s.\n\nThis is a bug in the provider, which should be reported in the provider's own issue tracker.",
+					n.ResolvedProvider.String(), n.Addr.String(),
+				),
+			))
+		}
 	}
 
 	var conformDiags tfdiags.Diagnostics