Updated signatures to consistently use diff objects

tools/diff-processor/breaking_changes/breaking_changes.go

@@ -27,7 +27,7 @@ func ComputeBreakingChanges(schemaDiff diff.SchemaDiff) []BreakingChange {
 	var breakingChanges []BreakingChange
 	for resource, resourceDiff := range schemaDiff {
 		for _, rule := range ResourceConfigDiffRules {
-			for _, message := range rule.Messages(resource, resourceDiff.ResourceConfig.Old, resourceDiff.ResourceConfig.New) {
+			for _, message := range rule.Messages(resource, resourceDiff.ResourceConfig) {
 				breakingChanges = append(breakingChanges, NewBreakingChange(message, rule.Identifier))
 			}
 		}
@@ -45,7 +45,7 @@ func ComputeBreakingChanges(schemaDiff diff.SchemaDiff) []BreakingChange {
 
 		for field, fieldDiff := range resourceDiff.Fields {
 			for _, rule := range FieldDiffRules {
-				for _, message := range rule.Messages(resource, field, fieldDiff.Old, fieldDiff.New) {
+				for _, message := range rule.Messages(resource, field, fieldDiff) {
 					breakingChanges = append(breakingChanges, NewBreakingChange(message, rule.Identifier))
 				}
 			}

tools/diff-processor/breaking_changes/field_diff.go

@@ -5,14 +5,15 @@ import (
 	"strconv"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+
+	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
 )
 
 // FieldDiffRule provides structure for rules
 // regarding field attribute changes
 type FieldDiffRule struct {
 	Identifier string
-	// TODO: change signature to take FieldDiff instead of old, new.
-	Messages func(resource, field string, old, new *schema.Schema) []string
+	Messages func(resource, field string, fieldDiff diff.FieldDiff) []string
 }
 
 // FieldDiffRules is a list of FieldDiffRule
@@ -34,23 +35,23 @@ var FieldChangingType = FieldDiffRule{
 	Messages:   FieldChangingTypeMessages,
 }
 
-func FieldChangingTypeMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldChangingTypeMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// Type change doesn't matter for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	tmpl := "Field `%s` changed from %s to %s on `%s`"
-	if old.Type != new.Type {
-		oldType := getValueType(old.Type)
-		newType := getValueType(new.Type)
+	if fieldDiff.Old.Type != fieldDiff.New.Type {
+		oldType := getValueType(fieldDiff.Old.Type)
+		newType := getValueType(fieldDiff.New.Type)
 		return []string{fmt.Sprintf(tmpl, field, oldType, newType, resource)}
 	}
 
-	oldCasted, _ := old.Elem.(*schema.Schema)
-	newCasted, _ := new.Elem.(*schema.Schema)
+	oldCasted, _ := fieldDiff.Old.Elem.(*schema.Schema)
+	newCasted, _ := fieldDiff.New.Elem.(*schema.Schema)
 	if oldCasted != nil && newCasted != nil && oldCasted.Type != newCasted.Type {
-		oldType := getValueType(old.Type) + "." + getValueType(oldCasted.Type)
-		newType := getValueType(new.Type) + "." + getValueType(newCasted.Type)
+		oldType := getValueType(fieldDiff.Old.Type) + "." + getValueType(oldCasted.Type)
+		newType := getValueType(fieldDiff.New.Type) + "." + getValueType(newCasted.Type)
 		return []string{fmt.Sprintf(tmpl, field, oldType, newType, resource)}
 	}
 
@@ -62,13 +63,13 @@ var FieldBecomingRequired = FieldDiffRule{
 	Messages:   FieldBecomingRequiredMessages,
 }
 
-func FieldBecomingRequiredMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldBecomingRequiredMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// Ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	tmpl := "Field `%s` changed from optional to required on `%s`"
-	if !old.Required && new.Required {
+	if !fieldDiff.Old.Required && fieldDiff.New.Required {
 		return []string{fmt.Sprintf(tmpl, field, resource)}
 	}
 
@@ -80,19 +81,19 @@ var FieldBecomingComputedOnly = FieldDiffRule{
 	Messages:   FieldBecomingComputedOnlyMessages,
 }
 
-func FieldBecomingComputedOnlyMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldBecomingComputedOnlyMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	// if the field is computed only already
 	// this rule doesn't apply
-	if old.Computed && !old.Optional {
+	if fieldDiff.Old.Computed && !fieldDiff.Old.Optional {
 		return nil
 	}
 
 	tmpl := "Field `%s` became Computed only on `%s`"
-	if new.Computed && !new.Optional {
+	if fieldDiff.New.Computed && !fieldDiff.New.Optional {
 		return []string{fmt.Sprintf(tmpl, field, resource)}
 	}
 	return nil
@@ -103,13 +104,13 @@ var FieldOptionalComputedToOptional = FieldDiffRule{
 	Messages:   FieldOptionalComputedToOptionalMessages,
 }
 
-func FieldOptionalComputedToOptionalMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldOptionalComputedToOptionalMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	tmpl := "Field `%s` transitioned from optional+computed to optional `%s`"
-	if (old.Computed && old.Optional) && (new.Optional && !new.Computed) {
+	if (fieldDiff.Old.Computed && fieldDiff.Old.Optional) && (fieldDiff.New.Optional && !fieldDiff.New.Computed) {
 		return []string{fmt.Sprintf(tmpl, field, resource)}
 	}
 	return nil
@@ -120,15 +121,15 @@ var FieldDefaultModification = FieldDiffRule{
 	Messages:   FieldDefaultModificationMessages,
 }
 
-func FieldDefaultModificationMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldDefaultModificationMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	tmpl := "Field `%s` default value changed from %s to %s on `%s`"
-	if old.Default != new.Default {
-		oldDefault := fmt.Sprintf("%v", old.Default)
-		newDefault := fmt.Sprintf("%v", new.Default)
+	if fieldDiff.Old.Default != fieldDiff.New.Default {
+		oldDefault := fmt.Sprintf("%v", fieldDiff.Old.Default)
+		newDefault := fmt.Sprintf("%v", fieldDiff.New.Default)
 		return []string{fmt.Sprintf(tmpl, field, oldDefault, newDefault, resource)}
 	}
 
@@ -140,18 +141,18 @@ var FieldGrowingMin = FieldDiffRule{
 	Messages:   FieldGrowingMinMessages,
 }
 
-func FieldGrowingMinMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldGrowingMinMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	tmpl := "Field `%s` MinItems went from %s to %s on `%s`"
-	if old.MinItems < new.MinItems || old.MinItems == 0 && new.MinItems > 0 {
-		oldMin := strconv.Itoa(old.MinItems)
-		if old.MinItems == 0 {
+	if fieldDiff.Old.MinItems < fieldDiff.New.MinItems || fieldDiff.Old.MinItems == 0 && fieldDiff.New.MinItems > 0 {
+		oldMin := strconv.Itoa(fieldDiff.Old.MinItems)
+		if fieldDiff.Old.MinItems == 0 {
 			oldMin = "unset"
 		}
-		newMin := strconv.Itoa(new.MinItems)
+		newMin := strconv.Itoa(fieldDiff.New.MinItems)
 		return []string{fmt.Sprintf(tmpl, field, oldMin, newMin, resource)}
 	}
 	return nil
@@ -162,18 +163,18 @@ var FieldShrinkingMax = FieldDiffRule{
 	Messages:   FieldShrinkingMaxMessages,
 }
 
-func FieldShrinkingMaxMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldShrinkingMaxMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	tmpl := "Field `%s` MinItems went from %s to %s on `%s`"
-	if old.MaxItems > new.MaxItems || old.MaxItems == 0 && new.MaxItems > 0 {
-		oldMax := strconv.Itoa(old.MaxItems)
-		if old.MaxItems == 0 {
+	if fieldDiff.Old.MaxItems > fieldDiff.New.MaxItems || fieldDiff.Old.MaxItems == 0 && fieldDiff.New.MaxItems > 0 {
+		oldMax := strconv.Itoa(fieldDiff.Old.MaxItems)
+		if fieldDiff.Old.MaxItems == 0 {
 			oldMax = "unset"
 		}
-		newMax := strconv.Itoa(new.MaxItems)
+		newMax := strconv.Itoa(fieldDiff.New.MaxItems)
 		return []string{fmt.Sprintf(tmpl, field, oldMax, newMax, resource)}
 	}
 	return nil
@@ -184,14 +185,14 @@ var FieldRemovingDiffSuppress = FieldDiffRule{
 	Messages:   FieldRemovingDiffSuppressMessages,
 }
 
-func FieldRemovingDiffSuppressMessages(resource, field string, old, new *schema.Schema) []string {
+func FieldRemovingDiffSuppressMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
 	// ignore for added / removed fields
-	if old == nil || new == nil {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
 	// TODO: Add resource to this message
 	tmpl := "Field `%s` lost its diff suppress function"
-	if old.DiffSuppressFunc != nil && new.DiffSuppressFunc == nil {
+	if fieldDiff.Old.DiffSuppressFunc != nil && fieldDiff.New.DiffSuppressFunc == nil {
 		return []string{fmt.Sprintf(tmpl, field)}
 	}
 	return nil
@@ -202,16 +203,16 @@ var FieldAddingSubfieldToConfigModeAttr = FieldDiffRule{
 	Messages:   FieldAddingSubfieldToConfigModeAttrMessages,
 }
 
-func FieldAddingSubfieldToConfigModeAttrMessages(resource, field string, old, new *schema.Schema) []string {
-	if old == nil || new == nil {
+func FieldAddingSubfieldToConfigModeAttrMessages(resource, field string, fieldDiff diff.FieldDiff) []string {
+	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
-	if new.ConfigMode == schema.SchemaConfigModeAttr {
-		newObj, ok := new.Elem.(*schema.Resource)
+	if fieldDiff.New.ConfigMode == schema.SchemaConfigModeAttr {
+		newObj, ok := fieldDiff.New.Elem.(*schema.Resource)
 		if !ok {
 			return nil
 		}
-		oldObj, ok := old.Elem.(*schema.Resource)
+		oldObj, ok := fieldDiff.Old.Elem.(*schema.Resource)
 		if !ok {
 			return nil
 		}

tools/diff-processor/breaking_changes/field_diff_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
 )
 
 type fieldTestCase struct {
@@ -635,7 +636,7 @@ var FieldAddingSubfieldToConfigModeAttrTestCases = []fieldTestCase{
 }
 
 func (tc *fieldTestCase) check(rule FieldDiffRule, t *testing.T) {
-	messages := rule.Messages("resource", "field", tc.oldField, tc.newField)
+	messages := rule.Messages("resource", "field", diff.FieldDiff{Old: tc.oldField, New: tc.newField})
 
 	violation := len(messages) > 0
 	if tc.expectedViolation != violation {

tools/diff-processor/breaking_changes/resource_config_diff.go

@@ -3,19 +3,17 @@ package breaking_changes
 import (
 	"fmt"
 
-	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
 )
 
-// ResourceInventoryRule provides
-// structure for rules regarding resource
-// inventory changes
+// ResourceConfigDiffRule provides
+// structure for rules regarding resource config changes
 type ResourceConfigDiffRule struct {
 	Identifier string
-	// TODO: Make this take a ResourceConfigDiff instead of old, new.
-	Messages func(resource string, old, new *schema.Resource) []string
+	Messages func(resource string, resourceConfigDiff diff.ResourceConfigDiff) []string
 }
 
-// ResourceInventoryRules is a list of ResourceInventoryRule
+// ResourceConfigDiffRules is a list of ResourceConfigDiffRule
 // guarding against provider breaking changes
 var ResourceConfigDiffRules = []ResourceConfigDiffRule{ResourceConfigRemovingAResource}
 
@@ -24,8 +22,8 @@ var ResourceConfigRemovingAResource = ResourceConfigDiffRule{
 	Messages:   ResourceConfigRemovingAResourceMessages,
 }
 
-func ResourceConfigRemovingAResourceMessages(resource string, old, new *schema.Resource) []string {
-	if new == nil && old != nil {
+func ResourceConfigRemovingAResourceMessages(resource string, resourceConfigDiff diff.ResourceConfigDiff) []string {
+	if resourceConfigDiff.New == nil && resourceConfigDiff.Old != nil {
 		tmpl := "Resource `%s` was either removed or renamed"
 		return []string{fmt.Sprintf(tmpl, resource)}
 	}

tools/diff-processor/breaking_changes/resource_config_diff_test.go

@@ -4,6 +4,8 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+
+	"github.com/GoogleCloudPlatform/magic-modules/tools/diff-processor/diff"
 )
 
 type resourceInventoryTestCase struct {
@@ -15,7 +17,7 @@ type resourceInventoryTestCase struct {
 
 func TestResourceInventoryRule_RemovingAResource(t *testing.T) {
 	for _, tc := range resourceConfigRemovingAResourceTestCases {
-		got := ResourceConfigRemovingAResource.Messages("resource", tc.old, tc.new)
+		got := ResourceConfigRemovingAResource.Messages("resource", diff.ResourceConfigDiff{Old: tc.old, New: tc.new})
 		gotViolations := len(got) > 0
 		if tc.wantViolations != gotViolations {
 			t.Errorf("ResourceConfigRemovingAResource.Messages(%v) violations not expected. Got %v, want %v", tc.name, gotViolations, tc.wantViolations)