fix sorting of module resources during state mv

Module resource were being sorted lexically by name by the state filter.
If there are 10 or more resources, the order won't match the index
order, and resources will have different indexes in their new location.

Sort the FilterResults by index numerically when the names match.

Clean up the module String output for visual inspection by sorting
Resource name parts numerically when they are an integer value.

command/state_mv_test.go

@@ -370,6 +370,184 @@ func TestStateMv_stateOutNew_count(t *testing.T) {
 	testStateOutput(t, backups[0], testStateMvCount_stateOutOriginal)
 }
 
+// Modules with more than 10 resources were sorted lexically, causing the
+// indexes in the new location to change.
+func TestStateMv_stateOutNew_largeCount(t *testing.T) {
+	state := &terraform.State{
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_instance.foo.0": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo0",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.1": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo1",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.2": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo2",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.3": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo3",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.4": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo4",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.5": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo5",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.6": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo6",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.7": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo7",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.8": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo8",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.9": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo9",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.foo.10": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "foo10",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+
+					"test_instance.bar": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+							Attributes: map[string]string{
+								"foo": "value",
+								"bar": "value",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	statePath := testStateFile(t, state)
+	stateOutPath := statePath + ".out"
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &StateMvCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		"-state-out", stateOutPath,
+		"test_instance.foo",
+		"test_instance.bar",
+	}
+	if code := c.Run(args); code != 0 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// Test it is correct
+	testStateOutput(t, stateOutPath, testStateMvLargeCount_stateOut)
+	testStateOutput(t, statePath, testStateMvLargeCount_stateOutSrc)
+
+	// Test we have backups
+	backups := testStateBackups(t, filepath.Dir(statePath))
+	if len(backups) != 1 {
+		t.Fatalf("bad: %#v", backups)
+	}
+	testStateOutput(t, backups[0], testStateMvLargeCount_stateOutOriginal)
+}
+
 func TestStateMv_stateOutNew_nestedModule(t *testing.T) {
 	state := &terraform.State{
 		Modules: []*terraform.ModuleState{
@@ -506,6 +684,111 @@ test_instance.foo.1:
   foo = value
 `
 
+const testStateMvLargeCount_stateOut = `
+test_instance.bar.0:
+  ID = foo0
+  bar = value
+  foo = value
+test_instance.bar.1:
+  ID = foo1
+  bar = value
+  foo = value
+test_instance.bar.2:
+  ID = foo2
+  bar = value
+  foo = value
+test_instance.bar.3:
+  ID = foo3
+  bar = value
+  foo = value
+test_instance.bar.4:
+  ID = foo4
+  bar = value
+  foo = value
+test_instance.bar.5:
+  ID = foo5
+  bar = value
+  foo = value
+test_instance.bar.6:
+  ID = foo6
+  bar = value
+  foo = value
+test_instance.bar.7:
+  ID = foo7
+  bar = value
+  foo = value
+test_instance.bar.8:
+  ID = foo8
+  bar = value
+  foo = value
+test_instance.bar.9:
+  ID = foo9
+  bar = value
+  foo = value
+test_instance.bar.10:
+  ID = foo10
+  bar = value
+  foo = value
+`
+
+const testStateMvLargeCount_stateOutSrc = `
+test_instance.bar:
+  ID = bar
+  bar = value
+  foo = value
+`
+
+const testStateMvLargeCount_stateOutOriginal = `
+test_instance.bar:
+  ID = bar
+  bar = value
+  foo = value
+test_instance.foo.0:
+  ID = foo0
+  bar = value
+  foo = value
+test_instance.foo.1:
+  ID = foo1
+  bar = value
+  foo = value
+test_instance.foo.2:
+  ID = foo2
+  bar = value
+  foo = value
+test_instance.foo.3:
+  ID = foo3
+  bar = value
+  foo = value
+test_instance.foo.4:
+  ID = foo4
+  bar = value
+  foo = value
+test_instance.foo.5:
+  ID = foo5
+  bar = value
+  foo = value
+test_instance.foo.6:
+  ID = foo6
+  bar = value
+  foo = value
+test_instance.foo.7:
+  ID = foo7
+  bar = value
+  foo = value
+test_instance.foo.8:
+  ID = foo8
+  bar = value
+  foo = value
+test_instance.foo.9:
+  ID = foo9
+  bar = value
+  foo = value
+test_instance.foo.10:
+  ID = foo10
+  bar = value
+  foo = value
+`
+
 const testStateMvNestedModule_stateOut = `
 <no state>
 module.bar:

terraform/state.go

@@ -1164,7 +1164,8 @@ func (m *ModuleState) String() string {
 	for name, _ := range m.Resources {
 		names = append(names, name)
 	}
-	sort.Strings(names)
+
+	sort.Sort(resourceNameSort(names))
 
 	for _, k := range names {
 		rs := m.Resources[k]
@@ -1204,6 +1205,7 @@ func (m *ModuleState) String() string {
 
 			attrKeys = append(attrKeys, ak)
 		}
+
 		sort.Strings(attrKeys)
 
 		for _, ak := range attrKeys {
@@ -1234,6 +1236,7 @@ func (m *ModuleState) String() string {
 		for k, _ := range m.Outputs {
 			ks = append(ks, k)
 		}
+
 		sort.Strings(ks)
 
 		for _, k := range ks {
@@ -2034,6 +2037,48 @@ func WriteState(d *State, dst io.Writer) error {
 	return nil
 }
 
+// resourceNameSort implements the sort.Interface to sort name parts lexically for
+// strings and numerically for integer indexes.
+type resourceNameSort []string
+
+func (r resourceNameSort) Len() int      { return len(r) }
+func (r resourceNameSort) Swap(i, j int) { r[i], r[j] = r[j], r[i] }
+
+func (r resourceNameSort) Less(i, j int) bool {
+	iParts := strings.Split(r[i], ".")
+	jParts := strings.Split(r[j], ".")
+
+	end := len(iParts)
+	if len(jParts) < end {
+		end = len(jParts)
+	}
+
+	for idx := 0; idx < end; idx++ {
+		if iParts[idx] == jParts[idx] {
+			continue
+		}
+
+		// sort on the first non-matching part
+		iInt, iIntErr := strconv.Atoi(iParts[idx])
+		jInt, jIntErr := strconv.Atoi(jParts[idx])
+
+		switch {
+		case iIntErr == nil && jIntErr == nil:
+			// sort numerically if both parts are integers
+			return iInt < jInt
+		case iIntErr == nil:
+			// numbers sort before strings
+			return true
+		case jIntErr == nil:
+			return false
+		default:
+			return iParts[idx] < jParts[idx]
+		}
+	}
+
+	return false
+}
+
 // moduleStateSort implements sort.Interface to sort module states
 type moduleStateSort []*ModuleState
 

terraform/state_filter.go

@@ -34,7 +34,7 @@ func (f *StateFilter) Filter(fs ...string) ([]*StateFilterResult, error) {
 		as[i] = a
 	}
 
-	// If we werent given any filters, then we list all
+	// If we weren't given any filters, then we list all
 	if len(fs) == 0 {
 		as = append(as, &ResourceAddress{Index: -1})
 	}
@@ -250,6 +250,13 @@ func (s StateFilterResultSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
 func (s StateFilterResultSlice) Less(i, j int) bool {
 	a, b := s[i], s[j]
 
+	// if these address contain an index, we want to sort by index rather than name
+	addrA, errA := ParseResourceAddress(a.Address)
+	addrB, errB := ParseResourceAddress(b.Address)
+	if errA == nil && errB == nil && addrA.Name == addrB.Name && addrA.Index != addrB.Index {
+		return addrA.Index < addrB.Index
+	}
+
 	// If the addresses are different it is just lexographic sorting
 	if a.Address != b.Address {
 		return a.Address < b.Address