refactor state-rm command argument parsing

internal/command/arguments/state_rm.go

@@ -0,0 +1,76 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package arguments
+
+import (
+	"time"
+
+	"github.com/hashicorp/terraform/internal/tfdiags"
+)
+
+// StateRm represents the command-line arguments for the state rm command.
+type StateRm struct {
+	// DryRun, if true, prints out what would be removed without actually
+	// removing anything.
+	DryRun bool
+
+	// BackupPath is the path where Terraform should write the backup state.
+	BackupPath string
+
+	// StateLock, if true, requests that the backend lock the state for this
+	// operation.
+	StateLock bool
+
+	// StateLockTimeout is the duration to retry a state lock.
+	StateLockTimeout time.Duration
+
+	// StatePath is an optional path to a local state file.
+	StatePath string
+
+	// IgnoreRemoteVersion, if true, continues even if remote and local
+	// Terraform versions are incompatible.
+	IgnoreRemoteVersion bool
+
+	// Addrs are the resource instance addresses to remove.
+	Addrs []string
+}
+
+// ParseStateRm processes CLI arguments, returning a StateRm value and
+// diagnostics. If errors are encountered, a StateRm value is still returned
+// representing the best effort interpretation of the arguments.
+func ParseStateRm(args []string) (*StateRm, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
+	rm := &StateRm{
+		StateLock: true,
+	}
+
+	cmdFlags := defaultFlagSet("state rm")
+	cmdFlags.BoolVar(&rm.DryRun, "dry-run", false, "dry run")
+	cmdFlags.StringVar(&rm.BackupPath, "backup", "-", "backup")
+	cmdFlags.BoolVar(&rm.StateLock, "lock", true, "lock state")
+	cmdFlags.DurationVar(&rm.StateLockTimeout, "lock-timeout", 0, "lock timeout")
+	cmdFlags.StringVar(&rm.StatePath, "state", "", "path")
+	cmdFlags.BoolVar(&rm.IgnoreRemoteVersion, "ignore-remote-version", false, "continue even if remote and local Terraform versions are incompatible")
+
+	if err := cmdFlags.Parse(args); err != nil {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Error,
+			"Failed to parse command-line flags",
+			err.Error(),
+		))
+	}
+
+	args = cmdFlags.Args()
+	if len(args) < 1 {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Error,
+			"Required argument missing",
+			"At least one address is required.",
+		))
+	}
+
+	rm.Addrs = args
+
+	return rm, diags
+}

internal/command/arguments/state_rm_test.go

@@ -0,0 +1,126 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: BUSL-1.1
+
+package arguments
+
+import (
+	"testing"
+	"time"
+
+	"github.com/hashicorp/terraform/internal/tfdiags"
+)
+
+func TestParseStateRm_valid(t *testing.T) {
+	testCases := map[string]struct {
+		args []string
+		want *StateRm
+	}{
+		"single address": {
+			[]string{"test_instance.foo"},
+			&StateRm{
+				DryRun:              false,
+				BackupPath:          "-",
+				StateLock:           true,
+				StateLockTimeout:    0,
+				StatePath:           "",
+				IgnoreRemoteVersion: false,
+				Addrs:               []string{"test_instance.foo"},
+			},
+		},
+		"multiple addresses": {
+			[]string{"test_instance.foo", "test_instance.bar"},
+			&StateRm{
+				DryRun:              false,
+				BackupPath:          "-",
+				StateLock:           true,
+				StateLockTimeout:    0,
+				StatePath:           "",
+				IgnoreRemoteVersion: false,
+				Addrs:               []string{"test_instance.foo", "test_instance.bar"},
+			},
+		},
+		"all options": {
+			[]string{"-dry-run", "-backup=backup.tfstate", "-lock=false", "-lock-timeout=5s", "-state=state.tfstate", "-ignore-remote-version", "test_instance.foo"},
+			&StateRm{
+				DryRun:              true,
+				BackupPath:          "backup.tfstate",
+				StateLock:           false,
+				StateLockTimeout:    5 * time.Second,
+				StatePath:           "state.tfstate",
+				IgnoreRemoteVersion: true,
+				Addrs:               []string{"test_instance.foo"},
+			},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			got, diags := ParseStateRm(tc.args)
+			if len(diags) > 0 {
+				t.Fatalf("unexpected diags: %v", diags)
+			}
+			if got.DryRun != tc.want.DryRun ||
+				got.BackupPath != tc.want.BackupPath ||
+				got.StateLock != tc.want.StateLock ||
+				got.StateLockTimeout != tc.want.StateLockTimeout ||
+				got.StatePath != tc.want.StatePath ||
+				got.IgnoreRemoteVersion != tc.want.IgnoreRemoteVersion {
+				t.Fatalf("unexpected result\n got: %#v\nwant: %#v", got, tc.want)
+			}
+			if len(got.Addrs) != len(tc.want.Addrs) {
+				t.Fatalf("unexpected Addrs length\n got: %d\nwant: %d", len(got.Addrs), len(tc.want.Addrs))
+			}
+			for i := range got.Addrs {
+				if got.Addrs[i] != tc.want.Addrs[i] {
+					t.Fatalf("unexpected Addrs[%d]\n got: %q\nwant: %q", i, got.Addrs[i], tc.want.Addrs[i])
+				}
+			}
+		})
+	}
+}
+
+func TestParseStateRm_invalid(t *testing.T) {
+	testCases := map[string]struct {
+		args      []string
+		wantAddrs int
+		wantDiags tfdiags.Diagnostics
+	}{
+		"no arguments": {
+			nil,
+			0,
+			tfdiags.Diagnostics{
+				tfdiags.Sourceless(
+					tfdiags.Error,
+					"Required argument missing",
+					"At least one address is required.",
+				),
+			},
+		},
+		"unknown flag": {
+			[]string{"-boop"},
+			0,
+			tfdiags.Diagnostics{
+				tfdiags.Sourceless(
+					tfdiags.Error,
+					"Failed to parse command-line flags",
+					"flag provided but not defined: -boop",
+				),
+				tfdiags.Sourceless(
+					tfdiags.Error,
+					"Required argument missing",
+					"At least one address is required.",
+				),
+			},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			got, gotDiags := ParseStateRm(tc.args)
+			if len(got.Addrs) != tc.wantAddrs {
+				t.Fatalf("unexpected Addrs length\n got: %d\nwant: %d", len(got.Addrs), tc.wantAddrs)
+			}
+			tfdiags.AssertDiagnosticsMatch(t, gotDiags, tc.wantDiags)
+		})
+	}
+}

