Fix logic for shrinking max items rule (GoogleCloudPlatform#13357)

Author: trodge

tools/diff-processor/breaking_changes/breaking_changes_test.go

@@ -298,7 +298,7 @@ func TestComputeBreakingChanges(t *testing.T) {
 			},
 			wantViolations: []BreakingChange{
 				{
-					Message:                "Field `field-a.sub-field-1` MinItems went from 100 to 25 on `google-x`",
+					Message:                "Field `field-a.sub-field-1` MaxItems went from 100 to 25 on `google-x`",
 					DocumentationReference: "https://googlecloudplatform.github.io/magic-modules/breaking-changes/breaking-changes#field-shrinking-max",
 				},
 			},
@@ -343,7 +343,7 @@ func TestComputeBreakingChanges(t *testing.T) {
 			},
 			wantViolations: []BreakingChange{
 				{
-					Message:                "Field `field-a.sub-field-1` MinItems went from 100 to 25 on `google-x`",
+					Message:                "Field `field-a.sub-field-1` MaxItems went from 100 to 25 on `google-x`",
 					DocumentationReference: "https://googlecloudplatform.github.io/magic-modules/breaking-changes/breaking-changes#field-shrinking-max",
 				},
 			},

tools/diff-processor/breaking_changes/field_diff.go

@@ -146,7 +146,7 @@ func FieldGrowingMinMessages(resource, field string, fieldDiff diff.FieldDiff) [
 		return nil
 	}
 	tmpl := "Field `%s` MinItems went from %s to %s on `%s`"
-	if fieldDiff.Old.MinItems < fieldDiff.New.MinItems || fieldDiff.Old.MinItems == 0 && fieldDiff.New.MinItems > 0 {
+	if fieldDiff.Old.MinItems < fieldDiff.New.MinItems {
 		oldMin := strconv.Itoa(fieldDiff.Old.MinItems)
 		if fieldDiff.Old.MinItems == 0 {
 			oldMin = "unset"
@@ -167,13 +167,16 @@ func FieldShrinkingMaxMessages(resource, field string, fieldDiff diff.FieldDiff)
 	if fieldDiff.Old == nil || fieldDiff.New == nil {
 		return nil
 	}
-	tmpl := "Field `%s` MinItems went from %s to %s on `%s`"
-	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(fieldDiff.New.MaxItems)
+	tmpl := "Field `%s` MaxItems went from %s to %s on `%s`"
+	if fieldDiff.New.MaxItems == 0 {
+		return nil
+	}
+	newMax := strconv.Itoa(fieldDiff.New.MaxItems)
+	if fieldDiff.Old.MaxItems == 0 {
+		return []string{fmt.Sprintf(tmpl, field, "unset", newMax, resource)}
+	}
+	oldMax := strconv.Itoa(fieldDiff.Old.MaxItems)
+	if fieldDiff.Old.MaxItems > fieldDiff.New.MaxItems {
 		return []string{fmt.Sprintf(tmpl, field, oldMax, newMax, resource)}
 	}
 	return nil

tools/diff-processor/breaking_changes/field_diff_test.go

@@ -558,6 +558,14 @@ var FieldShrinkingMaxTestCases = []fieldTestCase{
 		},
 		expectedViolation: true,
 	},
+	{
+		name: "max defined to unset",
+		oldField: &schema.Schema{
+			MaxItems: 2,
+		},
+		newField:          &schema.Schema{},
+		expectedViolation: false,
+	},
 }
 
 func (tc *fieldTestCase) check(rule FieldDiffRule, t *testing.T) {