Skip to content

Commit

Permalink
fix: skip linter without languages related
Browse files Browse the repository at this point in the history
  • Loading branch information
CarlJi committed Jun 24, 2024
1 parent 5b265dc commit fb40db7
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 29 deletions.
31 changes: 31 additions & 0 deletions internal/linters/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ func Filters(log *xlog.Logger, a Agent, linterResults map[string][]LinterOutput)
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.

Check warning on line 32 in internal/linters/filters.go

View check run for this annotation

qiniu-x / golangci-lint

internal/linters/filters.go#L32

line is 142 characters (lll)
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 {
Expand Down Expand Up @@ -143,3 +150,27 @@ func isGoZeroCustomTag(jsonOption string) bool {

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 {

Check warning on line 158 in internal/linters/filters.go

View check run for this annotation

qiniu-x / golangci-lint

internal/linters/filters.go#L158

ranges should only be cuddled with assignments used in the iteration (wsl)
ext := filepath.Ext(change.GetFilename())
if ext == "" {
continue

Check warning on line 161 in internal/linters/filters.go

View check run for this annotation

Codecov / codecov/patch

internal/linters/filters.go#L161

Added line #L161 was not covered by tests
}
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
}
76 changes: 76 additions & 0 deletions internal/linters/filters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,79 @@ func TestFilters(t *testing.T) {
})
}
}

func TestLinterRelated(t *testing.T) {

Check warning on line 201 in internal/linters/filters_test.go

View check run for this annotation

qiniu-x / golangci-lint

internal/linters/filters_test.go#L201

Function 'TestLinterRelated' is too long (73 > 60) (funlen)
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)
}
})
}
}
30 changes: 1 addition & 29 deletions internal/linters/linters.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"os"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -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)))
}
Expand Down Expand Up @@ -268,6 +260,7 @@ func Parse(log *xlog.Logger, output []byte, lineParser LineParser) (map[string][
}
output, err := lineParser(line)
if err != nil {
log.Warnf("failed to parse line: %v, err: %v", line, err)

Check warning on line 263 in internal/linters/linters.go

View check run for this annotation

Codecov / codecov/patch

internal/linters/linters.go#L263

Added line #L263 was not covered by tests
unexpectedLines = append(unexpectedLines, line)
continue
}
Expand Down Expand Up @@ -380,27 +373,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 {
Expand Down
5 changes: 5 additions & 0 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit fb40db7

Please sign in to comment.