Merge pull request #29330 from hashicorp/jbardin/move

refactoring: CanChainFrom and NestedWithin

internal/addrs/module_call.go

@@ -37,6 +37,10 @@ func (c ModuleCall) Absolute(moduleAddr ModuleInstance) AbsModuleCall {
 	}
 }
 
+func (c ModuleCall) Equal(other ModuleCall) bool {
+	return c.Name == other.Name
+}
+
 // AbsModuleCall is the address of a "module" block relative to the root
 // of the configuration.
 //
@@ -70,6 +74,10 @@ func (c AbsModuleCall) Instance(key InstanceKey) ModuleInstance {
 	return ret
 }
 
+func (c AbsModuleCall) Equal(other AbsModuleCall) bool {
+	return c.Module.Equal(other.Module) && c.Call.Equal(other.Call)
+}
+
 type absModuleCallInstanceKey string
 
 func (c AbsModuleCall) UniqueKey() UniqueKey {

internal/addrs/move_endpoint_module.go

@@ -192,6 +192,8 @@ func (e *MoveEndpointInModule) SelectsModule(addr ModuleInstance) bool {
 		if callPart.Name != relAddr.Call.Name {
 			return false
 		}
+
+		relMatch = relAddr.Module.Child(relAddr.Call.Name, callPart.InstanceKey)
 	case AbsResource:
 		relMatch = relAddr.Module
 	case AbsResourceInstance:
@@ -203,11 +205,13 @@ func (e *MoveEndpointInModule) SelectsModule(addr ModuleInstance) bool {
 	if len(relPart) != len(relMatch) {
 		return false
 	}
+
 	for i := range relMatch {
 		if relPart[i] != relMatch[i] {
 			return false
 		}
 	}
+
 	return true
 }
 
@@ -218,7 +222,35 @@ func (e *MoveEndpointInModule) SelectsModule(addr ModuleInstance) bool {
 // the reciever is the "to" from one statement and the other given address
 // is the "from" of another statement.
 func (e *MoveEndpointInModule) CanChainFrom(other *MoveEndpointInModule) bool {
-	// TODO: implement
+	eSub := e.relSubject
+	oSub := other.relSubject
+
+	switch oSub := oSub.(type) {
+	case AbsModuleCall:
+		switch eSub := eSub.(type) {
+		case AbsModuleCall:
+			return eSub.Equal(oSub)
+		}
+
+	case ModuleInstance:
+		switch eSub := eSub.(type) {
+		case ModuleInstance:
+			return eSub.Equal(oSub)
+		}
+
+	case AbsResource:
+		switch eSub := eSub.(type) {
+		case AbsResource:
+			return eSub.Equal(oSub)
+		}
+
+	case AbsResourceInstance:
+		switch eSub := eSub.(type) {
+		case AbsResourceInstance:
+			return eSub.Equal(oSub)
+		}
+	}
+
 	return false
 }
 
@@ -226,7 +258,52 @@ func (e *MoveEndpointInModule) CanChainFrom(other *MoveEndpointInModule) bool {
 // contained within one of the objects that the given other address could
 // select.
 func (e *MoveEndpointInModule) NestedWithin(other *MoveEndpointInModule) bool {
-	// TODO: implement
+	eSub := e.relSubject
+	oSub := other.relSubject
+
+	switch oSub := oSub.(type) {
+	case AbsModuleCall:
+		withinModuleCall := func(mod ModuleInstance, call AbsModuleCall) bool {
+			// parent modules don't match at all
+			if !call.Module.IsAncestor(mod) {
+				return false
+			}
+
+			rem := mod[len(call.Module):]
+			return rem[0].Name == call.Call.Name
+		}
+
+		// Module calls can contain module instances, resources, and resource
+		// instances.
+		switch eSub := eSub.(type) {
+		case AbsResource:
+			return withinModuleCall(eSub.Module, oSub)
+
+		case AbsResourceInstance:
+			return withinModuleCall(eSub.Module, oSub)
+
+		case ModuleInstance:
+			return withinModuleCall(eSub, oSub)
+		}
+
+	case ModuleInstance:
+		// Module instances can contain resources and resource instances.
+		switch eSub := eSub.(type) {
+		case AbsResource:
+			return eSub.Module.Equal(oSub) || oSub.IsAncestor(eSub.Module)
+
+		case AbsResourceInstance:
+			return eSub.Module.Equal(oSub) || oSub.IsAncestor(eSub.Module)
+		}
+
+	case AbsResource:
+		// A resource can only contain a resource instance.
+		switch eSub := eSub.(type) {
+		case AbsResourceInstance:
+			return eSub.ContainingResource().Equal(oSub)
+		}
+	}
+
 	return false
 }
 

internal/addrs/move_endpoint_module_test.go

@@ -1074,3 +1074,284 @@ func TestAbsResourceMoveDestination(t *testing.T) {
 		)
 	}
 }
+
+func TestMoveEndpointChainAndNested(t *testing.T) {
+	tests := []struct {
+		Endpoint, Other            AbsMoveable
+		CanChainFrom, NestedWithin bool
+	}{
+		{
+			Endpoint: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			Other: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			CanChainFrom: true,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint: mustParseModuleInstanceStr("module.foo[2]"),
+			Other: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			CanChainFrom: false,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint: mustParseModuleInstanceStr("module.foo[2].module.bar[2]"),
+			Other: AbsModuleCall{
+				Module: RootModuleInstance,
+				Call:   ModuleCall{Name: "foo"},
+			},
+			CanChainFrom: false,
+			NestedWithin: true,
+		},
+
+		{
+			Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].module.bar.resource.baz").ContainingResource(),
+			Other: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			CanChainFrom: false,
+			NestedWithin: true,
+		},
+
+		{
+			Endpoint: mustParseAbsResourceInstanceStr("module.foo[2].module.bar[3].resource.baz[2]"),
+			Other: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			CanChainFrom: false,
+			NestedWithin: true,
+		},
+
+		{
+			Endpoint: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			Other:        mustParseModuleInstanceStr("module.foo[2]"),
+			CanChainFrom: false,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint:     mustParseModuleInstanceStr("module.foo[2]"),
+			Other:        mustParseModuleInstanceStr("module.foo[2]"),
+			CanChainFrom: true,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint:     mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(),
+			Other:        mustParseModuleInstanceStr("module.foo[2]"),
+			CanChainFrom: false,
+			NestedWithin: true,
+		},
+
+		{
+			Endpoint:     mustParseAbsResourceInstanceStr("module.foo[2].module.bar.resource.baz"),
+			Other:        mustParseModuleInstanceStr("module.foo[2]"),
+			CanChainFrom: false,
+			NestedWithin: true,
+		},
+
+		{
+			Endpoint: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(),
+			CanChainFrom: false,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint:     mustParseModuleInstanceStr("module.foo[2]"),
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(),
+			CanChainFrom: false,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint:     mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(),
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(),
+			CanChainFrom: true,
+			NestedWithin: false,
+		},
+
+		{
+			Endpoint:     mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"),
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz[2]").ContainingResource(),
+			CanChainFrom: false,
+			NestedWithin: true,
+		},
+
+		{
+			Endpoint: AbsModuleCall{
+				Module: mustParseModuleInstanceStr("module.foo[2]"),
+				Call:   ModuleCall{Name: "bar"},
+			},
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"),
+			CanChainFrom: false,
+		},
+		{
+			Endpoint:     mustParseModuleInstanceStr("module.foo[2]"),
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"),
+			CanChainFrom: false,
+		},
+		{
+			Endpoint:     mustParseAbsResourceInstanceStr("module.foo[2].resource.baz").ContainingResource(),
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"),
+			CanChainFrom: false,
+		},
+		{
+			Endpoint:     mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"),
+			Other:        mustParseAbsResourceInstanceStr("module.foo[2].resource.baz"),
+			CanChainFrom: true,
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("[%02d]%s.CanChainFrom(%s)", i, test.Endpoint, test.Other),
+			func(t *testing.T) {
+				endpoint := &MoveEndpointInModule{
+					relSubject: test.Endpoint,
+				}
+
+				other := &MoveEndpointInModule{
+					relSubject: test.Other,
+				}
+
+				if endpoint.CanChainFrom(other) != test.CanChainFrom {
+					t.Errorf("expected %s CanChainFrom %s == %t", test.Endpoint, test.Other, test.CanChainFrom)
+				}
+
+				if endpoint.NestedWithin(other) != test.NestedWithin {
+					t.Errorf("expected %s NestedWithin %s == %t", test.Endpoint, test.Other, test.NestedWithin)
+				}
+			},
+		)
+	}
+}
+
+func TestSelectsModule(t *testing.T) {
+	tests := []struct {
+		Endpoint *MoveEndpointInModule
+		Addr     ModuleInstance
+		Selects  bool
+	}{
+		{
+			Endpoint: &MoveEndpointInModule{
+				relSubject: AbsModuleCall{
+					Module: mustParseModuleInstanceStr("module.foo[2]"),
+					Call:   ModuleCall{Name: "bar"},
+				},
+			},
+			Addr:    mustParseModuleInstanceStr("module.foo[2].module.bar[1]"),
+			Selects: true,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				module: mustParseModuleInstanceStr("module.foo").Module(),
+				relSubject: AbsModuleCall{
+					Module: mustParseModuleInstanceStr("module.bar[2]"),
+					Call:   ModuleCall{Name: "baz"},
+				},
+			},
+			Addr:    mustParseModuleInstanceStr("module.foo[2].module.bar[2].module.baz"),
+			Selects: true,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				module: mustParseModuleInstanceStr("module.foo").Module(),
+				relSubject: AbsModuleCall{
+					Module: mustParseModuleInstanceStr("module.bar[2]"),
+					Call:   ModuleCall{Name: "baz"},
+				},
+			},
+			Addr:    mustParseModuleInstanceStr("module.foo[2].module.bar[1].module.baz"),
+			Selects: false,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				relSubject: AbsModuleCall{
+					Module: mustParseModuleInstanceStr("module.bar"),
+					Call:   ModuleCall{Name: "baz"},
+				},
+			},
+			Addr:    mustParseModuleInstanceStr("module.bar[1].module.baz"),
+			Selects: false,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				module:     mustParseModuleInstanceStr("module.foo").Module(),
+				relSubject: mustParseAbsResourceInstanceStr(`module.bar.resource.name["key"]`),
+			},
+			Addr:    mustParseModuleInstanceStr(`module.foo[1].module.bar`),
+			Selects: true,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				relSubject: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`),
+			},
+			Addr:    mustParseModuleInstanceStr(`module.bar.module.baz["key"]`),
+			Selects: true,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				relSubject: mustParseAbsResourceInstanceStr(`module.bar.module.baz["key"].resource.name`).ContainingResource(),
+			},
+			Addr:    mustParseModuleInstanceStr(`module.bar.module.baz["key"]`),
+			Selects: true,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				module:     mustParseModuleInstanceStr("module.nope").Module(),
+				relSubject: mustParseAbsResourceInstanceStr(`module.bar.resource.name["key"]`),
+			},
+			Addr:    mustParseModuleInstanceStr(`module.foo[1].module.bar`),
+			Selects: false,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				relSubject: mustParseModuleInstanceStr(`module.bar.module.baz["key"]`),
+			},
+			Addr:    mustParseModuleInstanceStr(`module.bar.module.baz["nope"]`),
+			Selects: false,
+		},
+		{
+			Endpoint: &MoveEndpointInModule{
+				relSubject: mustParseAbsResourceInstanceStr(`module.nope.module.baz["key"].resource.name`).ContainingResource(),
+			},
+			Addr:    mustParseModuleInstanceStr(`module.bar.module.baz["key"]`),
+			Selects: false,
+		},
+	}
+
+	for i, test := range tests {
+		t.Run(fmt.Sprintf("[%02d]%s.SelectsModule(%s)", i, test.Endpoint, test.Addr),
+			func(t *testing.T) {
+				if test.Endpoint.SelectsModule(test.Addr) != test.Selects {
+					t.Errorf("expected %s SelectsModule %s == %t", test.Endpoint, test.Addr, test.Selects)
+				}
+			},
+		)
+	}
+}
+
+func mustParseAbsResourceInstanceStr(s string) AbsResourceInstance {
+	r, diags := ParseAbsResourceInstanceStr(s)
+	if diags.HasErrors() {
+		panic(diags.ErrWithWarnings().Error())
+	}
+	return r
+}

internal/refactoring/move_execute.go

@@ -28,6 +28,8 @@ type MoveResult struct {
 // ApplyMoves expects exclusive access to the given state while it's running.
 // Don't read or write any part of the state structure until ApplyMoves returns.
 func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]MoveResult {
+	results := make(map[addrs.UniqueKey]MoveResult)
+
 	// The methodology here is to construct a small graph of all of the move
 	// statements where the edges represent where a particular statement
 	// is either chained from or nested inside the effect of another statement.
@@ -40,19 +42,18 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
 	// at all. The separate validation step should detect this and return
 	// an error.
 	if len(g.Cycles()) != 0 {
-		return nil
+		return results
 	}
 
 	// The starting nodes are the ones that don't depend on any other nodes.
 	startNodes := make(dag.Set, len(stmts))
 	for _, v := range g.Vertices() {
-		if len(g.UpEdges(v)) == 0 {
+		if len(g.DownEdges(v)) == 0 {
 			startNodes.Add(v)
 		}
 	}
 
-	results := make(map[addrs.UniqueKey]MoveResult)
+	g.ReverseDepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error {
-	g.DepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error {
 		stmt := v.(*MoveStatement)
 
 		for _, ms := range state.Modules {
@@ -147,9 +148,9 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) map[addrs.UniqueKey]
 // may contain cycles and other sorts of invalidity.
 func buildMoveStatementGraph(stmts []MoveStatement) *dag.AcyclicGraph {
 	g := &dag.AcyclicGraph{}
-	for _, stmt := range stmts {
+	for i := range stmts {
 		// The graph nodes are pointers to the actual statements directly.
-		g.Add(&stmt)
+		g.Add(&stmts[i])
 	}
 
 	// Now we'll add the edges representing chaining and nesting relationships.

internal/refactoring/move_execute_test.go

@@ -15,39 +15,108 @@ import (
 )
 
 func TestApplyMoves(t *testing.T) {
-	// TODO: Renable this once we're ready to implement the intended behaviors
-	// it is describing.
-	t.Skip("ApplyMoves is not yet fully implemented")
-
 	providerAddr := addrs.AbsProviderConfig{
 		Module:   addrs.RootModule,
 		Provider: addrs.MustParseProviderSourceString("example.com/foo/bar"),
 	}
-	rootNoKeyResourceAddr := [...]addrs.AbsResourceInstance{
+
-		addrs.Resource{
+	moduleBoo, _ := addrs.ParseModuleInstanceStr("module.boo")
+	moduleBarKey, _ := addrs.ParseModuleInstanceStr("module.bar[0]")
+
+	instAddrs := map[string]addrs.AbsResourceInstance{
+		"foo.from": addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
 			Type: "foo",
 			Name: "from",
 		}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
-		addrs.Resource{
+
+		"foo.mid": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "mid",
+		}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
+
+		"foo.to": addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
 			Type: "foo",
 			Name: "to",
 		}.Instance(addrs.NoKey).Absolute(addrs.RootModuleInstance),
-	}
+
-	rootIntKeyResourceAddr := [...]addrs.AbsResourceInstance{
+		"foo.from[0]": addrs.Resource{
-		addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
 			Type: "foo",
 			Name: "from",
 		}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance),
-		addrs.Resource{
+
+		"foo.to[0]": addrs.Resource{
 			Mode: addrs.ManagedResourceMode,
 			Type: "foo",
 			Name: "to",
 		}.Instance(addrs.IntKey(0)).Absolute(addrs.RootModuleInstance),
+
+		"module.boo.foo.from": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.NoKey).Absolute(moduleBoo),
+
+		"module.boo.foo.mid": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "mid",
+		}.Instance(addrs.NoKey).Absolute(moduleBoo),
+
+		"module.boo.foo.to": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "to",
+		}.Instance(addrs.NoKey).Absolute(moduleBoo),
+
+		"module.boo.foo.from[0]": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.IntKey(0)).Absolute(moduleBoo),
+
+		"module.boo.foo.to[0]": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "to",
+		}.Instance(addrs.IntKey(0)).Absolute(moduleBoo),
+
+		"module.bar[0].foo.from": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.NoKey).Absolute(moduleBarKey),
+
+		"module.bar[0].foo.mid": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "mid",
+		}.Instance(addrs.NoKey).Absolute(moduleBarKey),
+
+		"module.bar[0].foo.to": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "to",
+		}.Instance(addrs.NoKey).Absolute(moduleBarKey),
+
+		"module.bar[0].foo.from[0]": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "from",
+		}.Instance(addrs.IntKey(0)).Absolute(moduleBarKey),
+
+		"module.bar[0].foo.to[0]": addrs.Resource{
+			Mode: addrs.ManagedResourceMode,
+			Type: "foo",
+			Name: "to",
+		}.Instance(addrs.IntKey(0)).Absolute(moduleBarKey),
 	}
 
+	emptyResults := map[addrs.UniqueKey]MoveResult{}
+
 	tests := map[string]struct {
 		Stmts []MoveStatement
 		State *states.State
@@ -58,14 +127,14 @@ func TestApplyMoves(t *testing.T) {
 		"no moves and empty state": {
 			[]MoveStatement{},
 			states.NewState(),
-			nil,
+			emptyResults,
 			nil,
 		},
 		"no moves": {
 			[]MoveStatement{},
 			states.BuildState(func(s *states.SyncState) {
 				s.SetResourceInstanceCurrent(
-					rootNoKeyResourceAddr[0],
+					instAddrs["foo.from"],
 					&states.ResourceInstanceObjectSrc{
 						Status:    states.ObjectReady,
 						AttrsJSON: []byte(`{}`),
@@ -73,7 +142,7 @@ func TestApplyMoves(t *testing.T) {
 					providerAddr,
 				)
 			}),
-			nil,
+			emptyResults,
 			[]string{
 				`foo.from`,
 			},
@@ -84,7 +153,7 @@ func TestApplyMoves(t *testing.T) {
 			},
 			states.BuildState(func(s *states.SyncState) {
 				s.SetResourceInstanceCurrent(
-					rootNoKeyResourceAddr[0],
+					instAddrs["foo.from"],
 					&states.ResourceInstanceObjectSrc{
 						Status:    states.ObjectReady,
 						AttrsJSON: []byte(`{}`),
@@ -93,13 +162,13 @@ func TestApplyMoves(t *testing.T) {
 				)
 			}),
 			map[addrs.UniqueKey]MoveResult{
-				rootNoKeyResourceAddr[0].UniqueKey(): {
+				instAddrs["foo.from"].UniqueKey(): {
-					From: rootNoKeyResourceAddr[0],
+					From: instAddrs["foo.from"],
-					To:   rootNoKeyResourceAddr[1],
+					To:   instAddrs["foo.to"],
 				},
-				rootNoKeyResourceAddr[1].UniqueKey(): {
+				instAddrs["foo.to"].UniqueKey(): {
-					From: rootNoKeyResourceAddr[1],
+					From: instAddrs["foo.from"],
-					To:   rootNoKeyResourceAddr[1],
+					To:   instAddrs["foo.to"],
 				},
 			},
 			[]string{
@@ -112,7 +181,7 @@ func TestApplyMoves(t *testing.T) {
 			},
 			states.BuildState(func(s *states.SyncState) {
 				s.SetResourceInstanceCurrent(
-					rootIntKeyResourceAddr[0],
+					instAddrs["foo.from[0]"],
 					&states.ResourceInstanceObjectSrc{
 						Status:    states.ObjectReady,
 						AttrsJSON: []byte(`{}`),
@@ -121,26 +190,206 @@ func TestApplyMoves(t *testing.T) {
 				)
 			}),
 			map[addrs.UniqueKey]MoveResult{
-				rootNoKeyResourceAddr[0].UniqueKey(): {
+				instAddrs["foo.from[0]"].UniqueKey(): {
-					From: rootIntKeyResourceAddr[0],
+					From: instAddrs["foo.from[0]"],
-					To:   rootIntKeyResourceAddr[1],
+					To:   instAddrs["foo.to[0]"],
 				},
-				rootNoKeyResourceAddr[1].UniqueKey(): {
+				instAddrs["foo.to[0]"].UniqueKey(): {
-					From: rootIntKeyResourceAddr[0],
+					From: instAddrs["foo.from[0]"],
-					To:   rootIntKeyResourceAddr[1],
+					To:   instAddrs["foo.to[0]"],
 				},
 			},
 			[]string{
 				`foo.to[0]`,
 			},
 		},
+		"chained move of whole singleton resource": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "foo.from", "foo.mid"),
+				testMoveStatement(t, "", "foo.mid", "foo.to"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["foo.from"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			map[addrs.UniqueKey]MoveResult{
+				instAddrs["foo.from"].UniqueKey(): {
+					From: instAddrs["foo.from"],
+					To:   instAddrs["foo.mid"],
+				},
+				instAddrs["foo.mid"].UniqueKey(): {
+					From: instAddrs["foo.mid"],
+					To:   instAddrs["foo.to"],
+				},
+				instAddrs["foo.to"].UniqueKey(): {
+					From: instAddrs["foo.mid"],
+					To:   instAddrs["foo.to"],
+				},
+			},
+			[]string{
+				`foo.to`,
+			},
+		},
+
+		"move whole resource into module": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "foo.from", "module.boo.foo.to"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["foo.from[0]"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			map[addrs.UniqueKey]MoveResult{
+				instAddrs["foo.from[0]"].UniqueKey(): {
+					From: instAddrs["foo.from[0]"],
+					To:   instAddrs["module.boo.foo.to[0]"],
+				},
+				instAddrs["module.boo.foo.to[0]"].UniqueKey(): {
+					From: instAddrs["foo.from[0]"],
+					To:   instAddrs["module.boo.foo.to[0]"],
+				},
+			},
+			[]string{
+				`module.boo.foo.to[0]`,
+			},
+		},
+
+		"move resource instance between modules": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "module.boo.foo.from[0]", "module.bar[0].foo.to[0]"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["module.boo.foo.from[0]"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			map[addrs.UniqueKey]MoveResult{
+				instAddrs["module.boo.foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.to[0]"],
+				},
+				instAddrs["module.bar[0].foo.to[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.to[0]"],
+				},
+			},
+			[]string{
+				`module.bar[0].foo.to[0]`,
+			},
+		},
+
+		"move whole single module to indexed module": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "module.boo", "module.bar[0]"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["module.boo.foo.from[0]"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			map[addrs.UniqueKey]MoveResult{
+				instAddrs["module.boo.foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.from[0]"],
+				},
+				instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.from[0]"],
+				},
+			},
+			[]string{
+				`module.bar[0].foo.from[0]`,
+			},
+		},
+
+		"move whole module to within indexed module and instance chained": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "module.boo", "module.bar[0]"),
+				testMoveStatement(t, "module.bar[0]", "foo.from[0]", "foo.too[0]"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["module.boo.foo.from[0]"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			map[addrs.UniqueKey]MoveResult{
+				instAddrs["module.boo.foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.from[0]"],
+				},
+				instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.from[0]"],
+				},
+			},
+			[]string{
+				`module.bar[0].foo.from[0]`,
+			},
+		},
+
+		"move instance to indexed module and instance chained": {
+			[]MoveStatement{
+				testMoveStatement(t, "", "module.boo.foo.from[0]", "module.bar[0].foo.from[0]"),
+				testMoveStatement(t, "module.bar[0]", "foo.from[0]", "foo.too[0]"),
+			},
+			states.BuildState(func(s *states.SyncState) {
+				s.SetResourceInstanceCurrent(
+					instAddrs["module.boo.foo.from[0]"],
+					&states.ResourceInstanceObjectSrc{
+						Status:    states.ObjectReady,
+						AttrsJSON: []byte(`{}`),
+					},
+					providerAddr,
+				)
+			}),
+			map[addrs.UniqueKey]MoveResult{
+				instAddrs["module.boo.foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.from[0]"],
+				},
+				instAddrs["module.bar[0].foo.from[0]"].UniqueKey(): {
+					From: instAddrs["module.boo.foo.from[0]"],
+					To:   instAddrs["module.bar[0].foo.from[0]"],
+				},
+			},
+			[]string{
+				`module.bar[0].foo.from[0]`,
+			},
+		},
 	}
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
 			var stmtsBuf strings.Builder
 			for _, stmt := range test.Stmts {
-				fmt.Fprintf(&stmtsBuf, "- from: %s\n  to:   %s", stmt.From, stmt.To)
+				fmt.Fprintf(&stmtsBuf, "- from: %s\n  to:   %s\n", stmt.From, stmt.To)
 			}
 			t.Logf("move statements:\n%s", stmtsBuf.String())
 

internal/refactoring/move_statement.go

@@ -50,3 +50,8 @@ func (s *MoveStatement) ObjectKind() addrs.MoveEndpointKind {
 	// match it.
 	return s.From.ObjectKind()
 }
+
+// Name is used internally for displaying the statement graph
+func (s *MoveStatement) Name() string {
+	return fmt.Sprintf("%s->%s", s.From, s.To)
+}