decode change before creating diff

This is functionally equivalent, but will allow us to filter the change
values directly for reduced drift output.

internal/command/format/diff.go

@@ -45,7 +45,7 @@ const (
 // If "color" is non-nil, it will be used to color the result. Otherwise,
 // no color codes will be included.
 func ResourceChange(
-	change *plans.ResourceInstanceChangeSrc,
+	change *plans.ResourceInstanceChange,
 	schema *configschema.Block,
 	color *colorstring.Colorize,
 	language DiffLanguage,
@@ -187,24 +187,7 @@ func ResourceChange(
 	// structures.
 	path := make(cty.Path, 0, 3)
 
-	changeV, err := change.Decode(schema.ImpliedType())
+	result := p.writeBlockBodyDiff(schema, change.Before, change.After, 6, path)
-	if err != nil {
-		// Should never happen in here, since we've already been through
-		// loads of layers of encode/decode of the planned changes before now.
-		panic(fmt.Sprintf("failed to decode plan for %s while rendering diff: %s", addr, err))
-	}
-
-	// We currently have an opt-out that permits the legacy SDK to return values
-	// that defy our usual conventions around handling of nesting blocks. To
-	// avoid the rendering code from needing to handle all of these, we'll
-	// normalize first.
-	// (Ideally we'd do this as part of the SDK opt-out implementation in core,
-	// but we've added it here for now to reduce risk of unexpected impacts
-	// on other code in core.)
-	changeV.Change.Before = objchange.NormalizeObjectFromLegacySDK(changeV.Change.Before, schema)
-	changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema)
-
-	result := p.writeBlockBodyDiff(schema, changeV.Before, changeV.After, 6, path)
 	if result.bodyWritten {
 		buf.WriteString("\n")
 		buf.WriteString(strings.Repeat(" ", 4))

internal/command/format/diff_test.go

@@ -4857,10 +4857,6 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 			case !beforeVal.IsKnown():
 				beforeVal = cty.UnknownVal(ty) // allow mistyped unknowns
 			}
-			before, err := plans.NewDynamicValue(beforeVal, ty)
-			if err != nil {
-				t.Fatal(err)
-			}
 
 			afterVal := tc.After
 			switch { // Some fixups to make the test cases a little easier to write
@@ -4869,10 +4865,6 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 			case !afterVal.IsKnown():
 				afterVal = cty.UnknownVal(ty) // allow mistyped unknowns
 			}
-			after, err := plans.NewDynamicValue(afterVal, ty)
-			if err != nil {
-				t.Fatal(err)
-			}
 
 			addr := addrs.Resource{
 				Mode: tc.Mode,
@@ -4887,7 +4879,7 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 				prevRunAddr = addr
 			}
 
-			change := &plans.ResourceInstanceChangeSrc{
+			change := &plans.ResourceInstanceChange{
 				Addr:        addr,
 				PrevRunAddr: prevRunAddr,
 				DeposedKey:  tc.DeposedKey,
@@ -4895,12 +4887,10 @@ func runTestCases(t *testing.T, testCases map[string]testCase) {
 					Provider: addrs.NewDefaultProvider("test"),
 					Module:   addrs.RootModule,
 				},
-				ChangeSrc: plans.ChangeSrc{
+				Change: plans.Change{
 					Action: tc.Action,
-					Before:         before,
+					Before: beforeVal.MarkWithPaths(tc.BeforeValMarks),
-					After:          after,
+					After:  afterVal.MarkWithPaths(tc.AfterValMarks),
-					BeforeValMarks: tc.BeforeValMarks,
-					AfterValMarks:  tc.AfterValMarks,
 				},
 				ActionReason:    tc.ActionReason,
 				RequiredReplace: tc.RequiredReplace,

internal/command/views/plan.go

@@ -9,7 +9,9 @@ import (
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/command/arguments"
 	"github.com/hashicorp/terraform/internal/command/format"
+	"github.com/hashicorp/terraform/internal/configs/configschema"
 	"github.com/hashicorp/terraform/internal/plans"
+	"github.com/hashicorp/terraform/internal/plans/objchange"
 	"github.com/hashicorp/terraform/internal/terraform"
 	"github.com/hashicorp/terraform/internal/tfdiags"
 )
@@ -274,7 +276,7 @@ func renderPlan(plan *plans.Plan, schemas *terraform.Schemas, view *View) {
 			}
 
 			view.streams.Println(format.ResourceChange(
-				rcs,
+				decodeChange(rcs, rSchema),
 				rSchema,
 				view.colorize,
 				format.DiffLanguageProposedChange,
@@ -401,7 +403,7 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas
 		}
 
 		view.streams.Println(format.ResourceChange(
-			rcs,
+			decodeChange(rcs, rSchema),
 			rSchema,
 			view.colorize,
 			format.DiffLanguageDetectedDrift,
@@ -424,6 +426,26 @@ func renderChangesDetectedByRefresh(plan *plans.Plan, schemas *terraform.Schemas
 	return true
 }
 
+func decodeChange(change *plans.ResourceInstanceChangeSrc, schema *configschema.Block) *plans.ResourceInstanceChange {
+	changeV, err := change.Decode(schema.ImpliedType())
+	if err != nil {
+		// Should never happen in here, since we've already been through
+		// loads of layers of encode/decode of the planned changes before now.
+		panic(fmt.Sprintf("failed to decode plan for %s while rendering diff: %s", change.Addr, err))
+	}
+
+	// We currently have an opt-out that permits the legacy SDK to return values
+	// that defy our usual conventions around handling of nesting blocks. To
+	// avoid the rendering code from needing to handle all of these, we'll
+	// normalize first.
+	// (Ideally we'd do this as part of the SDK opt-out implementation in core,
+	// but we've added it here for now to reduce risk of unexpected impacts
+	// on other code in core.)
+	changeV.Change.Before = objchange.NormalizeObjectFromLegacySDK(changeV.Change.Before, schema)
+	changeV.Change.After = objchange.NormalizeObjectFromLegacySDK(changeV.Change.After, schema)
+	return changeV
+}
+
 const planHeaderIntro = `
 Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
 `

internal/plans/changes.go

@@ -234,6 +234,10 @@ func (rc *ResourceInstanceChange) Encode(ty cty.Type) (*ResourceInstanceChangeSr
 	}, err
 }
 
+func (rc *ResourceInstanceChange) Moved() bool {
+	return !rc.Addr.Equal(rc.PrevRunAddr)
+}
+
 // Simplify will, where possible, produce a change with a simpler action than
 // the receiever given a flag indicating whether the caller is dealing with
 // a normal apply or a destroy. This flag deals with the fact that Terraform