refactoring: Use addrs.Map for maps with addresses as keys

We introduced the addrs.UniqueKey and addrs.UniqueKeyer mechanics as part
of implementing the ValidateMoves and ApplyMoves functions, as a way to
better encapsulate the solution to the problem that lots of our address
types aren't comparable and so cannot be used directly as map keys.

However, exposing addrs.UniqueKey handling directly in the logic adds
various noise to the algorithms and, in particular, obscures the fact that
MoveResults.Changes and MoveResult.Blocked both have different map key
types.

Here then we'll use the new addrs.Map helper type, which encapsulates the
idea of a map from an addrs.UniqueKeyer type to an arbitrary value type,
using the unique keys as the map keys internally. This does unfortunately
mean that we lose the conventional Go map access syntax and have to use
a method-based API instead, but I (subjectively) think that's an okay
compromise in return for avoiding the need to keep track inline of which
addrs.UniqueKey values correspond with which real addresses.

This is intended as an entirely-mechanical change, with equivalent
behavior to what it replaced. If anything here is doing something
materially different than what it replaced then that's a mistake.

internal/addrs/set.go

@@ -7,24 +7,24 @@ package addrs
 // by ranging over the map values, but making direct modifications could
 // potentially make the set data invalid or inconsistent, leading to undefined
 // behavior elsewhere.
-type Set[T UniqueKeyer] map[UniqueKey]UniqueKeyer
+type Set[T UniqueKeyer] map[UniqueKey]T
 
 // Has returns true if and only if the set includes the given address.
-func (s Set[T]) Has(addr UniqueKeyer) bool {
+func (s Set[T]) Has(addr T) bool {
 	_, exists := s[addr.UniqueKey()]
 	return exists
 }
 
 // Add inserts the given address into the set, if not already present. If
 // an equivalent address is already in the set, this replaces that address
 // with the new value.
-func (s Set[T]) Add(addr UniqueKeyer) {
+func (s Set[T]) Add(addr T) {
 	s[addr.UniqueKey()] = addr
 }
 
 // Remove deletes the given address from the set, if present. If not present,
 // this is a no-op.
-func (s Set[T]) Remove(addr UniqueKeyer) {
+func (s Set[T]) Remove(addr T) {
 	delete(s, addr.UniqueKey())
 }
 

internal/refactoring/move_execute.go

@@ -26,10 +26,7 @@ import (
 // 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) MoveResults {
-	ret := MoveResults{
-		Changes: make(map[addrs.UniqueKey]MoveSuccess),
-		Blocked: make(map[addrs.UniqueKey]MoveBlocked),
-	}
+	ret := makeMoveResults()
 
 	if len(stmts) == 0 {
 		return ret
@@ -71,25 +68,23 @@ func ApplyMoves(stmts []MoveStatement, state *states.State) MoveResults {
 	log.Printf("[TRACE] refactoring.ApplyMoves: Processing 'moved' statements in the configuration\n%s", logging.Indent(g.String()))
 
 	recordOldAddr := func(oldAddr, newAddr addrs.AbsResourceInstance) {
-		oldAddrKey := oldAddr.UniqueKey()
-		newAddrKey := newAddr.UniqueKey()
-		if prevMove, exists := ret.Changes[oldAddrKey]; exists {
+		if prevMove, exists := ret.Changes.GetOk(oldAddr); exists {
 			// If the old address was _already_ the result of a move then
 			// we'll replace that entry so that our results summarize a chain
 			// of moves into a single entry.
-			delete(ret.Changes, oldAddrKey)
+			ret.Changes.Remove(oldAddr)
 			oldAddr = prevMove.From
 		}
-		ret.Changes[newAddrKey] = MoveSuccess{
+		ret.Changes.Put(newAddr, MoveSuccess{
 			From: oldAddr,
 			To:   newAddr,
-		}
+		})
 	}
 	recordBlockage := func(newAddr, wantedAddr addrs.AbsMoveable) {
-		ret.Blocked[newAddr.UniqueKey()] = MoveBlocked{
+		ret.Blocked.Put(newAddr, MoveBlocked{
 			Wanted: wantedAddr,
 			Actual: newAddr,
-		}
+		})
 	}
 
 	g.ReverseDepthFirstWalk(startNodes, func(v dag.Vertex, depth int) error {
@@ -292,7 +287,7 @@ type MoveResults struct {
 	//
 	// In the return value from ApplyMoves, all of the keys are guaranteed to
 	// be unique keys derived from addrs.AbsResourceInstance values.
-	Changes map[addrs.UniqueKey]MoveSuccess
+	Changes addrs.Map[addrs.AbsResourceInstance, MoveSuccess]
 
 	// Blocked is a map from the unique keys of the final new
 	// resource instances addresses to information about where they "wanted"
@@ -308,7 +303,14 @@ type MoveResults struct {
 	//
 	// In the return value from ApplyMoves, all of the keys are guaranteed to
 	// be unique keys derived from values of addrs.AbsMoveable types.
-	Blocked map[addrs.UniqueKey]MoveBlocked
+	Blocked addrs.Map[addrs.AbsMoveable, MoveBlocked]
+}
+
+func makeMoveResults() MoveResults {
+	return MoveResults{
+		Changes: addrs.MakeMap[addrs.AbsResourceInstance, MoveSuccess](),
+		Blocked: addrs.MakeMap[addrs.AbsMoveable, MoveBlocked](),
+	}
 }
 
 type MoveSuccess struct {
@@ -327,15 +329,14 @@ type MoveBlocked struct {
 // If AddrMoved returns true, you can pass the same address to method OldAddr
 // to find its original address prior to moving.
 func (rs MoveResults) AddrMoved(newAddr addrs.AbsResourceInstance) bool {
-	_, ok := rs.Changes[newAddr.UniqueKey()]
-	return ok
+	return rs.Changes.Has(newAddr)
 }
 
 // OldAddr returns the old address of the given resource instance address, or
 // just returns back the same address if the given instance wasn't affected by
 // any move statements.
 func (rs MoveResults) OldAddr(newAddr addrs.AbsResourceInstance) addrs.AbsResourceInstance {
-	change, ok := rs.Changes[newAddr.UniqueKey()]
+	change, ok := rs.Changes.GetOk(newAddr)
 	if !ok {
 		return newAddr
 	}