helper/schema: diff should include removed set items [GH-1823]

helper/schema/field_reader_diff.go

@@ -144,7 +144,11 @@ func (r *DiffFieldReader) readSet(
 	set := &Set{F: schema.Set}
 
 	// Go through the map and find all the set items
-	for k, _ := range r.Diff.Attributes {
+	for k, d := range r.Diff.Attributes {
+		if d.NewRemoved {
+			// If the field is removed, we always ignore it
+			continue
+		}
 		if !strings.HasPrefix(k, prefix) {
 			continue
 		}

helper/schema/resource_data_test.go

@@ -688,6 +688,50 @@ func TestResourceDataGet(t *testing.T) {
 
 			Value: 33.0,
 		},
+
+		// #23 Sets with removed elements
+		{
+			Schema: map[string]*Schema{
+				"ports": &Schema{
+					Type:     TypeSet,
+					Optional: true,
+					Computed: true,
+					Elem:     &Schema{Type: TypeInt},
+					Set: func(a interface{}) int {
+						return a.(int)
+					},
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"ports.#":  "1",
+					"ports.80": "80",
+				},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"ports.#": &terraform.ResourceAttrDiff{
+						Old: "2",
+						New: "1",
+					},
+					"ports.80": &terraform.ResourceAttrDiff{
+						Old: "80",
+						New: "80",
+					},
+					"ports.8080": &terraform.ResourceAttrDiff{
+						Old:        "8080",
+						New:        "0",
+						NewRemoved: true,
+					},
+				},
+			},
+
+			Key: "ports",
+
+			Value: []interface{}{80},
+		},
 	}
 
 	for i, tc := range cases {

helper/schema/schema.go

@@ -842,6 +842,43 @@ func (m schemaMap) diffSet(
 		})
 	}
 
+	removed := os.Difference(ns)
+	for _, code := range removed.listCode() {
+		// If the code is negative (first character is -) then
+		// replace it with "~" for our computed set stuff.
+		codeStr := strconv.Itoa(code)
+		if codeStr[0] == '-' {
+			codeStr = string('~') + codeStr[1:]
+		}
+
+		switch t := schema.Elem.(type) {
+		case *Resource:
+			// This is a complex resource
+			for k2, schema := range t.Schema {
+				subK := fmt.Sprintf("%s.%s.%s", k, codeStr, k2)
+				err := m.diff(subK, schema, diff, d, true)
+				if err != nil {
+					return err
+				}
+			}
+		case *Schema:
+			// Copy the schema so that we can set Computed/ForceNew from
+			// the parent schema (the TypeSet).
+			t2 := *t
+			t2.ForceNew = schema.ForceNew
+
+			// This is just a primitive element, so go through each and
+			// just diff each.
+			subK := fmt.Sprintf("%s.%s", k, codeStr)
+			err := m.diff(subK, &t2, diff, d, true)
+			if err != nil {
+				return err
+			}
+		default:
+			return fmt.Errorf("%s: unknown element type (internal)", k)
+		}
+	}
+
 	for _, code := range ns.listCode() {
 		// If the code is negative (first character is -) then
 		// replace it with "~" for our computed set stuff.

helper/schema/schema_test.go

@@ -1047,6 +1047,16 @@ func TestSchemaMap_Diff(t *testing.T) {
 						Old: "2",
 						New: "0",
 					},
+					"ports.1": &terraform.ResourceAttrDiff{
+						Old:        "1",
+						New:        "0",
+						NewRemoved: true,
+					},
+					"ports.2": &terraform.ResourceAttrDiff{
+						Old:        "2",
+						New:        "0",
+						NewRemoved: true,
+					},
 				},
 			},
 
@@ -2064,6 +2074,11 @@ func TestSchemaMap_Diff(t *testing.T) {
 						New:         "0",
 						RequiresNew: true,
 					},
+					"instances.3": &terraform.ResourceAttrDiff{
+						Old:        "foo",
+						New:        "",
+						NewRemoved: true,
+					},
 				},
 			},
 
@@ -2348,6 +2363,59 @@ func TestSchemaMap_Diff(t *testing.T) {
 
 			Err: false,
 		},
+
+		// #60 - Removing set elements
+		{
+			Schema: map[string]*Schema{
+				"instances": &Schema{
+					Type:     TypeSet,
+					Elem:     &Schema{Type: TypeString},
+					Optional: true,
+					ForceNew: true,
+					Set: func(v interface{}) int {
+						return len(v.(string))
+					},
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"instances.#": "2",
+					"instances.3": "333",
+					"instances.2": "22",
+				},
+			},
+
+			Config: map[string]interface{}{
+				"instances": []interface{}{"333", "4444"},
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"instances.#": &terraform.ResourceAttrDiff{
+						Old: "2",
+						New: "2",
+					},
+					"instances.2": &terraform.ResourceAttrDiff{
+						Old:        "22",
+						New:        "",
+						NewRemoved: true,
+					},
+					"instances.3": &terraform.ResourceAttrDiff{
+						Old:         "333",
+						New:         "333",
+						RequiresNew: true,
+					},
+					"instances.4": &terraform.ResourceAttrDiff{
+						Old:         "",
+						New:         "4444",
+						RequiresNew: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
 	}
 
 	for i, tc := range cases {