Repository: reviewdog/reviewdog
Pull request #513 (2020-04-12 11:03:27)

[ProjectRun] exit 1 if a tool fails with zero-findings
If a tool fails and reviewdog **cannot** capture any findings, the tool
should have been failed unexpectedly, so reviewdog will also exit with 1.

If a tool failes and reviewdog **can** capture at least one finding,
reviewdog exit with 0 because most linters exit 1 if it finds errors.

related: #446


Patch:
diff --git a/go.sum b/go.sum
index cafc57ec..569427e5 100644
--- a/go.sum
+++ b/go.sum
@@ -316,6 +316,7 @@ golang.org/x/sys v0.0.0-20200317113312-5766fd39f98d h1:62ap6LNOjDU6uGmKXHJbSfciM
 golang.org/x/sys v0.0.0-20200317113312-5766fd39f98d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd h1:xhmwyvizuTgC2qz7ZlMluP20uW+C3Rm0FD/WLDX8884=
 golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
+golang.org/x/sys v0.0.0-20200331124033-c3d80250170d h1:nc5K6ox/4lTFbMVSL9WRR81ixkcwXThoiF6yf+R9scA=
 golang.org/x/sys v0.0.0-20200331124033-c3d80250170d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
diff --git a/project/run.go b/project/run.go
index e6cc81de..7168d4f3 100644
--- a/project/run.go
+++ b/project/run.go
@@ -29,17 +29,18 @@ func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, def
 		semaphoreNum = 1
 	}
 	semaphore := make(chan int, semaphoreNum)
