fix: skip linter without languages related

config/linters-config/.golangci.yml

@@ -26,4 +26,5 @@ linters:
     - exhaustruct # seems too arbitrary
     - execinquery # deprecated
     - gomnd # deprecated
+    - funlen # seems too arbitrary
 

internal/linters/filters.go

@@ -28,6 +28,14 @@
 	return results, nil
 }
 
+// LinterRelated checks if the linter is related to the PR.
+// Each linter has a list of languages that it supports and the file extensions are used to determine
+// whether the linter is related to the PR.
+func LinterRelated(linterName string, a Agent) bool {
+	exts := exts(a.PullRequestChangedFiles)
+	return languageRelated(linterName, exts)
+}
+
 // cleanLintResults cleans the file path in lint results.
 // It removes the workdir prefix from the file path.
 func cleanLintResults(workdir string, lintResults map[string][]LinterOutput) map[string][]LinterOutput {
@@ -143,3 +151,27 @@
 
 	return found
 }
+
+// exts returns the file extensions of the changes.
+// file extensions are used to determine whether the linter is related to the PR.
+func exts(changes []*github.CommitFile) map[string]bool {
+	var exts = make(map[string]bool)
+	for _, change := range changes {
+		ext := filepath.Ext(change.GetFilename())
+		if ext == "" {
+			continue
+		}
+		exts[ext] = true
+	}
+	return exts
+}
+
+func languageRelated(linterName string, exts map[string]bool) bool {
+	langs := Languages(linterName)
+	for _, language := range langs {
+		if language == "*" || exts[language] {
+			return true
+		}
+	}
+	return false
+}

internal/linters/filters_test.go

@@ -197,3 +197,79 @@ func TestFilters(t *testing.T) {
 		})
 	}
 }
+
+func TestLinterRelated(t *testing.T) {
+	tcs := []struct {
+		name     string
+		linter   string
+		a        Agent
+		langs    []string
+		expected bool
+	}{
+		{
+			name:   "related",
+			linter: "golangci-lint",
+			a: Agent{
+				PullRequestChangedFiles: []*github.CommitFile{
+					{
+						Filename: github.String(".testdata/xxx_1.go"),
+					},
+				},
+			},
+			langs:    []string{".go"},
+			expected: true,
+		},
+		{
+			name:   "not related",
+			linter: "golangci-lint",
+			a: Agent{
+				PullRequestChangedFiles: []*github.CommitFile{
+					{
+						Filename: github.String("abc.sh"),
+					},
+				},
+			},
+			langs:    []string{".go"},
+			expected: false,
+		},
+		{
+			name:   "related",
+			linter: "git-flow",
+			a: Agent{
+				PullRequestChangedFiles: []*github.CommitFile{
+					{
+						Filename: github.String("abc.sh"),
+					},
+				},
+			},
+			langs:    []string{"*"},
+			expected: true,
+		},
+		{
+			name:   "related",
+			linter: "git-flow",
+			a: Agent{
+				PullRequestChangedFiles: []*github.CommitFile{
+					{
+						Filename: github.String("main.c"),
+					},
+					{
+						Filename: github.String("main.cpp"),
+					},
+				},
+			},
+			langs:    []string{".c", ".go"},
+			expected: true,
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			RegisterLinterLanguages(tc.linter, tc.langs)
+			actual := LinterRelated(tc.linter, tc.a)
+			if actual != tc.expected {
+				t.Errorf("expected %v, got %v, linter: %v, PR: %v", tc.expected, actual, tc.linter, tc.a.PullRequestChangedFiles)
+			}
+		})
+	}
+}

internal/linters/linters.go

@@ -21,7 +21,6 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -180,13 +179,6 @@ func Report(log *xlog.Logger, a Agent, lintResults map[string][]LinterOutput) er
 
 	log.Infof("[%s] found %d files with valid %d linter errors related to this PR %d (%s) \n", linterName, len(lintResults), countLinterErrors(lintResults), num, orgRepo)
 
-	// only not report when there is no lint errors and the linter is not related to the PR
-	// see https://github.com/qiniu/reviewbot/issues/108#issuecomment-2042217108
-	if len(lintResults) == 0 && !languageRelated(linterName, exts(a.PullRequestChangedFiles)) {
-		log.Debugf("[%s] no lint errors found and not languages related, skip report", linterName)
-		return nil
-	}
-
 	if len(lintResults) > 0 {
 		metric.IncIssueCounter(orgRepo, linterName, a.PullRequestEvent.PullRequest.GetHTMLURL(), a.PullRequestEvent.GetPullRequest().GetHead().GetSHA(), float64(countLinterErrors(lintResults)))
 	}
@@ -380,27 +372,6 @@ func IsEmpty(args ...string) bool {
 	return true
 }
 
-func exts(changes []*github.CommitFile) map[string]bool {
-	var exts = make(map[string]bool)
-	for _, change := range changes {
-		ext := filepath.Ext(change.GetFilename())
-		if ext == "" {
-			continue
-		}
-		exts[ext] = true
-	}
-	return exts
-}
-
-func languageRelated(linterName string, exts map[string]bool) bool {
-	for _, language := range Languages(linterName) {
-		if exts[language] {
-			return true
-		}
-	}
-	return false
-}
-
 func countLinterErrors(lintResults map[string][]LinterOutput) int {
 	var count int
 	for _, outputs := range lintResults {

server.go

@@ -145,6 +145,11 @@ func (s *Server) handle(log *xlog.Logger, ctx context.Context, event *github.Pul
 			PullRequestChangedFiles: pullRequestAffectedFiles,
 		}
 
+		if !linters.LinterRelated(name, agent) {
+			log.Infof("[%s] linter is not related to the PR, skipping", name)
+			continue
+		}
+
 		if err := fn(log, agent); err != nil {
 			log.Errorf("failed to run linter: %v", err)
 			// continue to run other linters