Merge pull request #151 from CarlJi/feat/golangci-lint

feat(config): support linter's config path
qiniu · Jun 7, 2024 · 74f8d3f · 74f8d3f
2 parents e619b44 + ab5fba7
commit 74f8d3f
Show file tree

Hide file tree

Showing 6 changed files with 166 additions and 13 deletions.
diff --git a/config/config.go b/config/config.go
@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"sigs.k8s.io/yaml"
 )
@@ -21,6 +22,11 @@ type GlobalConfig struct {
 	// GithubReportType is the format of the report, will be used if linterConfig.ReportFormat is empty.
 	// e.g. "github_checks", "github_pr_review"
 	GithubReportType GithubReportType `json:"githubReportType,omitempty"`
+
+	// GolangciLintConfig is the path of golangci-lint config file to run golangci-lint globally.
+	// if not empty, use the config to run golangci-lint.
+	// it can be overridden by linter.ConfigPath.
+	GolangCiLintConfig string `json:"golangciLintConfig,omitempty"`
 }
 
 type Linter struct {
@@ -41,10 +47,16 @@ type Linter struct {
 	// Note:
 	// * github_check_run only support on Github Apps, not support on Github OAuth Apps or authenticated users.
 	ReportFormat GithubReportType `json:"githubReportType,omitempty"`
+
+	// ConfigPath is the path of the linter config file.
+	// If not empty, use the config to run the linter.
+	ConfigPath string `json:"configPath,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)
+	return fmt.Sprintf(
+		"Linter{Enable: %v, WorkDir: %v, Command: %v, Args: %v, ReportFormat: %v, ConfigPath: %v}",
+		*l.Enable, l.WorkDir, l.Command, l.Args, l.ReportFormat, l.ConfigPath)
 }
 
 // NewConfig returns a new Config.
@@ -64,6 +76,18 @@ func NewConfig(conf string) (Config, error) {
 		c.GlobalDefaultConfig.GithubReportType = GithubPRReview
 	}
 
+	// check golangci-lint config path
+	if c.GlobalDefaultConfig.GolangCiLintConfig != "" {
+		absPath, err := os.Getwd()
+		if err != nil {
+			return c, err
+		}
+		c.GlobalDefaultConfig.GolangCiLintConfig = filepath.Join(absPath, c.GlobalDefaultConfig.GolangCiLintConfig)
+		if _, err := os.Stat(c.GlobalDefaultConfig.GolangCiLintConfig); err != nil {
+			return c, fmt.Errorf("golangci-lint config file not found: %v", c.GlobalDefaultConfig.GolangCiLintConfig)
+		}
+	}
+
 	return c, nil
 }
 
@@ -74,6 +98,11 @@ func (c Config) Get(org, repo, ln string) Linter {
 		Command:      ln,
 	}
 
+	// set golangci-lint config path if exists
+	if c.GlobalDefaultConfig.GolangCiLintConfig != "" && ln == "golangci-lint" {
+		linter.ConfigPath = c.GlobalDefaultConfig.GolangCiLintConfig
+	}
+
 	if orgConfig, ok := c.CustomConfig[org]; ok {
 		if l, ok := orgConfig[ln]; ok {
 			linter = applyCustomConfig(linter, l)
@@ -102,14 +131,18 @@ func applyCustomConfig(legacy, custom Linter) Linter {
 		legacy.Command = custom.Command
 	}
 
-	if len(custom.Args) != 0 {
+	if custom.Args != nil {
 		legacy.Args = custom.Args
 	}
 
 	if custom.ReportFormat != "" {
 		legacy.ReportFormat = custom.ReportFormat
 	}
 
+	if custom.ConfigPath != "" {
+		legacy.ConfigPath = custom.ConfigPath
+	}
+
 	return legacy
 }
 

diff --git a/config/config.yaml b/config/config.yaml
@@ -6,6 +6,7 @@
 
 globalDefaultConfig: # global default settings, will be overridden by qbox org and repo specific settings if they exist
   githubReportType: "github_check_run" # github_pr_review, github_check_run
+  golangcilintConfig: "config/linters-config/.golangci.yml" # golangci-lint config file to use
 
 customConfig: # custom config for specific orgs or repos
   qbox: # github organization name

diff --git a/config/config_test.go b/config/config_test.go
@@ -3,6 +3,8 @@ package config
 import (
 	"os"
 	"path/filepath"
+	"reflect"
+	"strings"
 	"testing"
 )
 
@@ -14,7 +16,7 @@ func TestConfig(t *testing.T) {
 		expected    Config
 	}{
 		{
-			name:        "default config",
+			name:        "no config",
 			expectError: false,
 			rawConfig:   ``,
 			expected: Config{
@@ -61,6 +63,46 @@ customConfig: # custom config for specific orgs or repos
 				},
 			},
 		},
+		{
+			name:        "config with golangci-lint config path",
+			expectError: false,
+			rawConfig: `
+globalDefaultConfig: # global default settings, will be overridden by qbox org and repo specific settings if they exist
+  githubReportType: "github_check_run" # github_pr_review, github_check_run
+  golangciLintConfig: "linters-config/.golangci.yml"
+
+customConfig: # custom config for specific orgs or repos
+  qbox: # github organization name
+    golangci-lint:
+      enable: true
+      args: ["run", "-D", "staticcheck"] # disable staticcheck globally since we have a separate linter for it
+
+  qbox/net-cache:
+    luacheck:
+      enable: true
+      workDir: "nginx" # only run in the nginx directory since there are .luacheckrc files in this directory
+`,
+			expected: Config{
+				GlobalDefaultConfig: GlobalConfig{
+					GithubReportType:   GithubCheckRuns,
+					GolangCiLintConfig: "linters-config/.golangci.yml",
+				},
+				CustomConfig: map[string]map[string]Linter{
+					"qbox": {
+						"golangci-lint": {
+							Enable: boolPtr(true),
+							Args:   []string{"run", "-D", "staticcheck"},
+						},
+					},
+					"qbox/net-cache": {
+						"luacheck": {
+							Enable:  boolPtr(true),
+							WorkDir: "nginx",
+						},
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -86,8 +128,12 @@ customConfig: # custom config for specific orgs or repos
 					t.Errorf("expected %v, got %v", tc.expected.GlobalDefaultConfig.GithubReportType, c.GlobalDefaultConfig.GithubReportType)
 				}
 
-				if len(c.CustomConfig) != len(tc.expected.CustomConfig) {
-					t.Errorf("expected %v, got %v", len(tc.expected.CustomConfig), len(c.CustomConfig))
+				if !strings.HasSuffix(c.GlobalDefaultConfig.GolangCiLintConfig, tc.expected.GlobalDefaultConfig.GolangCiLintConfig) {
+					t.Errorf("expected %v, got %v", tc.expected.GlobalDefaultConfig.GolangCiLintConfig, c.GlobalDefaultConfig.GolangCiLintConfig)
+				}
+
+				if !reflect.DeepEqual(c.CustomConfig, tc.expected.CustomConfig) {
+					t.Errorf("expected %v, got %v", tc.expected.CustomConfig, c.CustomConfig)
 				}
 			}
 		})
@@ -100,6 +146,7 @@ func TestGet(t *testing.T) {
 	rawConfig := `
 globalDefaultConfig:
   githubReportType: "github_check_run" # github_pr_review, github_check_run
+  golangciLintConfig: "linters-config/.golangci.yml"
 
 customConfig: # custom config for specific orgs or repos
   qbox: # github organization name
@@ -116,6 +163,16 @@ customConfig: # custom config for specific orgs or repos
     staticcheck:
       enable: true
       workDir: "src/qiniu.com/kodo"
+  
+  qbox/net-common:	  
+    golangci-lint:
+      enable: true
+      args: []
+      configPath: "repo.golangci.yml"
+
+  qbox/net-tools:	  
+    golangci-lint:
+      enable: false
 `
 
 	if err := os.WriteFile(configPath, []byte(rawConfig), 0666); err != nil {
@@ -151,8 +208,9 @@ customConfig: # custom config for specific orgs or repos
 			repo:   "net-cache",
 			linter: "golangci-lint",
 			want: Linter{
-				Enable: boolPtr(true),
-				Args:   []string{"run", "-D", "staticcheck"},
+				Enable:     boolPtr(true),
+				Args:       []string{"run", "-D", "staticcheck"},
+				ConfigPath: "linters-config/.golangci.yml",
 			},
 		},
 		{
@@ -188,8 +246,9 @@ customConfig: # custom config for specific orgs or repos
 			repo:   "net-gslb",
 			linter: "golangci-lint",
 			want: Linter{
-				Enable: boolPtr(true),
-				Args:   []string{"run", "-D", "staticcheck"},
+				Enable:     boolPtr(true),
+				Args:       []string{"run", "-D", "staticcheck"},
+				ConfigPath: "linters-config/.golangci.yml",
 			},
 		},
 		{
@@ -198,8 +257,9 @@ customConfig: # custom config for specific orgs or repos
 			repo:   "net-gslb",
 			linter: "golangci-lint",
 			want: Linter{
-				Enable: boolPtr(true),
-				Args:   []string{},
+				Enable:     boolPtr(true),
+				Args:       []string{},
+				ConfigPath: "linters-config/.golangci.yml",
 			},
 		},
 		{
@@ -212,6 +272,26 @@ customConfig: # custom config for specific orgs or repos
 				WorkDir: "src/qiniu.com/kodo",
 			},
 		},
+		{
+			name:   "case9 - repo configuration will override the default global configurations.",
+			org:    "qbox",
+			repo:   "net-common",
+			linter: "golangci-lint",
+			want: Linter{
+				Enable:     boolPtr(true),
+				ConfigPath: "repo.golangci.yml",
+				Args:       []string{},
+			},
+		},
+		{
+			name:   "case10 - turn off golangci-lint for net-tools",
+			org:    "qbox",
+			repo:   "net-tools",
+			linter: "golangci-lint",
+			want: Linter{
+				Enable: boolPtr(false),
+			},
+		},
 	}
 
 	for _, tc := range tcs {
@@ -225,9 +305,13 @@ customConfig: # custom config for specific orgs or repos
 				t.Errorf("expected %v, got %v", tc.want.WorkDir, got.WorkDir)
 			}
 
-			if len(got.Args) != len(tc.want.Args) {
+			if tc.want.Args != nil && len(got.Args) != len(tc.want.Args) {
 				t.Errorf("expected %v, got %v", tc.want.Args, got.Args)
 			}
+
+			if !strings.HasSuffix(got.ConfigPath, tc.want.ConfigPath) {
+				t.Errorf("expected %v, got %v", tc.want.ConfigPath, got.ConfigPath)
+			}
 		})
 	}
 }
diff --git a/config/linters-config/.golangci.yml b/config/linters-config/.golangci.yml
@@ -0,0 +1,30 @@
+# 这是我们当前内部使用的配置文件，会基于实际的变化和我们的认知而迭代，仅供参考
+
+linters-settings:
+  paralleltest:
+    # Ignore missing calls to `t.Parallel()` and only report incorrect uses of it.
+    # Default: false
+    # see: https://github.com/qiniu/reviewbot/issues/149
+    ignore-missing: true
+    # Ignore missing calls to `t.Parallel()` in subtests. Top-level tests are
+    # still required to have `t.Parallel`, but subtests are allowed to skip it.
+    # Default: false
+    ignore-missing-subtests: true
+
+linters:
+  # Enable all available linters.
+  enable-all: true
+  # Disable specific linter
+  disable:
+    - nlreturn # see https://github.com/qiniu/reviewbot/issues/148
+    - deadcode # Deprecated
+    - exhaustivestruct # Deprecated
+    - golint # Deprecated
+    - ifshort # Deprecated
+    - interfacer # Deprecated
+    - maligned # Deprecated
+    - gomnd # Deprecated
+    - nosnakecase # Deprecated
+    - scopelint # Deprecated
+    - structcheck # Deprecated
+    - varcheck # Deprecated
diff --git a/internal/linters/go/golangci_lint/golangci_lint.go b/internal/linters/go/golangci_lint/golangci_lint.go
@@ -22,6 +22,10 @@ func golangciLintHandler(log *xlog.Logger, a linters.Agent) error {
 		a.LinterConfig.Args = append([]string{}, "run", "--timeout=5m0s", "--allow-parallel-runners=true")
 	}
 
+	if a.LinterConfig.ConfigPath != "" {
+		a.LinterConfig.Args = append(a.LinterConfig.Args, "--config", a.LinterConfig.ConfigPath)
+	}
+
 	return linters.GeneralHandler(log, a, golangciLintParse)
 }
 

diff --git a/internal/linters/linters.go b/internal/linters/linters.go
@@ -146,6 +146,7 @@ func ExecRun(workDir, command string, args ...string) ([]byte, error) {
 		return nil, err
 	}
 
+	log.Infof("exec command: %v %v", g, args)
 	c := exec.Command(g, args...)
 	c.Dir = workDir
 
@@ -247,7 +248,7 @@ func Parse(log *xlog.Logger, output []byte, lineParser LineParser) (map[string][
 		}
 		output, err := lineParser(line)
 		if err != nil {
-			log.Warnf("unexpected linter check output: %v", line)
+			log.Debugf("unexpected linter check output: %v", line)
 			continue
 		}