-	for _, runner := range conf.Runner {
+	for key, runner := range conf.Runner {
 		runner := runner
-		if len(runners) != 0 && !runners[runner.Name] {
+		runnerName := getRunnerName(key, runner)
+		if len(runners) != 0 && !runners[runnerName] {
 			continue // Skip this runner.
 		}
-		usedRunners = append(usedRunners, runner.Name)
+		usedRunners = append(usedRunners, runnerName)
 		semaphore <- 1
-		log.Printf("reviewdog: [start]\trunner=%s", runner.Name)
+		log.Printf("reviewdog: [start]\trunner=%s", runnerName)
 		fname := runner.Format
 		if fname == "" && len(runner.Errorformat) == 0 {
-			fname = runner.Name
+			fname = runnerName
 		}
 		opt := &reviewdog.ParserOpt{FormatName: fname, Errorformat: runner.Errorformat}
 		p, err := reviewdog.NewParser(opt)
@@ -59,12 +60,21 @@ func RunAndParse(ctx context.Context, conf *Config, runners map[string]bool, def
 			if err != nil {
 				return err
 			}
-			log.Printf("reviewdog: [finish]\trunner=%s", runner.Name)
 			level := runner.Level
 			if level == "" {
 				level = defaultLevel
 			}
-			results.Store(runner.Name, &reviewdog.Result{Level: level, CheckResults: rs})
+			cmdErr := cmd.Wait()
+			results.Store(runnerName, &reviewdog.Result{
+				Level:        level,
+				CheckResults: rs,
+				CmdErr:       cmdErr,
+			})
+			msg := fmt.Sprintf("reviewdog: [finish]\trunner=%s", runnerName)
+			if cmdErr != nil {
+				msg += fmt.Sprintf("\terror=%v", cmdErr)
+			}
+			log.Println(msg)
 			return nil
 		})
 	}
@@ -86,6 +96,10 @@ func Run(ctx context.Context, conf *Config, runners map[string]bool, c reviewdog
 	if results.Len() == 0 {
 		return nil
 	}
+	if err := checkUnexpectedFailures(results); err != nil {
+		return err
+	}
+
 	b, err := d.Diff(ctx)
 	if err != nil {
 		return err
@@ -138,3 +152,25 @@ func checkUnknownRunner(specifiedRunners map[string]bool, usedRunners []string)
 	}
 	return nil
 }
+
+func checkUnexpectedFailures(results *reviewdog.ResultMap) error {
+	var err error
+	results.Range(func(toolname string, result *reviewdog.Result) {
+		// Skip if err is already found.
+		if err != nil {
+			return
+		}
+		if result.CmdErr != nil && len(result.CheckResults) == 0 {
+			err = fmt.Errorf("%s failed with zero findings: The command itself "+
+				"failed (%v) or reviewdog cannot parse the results", toolname, result.CmdErr)
+		}
+	})
+	return err
+}
+
+func getRunnerName(key string, runner *Runner) string {
+	if runner.Name != "" {
+		return runner.Name
+	}
+	return key
+}
diff --git a/resultmap.go b/resultmap.go
index f07690ae..4c9ab6e3 100644
--- a/resultmap.go
+++ b/resultmap.go
@@ -14,6 +14,9 @@ type ResultMap struct {
 type Result struct {
 	Level        string
 	CheckResults []*CheckResult
+
+	// Optional. Report an error of the command execution.
+	CmdErr error
 }
 
 // Store saves a new *Result into ResultMap.


Test patch:
diff --git a/project/run_test.go b/project/run_test.go
index 7140d37e..41ac735b 100644
--- a/project/run_test.go
+++ b/project/run_test.go
@@ -65,7 +65,7 @@ func TestRun(t *testing.T) {
 		conf := &Config{
 			Runner: map[string]*Runner{
 				"test": {
-					Cmd:         "not found",
+					Cmd:         "echo 'hi'",
 					Errorformat: []string{`%f:%l:%c:%m`},
 				},
 			},
@@ -77,7 +77,7 @@ func TestRun(t *testing.T) {
 		}
 	})
 
-	t.Run("cmd error (not for reviewdog to exit with error)", func(t *testing.T) {
+	t.Run("cmd error with findings (not for reviewdog to exit with error)", func(t *testing.T) {
 		buf := new(bytes.Buffer)
 		defaultTeeStderr = buf
 		ds := &fakeDiffService{
@@ -93,7 +93,7 @@ func TestRun(t *testing.T) {
 		conf := &Config{
 			Runner: map[string]*Runner{
 				"test": {
-					Cmd:         "not found",
+					Cmd:         "echo 'file:14:14:message'; exit 1",
 					Errorformat: []string{`%f:%l:%c:%m`},
 				},
 			},
@@ -107,6 +107,34 @@ func TestRun(t *testing.T) {
 		}
 	})
 
+	t.Run("unexpected cmd error (reviewdog exits with error)", func(t *testing.T) {
+		buf := new(bytes.Buffer)
+		defaultTeeStderr = buf
+		ds := &fakeDiffService{
+			FakeDiff: func() ([]byte, error) {
+				return []byte(""), nil
+			},
+		}
+		cs := &fakeCommentService{
+			FakePost: func(c *reviewdog.Comment) error {
+				return nil
+			},
+		}
+		conf := &Config{
+			Runner: map[string]*Runner{
+				"test": {
+					Cmd:         "not found",
+					Errorformat: []string{`%f:%l:%c:%m`},
+				},
+			},
+		}
+		if err := Run(ctx, conf, nil, cs, ds, false, reviewdog.FilterModeAdded); err == nil {
+			t.Error("want error, got nil")
+		} else {
+			t.Log(err)
+		}
+	})
+
 	t.Run("cmd error with tee", func(t *testing.T) {
 		buf := new(bytes.Buffer)
 		defaultTeeStderr = buf
@@ -128,8 +156,10 @@ func TestRun(t *testing.T) {
 				},
 			},
 		}
-		if err := Run(ctx, conf, nil, cs, ds, true, reviewdog.FilterModeAdded); err != nil {
-			t.Error(err)
+		if err := Run(ctx, conf, nil, cs, ds, true, reviewdog.FilterModeAdded); err == nil {
+			t.Error("want error, got nil")
+		} else {
+			t.Log(err)
 		}
 		want := "sh: 1: not: not found\n"
 		if got := buf.String(); got != want {