refactor hunk logic

internal/linters/hunk.go

@@ -20,86 +20,26 @@ import (
 	"fmt"
 	"regexp"
 	"strconv"
-
-	"github.com/google/go-github/v57/github"
-	"github.com/qiniu/x/errors"
-	"github.com/qiniu/x/log"
-	"github.com/xanzy/go-gitlab"
 )
 
 type HunkChecker interface {
 	InHunk(file string, line int) bool
 }
 
 type FileHunkChecker struct {
-	// map[file][]hunks
 	Hunks map[string][]Hunk
 }
 
-var (
-	errCommitFile = errors.New("commit file error")
-)
-
-func NewFileHunkChecker(commitFiles []*github.CommitFile) (*FileHunkChecker, error) {
-	hunks := make(map[string][]Hunk)
-	for _, commitFile := range commitFiles {
-		if commitFile == nil || commitFile.GetPatch() == "" {
-			continue
-		}
-
-		if commitFile.GetStatus() == "removed" {
-			continue
-		}
-
-		fileHunks, err := DiffHunks(commitFile)
-		if err != nil {
-			return nil, err
-		}
-
-		v, ok := hunks[commitFile.GetFilename()]
-		if ok {
-			log.Warnf("duplicate commitFiles: %v, %v", commitFile, v)
-			continue
-		}
-
-		hunks[commitFile.GetFilename()] = fileHunks
-	}
-
-	return &FileHunkChecker{
-		Hunks: hunks,
-	}, nil
+type Hunk struct {
+	StartLine int
+	EndLine   int
 }
-func NewGitLabCommitFileHunkChecker(commitFiles []*gitlab.MergeRequestDiff) (*FileHunkChecker, error) {
-	hunks := make(map[string][]Hunk)
-	for _, commitFile := range commitFiles {
-		if commitFile == nil || commitFile.NewPath == "" {
-			continue
-		}
-		if commitFile.DeletedFile {
-			continue
-		}
-		fileHunks, err := DiffHunksMerge(commitFile)
-		if err != nil {
-			return nil, err
-		}
-		v, ok := hunks[commitFile.NewPath]
-		if ok {
-			log.Warnf("duplicate commitFiles: %v, %v", commitFile, v)
-			continue
-		}
-		hunks[commitFile.NewPath] = fileHunks
-	}
 
+// NewFileHunkChecker creates a new FileHunkChecker with given hunks map.
+func NewFileHunkChecker(hunks map[string][]Hunk) *FileHunkChecker {
 	return &FileHunkChecker{
 		Hunks: hunks,
-	}, nil
-}
-func DiffHunksMerge(commitFile *gitlab.MergeRequestDiff) ([]Hunk, error) {
-	if commitFile == nil || commitFile.NewPath == "" {
-		log.Errorf("invalid commitFile: %v", commitFile)
-		return nil, errCommitFile
 	}
-	return ParsePatch(commitFile.Diff)
 }
 
 func (c *FileHunkChecker) InHunk(file string, line, startLine int) bool {
@@ -114,14 +54,12 @@ func (c *FileHunkChecker) InHunk(file string, line, startLine int) bool {
 			}
 		}
 	}
-
 	return false
 }
 
