Fix for TypeSet applying with unexpected empty element (#2)

hashicorp/terraform-plugin-sdk#895

When calculating a diff, sets are added as delete of all old attributes
and a create of all new attributes. When DiffSuppressFunc is used, the
delete attribute drops the `NewRemoved` field, which results in sending
a diff that ends up creating a new empty set element.

Without the fix, the test fails:
```
=== RUN   TestSchemaMap_Diff/30-Set_with_DiffSuppressFunc
    schema_test.go:3188: expected:
        *terraform.InstanceDiff{ [...]"rule.80.duration":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:false, NewRemoved:true, [...]

        got:
        *terraform.InstanceDiff{ [...]"rule.80.duration":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:false, [...] NewRemoved:false, [...]
```

Previously, `NewRemoved` was set to false for the sustain field even though it belonged to an element being removed.

Author: prashantv

helper/schema/schema.go

@@ -1149,10 +1149,10 @@ func (m schemaMap) diff(
 				}
 
 				logging.HelperSchemaDebug(ctx, "Ignoring change due to DiffSuppressFunc", map[string]interface{}{logging.KeyAttributePath: attrK})
-				attrV = &terraform.ResourceAttrDiff{
-					Old: attrV.Old,
-					New: attrV.Old,
-				}
+				// If the diff is suppressed, we want Old = New, but retain the other properties.
+				updatedAttr := *attrV
+				updatedAttr.New = attrV.Old
+				attrV = &updatedAttr
 			}
 		}
 		diff.Attributes[attrK] = attrV

helper/schema/schema_test.go

@@ -1155,6 +1155,80 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		{
+			Name: "Set with DiffSuppressFunc",
+			Schema: map[string]*Schema{
+				"rule": {
+					Type:     TypeSet,
+					Required: true,
+					Elem: &Resource{
+						Schema: map[string]*Schema{
+							"port": {
+								Type:     TypeInt,
+								Required: true,
+							},
+							"duration": {
+								Type:     TypeString,
+								Optional: true,
+								DiffSuppressFunc: func(k, oldValue, newValue string, d *ResourceData) bool {
+									// Adding a DiffSuppressFunc to an element in a set changes behaviour.
+									// The actual suppress func doesn't matter.
+									return oldValue == newValue
+								},
+							},
+						},
+					},
+					Set: func(v interface{}) int {
+						m := v.(map[string]interface{})
+						port := m["port"].(int)
+						return port
+					},
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"rule.#":           "1",
+					"rule.80.port":     "80",
+					"rule.80.duration": "",
+				},
+			},
+
+			Config: map[string]interface{}{
+				"rule": []interface{}{
+					map[string]interface{}{
+						"port":     90,
+						"duration": "30s",
+					},
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"rule.80.port": {
+						Old:        "80",
+						New:        "0",
+						NewRemoved: true,
+					},
+					"rule.80.duration": {
+						Old:        "",
+						New:        "",
+						NewRemoved: true,
+					},
+					"rule.90.port": {
+						Old: "",
+						New: "90",
+					},
+					"rule.90.duration": {
+						Old: "",
+						New: "30s",
+					},
+				},
+			},
+
+			Err: false,
+		},
+
 		{
 			Name: "List of structure decode",
 			Schema: map[string]*Schema{