core: do not refresh when forgetting instances

internal/terraform/context_apply2_test.go

@@ -2743,6 +2743,11 @@ removed {
 		t.Fatalf("diags: %s", diags.Err())
 	}
 
+	// check that the provider was not asked to refresh the resource
+	if p.ReadResourceCalled {
+		t.Fatalf("Expected ReadResource not to be called, but it was called")
+	}
+
 	// check that the provider was not asked to destroy the resource
 	if p.ApplyResourceChangeCalled {
 		t.Fatalf("Expected ApplyResourceChange not to be called, but it was called")
@@ -2789,6 +2794,11 @@ removed {
 		t.Fatalf("diags: %s", diags.Err())
 	}
 
+	// check that the provider was not asked to refresh the resource
+	if p.ReadResourceCalled {
+		t.Fatalf("Expected ReadResource not to be called, but it was called")
+	}
+
 	// check that the provider was not asked to destroy the resource
 	if p.ApplyResourceChangeCalled {
 		t.Fatalf("Expected ApplyResourceChange not to be called, but it was called")

internal/terraform/node_resource_destroy_deposed.go

@@ -108,12 +108,26 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk
 		return diags
 	}
 
+	var forget bool
+	for _, ft := range n.forgetResources {
+		if ft.Equal(n.ResourceAddr()) {
+			forget = true
+		}
+	}
+	for _, fm := range n.forgetModules {
+		if fm.TargetContains(n.Addr) {
+			forget = true
+		}
+	}
+
 	// We don't refresh during the planDestroy walk, since that is only adding
 	// the destroy changes to the plan and the provider will not be configured
 	// at this point. The other nodes use separate types for plan and destroy,
 	// while deposed instances are always a destroy or forget operation, so the
 	// logic here is a bit overloaded.
-	if !n.skipRefresh && op != walkPlanDestroy {
+	//
+	// We also don't refresh when forgetting instances, as it is unnecessary.
+	if !n.skipRefresh && op != walkPlanDestroy && !forget {
 		// Refresh this object even though it may be destroyed, in
 		// case it's already been deleted outside of Terraform. If this is a
 		// normal plan, providers expect a Read request to remove missing
@@ -149,17 +163,6 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk
 	}
 
 	if !n.skipPlanChanges {
-		var forget bool
-		for _, ft := range n.forgetResources {
-			if ft.Equal(n.ResourceAddr()) {
-				forget = true
-			}
-		}
-		for _, fm := range n.forgetModules {
-			if fm.TargetContains(n.Addr) {
-				forget = true
-			}
-		}
 		var change *plans.ResourceInstanceChange
 		var pDiags tfdiags.Diagnostics
 		var deferred *providers.Deferred

internal/terraform/node_resource_plan_orphan.go

@@ -110,7 +110,40 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon
 		return diags
 	}
 
-	if !n.skipRefresh {
+	var forget bool
+	for _, ft := range n.forgetResources {
+		if ft.Equal(n.ResourceAddr()) {
+			forget = true
+		}
+	}
+	for _, fm := range n.forgetModules {
+		if fm.TargetContains(n.Addr) {
+			forget = true
+		}
+	}
+	var change *plans.ResourceInstanceChange
+	var pDiags tfdiags.Diagnostics
+	var deferred *providers.Deferred
+	if forget {
+		change, pDiags = n.planForget(ctx, oldState, "")
+		diags = diags.Append(pDiags)
+	} else {
+		change, deferred, pDiags = n.planDestroy(ctx, oldState, "")
+		diags = diags.Append(pDiags)
+
+		if deferred != nil {
+			ctx.Deferrals().ReportResourceInstanceDeferred(n.Addr, deferred.Reason, &plans.ResourceInstanceChange{
+				Addr:   n.Addr,
+				Change: change.Change,
+			})
+			return diags
+		}
+	}
+	if diags.HasErrors() {
+		return diags
+	}
+
+	if !n.skipRefresh && !forget {
 		// Refresh this instance even though it is going to be destroyed, in
 		// order to catch missing resources. If this is a normal plan,
 		// providers expect a Read request to remove missing resources from the
@@ -152,39 +185,6 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon
 		return diags.Append(n.writeResourceInstanceState(ctx, oldState, workingState))
 	}
 
-	var forget bool
-	for _, ft := range n.forgetResources {
-		if ft.Equal(n.ResourceAddr()) {
-			forget = true
-		}
-	}
-	for _, fm := range n.forgetModules {
-		if fm.TargetContains(n.Addr) {
-			forget = true
-		}
-	}
-	var change *plans.ResourceInstanceChange
-	var pDiags tfdiags.Diagnostics
-	var deferred *providers.Deferred
-	if forget {
-		change, pDiags = n.planForget(ctx, oldState, "")
-		diags = diags.Append(pDiags)
-	} else {
-		change, deferred, pDiags = n.planDestroy(ctx, oldState, "")
-		diags = diags.Append(pDiags)
-
-		if deferred != nil {
-			ctx.Deferrals().ReportResourceInstanceDeferred(n.Addr, deferred.Reason, &plans.ResourceInstanceChange{
-				Addr:   n.Addr,
-				Change: change.Change,
-			})
-			return diags
-		}
-	}
-	if diags.HasErrors() {
-		return diags
-	}
-
 	// We might be able to offer an approximate reason for why we are
 	// planning to delete this object. (This is best-effort; we might
 	// sometimes not have a reason.)