refactoring: ImpliedMoveStatements function

This new function complements the existing function FindMoveStatements
by potentially generating additional "implied" move statements that aren't
written explicit in the configuration but that we'll infer by comparing
the configuration and te previous run state.

The goal here is to infer only enough to replicate the effect of the
"count boundary fixup" graph node (terraform.NodeCountBoundary) that we
currently use to deal with this concern of preserving the zero-instance
when switching between "count" and not "count".

This is just dead code for now. A subsequent commit will introduce this
into the "terraform" package while also removing
terraform.NodeCountBoundary, thus achieving the same effect as before but
in a way that'll get reported in the UI as a move, using the same language
that we'd use for an explicit move statement.

internal/addrs/move_endpoint_module.go

@@ -2,6 +2,7 @@ package addrs
 
 import (
 	"fmt"
+	"reflect"
 	"strings"
 
 	"github.com/hashicorp/terraform/internal/tfdiags"
@@ -51,6 +52,27 @@ type MoveEndpointInModule struct {
 	relSubject AbsMoveable
 }
 
+// ImpliedMoveStatementEndpoint is a special constructor for MoveEndpointInModule
+// which is suitable only for constructing "implied" move statements, which
+// means that we inferred the statement automatically rather than building it
+// from an explicit block in the configuration.
+//
+// Implied move endpoints, just as for the statements they are embedded in,
+// have somewhat-related-but-imprecise source ranges, typically referring to
+// some general configuration construct that implied the statement, because
+// by definition there is no explicit move endpoint expression in this case.
+func ImpliedMoveStatementEndpoint(addr AbsResourceInstance, rng tfdiags.SourceRange) *MoveEndpointInModule {
+	// implied move endpoints always belong to the root module, because each
+	// one refers to a single resource instance inside a specific module
+	// instance, rather than all instances of the module where the resource
+	// was declared.
+	return &MoveEndpointInModule{
+		SourceRange: rng,
+		module:      RootModule,
+		relSubject:  addr,
+	}
+}
+
 func (e *MoveEndpointInModule) ObjectKind() MoveEndpointKind {
 	return absMoveableEndpointKind(e.relSubject)
 }
@@ -85,6 +107,24 @@ func (e *MoveEndpointInModule) String() string {
 	return buf.String()
 }
 
+// Equal returns true if the reciever represents the same matching pattern
+// as the other given endpoint, ignoring the source location information.
+//
+// This is not an optimized function and is here primarily to help with
+// writing concise assertions in test code.
+func (e *MoveEndpointInModule) Equal(other *MoveEndpointInModule) bool {
+	if (e == nil) != (other == nil) {
+		return false
+	}
+	if !e.module.Equal(other.module) {
+		return false
+	}
+	// This assumes that all of our possible "movables" are trivially
+	// comparable with reflect, which is true for all of them at the time
+	// of writing.
+	return reflect.DeepEqual(e.relSubject, other.relSubject)
+}
+
 // Module returns the address of the module where the receiving address was
 // declared.
 func (e *MoveEndpointInModule) Module() Module {

internal/refactoring/move_statement.go

@@ -5,12 +5,26 @@ import (
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/configs"
+	"github.com/hashicorp/terraform/internal/states"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
 
 type MoveStatement struct {
 	From, To  *addrs.MoveEndpointInModule
 	DeclRange tfdiags.SourceRange
+
+	// Implied is true for statements produced by ImpliedMoveStatements, and
+	// false for statements produced by FindMoveStatements.
+	//
+	// An "implied" statement is one that has no explicit "moved" block in
+	// the configuration and was instead generated automatically based on a
+	// comparison between current configuration and previous run state.
+	// For implied statements, the DeclRange field contains the source location
+	// of something in the source code that implied the statement, in which
+	// case it would probably be confusing to show that source range to the
+	// user, e.g. in an error message, without clearly mentioning that it's
+	// related to an implied move statement.
+	Implied bool
 }
 
 // FindMoveStatements recurses through the modules of the given configuration
@@ -34,6 +48,7 @@ func findMoveStatements(cfg *configs.Config, into []MoveStatement) []MoveStateme
 			From:      fromAddr,
 			To:        toAddr,
 			DeclRange: tfdiags.SourceRangeFromHCL(mc.DeclRange),
+			Implied:   false,
 		})
 	}
 