-var patchRegex = regexp.MustCompile(`@@ \-(\d+),(\d+) \+(\d+),(\d+) @@`)
-
+// ParsePatch parses a unified diff patch string and returns hunks.
 func ParsePatch(patch string) ([]Hunk, error) {
-	var hunks []Hunk
+	hunks := make([]Hunk, 0)
 
 	groups := patchRegex.FindAllStringSubmatch(patch, -1)
 	for _, group := range groups {
@@ -147,15 +85,4 @@ func ParsePatch(patch string) ([]Hunk, error) {
 	return hunks, nil
 }
 
-type Hunk struct {
-	StartLine int
-	EndLine   int
-}
-
-func DiffHunks(commitFile *github.CommitFile) ([]Hunk, error) {
-	if commitFile == nil || commitFile.GetPatch() == "" {
-		return nil, fmt.Errorf("invalid commitFile: %v", commitFile)
-	}
-
-	return ParsePatch(commitFile.GetPatch())
-}
+var patchRegex = regexp.MustCompile(`@@ \-(\d+),(\d+) \+(\d+),(\d+) @@`)

internal/linters/providergithub.go

@@ -238,7 +238,7 @@ type GithubProvider struct {
 }
 
 func NewGithubProvider(githubClient *github.Client, gitClient gitv2.ClientFactory, pullRequestChangedFiles []*github.CommitFile, pullRequestEvent github.PullRequestEvent) (*GithubProvider, error) {
-	checker, err := NewFileHunkChecker(pullRequestChangedFiles)
+	checker, err := newGithubHunkChecker(pullRequestChangedFiles)
 	if err != nil {
 		return nil, err
 	}
@@ -717,3 +717,34 @@ func newMixCheckRun(a Agent, lintErrs map[string][]LinterOutput) github.CreateCh
 	check.Output.Text = github.String(b.String())
 	return check
 }
+
+func newGithubHunkChecker(commitFiles []*github.CommitFile) (*FileHunkChecker, error) {
+	hunks := make(map[string][]Hunk)
+	for _, commitFile := range commitFiles {
+		if !isValidGithubCommitFile(commitFile) {
+			continue
+		}
+
+		fileHunks, err := parseGithubPatch(commitFile.GetPatch())
+		if err != nil {
+			return nil, err
+		}
+
+		if existing, ok := hunks[commitFile.GetFilename()]; ok {
+			log.Warnf("duplicate commitFiles: %v, %v", commitFile, existing)
+			continue
+		}
+
+		hunks[commitFile.GetFilename()] = fileHunks
+	}
+
+	return NewFileHunkChecker(hunks), nil
+}
+
+func isValidGithubCommitFile(file *github.CommitFile) bool {
+	return file != nil && file.GetPatch() != "" && file.GetStatus() != "removed"
+}
+
+func parseGithubPatch(patch string) ([]Hunk, error) {
+	return ParsePatch(patch)
+}

internal/linters/providergitlab.go

@@ -48,7 +48,8 @@ type DiffSha struct {
 }
 
 func ListMergeRequestsFiles(ctx context.Context, gc *gitlab.Client, owner string, repo string, pid int, number int) ([]*gitlab.MergeRequestDiff, *gitlab.Response, error) {
-	// For version compatibility,because verion below 10.8 not support ListMergeRequestDiffs
+	// For version compatibility,because version below 10.8 not support ListMergeRequestDiffs
+	//nolint:staticcheck
 	files, response, err := gc.MergeRequests.GetMergeRequestChanges(pid, number, nil)
 	if err != nil {
 		return nil, nil, err
@@ -177,7 +178,7 @@ func (g *GitlabProvider) GetCodeReviewInfo() CodeReview {
 }
 
 func NewGitlabProvider(gitlabClient *gitlab.Client, gitClient gitv2.ClientFactory, mergeRequestChangedFiles []*gitlab.MergeRequestDiff, mergeRequestEvent gitlab.MergeEvent) (*GitlabProvider, error) {
-	checker, err := NewGitLabCommitFileHunkChecker(mergeRequestChangedFiles)
+	checker, err := newGitlabHunkChecker(mergeRequestChangedFiles)
 	if err != nil {
 		log.Fatalf("Failed to create client: %v", err)
 	}
@@ -580,3 +581,34 @@ func constructMergeRequestDiscussion(linterOutputs map[string][]LinterOutput, li
 	}
 	return comments
 }
+
+func newGitlabHunkChecker(commitFiles []*gitlab.MergeRequestDiff) (*FileHunkChecker, error) {
+	hunks := make(map[string][]Hunk)
+	for _, commitFile := range commitFiles {
+		if !isValidGitlabCommitFile(commitFile) {
+			continue
+		}
+
+		fileHunks, err := parseGitlabPatch(commitFile.Diff)
+		if err != nil {
+			return nil, err
+		}
+
+		if existing, ok := hunks[commitFile.NewPath]; ok {
+			log.Warnf("duplicate commitFiles: %v, %v", commitFile, existing)
+			continue
+		}
+
+		hunks[commitFile.NewPath] = fileHunks
+	}
+
+	return NewFileHunkChecker(hunks), nil
+}
+
+func isValidGitlabCommitFile(file *gitlab.MergeRequestDiff) bool {
+	return file != nil && file.NewPath != "" && !file.DeletedFile
+}
+
+func parseGitlabPatch(patch string) ([]Hunk, error) {
+	return ParsePatch(patch)
+}