cmd/cue: replace most fatal exitOnErr calls with error returns

exitOnErr prints an error to stderr if there is any,
and if fatal is true, it also stops the command via panicExit,
which is recovered later in Command.Run via recoverError.

In a way, this avoids a small amount of verbosity, as we use a single
line rather than three to handle errors. However, it makes our error
handling inconsistent and subtle, to the point that it is hard
to see where a command function actually stops running.

Moreover, most of the function bodies we change here already returned
an error value, so it's even more confusing that a function
might sometimes choose to panicExit with an error rather than return it.
The behavior ends up being the same, as any call sites with error checks
don't run any code before returning a non-nil error.

Replace nearly all uses of exitOnErr with fatal=true with error returns.
Only buildInstances remains, but that will require a larger refactor
as the function and its callers do not return errors yet.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Ib4dbe45e7e87b911badb57665bff77621110e1e2
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194537
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Noam Dolovich <[email protected]>

cmd/cue/cmd/custom.go

@@ -158,10 +158,7 @@ func doTasks(cmd *Command, typ, command string, root *cue.Instance) error {
 
 	c := flow.New(cfg, root, newTaskFunc(cmd))
 
-	err := c.Run(backgroundContext())
-	exitOnErr(cmd, err, true)
-
-	return err
+	return c.Run(backgroundContext())
 }
 
 // func (r *customRunner) tagReference(t *task, ref cue.Value) error {

cmd/cue/cmd/def.go

@@ -53,10 +53,14 @@ The --expression flag is used to only print parts of a configuration.
 
 func runDef(cmd *Command, args []string) error {
 	b, err := parseArgs(cmd, args, &config{outMode: filetypes.Def})
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	e, err := encoding.NewEncoder(cmd.ctx, b.outFile, b.encConfig)
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	iter := b.instances()
 	defer iter.close()
@@ -67,12 +71,16 @@ func runDef(cmd *Command, args []string) error {
 		} else {
 			err = e.Encode(iter.value())
 		}
-		exitOnErr(cmd, err, true)
+		if err != nil {
+			return err
+		}
+	}
+	if err := iter.err(); err != nil {
+		return err
 	}
-	exitOnErr(cmd, iter.err(), true)
-
-	err = e.Close()
-	exitOnErr(cmd, err, true)
 
+	if err := e.Close(); err != nil {
+		return err
+	}
 	return nil
 }

cmd/cue/cmd/eval.go

