refactor: define new linter workflow

config/config.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"os"
 
 	"sigs.k8s.io/yaml"
@@ -11,10 +12,14 @@ type Config map[string]map[string]Linter
 
 type Linter struct {
 	// Enable is whether to enable this linter, if false, linter still run but not report.
-	Enable  *bool    `json:"enable,omitempty"`
-	WorkDir string   `json:"workDir,omitempty"`
-	Command string   `json:"command,omitempty"`
-	Args    []string `json:"args,omitempty"`
+	Enable *bool `json:"enable,omitempty"`
+	// WorkDir is the working directory of the linter.
+	WorkDir string `json:"workDir,omitempty"`
+	// Command is the command to run the linter. e.g. "golangci-lint", "staticcheck"
+	// If empty, use the linter name as the command.
+	Command string `json:"command,omitempty"`
+	// Args is the arguments of the command.
+	Args []string `json:"args,omitempty"`
 
 	// ReportFormat is the format of the report, if empty, use github_checks by default.
 	// e.g. "github_checks", "github_pr_review"
@@ -24,6 +29,10 @@ type Linter struct {
 	ReportFormat string `json:"reportFormat,omitempty"`
 }
 
+func (l Linter) String() string {
+	return fmt.Sprintf("Linter{Enable: %v, WorkDir: %v, Command: %v, Args: %v, ReportFormat: %v}", *l.Enable, l.WorkDir, l.Command, l.Args, l.ReportFormat)
+}
+
 // NewConfig returns a new Config.
 func NewConfig(conf string) (Config, error) {
 	var c Config
@@ -36,7 +45,7 @@ func NewConfig(conf string) (Config, error) {
 		return nil, err
 	}
 
-	c = fixConfig(c)
+	c = FixConfig(c)
 
 	return c, nil
 }
@@ -53,24 +62,37 @@ func (c Config) CustomLinterConfigs(org, repo string) map[string]Linter {
 	return nil
 }
 
-// fixConfig fix the config
+// FixConfig fix the config
 // 1. if linterConfig.Enable is nil, set it to true
 // 2. if linterConfig.ReportFormat is empty, set it to "github_checks"
-func fixConfig(c Config) Config {
-	for org, repoConfig := range c {
-		for repo, linterConfig := range repoConfig {
-			if linterConfig.Enable == nil {
-				enable := true
-				linterConfig.Enable = &enable
-			}
-
-			if linterConfig.ReportFormat == "" {
-				linterConfig.ReportFormat = "github_checks"
-			}
-
-			c[org][repo] = linterConfig
+// 3. if linterConfig.Command is empty, set it to linterName
+func FixConfig(c Config) Config {
+	for repo, repoConfig := range c {
+		for linterName, linterConfig := range repoConfig {
+			c[repo][linterName] = FixLinterConfig(linterConfig, linterName)
 		}
 	}
 
 	return c
 }
+
+// FixLinterConfig fix the linter config
+func FixLinterConfig(linterConfig Linter, linterName string) Linter {
+	// if linterConfig.Enable is nil, set it to true
+	if linterConfig.Enable == nil {
+		enable := true
+		linterConfig.Enable = &enable
+	}
+
+	// if linterConfig.ReportFormat is empty, set it to "github_checks"
+	if linterConfig.ReportFormat == "" {
+		linterConfig.ReportFormat = "github_checks"
+	}
+
+	// if linterConfig.Command is empty, set it to linterName
+	if linterConfig.Command == "" {
+		linterConfig.Command = linterName
+	}
+
+	return linterConfig
+}

internal/github/github.go → internal/linters/github.go

@@ -14,7 +14,7 @@
  limitations under the License.
 */
 
-package github
+package linters
 
 import (
 	"context"
@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"github.com/google/go-github/v57/github"
-	"github.com/qiniu/reviewbot/internal/linters"
 	"github.com/qiniu/x/log"
 )
 
@@ -139,7 +138,7 @@ func GetCommitIDFromContentsURL(contentsURL string) (string, error) {
 	return matches[1], nil
 }
 
-func BuildPullRequestCommentBody(linterName string, lintErrs map[string][]linters.LinterOutput, commitFiles []*github.CommitFile) ([]*github.PullRequestComment, error) {
+func BuildPullRequestCommentBody(linterName string, lintErrs map[string][]LinterOutput, commitFiles []*github.CommitFile) ([]*github.PullRequestComment, error) {
 	var comments []*github.PullRequestComment
 	hunkChecker, err := NewGithubCommitFileHunkChecker(commitFiles)
 	if err != nil {
@@ -172,7 +171,7 @@ func BuildPullRequestCommentBody(linterName string, lintErrs map[string][]linter
 				}
 
 				message := fmt.Sprintf("[%s] %s\n%s",
-					linterName, lintErr.Message, linters.CommentFooter)
+					linterName, lintErr.Message, CommentFooter)
 
 				comments = append(comments, &github.PullRequestComment{
 					Body:     github.String(message),

internal/linters/go/staticcheck/staticcheck.go

@@ -17,124 +17,22 @@
 package staticcheck
 
 import (
-	"context"
-	"os/exec"
-
-	gh "github.com/qiniu/reviewbot/internal/github"
 	"github.com/qiniu/reviewbot/internal/linters"
-	"github.com/qiniu/x/log"
 	"github.com/qiniu/x/xlog"
 )
 
-var lintName = "staticcheck"
+// refer to https://staticcheck.io/docs/
+const linterName = "staticcheck"
 
 func init() {
-	linters.RegisterPullRequestHandler(lintName, staticcheckHandler)
+	linters.RegisterPullRequestHandler(linterName, staticcheckHandler)
 }
 
 func staticcheckHandler(log *xlog.Logger, a linters.Agent) error {
-	var (
-		org     = a.PullRequestEvent.GetRepo().GetOwner().GetLogin()
-		repo    = a.PullRequestEvent.GetRepo().GetName()
-		num     = a.PullRequestEvent.GetNumber()
-		orgRepo = org + "/" + repo
-	)
-
-	executor, err := NewStaticcheckExecutor(a.LinterConfig.WorkDir)
-	if err != nil {
-		log.Errorf("init staticcheck executor failed: %v", err)
-		return err
-	}
-
 	if linters.IsEmpty(a.LinterConfig.Args...) {
 		// turn off compile errors by default
 		a.LinterConfig.Args = append([]string{}, "-debug.no-compile-errors=true", "./...")
 	}
 
-	output, err := executor.Run(log, a.LinterConfig.Args...)
-	if err != nil {
-		log.Errorf("staticcheck run failed: %v", err)
-		return err
-	}
-
-	lintResults, err := executor.Parse(log, output)
-	if err != nil {
-		log.Errorf("staticcheck parse output failed: %v", err)
-		return err
-	}
-
-	if len(lintResults) == 0 {
-		return nil
-	}
-
-	log.Infof("[%s] found total %d files with lint errors on repo %v", lintName, len(lintResults), orgRepo)
-	comments, err := gh.BuildPullRequestCommentBody(lintName, lintResults, a.PullRequestChangedFiles)
-	if err != nil {
-		log.Errorf("failed to build pull request comment body: %v", err)
-		return err
-	}
-
-	if len(comments) == 0 {
-		// no related comments to post, continue to run other linters
-		return nil
-	}
-
-	log.Infof("[%s] found valid %d comments related to this PR %d (%s) \n", lintName, len(comments), num, orgRepo)
-	if err := gh.PostPullReviewCommentsWithRetry(context.Background(), a.GithubClient, org, repo, num, comments); err != nil {
-		log.Errorf("failed to post comments: %v", err)
-		return err
-	}
-	return nil
-}
-
-// Staticcheck is an executor that knows how to execute Staticcheck commands.
-type Staticcheck struct {
-	// dir is the location of this repo.
-	dir string
-	// staticcheck is the path to the staticcheck binary.
-	staticcheck string
-	// execute executes a command
-	execute func(dir, command string, args ...string) ([]byte, error)
-}
-
-// NewStaticcheckExecutor returns a new executor that knows how to execute staticcheck commands
-// TODO: with config
-func NewStaticcheckExecutor(dir string) (linters.Linter, error) {
-	log.Infof("staticcheck executor init")
-	g, err := exec.LookPath("staticcheck")
-	if err != nil {
-		return nil, err
-	}
-	return &Staticcheck{
-		dir:         dir,
-		staticcheck: g,
-		execute: func(dir, command string, args ...string) ([]byte, error) {
-			c := exec.Command(command, args...)
-			c.Dir = dir
-			log.Printf("final command:  %v \n", c)
-			return c.Output()
-		},
-	}, nil
-}
-
-func (e *Staticcheck) Run(log *xlog.Logger, args ...string) ([]byte, error) {
-	b, err := e.execute(e.dir, e.staticcheck, args...)
-	if err != nil {
-		log.Errorf("staticcheck run with status: %v, mark and continue", err)
-	} else {
-		log.Infof("staticcheck succeeded")
-	}
-
-	return b, nil
-}
-
-// formatStaticcheckOutput formats the output of staticcheck
-// example：
-// gslb-api/utils.go:149:6: func dealWithRecordVipsId is unused (U1000)
-// gslb-api/utils.go:162:2: this value of err is never used (SA4006)
-// domain/repo/image.go:70:7: receiver name should be a reflection of its identity; don't use generic names such as "this" or "self" (ST1006)
-//
-//	output:  map[file][]linters.LinterOutput
-func (e *Staticcheck) Parse(log *xlog.Logger, output []byte) (map[string][]linters.LinterOutput, error) {
-	return linters.FormatLinterOutput(log, output, linters.FormatLinterLine)
+	return linters.GeneralHandler(log, a, linters.GeneralParse)
 }

internal/github/hunk.go → internal/linters/hunk.go

@@ -14,7 +14,7 @@
  limitations under the License.
 */
 
-package github
+package linters
 
 import (
 	"fmt"

internal/github/hunk_test.go → internal/linters/hunk_test.go

@@ -14,7 +14,7 @@
  limitations under the License.
 */
 
-package github
+package linters
 
 import (
 	"fmt"