diff --git a/cmd/cue/cmd/common.go b/cmd/cue/cmd/common.go index 590fcee2c33..88a48ecc261 100644 --- a/cmd/cue/cmd/common.go +++ b/cmd/cue/cmd/common.go @@ -15,17 +15,14 @@ package cmd import ( - "io" "os" "path/filepath" "regexp" "strconv" "strings" - "testing" "github.com/spf13/pflag" "golang.org/x/text/language" - "golang.org/x/text/message" "cuelang.org/go/cue" "cuelang.org/go/cue/ast" @@ -82,34 +79,6 @@ func getLang() language.Tag { return language.Make(loc) } -// exitOnErr uses cue/errors to print any error to stderr, -// and if fatal is true, it causes the command's exit code to be 1. -// Note that os.Exit is not called, as that would prevent defers from running. -// -// TODO(mvdan): can we avoid the panicError and recover shenanigans? -// They work, but they make the code flow somewhat confusing to follow. -func exitOnErr(cmd *Command, err error, fatal bool) { - if err == nil { - return - } - - // Link x/text as our localizer. - p := message.NewPrinter(getLang()) - format := func(w io.Writer, format string, args ...interface{}) { - p.Fprintf(w, format, args...) - } - - cwd, _ := os.Getwd() - errors.Print(cmd.Stderr(), err, &errors.Config{ - Format: format, - Cwd: cwd, - ToSlash: testing.Testing(), - }) - if fatal { - panicExit() - } -} - func loadFromArgs(args []string, cfg *load.Config) []*build.Instance { binst := load.Instances(args, cfg) if len(binst) == 0 { @@ -168,9 +137,11 @@ func (b *buildPlan) instances() iterator { case len(b.orphaned) > 0: i = newStreamingIterator(b) case len(b.insts) > 0: + insts, err := buildInstances(b.cmd, b.insts, false) i = &instanceIterator{ inst: b.instance, - a: buildInstances(b.cmd, b.insts, false), + a: insts, + e: err, i: -1, } default: @@ -625,11 +596,14 @@ func parseArgs(cmd *Command, args []string, cfg *config) (p *buildPlan, err erro // TODO: ignore errors here for now until reporting of concreteness // of errors is correct. // See https://github.com/cue-lang/cue/issues/1483. - inst := buildInstances( + insts, err := buildInstances( p.cmd, []*build.Instance{schema}, - true)[0] - + true) + if err != nil { + return nil, err + } + inst := insts[0] if err := inst.err; err != nil { return nil, err } @@ -729,12 +703,14 @@ func (b *buildPlan) parseFlags() (err error) { return nil } -func buildInstances(cmd *Command, binst []*build.Instance, ignoreErrors bool) []*instance { +func buildInstances(cmd *Command, binst []*build.Instance, ignoreErrors bool) ([]*instance, error) { // TODO: // If there are no files and User is true, then use those? // Always use all files in user mode? instances, err := cmd.ctx.BuildInstances(binst) - exitOnErr(cmd, err, true) + if err != nil { + return nil, err + } insts := make([]*instance, len(instances)) for i, v := range instances { @@ -748,15 +724,22 @@ func buildInstances(cmd *Command, binst []*build.Instance, ignoreErrors bool) [] // TODO: remove ignoreErrors flag and always return here, leaving it up to // clients to check for errors down the road. if ignoreErrors || flagIgnore.Bool(cmd) { - return insts + return insts, nil } for _, inst := range instances { // TODO: consider merging errors of multiple files, but ensure // duplicates are removed. - exitOnErr(cmd, inst.Validate(), !flagIgnore.Bool(cmd)) + err := inst.Validate() + if err != nil { + if flagIgnore.Bool(cmd) { + printError(cmd, err) + } else { + return nil, err + } + } } - return insts + return insts, nil } func buildToolInstances(binst []*build.Instance) ([]*cue.Instance, error) { diff --git a/cmd/cue/cmd/eval.go b/cmd/cue/cmd/eval.go index 9ef3f147f22..9b8afe0fe84 100644 --- a/cmd/cue/cmd/eval.go +++ b/cmd/cue/cmd/eval.go @@ -132,7 +132,7 @@ func runEval(cmd *Command, args []string) error { err := e.Encode(v) if err != nil { errHeader() - exitOnErr(cmd, err, false) + printError(cmd, err) } continue } @@ -160,7 +160,7 @@ func runEval(cmd *Command, args []string) error { if e.IsConcrete() || flagConcrete.Bool(cmd) { if err := v.Validate(cue.Concrete(true)); err != nil { errHeader() - exitOnErr(cmd, err, false) + printError(cmd, err) continue } } @@ -171,7 +171,7 @@ func runEval(cmd *Command, args []string) error { err := e.EncodeFile(f) if err != nil { errHeader() - exitOnErr(cmd, err, false) + printError(cmd, err) } } if err := iter.err(); err != nil { diff --git a/cmd/cue/cmd/fmt.go b/cmd/cue/cmd/fmt.go index e4d8ee9f3a2..a70c240e317 100644 --- a/cmd/cue/cmd/fmt.go +++ b/cmd/cue/cmd/fmt.go @@ -71,14 +71,13 @@ given as explicit arguments. } 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 { @@ -116,7 +115,9 @@ given as explicit arguments. if path == "-" { contents, err := io.ReadAll(cmd.InOrStdin()) - exitOnErr(cmd, err, false) + if err != nil { + return err + } file.Source = contents } @@ -131,13 +132,14 @@ given as explicit arguments. } for _, arg := range args { if arg == "-" { - err := processFile(arg) - exitOnErr(cmd, err, false) + if err := processFile(arg); err != nil { + return err + } continue } arg = filepath.Clean(arg) - err := filepath.WalkDir(arg, func(path string, d fs.DirEntry, err error) error { + if err := filepath.WalkDir(arg, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } @@ -157,8 +159,9 @@ given as explicit arguments. } return processFile(path) - }) - exitOnErr(cmd, err, false) + }); err != nil { + return err + } } } @@ -227,8 +230,9 @@ func formatFile(file *build.File, opts []format.Option, cwd string, doDiff, chec case file.Filename == "-": // already wrote the formatted source to stdout above default: - err := os.WriteFile(file.Filename, formatted, 0644) - exitOnErr(cmd, err, false) + if err := os.WriteFile(file.Filename, formatted, 0644); err != nil { + return false, err + } } return true, nil } diff --git a/cmd/cue/cmd/root.go b/cmd/cue/cmd/root.go index 53804baf406..104bdb708e9 100644 --- a/cmd/cue/cmd/root.go +++ b/cmd/cue/cmd/root.go @@ -25,6 +25,7 @@ import ( "testing" "github.com/spf13/cobra" + "golang.org/x/text/message" "cuelang.org/go/cue" "cuelang.org/go/cue/cuecontext" @@ -300,7 +301,7 @@ type errWriter Command func (w *errWriter) Write(b []byte) (int, error) { c := (*Command)(w) - c.hasErr = true + c.hasErr = len(b) > 0 return c.Command.OutOrStderr().Write(b) } @@ -309,6 +310,9 @@ func (w *errWriter) Write(b []byte) (int, error) { // be used directly. // Stderr returns a writer that should be used for error messages. +// Writing to it will result in the command's exit code being 1. +// +// TODO(mvdan): provide an alternative to write to stderr without setting the exit code to 1. func (c *Command) Stderr() io.Writer { return (*errWriter)(c) } @@ -331,17 +335,37 @@ func (c *Command) SetInput(r io.Reader) { c.root.SetIn(r) } -// ErrPrintedError indicates error messages have been printed to stderr. +// ErrPrintedError indicates error messages have been printed directly to stderr, +// and can be used so that the returned error itself isn't printed as well. var ErrPrintedError = errors.New("terminating because of errors") +// printError uses cue/errors to print an error to stderr when non-nil. +func printError(cmd *Command, err error) { + if err == nil { + return + } + + // Link x/text as our localizer. + p := message.NewPrinter(getLang()) + format := func(w io.Writer, format string, args ...interface{}) { + p.Fprintf(w, format, args...) + } + + // TODO(mvdan): call os.Getwd only once at the start of the process. + cwd, _ := os.Getwd() + errors.Print(cmd.Stderr(), err, &errors.Config{ + Format: format, + Cwd: cwd, + ToSlash: testing.Testing(), + }) +} + func (c *Command) Run(ctx context.Context) (err error) { // Three categories of commands: // - normal // - user defined // - help // For the latter two, we need to use the default loading. - defer recoverError(&err) - if err := c.root.Execute(); err != nil { return err } @@ -350,22 +374,3 @@ func (c *Command) Run(ctx context.Context) (err error) { } return nil } - -func recoverError(err *error) { - switch e := recover().(type) { - case nil: - case panicError: - *err = e.Err - default: - panic(e) - } - // We use panic to escape, instead of os.Exit -} - -type panicError struct { - Err error -} - -func panicExit() { - panic(panicError{ErrPrintedError}) -} diff --git a/cmd/cue/cmd/trim.go b/cmd/cue/cmd/trim.go index a263bd50ef2..bbee57fbb99 100644 --- a/cmd/cue/cmd/trim.go +++ b/cmd/cue/cmd/trim.go @@ -100,7 +100,10 @@ func runTrim(cmd *Command, args []string) error { if binst == nil { return nil } - instances := buildInstances(cmd, binst, false) + instances, err := buildInstances(cmd, binst, false) + if err != nil { + return err + } dst := flagOutFile.String(cmd) if dst != "" && dst != "-" && !flagForce.Bool(cmd) { @@ -130,7 +133,10 @@ func runTrim(cmd *Command, args []string) error { cfg := *defCfg.loadCfg cfg.Overlay = overlay - tinsts := buildInstances(cmd, load.Instances(args, &cfg), false) + tinsts, err := buildInstances(cmd, load.Instances(args, &cfg), false) + if err != nil { + return err + } if len(tinsts) != len(binst) { return errors.New("unexpected number of new instances") } diff --git a/cmd/cue/cmd/vet.go b/cmd/cue/cmd/vet.go index 7df0918f65b..2c39cc711be 100644 --- a/cmd/cue/cmd/vet.go +++ b/cmd/cue/cmd/vet.go @@ -134,7 +134,7 @@ func doVet(cmd *Command, args []string) error { "some instances are incomplete; use the -c flag to show errors or suppress this message") } } - exitOnErr(cmd, err, false) + printError(cmd, err) } if err := iter.err(); err != nil { return err @@ -156,7 +156,7 @@ func vetFiles(cmd *Command, b *buildPlan) error { // Always concrete when checking against concrete files. err := v.Validate(cue.Concrete(true)) - exitOnErr(cmd, err, false) + printError(cmd, err) } if err := iter.err(); err != nil { return err