internal/command/state_rm.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/hashicorp/cli"
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/command/arguments"
 	"github.com/hashicorp/terraform/internal/command/clistate"
@@ -23,24 +22,17 @@ type StateRmCommand struct {
 }
 
 func (c *StateRmCommand) Run(args []string) int {
-	args = c.Meta.process(args)
-	var dryRun bool
-	cmdFlags := c.Meta.ignoreRemoteVersionFlagSet("state rm")
-	cmdFlags.BoolVar(&dryRun, "dry-run", false, "dry run")
-	cmdFlags.StringVar(&c.backupPath, "backup", "-", "backup")
-	cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
-	cmdFlags.DurationVar(&c.Meta.stateLockTimeout, "lock-timeout", 0, "lock timeout")
-	cmdFlags.StringVar(&c.statePath, "state", "", "path")
-	if err := cmdFlags.Parse(args); err != nil {
-		c.Ui.Error(fmt.Sprintf("Error parsing command-line flags: %s\n", err.Error()))
+	parsedArgs, diags := arguments.ParseStateRm(c.Meta.process(args))
+	if diags.HasErrors() {
+		c.showDiagnostics(diags)
 		return 1
 	}
 
-	args = cmdFlags.Args()
-	if len(args) < 1 {
-		c.Ui.Error("At least one address is required.\n")
-		return cli.RunResultHelp
-	}
+	c.backupPath = parsedArgs.BackupPath
+	c.Meta.stateLock = parsedArgs.StateLock
+	c.Meta.stateLockTimeout = parsedArgs.StateLockTimeout
+	c.statePath = parsedArgs.StatePath
+	c.Meta.ignoreRemoteVersion = parsedArgs.IgnoreRemoteVersion
 
 	if diags := c.Meta.checkRequiredVersion(); diags != nil {
 		c.showDiagnostics(diags)
@@ -82,8 +74,7 @@ func (c *StateRmCommand) Run(args []string) int {
 	// This command primarily works with resource instances, though it will
 	// also clean up any modules and resources left empty by actions it takes.
 	var addrs []addrs.AbsResourceInstance
-	var diags tfdiags.Diagnostics
-	for _, addrStr := range args {
+	for _, addrStr := range parsedArgs.Addrs {
 		moreAddrs, moreDiags := c.lookupResourceInstanceAddr(state, true, addrStr)
 		addrs = append(addrs, moreAddrs...)
 		diags = diags.Append(moreDiags)
@@ -94,7 +85,7 @@ func (c *StateRmCommand) Run(args []string) int {
 	}
 
 	prefix := "Removed "
-	if dryRun {
+	if parsedArgs.DryRun {
 		prefix = "Would remove "
 	}
 
@@ -103,13 +94,13 @@ func (c *StateRmCommand) Run(args []string) int {
 	for _, addr := range addrs {
 		isCount++
 		c.Ui.Output(prefix + addr.String())
-		if !dryRun {
+		if !parsedArgs.DryRun {
 			ss.ForgetResourceInstanceAll(addr)
 			ss.RemoveResourceIfEmpty(addr.ContainingResource())
 		}
 	}
 
-	if dryRun {
+	if parsedArgs.DryRun {
 		if isCount == 0 {
 			c.Ui.Output("Would have removed nothing.")
 		}