From f932e11a5089754a1daa3786e17571dfabe9a821 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 12 Feb 2019 11:48:36 -0500
Subject: [PATCH 1/2] create a test that removes a RequiresReplace path

One of the paths that triggers RequiresReplace does not apply to the
new value.
---
 builtin/providers/test/resource_list.go      |  5 ++++
 builtin/providers/test/resource_list_test.go | 30 ++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/builtin/providers/test/resource_list.go b/builtin/providers/test/resource_list.go
index 4a0d4c92e..033e5f40c 100644
--- a/builtin/providers/test/resource_list.go
+++ b/builtin/providers/test/resource_list.go
@@ -25,6 +25,11 @@ func testResourceList() *schema.Resource {
 							Type:     schema.TypeInt,
 							Optional: true,
 						},
+						"force_new": {
+							Type:     schema.TypeString,
+							Optional: true,
+							ForceNew: true,
+						},
 						"sublist_block": {
 							Type:     schema.TypeList,
 							Optional: true,
diff --git a/builtin/providers/test/resource_list_test.go b/builtin/providers/test/resource_list_test.go
index e39b57521..c1d8eca61 100644
--- a/builtin/providers/test/resource_list_test.go
+++ b/builtin/providers/test/resource_list_test.go
@@ -188,3 +188,33 @@ resource "test_resource_list" "bar" {
 		},
 	})
 }
+
+func TestResourceList_removedForcesNew(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list" "foo" {
+	list_block {
+		force_new = "ok"
+	}
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(
+						"test_resource_list.foo", "list_block.0.force_new", "ok",
+					),
+				),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+resource "test_resource_list" "foo" {
+}
+				`),
+				Check: resource.ComposeTestCheckFunc(),
+			},
+		},
+	})
+}

From c6daf9fb24a8dcdc24303a85724633794560042c Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Tue, 12 Feb 2019 11:49:57 -0500
Subject: [PATCH 2/2] don't error on all invalid RequiresReplace paths

RequiresReplace paths with IndexSteps that have been added or removed
may fail to apply against one of the two state values. Only error out if
the path cannot be applied to both values.
---
 terraform/eval_diff.go | 44 +++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go
index 0f6b12d81..98927bd0e 100644
--- a/terraform/eval_diff.go
+++ b/terraform/eval_diff.go
@@ -236,16 +236,15 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 		for _, path := range resp.RequiresReplace {
 			if priorVal.IsNull() {
 				// If prior is null then we don't expect any RequiresReplace at all,
-				// because this is a Create action. (This is just to avoid errors
-				// when we use this value below, if the provider misbehaves.)
+				// because this is a Create action.
 				continue
 			}
-			plannedChangedVal, pathDiags := hcl.ApplyPath(plannedNewVal, path, nil)
-			if pathDiags.HasErrors() {
-				// This always indicates a provider bug, since RequiresReplace
-				// should always refer only to whole attributes (and not into
-				// attribute values themselves) and these should always be
-				// present, even though they might be null or unknown.
+
+			priorChangedVal, priorPathDiags := hcl.ApplyPath(priorVal, path, nil)
+			plannedChangedVal, plannedPathDiags := hcl.ApplyPath(plannedNewVal, path, nil)
+			if plannedPathDiags.HasErrors() && priorPathDiags.HasErrors() {
+				// This means the path was invalid in both the prior and new
+				// values, which is an error with the provider itself.
 				diags = diags.Append(tfdiags.Sourceless(
 					tfdiags.Error,
 					"Provider produced invalid plan",
@@ -256,17 +255,26 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) {
 				))
 				continue
 			}
-			priorChangedVal, err := path.Apply(priorVal)
-			if err != nil {
-				// Should never happen since prior and changed should be of
-				// the same type, but we'll allow it for robustness.
-				reqRep.Add(path)
+
+			// Make sure we have valid Values for both values.
+			// Note: if the opposing value was of the type
+			// cty.DynamicPseudoType, the type assigned here may not exactly
+			// match the schema. This is fine here, since we're only going to
+			// check for equality, but if the NullVal is to be used, we need to
+			// check the schema for th true type.
+			switch {
+			case priorChangedVal == cty.NilVal && plannedChangedVal == cty.NilVal:
+				// this should never happen without ApplyPath errors above
+				panic("requires replace path returned 2 nil values")
+			case priorChangedVal == cty.NilVal:
+				priorChangedVal = cty.NullVal(plannedChangedVal.Type())
+			case plannedChangedVal == cty.NilVal:
+				plannedChangedVal = cty.NullVal(priorChangedVal.Type())
 			}
-			if priorChangedVal != cty.NilVal {
-				eqV := plannedChangedVal.Equals(priorChangedVal)
-				if !eqV.IsKnown() || eqV.False() {
-					reqRep.Add(path)
-				}
+
+			eqV := plannedChangedVal.Equals(priorChangedVal)
+			if !eqV.IsKnown() || eqV.False() {
+				reqRep.Add(path)
 			}
 		}
 		if diags.HasErrors() {