enhance github mix report type

config/config.yaml → config/config.example.yaml

@@ -1,11 +1,12 @@
+# This is an example of the config file for reviewbot.
 # Reviewbot follows a zero-configuration approach in its design, where the default behavior implemented in the code should be suitable for the majority of scenarios.
 # Configuration of the file is only necessary in special cases, such as:
 # 1. Using specific commands for a particular repository.
 # 2. Modifying the default global configuration.
 # 3. Conducting specific gray testing for a particular feature.
 
 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
+  githubReportType: "github_mix" # github_pr_review, github_check_run, github_mix
   golangcilintConfig: "config/linters-config/.golangci.yml" # golangci-lint config file to use
   copySSHKeyToContainer: "/root/.ssh/id_rsa"
 
@@ -138,7 +139,7 @@ customConfig: # custom config for specific orgs or repos
 
 issueReferences:
   golangci-lint:
-    - pattern: 'ST1003'
+    - pattern: "ST1003"
       url: "https://github.com/qiniu/reviewbot/issues/398"
     - pattern: '^do not define dynamic errors, use wrapped static errors instead:.*\(err113\)$'
       url: "https://github.com/qiniu/reviewbot/issues/418"

config/config.go

@@ -379,6 +379,8 @@ const (
 	GithubCheckRuns ReportType = "github_check_run"
 	GithubPRReview  ReportType = "github_pr_review"
 	// GithubMixType is the type of the report that mix the github_check_run and github_pr_review.
+	// which use the github_check_run to report all lint results as a check run summary,
+	// but use the github_pr_review to report top 10 lint results to pull request review comments at most.
 	GithubMixType ReportType = "github_mix"
 	// for debug and testing.
 	Quiet ReportType = "quiet"

config/linters-config/.golangci.goplus.yml

@@ -39,6 +39,9 @@ linters-settings:
         disabled: true
       - name: cyclomatic
         arguments: [50]
+      # revive 的 unhandled-error 误报太多, 本身已有 errcheck 检查
+      - name: unhandled-error
+        disabled: true
 
   errcheck:
     exclude-functions:

config/linters-config/.golangci.yml

@@ -37,6 +37,9 @@ linters-settings:
         disabled: true
       - name: cyclomatic
         arguments: [30]
+      # revive 的 unhandled-error 误报太多, 本身已有 errcheck 检查
+      - name: unhandled-error
+        disabled: true
 
   errcheck:
     exclude-functions:

internal/linters/agent.go

@@ -18,6 +18,7 @@
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -33,6 +34,11 @@
 // issueCache is the issue references cache.
 var issueCache = cache.NewIssueReferencesCache(time.Minute * 10)
 
+var (
+	ErrIssueNumberIsZero         = errors.New("issue number is 0, this should not happen except for test cases")
+	ErrFailedToFetchIssueContent = errors.New("failed to fetch issue content")
+)
+
 // Agent knows necessary information in order to run linters.
 type Agent struct {
 	// ID each linter execution has a unique id.
@@ -55,64 +61,99 @@
 	IssueReferences []config.CompiledIssueReference
 }
 
-// ApplyIssueReferences applies the issue references to the lint results.
-func (a *Agent) ApplyIssueReferences(ctx context.Context, lintResults map[string][]LinterOutput) {
-	log := lintersutil.FromContext(ctx)
-	var msgFormat string
-	format := a.LinterConfig.ReportFormat
+// getMsgFormat returns the message format based on report type.
+func getMsgFormat(format config.ReportType) string {
 	switch format {
-	case config.GithubCheckRuns:
-		msgFormat = "%s\nmore info: %s"
-	case config.GithubPRReview:
-		msgFormat = "[%s](%s)"
-	case config.GithubMixType:
-		msgFormat = "[%s](%s)"
-	case config.Quiet:
-		return
+	case config.GithubPRReview, config.GithubMixType:
+		return "[%s](%s)"
 	default:
-		msgFormat = "%s\nmore info: %s"
+		return "%s\nmore info: %s"
 	}
-	for _, ref := range a.IssueReferences {
-		for file, outputs := range lintResults {
-			for i, o := range outputs {
-				if !ref.Pattern.MatchString(o.Message) {
-					continue
-				}
-				lintResults[file][i].Message = fmt.Sprintf(msgFormat, o.Message, ref.URL)
+}
 
-				if format != config.GithubPRReview && format != config.GithubMixType {
-					continue
-				}
+// getIssueContent fetches issue content with cache support.
+func (a *Agent) getIssueContent(ctx context.Context, ref config.CompiledIssueReference) (string, error) {
+	log := lintersutil.FromContext(ctx)
 
-				// specific for github pr review format
-				if issueContent, ok := issueCache.Get(ref.URL); ok && !issueCache.IsExpired(ref.URL) {
-					lintResults[file][i].Message += fmt.Sprintf(ReferenceFooter, issueContent)
-					continue
-				}
+	// Try cache first
+	if content, ok := issueCache.Get(ref.URL); ok && !issueCache.IsExpired(ref.URL) {
+		return content, nil
+	}
 
-				issue, resp, err := github.NewClient(http.DefaultClient).Issues.Get(ctx, "qiniu", "reviewbot", ref.IssueNumber)
-				if err != nil {
-					log.Errorf("failed to fetch issue content: %s", err)
-					// just log and continue.
-					continue
-				}
+	if ref.IssueNumber == 0 {
+		return "", ErrIssueNumberIsZero
+	}
 
-				if resp.StatusCode != http.StatusOK {
-					log.Errorf("failed to fetch issue content, resp: %v", resp)
-					// just log and continue.
-					continue
-				}
+	// Fetch from GitHub
+	issue, resp, err := github.NewClient(http.DefaultClient).Issues.Get(ctx, "qiniu", "reviewbot", ref.IssueNumber)
+	if err != nil {
+		log.Errorf("failed to fetch issue content: %s", err)
+		return "", err
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		log.Errorf("failed to fetch issue content, resp: %v", resp)
+		return "", ErrFailedToFetchIssueContent
+	}
+
+	content := issue.GetBody()
+	issueCache.Set(ref.URL, content)
+	return content, nil
+}
+
+// processOutput processes a single lint output.
+func (a *Agent) processOutput(ctx context.Context, output LinterOutput, ref config.CompiledIssueReference, msgFormat string) (LinterOutput, bool) {
+	if !ref.Pattern.MatchString(output.Message) {
+		return output, false
+	}
+
+	newOutput := output
+	newOutput.TypedMessage = fmt.Sprintf(msgFormat, output.Message, ref.URL)
+
+	// Add issue content for PR review formats
+	if a.LinterConfig.ReportFormat == config.GithubPRReview || a.LinterConfig.ReportFormat == config.GithubMixType {
+		if content, err := a.getIssueContent(ctx, ref); err == nil {
+			newOutput.TypedMessage += fmt.Sprintf(ReferenceFooter, content)
+		}
+	}
+
+	return newOutput, true
+}
+
+// ApplyTypedMessageByIssueReferences applies the issue references to the lint results with the typed message.
+func (a *Agent) ApplyTypedMessageByIssueReferences(ctx context.Context, lintResults map[string][]LinterOutput) map[string][]LinterOutput {
+	msgFormat := getMsgFormat(a.LinterConfig.ReportFormat)
+	newLintResults := make(map[string][]LinterOutput, len(lintResults))
 
-				issueContent := issue.GetBody()
-				issueCache.Set(ref.URL, issueContent)
-				lintResults[file][i].Message += fmt.Sprintf(ReferenceFooter, issueContent)
+	// Process each file's outputs
+	for file, outputs := range lintResults {
+		newOutputs := make([]LinterOutput, 0, len(outputs))
+
+		// Apply each reference pattern
+		for _, output := range outputs {
+			processed := false
+			for _, ref := range a.IssueReferences {
+				if newOutput, ok := a.processOutput(ctx, output, ref, msgFormat); ok {
+					newOutputs = append(newOutputs, newOutput)
+					processed = true
+					break
+				}
 			}
+			if !processed {
+				newOutputs = append(newOutputs, output)
+			}
+		}
+
+		if len(newOutputs) > 0 {
+			newLintResults[file] = newOutputs
 		}
 	}
+
+	return newLintResults
 }
 
 const ReferenceFooter = `
 <details>
-<summary>详细解释</summary>
+<summary>Details</summary>
 %s
 </details>`