diff --git a/cmd/format/format.go b/cmd/format/format.go index 4e877aee..f3cd281b 100644 --- a/cmd/format/format.go +++ b/cmd/format/format.go @@ -32,7 +32,10 @@ const ( BatchSize = 1024 ) -var ErrFailOnChange = errors.New("unexpected changes detected, --fail-on-change is enabled") +var ( + ErrFailOnChange = errors.New("unexpected changes detected, --fail-on-change is enabled") + ErrFormattingFailures = errors.New("formatting failures detected") +) func Run(v *viper.Viper, statz *stats.Stats, cmd *cobra.Command, paths []string) error { cmd.SilenceUsage = true @@ -404,6 +407,8 @@ func postProcessing( formattedCh chan *format.Task, ) func() error { return func() error { + var formattingFailures bool // track if there were any formatting failures + LOOP: for { select { @@ -420,8 +425,9 @@ func postProcessing( // grab the underlying file reference file := task.File - // check if there were any errors processing the file if len(task.Errors) > 0 { + formattingFailures = true + // release the file, passing the first task error // note: task errors are related to the batch in which a task was applied // this does not necessarily indicate this file had a problem being formatted, but this approach @@ -471,16 +477,21 @@ func postProcessing( } } - // if fail on change has been enabled, check that no files were actually changed, throwing an error if so - if cfg.FailOnChange && statz.Value(stats.Changed) != 0 { - return ErrFailOnChange - } - // print stats to stdout unless we are processing stdin and printing the results to stdout if !cfg.Stdin { statz.Print() } + // return an error if any formatting failures were detected + if formattingFailures { + return ErrFormattingFailures + } + + // if fail on change has been enabled, check that no files were actually changed, throwing an error if so + if cfg.FailOnChange && statz.Value(stats.Changed) != 0 { + return ErrFailOnChange + } + return nil } } diff --git a/cmd/root_test.go b/cmd/root_test.go index c860cce2..71d0b238 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -14,7 +14,7 @@ import ( "github.com/charmbracelet/log" "github.com/numtide/treefmt/cmd" - format2 "github.com/numtide/treefmt/cmd/format" + formatCmd "github.com/numtide/treefmt/cmd/format" "github.com/numtide/treefmt/config" "github.com/numtide/treefmt/format" "github.com/numtide/treefmt/stats" @@ -482,7 +482,7 @@ func TestCache(t *testing.T) { // running should match but not format anything _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) - as.NoError(err) + as.ErrorIs(err, formatCmd.ErrFormattingFailures) assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, @@ -492,8 +492,8 @@ func TestCache(t *testing.T) { }) // running again should provide the same result - _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir, "-vv") - as.NoError(err) + _, statz, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir) + as.ErrorIs(err, formatCmd.ErrFormattingFailures) assertStats(t, as, statz, map[stats.Type]int{ stats.Traversed: 32, @@ -588,13 +588,13 @@ func TestFailOnChange(t *testing.T) { test.WriteConfig(t, configPath, cfg) _, _, err := treefmt(t, "--fail-on-change", "--config-file", configPath, "--tree-root", tempDir) - as.ErrorIs(err, format2.ErrFailOnChange) + as.ErrorIs(err, formatCmd.ErrFailOnChange) // test with no cache t.Setenv("TREEFMT_FAIL_ON_CHANGE", "true") test.WriteConfig(t, configPath, cfg) _, _, err = treefmt(t, "--config-file", configPath, "--tree-root", tempDir, "--no-cache") - as.ErrorIs(err, format2.ErrFailOnChange) + as.ErrorIs(err, formatCmd.ErrFailOnChange) } func TestBustCacheOnFormatterChange(t *testing.T) { @@ -1027,7 +1027,7 @@ func TestStdin(t *testing.T) { // we get an error about the missing filename parameter. out, _, err := treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--stdin") as.EqualError(err, "exactly one path should be specified when using the --stdin flag") - as.Equal("", string(out)) + as.Equal("Error: exactly one path should be specified when using the --stdin flag\n", string(out)) // now pass along the filename parameter os.Stdin = test.TempFile(t, "", "stdin", &contents) @@ -1051,7 +1051,7 @@ func TestStdin(t *testing.T) { out, _, err = treefmt(t, "-C", tempDir, "--allow-missing-formatter", "--stdin", "../test.nix") as.Errorf(err, "path ../test.nix not inside the tree root %s", tempDir) - as.Equal("", string(out)) + as.Contains(string(out), "Error: path ../test.nix not inside the tree root") // try some markdown instead contents = ` @@ -1281,21 +1281,20 @@ func treefmt(t *testing.T, args ...string) ([]byte, *stats.Stats, error) { time.Sleep(time.Until(waitUntil)) }() - if err := root.Execute(); err != nil { - return nil, nil, err - } + // execute the command + cmdErr := root.Execute() // reset and read the temporary output - if _, err := tempOut.Seek(0, 0); err != nil { - return nil, nil, fmt.Errorf("failed to reset temp output for reading: %w", err) + if _, resetErr := tempOut.Seek(0, 0); resetErr != nil { + t.Fatal(fmt.Errorf("failed to reset temp output for reading: %w", resetErr)) } - out, err := io.ReadAll(tempOut) - if err != nil { - return nil, nil, fmt.Errorf("failed to read temp output: %w", err) + out, readErr := io.ReadAll(tempOut) + if readErr != nil { + t.Fatal(fmt.Errorf("failed to read temp output: %w", readErr)) } - return out, statz, nil + return out, statz, cmdErr } func assertStats( diff --git a/format/formatter.go b/format/formatter.go index 340bc28c..41648b5f 100644 --- a/format/formatter.go +++ b/format/formatter.go @@ -77,7 +77,7 @@ func (f *Formatter) Apply(ctx context.Context, tasks []*Task) error { f.log.Errorf("failed to apply with options '%v': %s", f.config.Options, err) if len(out) > 0 { - _, _ = fmt.Fprintf(os.Stderr, "%s error:\n%s\n", f.name, out) + _, _ = fmt.Fprintf(os.Stderr, "\n%s\n", out) } return fmt.Errorf("formatter '%s' with options '%v' failed to apply: %w", f.config.Command, f.config.Options, err)