@@ -86,7 +86,9 @@ const (
 
 func runEval(cmd *Command, args []string) error {
 	b, err := parseArgs(cmd, args, &config{outMode: filetypes.Eval})
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	syn := []cue.Option{
 		cue.Final(), // for backwards compatibility
@@ -108,7 +110,9 @@ func runEval(cmd *Command, args []string) error {
 	b.encConfig.Format = opts
 
 	e, err := encoding.NewEncoder(cmd.ctx, b.outFile, b.encConfig)
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	iter := b.instances()
 	defer iter.close()
@@ -170,10 +174,11 @@ func runEval(cmd *Command, args []string) error {
 			exitOnErr(cmd, err, false)
 		}
 	}
-	exitOnErr(cmd, iter.err(), true)
-
-	err = e.Close()
-	exitOnErr(cmd, err, true)
-
+	if err := iter.err(); err != nil {
+		return err
+	}
+	if err := e.Close(); err != nil {
+		return err
+	}
 	return nil
 }

cmd/cue/cmd/export.go

@@ -111,22 +111,29 @@ The following formats are recognized:
 
 func runExport(cmd *Command, args []string) error {
 	b, err := parseArgs(cmd, args, &config{outMode: filetypes.Export})
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	enc, err := encoding.NewEncoder(cmd.ctx, b.outFile, b.encConfig)
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	iter := b.instances()
 	defer iter.close()
 	for iter.scan() {
 		v := iter.value()
-		err = enc.Encode(v)
-		exitOnErr(cmd, err, true)
+		err := enc.Encode(v)
+		if err != nil {
+			return err
+		}
+	}
+	if err := iter.err(); err != nil {
+		return err
+	}
+	if err := enc.Close(); err != nil {
+		return err
 	}
-	exitOnErr(cmd, iter.err(), true)
-
-	err = enc.Close()
-	exitOnErr(cmd, err, true)
-
 	return nil
 }

cmd/cue/cmd/fmt.go

@@ -45,7 +45,7 @@ func newFmtCmd(c *Command) *cobra.Command {
 				Package:     "*",
 			})
 			if builds == nil {
-				exitOnErr(cmd, errors.Newf(token.NoPos, "invalid args"), true)
+				return errors.Newf(token.NoPos, "invalid args")
 			}
 
 			opts := []format.Option{}
@@ -60,14 +60,13 @@ func newFmtCmd(c *Command) *cobra.Command {
 			stdout := cmd.OutOrStdout()
 
 			for _, inst := range builds {
-				if inst.Err != nil {
+				if err := inst.Err; err != nil {
 					switch {
-					case errors.As(inst.Err, new(*load.PackageError)) && len(inst.BuildFiles) != 0:
+					case errors.As(err, new(*load.PackageError)) && len(inst.BuildFiles) != 0:
 						// Ignore package errors if there are files to format.
-					case errors.As(inst.Err, new(*load.NoFilesError)):
+					case errors.As(err, new(*load.NoFilesError)):
 					default:
-						exitOnErr(cmd, inst.Err, false)
-						continue
+						return err
 					}
 				}
 				for _, file := range inst.BuildFiles {
@@ -83,14 +82,20 @@ func newFmtCmd(c *Command) *cobra.Command {
 					if !ok {
 						var err error
 						src, err = source.ReadAll(file.Filename, file.Source)
-						exitOnErr(cmd, err, true)
+						if err != nil {
+							return err
+						}
 					}
 
 					file, err := parser.ParseFile(file.Filename, src, parser.ParseComments)
-					exitOnErr(cmd, err, true)
+					if err != nil {
+						return err
+					}
 
 					formatted, err := format.Node(file, opts...)
-					exitOnErr(cmd, err, true)
+					if err != nil {
+						return err
+					}
 
 					// Always write to stdout if the file is read from stdin.
 					if file.Filename == "-" && !doDiff && !check {
@@ -118,7 +123,7 @@ func newFmtCmd(c *Command) *cobra.Command {
 						// already wrote the formatted source to stdout above
 					default:
 						if err := os.WriteFile(file.Filename, formatted, 0644); err != nil {
-							exitOnErr(cmd, err, false)
+							return err
 						}
 					}
 				}

cmd/cue/cmd/import.go

@@ -312,17 +312,17 @@ func runImport(cmd *Command, args []string) (err error) {
 	}
 
 	b, err := parseArgs(cmd, args, c)
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	switch mode {
 	default:
 		err = genericMode(cmd, b)
 	case "proto":
 		err = protoMode(b)
 	}
-
-	exitOnErr(cmd, err, true)
-	return nil
+	return err
 }
 
 func protoMode(b *buildPlan) error {
@@ -418,8 +418,7 @@ func genericMode(cmd *Command, b *buildPlan) error {
 		}
 		// TODO: allow if there is a unique package name.
 		if pkgName == "" && len(b.insts) > 1 {
-			err := fmt.Errorf("must specify package name with the -p flag")
-			exitOnErr(cmd, err, true)
+			return fmt.Errorf("must specify package name with the -p flag")
 		}
 	}
 

cmd/cue/cmd/root.go

@@ -53,22 +53,21 @@ import (
 
 type runFunction func(cmd *Command, args []string) error
 
-func statsEncoder(cmd *Command) *encoding.Encoder {
+func statsEncoder(cmd *Command) (*encoding.Encoder, error) {
 	file := os.Getenv("CUE_STATS_FILE")
 	if file == "" {
-		return nil
+		return nil, nil
 	}
 
 	stats, err := filetypes.ParseFile(file, filetypes.Export)
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return nil, err
+	}
 
-	statsEnc, err := encoding.NewEncoder(cmd.ctx, stats, &encoding.Config{
+	return encoding.NewEncoder(cmd.ctx, stats, &encoding.Config{
 		Stdout: cmd.OutOrStderr(),
 		Force:  true,
 	})
-	exitOnErr(cmd, err, true)
-
-	return statsEnc
 }
 
 // Stats expands [stats.Counts] with counters obtained from other sources,
@@ -88,8 +87,10 @@ func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
 	return func(cmd *cobra.Command, args []string) error {
 		c.Command = cmd
 
-		statsEnc := statsEncoder(c)
-
+		statsEnc, err := statsEncoder(c)
+		if err != nil {
+			return err
+		}
 		if err := cueexperiment.Init(); err != nil {
 			return err
 		}
@@ -120,7 +121,7 @@ func mkRunE(c *Command, f runFunction) func(*cobra.Command, []string) error {
 			(*cueruntime.Runtime)(c.ctx).SetDebugOptions(&cuedebug.Flags)
 		}
 
-		err := f(c, args)
+		err = f(c, args)
 
 		// TODO(mvdan): support -memprofilerate like `go help testflag`.
 		if memprofile := flagMemProfile.String(c); memprofile != "" {
@@ -267,10 +268,10 @@ var rootContextOptions []cuecontext.Option
 
 // Main runs the cue tool and returns the code for passing to os.Exit.
 func Main() int {
-	cwd, _ := os.Getwd()
 	cmd, _ := New(os.Args[1:])
 	if err := cmd.Run(backgroundContext()); err != nil {
 		if err != ErrPrintedError {
+			cwd, _ := os.Getwd()
 			errors.Print(os.Stderr, err, &errors.Config{
 				Cwd:     cwd,
 				ToSlash: testing.Testing(),

cmd/cue/cmd/vet.go

@@ -91,14 +91,15 @@ func doVet(cmd *Command, args []string) error {
 	b, err := parseArgs(cmd, args, &config{
 		noMerge: true,
 	})
-	exitOnErr(cmd, err, true)
+	if err != nil {
+		return err
+	}
 
 	// Go into a special vet mode if the user explicitly specified non-cue
 	// files on the command line.
 	// TODO: unify these two modes.
 	if len(b.orphaned) > 0 {
-		vetFiles(cmd, b)
-		return nil
+		return vetFiles(cmd, b)
 	}
 
 	shown := false
@@ -135,15 +136,17 @@ func doVet(cmd *Command, args []string) error {
 		}
 		exitOnErr(cmd, err, false)
 	}
-	exitOnErr(cmd, iter.err(), true)
+	if err := iter.err(); err != nil {
+		return err
+	}
 	return nil
 }
 
-func vetFiles(cmd *Command, b *buildPlan) {
+func vetFiles(cmd *Command, b *buildPlan) error {
 	// Use -r type root, instead of -e
 
 	if !b.encConfig.Schema.Exists() {
-		exitOnErr(cmd, errors.New("data files specified without a schema"), true)
+		return errors.New("data files specified without a schema")
 	}
 
 	iter := b.instances()
@@ -155,5 +158,8 @@ func vetFiles(cmd *Command, b *buildPlan) {
 		err := v.Validate(cue.Concrete(true))
 		exitOnErr(cmd, err, false)
 	}
-	exitOnErr(cmd, iter.err(), false)
+	if err := iter.err(); err != nil {
+		return err
+	}
+	return nil
 }