cli: Better diagnostics for apply positional args

The previous changes removing support for using the trailing positional
argument as a working directory missed a spot in the apply/destroy
command implementation. We still support this argument for applying a
saved plan:

    terraform apply foo.tfplan

However, if you pass a positional path which doesn't "look like" a plan
(for example, the path to a configuration directory), Terraform would
silently ignore it and continue.

This commit fixes that by adding an error message if the user specifies
a path which the plan loader rejects as not "looking like" a plan. This
message includes a reference to the `-chdir` flag as a pointer about
what to do next.

We also rearrange the error message when calling `terraform destroy`
with a plan file argument, and add test coverage for the above. While
we're here, update the destroy tests to copy the fixture directory,
chdir, and defer cleanup.

command/apply.go

@@ -74,10 +74,21 @@ func (c *ApplyCommand) Run(args []string) int {
 			c.Ui.Error(err.Error())
 			return 1
 		}
-	}
+
-	if c.Destroy && planFile != nil {
+		// If the path doesn't look like a plan, both planFile and err will be
-		c.Ui.Error("Destroy can't be called with a plan file.")
+		// nil. In that case, the user is probably trying to use the positional
-		return 1
+		// argument to specify a configuration path. Point them at -chdir.
+		if planFile == nil {
+			c.Ui.Error(fmt.Sprintf("Failed to load %q as a plan file. Did you mean to use -chdir?", planPath))
+			return 1
+		}
+
+		// If we successfully loaded a plan but this is a destroy operation,
+		// explain that this is not supported.
+		if c.Destroy {
+			c.Ui.Error("Destroy can't be called with a plan file.")
+			return 1
+		}
 	}
 	if planFile != nil {
 		// Reset the config path for backend loading

command/apply_destroy_test.go

@@ -18,6 +18,12 @@ import (
 )
 
 func TestApply_destroy(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	testCopyDir(t, testFixturePath("apply"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
 	originalState := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(
 			addrs.Resource{
@@ -64,7 +70,6 @@ func TestApply_destroy(t *testing.T) {
 	args := []string{
 		"-auto-approve",
 		"-state", statePath,
-		testFixturePath("apply"),
 	}
 	if code := c.Run(args); code != 0 {
 		t.Log(ui.OutputWriter.String())
@@ -233,6 +238,12 @@ func TestApply_destroyApproveYes(t *testing.T) {
 }
 
 func TestApply_destroyLockedState(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	testCopyDir(t, testFixturePath("apply"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
 	originalState := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(
 			addrs.Resource{
@@ -272,7 +283,6 @@ func TestApply_destroyLockedState(t *testing.T) {
 	args := []string{
 		"-auto-approve",
 		"-state", statePath,
-		testFixturePath("apply"),
 	}
 
 	if code := c.Run(args); code == 0 {
@@ -286,6 +296,12 @@ func TestApply_destroyLockedState(t *testing.T) {
 }
 
 func TestApply_destroyPlan(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	testCopyDir(t, testFixturePath("apply"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
 	planPath := testPlanFileNoop(t)
 
 	p := testProvider()
@@ -305,9 +321,50 @@ func TestApply_destroyPlan(t *testing.T) {
 	if code := c.Run(args); code != 1 {
 		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
 	}
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "plan file") {
+		t.Fatal("expected command output to refer to plan file, but got:", output)
+	}
+}
+
+func TestApply_destroyPath(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	testCopyDir(t, testFixturePath("apply"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	p := applyFixtureProvider()
+
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		Destroy: true,
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			Ui:               ui,
+		},
+	}
+
+	args := []string{
+		"-auto-approve",
+		testFixturePath("apply"),
+	}
+	if code := c.Run(args); code != 1 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "-chdir") {
+		t.Fatal("expected command output to refer to -chdir flag, but got:", output)
+	}
 }
 
 func TestApply_destroyTargeted(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	testCopyDir(t, testFixturePath("apply-destroy-targeted"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
 	originalState := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(
 			addrs.Resource{
@@ -383,7 +440,6 @@ func TestApply_destroyTargeted(t *testing.T) {
 		"-auto-approve",
 		"-target", "test_instance.foo",
 		"-state", statePath,
-		testFixturePath("apply-destroy-targeted"),
 	}
 	if code := c.Run(args); code != 0 {
 		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())

command/apply_test.go

@@ -63,6 +63,36 @@ func TestApply(t *testing.T) {
 	}
 }
 
+func TestApply_path(t *testing.T) {
+	// Create a temporary working directory that is empty
+	td := tempDir(t)
+	testCopyDir(t, testFixturePath("apply"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	p := applyFixtureProvider()
+
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			Ui:               ui,
+		},
+	}
+
+	args := []string{
+		"-auto-approve",
+		testFixturePath("apply"),
+	}
+	if code := c.Run(args); code != 1 {
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "-chdir") {
+		t.Fatal("expected command output to refer to -chdir flag, but got:", output)
+	}
+}
+
 func TestApply_approveNo(t *testing.T) {
 	// Create a temporary working directory that is empty
 	td := tempDir(t)