core: Report a warning if any moves get blocked

In most cases Terraform will be able to automatically fully resolve all
of the pending move statements before creating a plan, but there are some
edge cases where we can end up wanting to move one object to a location
where another object is already declared.

One relatively-obvious example is if someone uses "terraform state mv" in
order to create a set of resource instance bindings that could never have
arising in normal Terraform use.

A less obvious example arises from the interactions between moves at
different levels of granularity. If we are both moving a module to a new
address and moving a resource into an instance of the new module at the
same time, the old module might well have already had a resource of the
same name and so the resource move will be unresolvable.

In these situations Terraform will move the objects as far as possible,
but because it's never valid for a move "from" address to still be
declared in the configuration Terraform will inevitably always plan to
destroy the objects that didn't find a final home. To give some additional
explanation for that result, here we'll add a warning which describes
what happened.

This is not a particularly actionable warning because we don't really
have enough information to guess what the user intended, but we do at
least prompt that they might be able to use the "terraform state" family
of subcommands to repair the ambiguous situation before planning, if they
want a different result than what Terraform proposed.

internal/terraform/context_plan.go

@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"bytes"
 	"fmt"
 	"log"
 	"sort"
@@ -417,6 +418,14 @@ func (c *Context) planWalk(config *configs.Config, prevRunState *states.State, r
 	diags = diags.Append(walker.NonFatalDiagnostics)
 	diags = diags.Append(walkDiags)
 	diags = diags.Append(c.postPlanValidateMoves(config, moveStmts, walker.InstanceExpander.AllInstances()))
+	if len(moveResults.Blocked) > 0 && !diags.HasErrors() {
+		// If we had blocked moves and we're not going to be returning errors
+		// then we'll report the blockers as a warning. We do this only in the
+		// absense of errors because invalid move statements might well be
+		// the root cause of the blockers, and so better to give an actionable
+		// error message than a less-actionable warning.
+		diags = diags.Append(blockedMovesWarningDiag(moveResults))
+	}
 
 	prevRunState = walker.PrevRunState.Close()
 	priorState := walker.RefreshState.Close()
@@ -633,3 +642,24 @@ func (c *Context) PlanGraphForUI(config *configs.Config, prevRunState *states.St
 	diags = diags.Append(moreDiags)
 	return graph, diags
 }
+
+func blockedMovesWarningDiag(results refactoring.MoveResults) tfdiags.Diagnostic {
+	if len(results.Blocked) < 1 {
+		// Caller should check first
+		panic("request to render blocked moves warning without any blocked moves")
+	}
+
+	var itemsBuf bytes.Buffer
+	for _, blocked := range results.Blocked {
+		fmt.Fprintf(&itemsBuf, "\n  - %s could not move to %s", blocked.Actual, blocked.Wanted)
+	}
+
+	return tfdiags.Sourceless(
+		tfdiags.Warning,
+		"Unresolved resource instance address changes",
+		fmt.Sprintf(
+			"Terraform tried to adjust resource instance addresses in the prior state based on change information recorded in the configuration, but some adjustments did not succeed due to existing objects already at the intended addresses:%s\n\nTerraform has planned to destroy these objects. If Terraform's proposed changes aren't appropriate, you must first resolve the conflicts using the \"terraform state\" subcommands and then create a new plan.",
+			itemsBuf.String(),
+		),
+	)
+}

internal/terraform/context_plan2_test.go

@@ -791,6 +791,102 @@ func TestContext2Plan_movedResourceBasic(t *testing.T) {
 	})
 }
 
+func TestContext2Plan_movedResourceCollision(t *testing.T) {
+	addrNoKey := mustResourceInstanceAddr("test_object.a")
+	addrZeroKey := mustResourceInstanceAddr("test_object.a[0]")
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+			resource "test_object" "a" {
+				# No "count" set, so test_object.a[0] will want
+				# to implicitly move to test_object.a, but will get
+				# blocked by the existing object at that address.
+			}
+		`,
+	})
+
+	state := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(addrNoKey, &states.ResourceInstanceObjectSrc{
+			AttrsJSON: []byte(`{}`),
+			Status:    states.ObjectReady,
+		}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
+		s.SetResourceInstanceCurrent(addrZeroKey, &states.ResourceInstanceObjectSrc{
+			AttrsJSON: []byte(`{}`),
+			Status:    states.ObjectReady,
+		}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`))
+	})
+
+	p := simpleMockProvider()
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(m, state, &PlanOpts{
+		Mode: plans.NormalMode,
+	})
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+	}
+
+	// We should have a warning, though! We'll lightly abuse the "for RPC"
+	// feature of diagnostics to get some more-readily-comparable diagnostic
+	// values.
+	gotDiags := diags.ForRPC()
+	wantDiags := tfdiags.Diagnostics{
+		tfdiags.Sourceless(
+			tfdiags.Warning,
+			"Unresolved resource instance address changes",
+			`Terraform tried to adjust resource instance addresses in the prior state based on change information recorded in the configuration, but some adjustments did not succeed due to existing objects already at the intended addresses:
+  - test_object.a[0] could not move to test_object.a
+
+Terraform has planned to destroy these objects. If Terraform's proposed changes aren't appropriate, you must first resolve the conflicts using the "terraform state" subcommands and then create a new plan.`,
+		),
+	}.ForRPC()
+	if diff := cmp.Diff(wantDiags, gotDiags); diff != "" {
+		t.Errorf("wrong diagnostics\n%s", diff)
+	}
+
+	t.Run(addrNoKey.String(), func(t *testing.T) {
+		instPlan := plan.Changes.ResourceInstance(addrNoKey)
+		if instPlan == nil {
+			t.Fatalf("no plan for %s at all", addrNoKey)
+		}
+
+		if got, want := instPlan.Addr, addrNoKey; !got.Equal(want) {
+			t.Errorf("wrong current address\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.PrevRunAddr, addrNoKey; !got.Equal(want) {
+			t.Errorf("wrong previous run address\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.Action, plans.NoOp; got != want {
+			t.Errorf("wrong planned action\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.ActionReason, plans.ResourceInstanceChangeNoReason; got != want {
+			t.Errorf("wrong action reason\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+	t.Run(addrZeroKey.String(), func(t *testing.T) {
+		instPlan := plan.Changes.ResourceInstance(addrZeroKey)
+		if instPlan == nil {
+			t.Fatalf("no plan for %s at all", addrZeroKey)
+		}
+
+		if got, want := instPlan.Addr, addrZeroKey; !got.Equal(want) {
+			t.Errorf("wrong current address\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.PrevRunAddr, addrZeroKey; !got.Equal(want) {
+			t.Errorf("wrong previous run address\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.Action, plans.Delete; got != want {
+			t.Errorf("wrong planned action\ngot:  %s\nwant: %s", got, want)
+		}
+		if got, want := instPlan.ActionReason, plans.ResourceInstanceDeleteBecauseWrongRepetition; got != want {
+			t.Errorf("wrong action reason\ngot:  %s\nwant: %s", got, want)
+		}
+	})
+}
+
 func TestContext2Plan_movedResourceUntargeted(t *testing.T) {
 	addrA := mustResourceInstanceAddr("test_object.a")
 	addrB := mustResourceInstanceAddr("test_object.b")