@@ -44,6 +59,102 @@ func findMoveStatements(cfg *configs.Config, into []MoveStatement) []MoveStateme
 	return into
 }
 
+// ImpliedMoveStatements compares addresses in the given state with addresses
+// in the given configuration and potentially returns additional MoveStatement
+// objects representing moves we infer automatically, even though they aren't
+// explicitly recorded in the configuration.
+//
+// We do this primarily for backward compatibility with behaviors of Terraform
+// versions prior to introducing explicit "moved" blocks. Specifically, this
+// function aims to achieve the same result as the "NodeCountBoundary"
+// heuristic from Terraform v1.0 and earlier, where adding or removing the
+// "count" meta-argument from an already-created resource can automatically
+// preserve the zeroth or the NoKey instance, depending on the direction of
+// the change. We do this only for resources that aren't mentioned already
+// in at least one explicit move statement.
+//
+// As with the previous-version heuristics it replaces, this is a best effort
+// and doesn't handle all situations. An explicit move statement is always
+// preferred, but our goal here is to match exactly the same cases that the
+// old heuristic would've matched, to retain compatibility for existing modules.
+//
+// We should think very hard before adding any _new_ implication rules for
+// moved statements.
+func ImpliedMoveStatements(rootCfg *configs.Config, prevRunState *states.State, explicitStmts []MoveStatement) []MoveStatement {
+	return impliedMoveStatements(rootCfg, prevRunState, explicitStmts, nil)
+}
+
+func impliedMoveStatements(cfg *configs.Config, prevRunState *states.State, explicitStmts []MoveStatement, into []MoveStatement) []MoveStatement {
+	modAddr := cfg.Path
+
+	// There can be potentially many instances of the module, so we need
+	// to consider each of them separately.
+	for _, modState := range prevRunState.ModuleInstances(modAddr) {
+		// What we're looking for here is either a no-key resource instance
+		// where the configuration has count set or a zero-key resource
+		// instance where the configuration _doesn't_ have count set.
+		// If so, we'll generate a statement replacing no-key with zero-key or
+		// vice-versa.
+		for _, rState := range modState.Resources {
+			rAddr := rState.Addr
+			rCfg := cfg.Module.ResourceByAddr(rAddr.Resource)
+			if rCfg == nil {
+				// If there's no configuration at all then there can't be any
+				// automatic move fixup to do.
+				continue
+			}
+			approxSrcRange := tfdiags.SourceRangeFromHCL(rCfg.DeclRange)
+
+			// NOTE: We're intentionally not checking to see whether the
+			// "to" addresses in our implied statements already have
+			// instances recorded in state, because ApplyMoves should
+			// deal with such conflicts in a deterministic way for both
+			// explicit and implicit moves, and we'd rather have that
+			// handled all in one place.
+
+			var fromKey, toKey addrs.InstanceKey
+
+			switch {
+			case rCfg.Count != nil:
+				// If we have a count expression then we'll use _that_ as
+				// a slightly-more-precise approximate source range.
+				approxSrcRange = tfdiags.SourceRangeFromHCL(rCfg.Count.Range())
+
+				if riState := rState.Instances[addrs.NoKey]; riState != nil {
+					fromKey = addrs.NoKey
+					toKey = addrs.IntKey(0)
+				}
+			default:
+				if riState := rState.Instances[addrs.IntKey(0)]; riState != nil {
+					fromKey = addrs.IntKey(0)
+					toKey = addrs.NoKey
+				}
+			}
+
+			if fromKey != toKey {
+				// We mustn't generate an impied statement if the user already
+				// wrote an explicit statement referring to this resource,
+				// because they may wish to select an instance key other than
+				// zero as the one to retain.
+				if !haveMoveStatementForResource(rAddr, explicitStmts) {
+					into = append(into, MoveStatement{
+						From:      addrs.ImpliedMoveStatementEndpoint(rAddr.Instance(fromKey), approxSrcRange),
+						To:        addrs.ImpliedMoveStatementEndpoint(rAddr.Instance(toKey), approxSrcRange),
+						DeclRange: approxSrcRange,
+						Implied:   true,
+					})
+				}
+			}
+		}
+	}
+
+	for _, childCfg := range cfg.Children {
+		into = findMoveStatements(childCfg, into)
+	}
+
+	return into
+}
+
 func (s *MoveStatement) ObjectKind() addrs.MoveEndpointKind {
 	// addrs.UnifyMoveEndpoints guarantees that both of our addresses have
 	// the same kind, so we can just arbitrary use From and assume To will
@@ -55,3 +166,21 @@ func (s *MoveStatement) ObjectKind() addrs.MoveEndpointKind {
 func (s *MoveStatement) Name() string {
 	return fmt.Sprintf("%s->%s", s.From, s.To)
 }
+
+func haveMoveStatementForResource(addr addrs.AbsResource, stmts []MoveStatement) bool {
+	// This is not a particularly optimal way to answer this question,
+	// particularly since our caller calls this function in a loop already,
+	// but we expect the total number of explicit statements to be small
+	// in any reasonable Terraform configuration and so a more complicated
+	// approach wouldn't be justified here.
+
+	for _, stmt := range stmts {
+		if stmt.From.SelectsResource(addr) {
+			return true
+		}
+		if stmt.To.SelectsResource(addr) {
+			return true
+		}
+	}
+	return false
+}

internal/refactoring/move_statement_test.go

@@ -0,0 +1,132 @@
+package refactoring
+
+import (
+	"sort"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform/internal/addrs"
+	"github.com/hashicorp/terraform/internal/states"
+	"github.com/hashicorp/terraform/internal/tfdiags"
+)
+
+func TestImpliedMoveStatements(t *testing.T) {
+	resourceAddr := func(name string) addrs.AbsResource {
+		return addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: name,
+		}.Absolute(addrs.RootModuleInstance)
+	}
+	instObjState := func() *states.ResourceInstanceObjectSrc {
+		return &states.ResourceInstanceObjectSrc{}
+	}
+	providerAddr := addrs.AbsProviderConfig{
+		Module:   addrs.RootModule,
+		Provider: addrs.MustParseProviderSourceString("hashicorp/foo"),
+	}
+
+	rootCfg, _ := loadRefactoringFixture(t, "testdata/move-statement-implied")
+	prevRunState := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(
+			resourceAddr("formerly_count").Instance(addrs.IntKey(0)),
+			instObjState(),
+			providerAddr,
+		)
+		s.SetResourceInstanceCurrent(
+			resourceAddr("formerly_count").Instance(addrs.IntKey(1)),
+			instObjState(),
+			providerAddr,
+		)
+		s.SetResourceInstanceCurrent(
+			resourceAddr("now_count").Instance(addrs.NoKey),
+			instObjState(),
+			providerAddr,
+		)
+		s.SetResourceInstanceCurrent(
+			resourceAddr("formerly_count_explicit").Instance(addrs.IntKey(0)),
+			instObjState(),
+			providerAddr,
+		)
+		s.SetResourceInstanceCurrent(
+			resourceAddr("formerly_count_explicit").Instance(addrs.IntKey(1)),
+			instObjState(),
+			providerAddr,
+		)
+		s.SetResourceInstanceCurrent(
+			resourceAddr("now_count_explicit").Instance(addrs.NoKey),
+			instObjState(),
+			providerAddr,
+		)
+
+		// This "ambiguous" resource is representing a rare but possible
+		// situation where we end up having a mixture of different index
+		// types in the state at the same time. The main way to get into
+		// this state would be to remove "count = 1" and then have the
+		// provider fail to destroy the zero-key instance even though we
+		// already created the no-key instance. Users can also get here
+		// by using "terraform state mv" in weird ways.
+		s.SetResourceInstanceCurrent(
+			resourceAddr("ambiguous").Instance(addrs.NoKey),
+			instObjState(),
+			providerAddr,
+		)
+		s.SetResourceInstanceCurrent(
+			resourceAddr("ambiguous").Instance(addrs.IntKey(0)),
+			instObjState(),
+			providerAddr,
+		)
+	})
+
+	explicitStmts := FindMoveStatements(rootCfg)
+	got := ImpliedMoveStatements(rootCfg, prevRunState, explicitStmts)
+	want := []MoveStatement{
+		{
+			From:    addrs.ImpliedMoveStatementEndpoint(resourceAddr("formerly_count").Instance(addrs.IntKey(0)), tfdiags.SourceRange{}),
+			To:      addrs.ImpliedMoveStatementEndpoint(resourceAddr("formerly_count").Instance(addrs.NoKey), tfdiags.SourceRange{}),
+			Implied: true,
+			DeclRange: tfdiags.SourceRange{
+				Filename: "testdata/move-statement-implied/move-statement-implied.tf",
+				Start:    tfdiags.SourcePos{Line: 9, Column: 1, Byte: 232},
+				End:      tfdiags.SourcePos{Line: 9, Column: 32, Byte: 263},
+			},
+		},
+		{
+			From:    addrs.ImpliedMoveStatementEndpoint(resourceAddr("now_count").Instance(addrs.NoKey), tfdiags.SourceRange{}),
+			To:      addrs.ImpliedMoveStatementEndpoint(resourceAddr("now_count").Instance(addrs.IntKey(0)), tfdiags.SourceRange{}),
+			Implied: true,
+			DeclRange: tfdiags.SourceRange{
+				Filename: "testdata/move-statement-implied/move-statement-implied.tf",
+				Start:    tfdiags.SourcePos{Line: 14, Column: 11, Byte: 334},
+				End:      tfdiags.SourcePos{Line: 14, Column: 12, Byte: 335},
+			},
+		},
+
+		// We generate foo.ambiguous[0] to foo.ambiguous here, even though
+		// there's already a foo.ambiguous in the state, because it's the
+		// responsibility of the later ApplyMoves step to deal with the
+		// situation where an object wants to move into an address already
+		// occupied by another object.
+		{
+			From:    addrs.ImpliedMoveStatementEndpoint(resourceAddr("ambiguous").Instance(addrs.IntKey(0)), tfdiags.SourceRange{}),
+			To:      addrs.ImpliedMoveStatementEndpoint(resourceAddr("ambiguous").Instance(addrs.NoKey), tfdiags.SourceRange{}),
+			Implied: true,
+			DeclRange: tfdiags.SourceRange{
+				Filename: "testdata/move-statement-implied/move-statement-implied.tf",
+				Start:    tfdiags.SourcePos{Line: 42, Column: 1, Byte: 709},
+				End:      tfdiags.SourcePos{Line: 42, Column: 27, Byte: 735},
+			},
+		},
+	}
+
+	sort.Slice(got, func(i, j int) bool {
+		// This is just an arbitrary sort to make the result consistent
+		// regardless of what order the ImpliedMoveStatements function
+		// visits the entries in the state/config.
+		return got[i].DeclRange.Start.Line < got[j].DeclRange.Start.Line
+	})
+
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("wrong result\n%s", diff)
+	}
+}

internal/refactoring/testdata/move-statement-implied/move-statement-implied.tf

@@ -0,0 +1,46 @@
+# This fixture is useful only in conjunction with a previous run state that
+# conforms to the statements encoded in the resource names. It's for
+# TestImpliedMoveStatements only.
+
+terraform {
+  experiments = [config_driven_move]
+}
+
+resource "foo" "formerly_count" {
+  # but not count anymore
+}
+
+resource "foo" "now_count" {
+  count = 2
+}
+
+resource "foo" "new_no_count" {
+}
+
+resource "foo" "new_count" {
+  count = 2
+}
+
+resource "foo" "formerly_count_explicit" {
+  # but not count anymore
+}
+
+moved {
+  from = foo.formerly_count_explicit[1]
+  to   = foo.formerly_count_explicit
+}
+
+resource "foo" "now_count_explicit" {
+  count = 2
+}
+
+moved {
+  from = foo.now_count_explicit
+  to   = foo.now_count_explicit[1]
+}
+
+resource "foo" "ambiguous" {
+  # this one doesn't have count in the config, but the test should
+  # set it up to have both no-key and zero-key instances in the
+  # state.
+}