Better error handling for -json-into argument

Signed-off-by: Christian Mesh <christianmesh1@gmail.com>

internal/command/arguments/plan.go

@@ -20,10 +20,6 @@ type Plan struct {
 	// changes, and success with no changes.
 	DetailedExitCode bool
 
-	// InputEnabled is used to disable interactive input for unspecified
-	// variable and backend config values. Default is true.
-	InputEnabled bool
-
 	// OutPath contains an optional path to store the plan file
 	OutPath string
 
@@ -32,10 +28,8 @@ type Plan struct {
 	// be written to.
 	GenerateConfigPath string
 
-	// ViewType specifies which output format to use
-	ViewType ViewType
-
-	JsonInto string
+	// ViewOptions specifies which view options to use
+	ViewOptions ViewOptions
 
 	// ShowSensitive is used to display the value of variables marked as sensitive.
 	ShowSensitive bool
@@ -54,14 +48,11 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) {
 
 	cmdFlags := extendedFlagSet("plan", plan.State, plan.Operation, plan.Vars)
 	cmdFlags.BoolVar(&plan.DetailedExitCode, "detailed-exitcode", false, "detailed-exitcode")
-	cmdFlags.BoolVar(&plan.InputEnabled, "input", true, "input")
 	cmdFlags.StringVar(&plan.OutPath, "out", "", "out")
 	cmdFlags.StringVar(&plan.GenerateConfigPath, "generate-config-out", "", "generate-config-out")
 	cmdFlags.BoolVar(&plan.ShowSensitive, "show-sensitive", false, "displays sensitive values")
 
-	var json bool
-	cmdFlags.BoolVar(&json, "json", false, "json")
-	cmdFlags.StringVar(&plan.JsonInto, "json-into", "", "json-into")
+	plan.ViewOptions.AddFlags(cmdFlags, true)
 
 	if err := cmdFlags.Parse(args); err != nil {
 		diags = diags.Append(tfdiags.Sourceless(
@@ -82,18 +73,7 @@ func ParsePlan(args []string) (*Plan, tfdiags.Diagnostics) {
 	}
 
 	diags = diags.Append(plan.Operation.Parse())
-
-	// JSON view currently does not support input, so we disable it here
-	if json {
-		plan.InputEnabled = false
-	}
-
-	switch {
-	case json || plan.JsonInto != "":
-		plan.ViewType = ViewJSON
-	default:
-		plan.ViewType = ViewHuman
-	}
+	diags = diags.Append(plan.ViewOptions.Parse())
 
 	return plan, diags
 }

internal/command/arguments/plan_test.go

@@ -27,11 +27,13 @@ func TestParsePlan_basicValid(t *testing.T) {
 			nil,
 			&Plan{
 				DetailedExitCode: false,
-				InputEnabled:     true,
-				OutPath:          "",
-				ViewType:         ViewHuman,
-				State:            &State{Lock: true},
-				Vars:             &Vars{},
+				ViewOptions: ViewOptions{
+					InputEnabled: true,
+					ViewType:     ViewHuman,
+				},
+				OutPath: "",
+				State:   &State{Lock: true},
+				Vars:    &Vars{},
 				Operation: &Operation{
 					PlanMode:    plans.NormalMode,
 					Parallelism: 10,
@@ -43,11 +45,13 @@ func TestParsePlan_basicValid(t *testing.T) {
 			[]string{"-destroy", "-detailed-exitcode", "-input=false", "-out=saved.tfplan"},
 			&Plan{
 				DetailedExitCode: true,
-				InputEnabled:     false,
-				OutPath:          "saved.tfplan",
-				ViewType:         ViewHuman,
-				State:            &State{Lock: true},
-				Vars:             &Vars{},
+				ViewOptions: ViewOptions{
+					InputEnabled: false,
+					ViewType:     ViewHuman,
+				},
+				OutPath: "saved.tfplan",
+				State:   &State{Lock: true},
+				Vars:    &Vars{},
 				Operation: &Operation{
 					PlanMode:    plans.DestroyMode,
 					Parallelism: 10,
@@ -59,11 +63,13 @@ func TestParsePlan_basicValid(t *testing.T) {
 			[]string{"-json"},
 			&Plan{
 				DetailedExitCode: false,
-				InputEnabled:     false,
-				OutPath:          "",
-				ViewType:         ViewJSON,
-				State:            &State{Lock: true},
-				Vars:             &Vars{},
+				ViewOptions: ViewOptions{
+					InputEnabled: false,
+					ViewType:     ViewJSON,
+				},
+				OutPath: "",
+				State:   &State{Lock: true},
+				Vars:    &Vars{},
 				Operation: &Operation{
 					PlanMode:    plans.NormalMode,
 					Parallelism: 10,
@@ -73,7 +79,7 @@ func TestParsePlan_basicValid(t *testing.T) {
 		},
 	}
 
-	cmpOpts := cmpopts.IgnoreUnexported(Operation{}, Vars{}, State{})
+	cmpOpts := cmpopts.IgnoreUnexported(Operation{}, Vars{}, State{}, ViewOptions{})
 
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
@@ -96,8 +102,8 @@ func TestParsePlan_invalid(t *testing.T) {
 	if got, want := diags.Err().Error(), "flag provided but not defined"; !strings.Contains(got, want) {
 		t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want)
 	}
-	if got.ViewType != ViewHuman {
-		t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman)
+	if got.ViewOptions.ViewType != ViewHuman {
+		t.Fatalf("wrong view type, got %#v, want %#v", got.ViewOptions.ViewType, ViewHuman)
 	}
 }
 
@@ -109,8 +115,8 @@ func TestParsePlan_tooManyArguments(t *testing.T) {
 	if got, want := diags.Err().Error(), "Too many command line arguments"; !strings.Contains(got, want) {
 		t.Fatalf("wrong diags\n got: %s\nwant: %s", got, want)
 	}
-	if got.ViewType != ViewHuman {
-		t.Fatalf("wrong view type, got %#v, want %#v", got.ViewType, ViewHuman)
+	if got.ViewOptions.ViewType != ViewHuman {
+		t.Fatalf("wrong view type, got %#v, want %#v", got.ViewOptions.ViewType, ViewHuman)
 	}
 }
 

internal/command/arguments/types.go

@@ -5,6 +5,14 @@
 
 package arguments
 
+import (
+	"flag"
+	"fmt"
+	"os"
+
+	"github.com/opentofu/opentofu/internal/tfdiags"
+)
+
 // ViewType represents which view layer to use for a given command. Not all
 // commands will support all view types, and validation that the type is
 // supported should happen in the view constructor.
@@ -31,3 +39,60 @@ func (vt ViewType) String() string {
 		return "unknown"
 	}
 }
+
+type ViewOptions struct {
+	// Raw cli flags
+	jsonFlag     bool
+	jsonIntoFlag string
+
+	// ViewType specifies which output format to use
+	ViewType ViewType
+
+	// InputEnabled is used to disable interactive input for unspecified
+	// variable and backend config values. Default is true.
+	InputEnabled bool
+
+	// Optional stream to write json data to
+	JSONInto *os.File
+}
+
+func (v *ViewOptions) AddFlags(cmdFlags *flag.FlagSet, input bool) {
+	if input {
+		cmdFlags.BoolVar(&v.InputEnabled, "input", true, "input")
+	}
+
+	cmdFlags.BoolVar(&v.jsonFlag, "json", false, "json")
+	cmdFlags.StringVar(&v.jsonIntoFlag, "json-into", "", "json-into")
+}
+
+func (v *ViewOptions) Parse() tfdiags.Diagnostics {
+	var diags tfdiags.Diagnostics
+
+	if v.jsonIntoFlag != "" {
+		var err error
+		v.JSONInto, err = os.OpenFile(v.jsonIntoFlag, os.O_TRUNC|os.O_WRONLY|os.O_CREATE, 0600)
+		if err != nil {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Invalid argument",
+				fmt.Sprintf("Unable to open the file %q specified by -json-into for writing: %s", v.jsonIntoFlag, err.Error()),
+			))
+		}
+	}
+
+	// Default to Human
+	v.ViewType = ViewHuman
+	if v.jsonFlag {
+		v.ViewType = ViewJSON
+		// JSON view currently does not support input, so we disable it here
+		v.InputEnabled = false
+		if v.jsonIntoFlag != "" {
+			diags = diags.Append(tfdiags.Sourceless(
+				tfdiags.Error,
+				"Conflicting command output options",
+				"The -json and -json-into arguments are mutually exclusive",
+			))
+		}
+	}
+	return diags
+}

internal/command/get.go

@@ -49,9 +49,16 @@ func (c *GetCommand) Run(args []string) int {
 	}
 
 	if c.outputJSONInto != "" {
-		out, err := os.OpenFile(c.outputJSONInto, os.O_RDWR|os.O_CREATE, 0600)
+		if c.outputInJSON {
+			// Not a valid combination
+			c.Ui.Error("The -json and -json-into options are mutually-exclusive in their use")
+			return 1
+		}
+
+		out, err := os.OpenFile(c.outputJSONInto, os.O_TRUNC|os.O_WRONLY|os.O_CREATE, 0600)
 		if err != nil {
-			panic(err)
+			c.Ui.Error(fmt.Sprintf("Unable to open the file %q specified by -json-into for writing: %s", c.outputJSONInto, err.Error()))
+			return 1
 		}
 		c.oldUi = c.Ui
 		c.Ui = &WrappedUi{

internal/command/init.go

@@ -92,9 +92,16 @@ func (c *InitCommand) Run(args []string) int {
 	}
 
 	if c.outputJSONInto != "" {
-		out, err := os.OpenFile(c.outputJSONInto, os.O_RDWR|os.O_CREATE, 0600)
+		if c.outputInJSON {
+			// Not a valid combination
+			c.Ui.Error("The -json and -json-into options are mutually-exclusive in their use")
+			return 1
+		}
+
+		out, err := os.OpenFile(c.outputJSONInto, os.O_TRUNC|os.O_WRONLY|os.O_CREATE, 0600)
 		if err != nil {
-			panic(err)
+			c.Ui.Error(fmt.Sprintf("Unable to open the file %q specified by -json-into for writing: %s", c.outputJSONInto, err.Error()))
+			return 1
 		}
 		c.oldUi = c.Ui
 		c.Ui = &WrappedUi{

internal/command/plan.go

@@ -62,7 +62,7 @@ func (c *PlanCommand) Run(rawArgs []string) int {
 	// FIXME: the -input flag value is needed to initialize the backend and the
 	// operation, but there is no clear path to pass this value down, so we
 	// continue to mutate the Meta object state for now.
-	c.Meta.input = args.InputEnabled
+	c.Meta.input = args.ViewOptions.InputEnabled
 
 	// FIXME: the -parallelism flag is used to control the concurrency of
 	// OpenTofu operations. At the moment, this value is used both to
@@ -86,7 +86,7 @@ func (c *PlanCommand) Run(rawArgs []string) int {
 	}
 
 	// Prepare the backend with the backend-specific arguments
-	be, beDiags := c.PrepareBackend(ctx, args.State, args.ViewType, enc)
+	be, beDiags := c.PrepareBackend(ctx, args.State, args.ViewOptions.ViewType, enc)
 	diags = diags.Append(beDiags)
 	if diags.HasErrors() {
 		view.Diagnostics(diags)
@@ -94,7 +94,7 @@ func (c *PlanCommand) Run(rawArgs []string) int {
 	}
 
 	// Build the operation request
-	opReq, opDiags := c.OperationRequest(ctx, be, view, args.ViewType, args.Operation, args.OutPath, args.GenerateConfigPath, enc)
+	opReq, opDiags := c.OperationRequest(ctx, be, view, args.ViewOptions.ViewType, args.Operation, args.OutPath, args.GenerateConfigPath, enc)
 	diags = diags.Append(opDiags)
 	if diags.HasErrors() {
 		view.Diagnostics(diags)

internal/command/views/plan.go

@@ -7,7 +7,6 @@ package views
 
 import (
 	"fmt"
-	"os"
 
 	"github.com/opentofu/opentofu/internal/command/arguments"
 	"github.com/opentofu/opentofu/internal/tfdiags"
@@ -25,29 +24,24 @@ type Plan interface {
 
 // NewPlan returns an initialized Plan implementation for the given ViewType.
 func NewPlan(args *arguments.Plan, view *View) Plan {
-	human := &PlanHuman{
-		view:         view,
-		inAutomation: view.RunningInAutomation(),
-	}
-
-	switch args.ViewType {
+	switch args.ViewOptions.ViewType {
 	case arguments.ViewJSON:
-		if args.JsonInto != "" {
-			out, err := os.OpenFile(args.JsonInto, os.O_RDWR|os.O_CREATE, 0600)
-			if err != nil {
-				panic(err)
-			}
-			return PlanMulti{human, &PlanJSON{
-				view: NewJSONView(view, out),
-			}}
-		}
 		return &PlanJSON{
 			view: NewJSONView(view, nil),
 		}
 	case arguments.ViewHuman:
+		human := &PlanHuman{
+			view:         view,
+			inAutomation: view.RunningInAutomation(),
+		}
+
+		if args.ViewOptions.JSONInto != nil {
+			return PlanMulti{human, &PlanJSON{view: NewJSONView(view, args.ViewOptions.JSONInto)}}
+		}
+
 		return human
 	default:
-		panic(fmt.Sprintf("unknown view type %v", args.ViewType))
+		panic(fmt.Sprintf("unknown view type %v", args.ViewOptions.ViewType))
 	}
 }
 

internal/command/views/plan_test.go

@@ -24,7 +24,7 @@ import (
 func TestPlanHuman_operation(t *testing.T) {
 	streams, done := terminal.StreamsForTesting(t)
 	defer done(t)
-	v := NewPlan(&arguments.Plan{ViewType: arguments.ViewHuman}, NewView(streams).SetRunningInAutomation(true)).Operation()
+	v := NewPlan(&arguments.Plan{ViewOptions: arguments.ViewOptions{ViewType: arguments.ViewHuman}}, NewView(streams).SetRunningInAutomation(true)).Operation()
 	if hv, ok := v.(*OperationHuman); !ok {
 		t.Fatalf("unexpected return type %t", v)
 	} else if hv.inAutomation != true {
@@ -36,7 +36,7 @@ func TestPlanHuman_operation(t *testing.T) {
 func TestPlanHuman_hooks(t *testing.T) {
 	streams, done := terminal.StreamsForTesting(t)
 	defer done(t)
-	v := NewPlan(&arguments.Plan{ViewType: arguments.ViewHuman}, NewView(streams).SetRunningInAutomation((true)))
+	v := NewPlan(&arguments.Plan{ViewOptions: arguments.ViewOptions{ViewType: arguments.ViewHuman}}, NewView(streams).SetRunningInAutomation((true)))
 	hooks := v.Hooks()
 
 	var uiHook *UiHook