fix: Return a non-zero error code if there are any warnings during fix. (#81)

Also, report warnings even if the file is already "fixed". This seems
like another bug that was lurking.

This change required less work than I was expecting. `go run` helpfully
prints `error status %d` to stderr for us.

Fixes #77

Author: JeffFaer

cmd/cmd.go

@@ -214,30 +214,36 @@ func Run(c *Config, files []string) (ok bool, err error) {
 }
 
 func fix(fixer *keepsorted.Fixer, filenames []string, modifiedLines []keepsorted.LineRange) (ok bool, err error) {
+	ok = true
+
 	for _, fn := range filenames {
 		contents, err := read(fn)
 		if err != nil {
 			return false, err
 		}
-		if want, alreadyFixed, warnings := fixer.Fix(fn, contents, modifiedLines); fn == stdin || !alreadyFixed {
+		want, alreadyFixed, warnings := fixer.Fix(fn, contents, modifiedLines)
+		if fn == stdin || !alreadyFixed {
 			if err := write(fn, want); err != nil {
 				return false, err
 			}
-			for _, warn := range warnings {
-				log := log.Warn()
-				if warn.Path != stdin {
-					log = log.Str("file", warn.Path)
-				}
-				if warn.Lines.Start == warn.Lines.End {
-					log = log.Int("line", warn.Lines.Start)
-				} else {
-					log = log.Int("start", warn.Lines.Start).Int("end", warn.Lines.End)
-				}
-				log.Msg(warn.Message)
+		}
+
+		for _, warn := range warnings {
+			ok = false
+			log := log.Warn()
+			if warn.Path != stdin {
+				log = log.Str("file", warn.Path)
 			}
+			if warn.Lines.Start == warn.Lines.End {
+				log = log.Int("line", warn.Lines.Start)
+			} else {
+				log = log.Int("start", warn.Lines.Start).Int("end", warn.Lines.End)
+			}
+			log.Msg(warn.Message)
 		}
 	}
-	return true, nil
+
+	return ok, nil
 }
 
 func lint(fixer *keepsorted.Fixer, filenames []string, modifiedLines []keepsorted.LineRange) (ok bool, err error) {

goldens/by_regex.err

@@ -1,2 +1,3 @@
 WRN while parsing option "by_regex": error parsing regexp: missing argument to repetition operator: `*` line=85
 WRN by_regex cannot be used with ignore_prefixes (consider adding a non-capturing group to the start of your regex instead of ignore_prefixes: "(?:foo|bar)") line=92
+exit status 1

goldens/generate-goldens.sh

@@ -25,7 +25,8 @@ for i in "$@"; do
   out="${i%%in}out"
   err="${i%%in}err"
 
-  go run "${git_dir}" --id=keep-sorted-test --omit-timestamps - <"${i}" >"${out}" 2>"${err}"
+  go run "${git_dir}" --id=keep-sorted-test --omit-timestamps - <"${i}" >"${out}" 2>"${err}" \
+    || true  # Ignore any non-zero exit codes.
   if (( $(wc -l < "${err}") == 0 )); then
     rm "${err}"
   fi

goldens/golden_test.go

@@ -91,7 +91,7 @@ func TestGoldens(t *testing.T) {
 				wantErr = []byte(strings.ReplaceAll(string(wantErr), "\r\n", "\n"))
 				wantErr = []byte(strings.ReplaceAll(string(wantErr), "\r", "\n"))
 
-				gotOut, gotErr, err := runKeepSorted(in)
+				gotOut, gotErr, exitCode, err := runKeepSorted(in)
 				if err != nil {
 					t.Errorf("Had trouble running keep-sorted: %v", err)
 				}
@@ -104,10 +104,13 @@ func TestGoldens(t *testing.T) {
 					needsRegen <- inFile
 				}
 
-				gotOut2, _, err := runKeepSorted(strings.NewReader(gotOut))
+				gotOut2, _, exitCode2, err := runKeepSorted(strings.NewReader(gotOut))
 				if err != nil {
 					t.Errorf("Had trouble running keep-sorted on keep-sorted output: %v", err)
 				}
+				if exitCode != exitCode2 {
+					t.Errorf("Running keep-sorted on keep-sorted output returned a different exit code (should be idempotent): got %d want %d", exitCode2, exitCode)
+				}
 				if diff := cmp.Diff(gotOut, gotOut2); diff != "" {
 					t.Errorf("keep-sorted diff on keep-sorted output (should be idempotent) (-want +got)\n%s", diff)
 				}
@@ -131,20 +134,20 @@ func showTopLevel(dir string) (string, error) {
 	return strings.TrimSpace(string(b)), err
 }
 
-func runKeepSorted(stdin io.Reader) (stdout, stderr string, err error) {
+func runKeepSorted(stdin io.Reader) (stdout, stderr string, exitCode int, err error) {
 	cmd := exec.Command("go", "run", gitDir, "--id=keep-sorted-test", "--omit-timestamps", "-")
 	cmd.Stdin = stdin
 	outPipe, err := cmd.StdoutPipe()
 	if err != nil {
-		return "", "", fmt.Errorf("could not create stdout pipe: %w", err)
+		return "", "", -1, fmt.Errorf("could not create stdout pipe: %w", err)
 	}
 	errPipe, err := cmd.StderrPipe()
 	if err != nil {
-		return "", "", fmt.Errorf("could not create stderr pipe: %w", err)
+		return "", "", -1, fmt.Errorf("could not create stderr pipe: %w", err)
 	}
 
 	if err := cmd.Start(); err != nil {
-		return "", "", fmt.Errorf("could not start keep-sorted: %w", err)
+		return "", "", -1, fmt.Errorf("could not start keep-sorted: %w", err)
 	}
 
 	var errs []error
@@ -159,8 +162,13 @@ func runKeepSorted(stdin io.Reader) (stdout, stderr string, err error) {
 	}
 
 	if err := cmd.Wait(); err != nil {
-		errs = append(errs, fmt.Errorf("keep-sorted failed: %w", err))
+		var ee *exec.ExitError
+		if errors.As(err, &ee) {
+			exitCode = ee.ExitCode()
+		} else {
+			errs = append(errs, fmt.Errorf("keep-sorted failed: %w", err))
+		}
 	}
 
-	return string(gotOut), string(gotErr), errors.Join(errs...)
+	return string(gotOut), string(gotErr), exitCode, errors.Join(errs...)
 }

goldens/simple.err

@@ -1,3 +1,4 @@
 WRN skip_lines has invalid value: -1 line=105
 WRN unrecognized option "foo" line=105
 WRN while parsing option "ignore_prefixes": content appears to be an unterminated YAML list: "[abc, foo" line=112
+exit status 1