Support SuggestedFixes

Conver the SuggesterFixes to Replacement by:
1. applying the fixes to a file in memory
2. creating a diff by comparing with original file
3. using `ExtractIssuesFromPatch` function to create the issues

Author: SVilgelm

go.mod

@@ -64,6 +64,7 @@ require (
 	github.com/nishanths/exhaustive v0.7.11
 	github.com/nishanths/predeclared v0.2.1
 	github.com/pkg/errors v0.9.1
+	github.com/pmezard/go-difflib v1.0.0
 	github.com/polyfloyd/go-errorlint v0.0.0-20211125173453-6d6d39c5bb8b
 	github.com/prometheus/procfs v0.6.0 // indirect
 	github.com/quasilyte/go-ruleguard/dsl v0.3.17

pkg/golinters/gofmt_common.go renamed to pkg/golinters/goanalysis/extract_issues.go

@@ -1,10 +1,11 @@
-package golinters
+package goanalysis
 
 import (
 	"bytes"
 	"fmt"
 	"go/token"
 	"strings"
+	"sync"
 
 	"github.com/pkg/errors"
 	diffpkg "github.com/sourcegraph/go-diff/diff"
@@ -207,29 +208,30 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change {
 	return p.ret
 }
 
+type TextModifier = func(lintCtx *linter.Context, text string) string
+
+var (
+	textModifier     = make(map[string]TextModifier)
+	textModifierLock = sync.RWMutex{}
+)
+
+func SetTextModifier(linterName string, modifier TextModifier) {
+	textModifierLock.Lock()
+	defer textModifierLock.Unlock()
+	textModifier[linterName] = modifier
+}
+
 func getErrorTextForLinter(lintCtx *linter.Context, linterName string) string {
 	text := "File is not formatted"
-	switch linterName {
-	case gofumptName:
-		text = "File is not `gofumpt`-ed"
-		if lintCtx.Settings().Gofumpt.ExtraRules {
-			text += " with `-extra`"
-		}
-	case gofmtName:
-		text = "File is not `gofmt`-ed"
-		if lintCtx.Settings().Gofmt.Simplify {
-			text += " with `-s`"
-		}
-	case goimportsName:
-		text = "File is not `goimports`-ed"
-		if lintCtx.Settings().Goimports.LocalPrefixes != "" {
-			text += " with -local " + lintCtx.Settings().Goimports.LocalPrefixes
-		}
+	textModifierLock.RLock()
+	defer textModifierLock.RUnlock()
+	if f, ok := textModifier[linterName]; ok {
+		return f(lintCtx, text)
 	}
 	return text
 }
 
-func extractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Context, linterName string) ([]result.Issue, error) {
+func ExtractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Context, linterName string) ([]result.Issue, error) {
 	diffs, err := diffpkg.ParseMultiFileDiff([]byte(patch))
 	if err != nil {
 		return nil, errors.Wrap(err, "can't parse patch")

pkg/golinters/gofmt_test.go renamed to pkg/golinters/goanalysis/extract_issues_test.go

@@ -1,4 +1,4 @@
-package golinters
+package goanalysis
 
 import (
 	"testing"

pkg/golinters/goanalysis/runners.go

@@ -59,7 +59,7 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss
 		}
 	}()
 
-	buildAllIssues := func() []result.Issue {
+	buildAllIssues := func() ([]result.Issue, error) {
 		var retIssues []result.Issue
 		reportedIssues := cfg.reportIssues(lintCtx)
 		for i := range reportedIssues {
@@ -69,8 +69,13 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss
 			}
 			retIssues = append(retIssues, *issue)
 		}
-		retIssues = append(retIssues, buildIssues(diags, cfg.getLinterNameForDiagnostic)...)
-		return retIssues
+		newIssues, err := buildIssues(lintCtx, diags, cfg.getLinterNameForDiagnostic)
+		if err != nil {
+			return nil, err
+		}
+
+		retIssues = append(retIssues, newIssues...)
+		return retIssues, nil
 	}
 
 	errIssues, err := buildIssuesFromIllTypedError(errs, lintCtx)
@@ -79,12 +84,16 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss
 	}
 
 	issues = append(issues, errIssues...)
-	issues = append(issues, buildAllIssues()...)
+	newIssues, err := buildAllIssues()
+	if err != nil {
+		return nil, err
+	}
+	issues = append(issues, newIssues...)
 
 	return issues, nil
 }
 
-func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) string) []result.Issue {
+func buildIssues(lintCtx *linter.Context, diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) string) ([]result.Issue, error) {
 	var issues []result.Issue
 	for i := range diags {
 		diag := &diags[i]
@@ -97,25 +106,31 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st
 			text = fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message)
 		}
 
-		issues = append(issues, result.Issue{
-			FromLinter: linterName,
-			Text:       text,
-			Pos:        diag.Position,
-			Pkg:        diag.Pkg,
-		})
-
-		if len(diag.Related) > 0 {
-			for _, info := range diag.Related {
-				issues = append(issues, result.Issue{
-					FromLinter: linterName,
-					Text:       fmt.Sprintf("%s(related information): %s", diag.Analyzer.Name, info.Message),
-					Pos:        diag.Pkg.Fset.Position(info.Pos),
-					Pkg:        diag.Pkg,
-				})
+		if lintCtx.Cfg.Issues.NeedFix && len(diag.SuggestedFixes) > 0 {
+			newIssues, err := convertSuggestedFixes(lintCtx, linterName, diag)
+			if err != nil {
+				return nil, err
 			}
+			issues = append(issues, newIssues...)
+		} else {
+			issues = append(issues, result.Issue{
+				FromLinter: linterName,
+				Text:       text,
+				Pos:        diag.Position,
+				Pkg:        diag.Pkg,
+			})
+		}
+
+		for _, info := range diag.Related {
+			issues = append(issues, result.Issue{
+				FromLinter: linterName,
+				Text:       fmt.Sprintf("%s(related information): %s", diag.Analyzer.Name, info.Message),
+				Pos:        diag.Pkg.Fset.Position(info.Pos),
+				Pkg:        diag.Pkg,
+			})
 		}
 	}
-	return issues
+	return issues, nil
 }
 
 func getIssuesCacheKey(analyzers []*analysis.Analyzer) string {

pkg/golinters/goanalysis/suggested_fixes.go

@@ -0,0 +1,104 @@
+package goanalysis
+
+import (
+	"bytes"
+	"fmt"
+	"go/token"
+	"io"
+	"os"
+
+	"github.com/pkg/errors"
+	"github.com/pmezard/go-difflib/difflib"
+
+	"github.com/golangci/golangci-lint/pkg/lint/linter"
+	"github.com/golangci/golangci-lint/pkg/result"
+)
+
+func convertSuggestedFixes(lintCtx *linter.Context, linterName string, diag *Diagnostic) ([]result.Issue, error) {
+	var issues []result.Issue
+
+	for _, fix := range diag.SuggestedFixes {
+		origContent := make(map[string][]byte)
+		readers := make(map[string]io.ReadSeeker)
+		bufs := make(map[string]*bytes.Buffer)
+
+		for _, edit := range fix.TextEdits {
+			if edit.End == token.NoPos {
+				edit.End = edit.Pos
+			}
+			start, end := diag.Pkg.Fset.Position(edit.Pos), diag.Pkg.Fset.Position(edit.End)
+			orig, ok := readers[start.Filename]
+			if !ok {
+				data, err := os.ReadFile(start.Filename)
+				if err != nil {
+					return nil, errors.Wrapf(err, "can't read file: %s", start.Filename)
+				}
+				origContent[start.Filename] = data
+				orig = bytes.NewReader(data)
+				readers[start.Filename] = orig
+				bufs[start.Filename] = &bytes.Buffer{}
+			}
+
+			buf := bufs[start.Filename]
+			cur, err := orig.Seek(0, io.SeekCurrent)
+			if err != nil {
+				return nil, errors.Wrapf(err, "can't get current position of reader of file %q", start.Filename)
+			}
+			if l := start.Offset - int(cur); l > 0 {
+				b := make([]byte, l)
+				if _, err := orig.Read(b); err != nil {
+					return nil, errors.Wrapf(err, "can't read form stored reader of file %q", start.Filename)
+				}
+				buf.Write(b)
+			}
+			buf.Write(edit.NewText)
+			if _, err := orig.Seek(int64(end.Offset), io.SeekStart); err != nil {
+				return nil, errors.Wrapf(err, "can't change position of reader of file %q", start.Filename)
+			}
+		}
+		for filename, f := range readers {
+			data, err := io.ReadAll(f)
+			if err != nil {
+				return nil, err
+			}
+			bufs[filename].Write(data)
+		}
+
+		for filename, data := range origContent {
+			newIssues, err := createPatchAndExtractIssues(lintCtx, linterName, filename, data, bufs[filename].Bytes())
+			if err != nil {
+				return nil, err
+			}
+			for i := range newIssues {
+				newIssues[i].Text = fmt.Sprintf("%s: %s", diag.Message, fix.Message)
+			}
+			issues = append(issues, newIssues...)
+		}
+	}
+
+	return issues, nil
+}
+
+func createPatchAndExtractIssues(lintCtx *linter.Context, linterName, filename string, src, dst []byte) ([]result.Issue, error) {
+	out := bytes.Buffer{}
+	if _, err := out.WriteString(fmt.Sprintf("--- %[1]s\n+++ %[1]s\n", filename)); err != nil {
+		return nil, errors.Wrap(err, "can't write diff header")
+	}
+
+	d := difflib.UnifiedDiff{
+		A:       difflib.SplitLines(string(src)),
+		B:       difflib.SplitLines(string(dst)),
+		Context: 3,
+	}
+
+	if err := difflib.WriteUnifiedDiff(&out, d); err != nil {
+		return nil, errors.Wrap(err, "can't create diff")
+	}
+
+	newIssues, err := ExtractIssuesFromPatch(out.String(), lintCtx.Log, lintCtx, linterName)
+	if err != nil {
+		return nil, errors.Wrap(err, "can't extract issues from diff")
+	}
+
+	return newIssues, nil
+}

pkg/golinters/gofmt.go

@@ -14,6 +14,14 @@ import (
 const gofmtName = "gofmt"
 
 func NewGofmt() *goanalysis.Linter {
+	goanalysis.SetTextModifier(gofmtName, func(lintCtx *linter.Context, _ string) string {
+		text := "File is not `gofmt`-ed"
+		if lintCtx.Settings().Gofmt.Simplify {
+			text += " with `-s`"
+		}
+		return text
+	})
+
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue
 
@@ -46,7 +54,7 @@ func NewGofmt() *goanalysis.Linter {
 					continue
 				}
 
-				is, err := extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, gofmtName)
+				is, err := goanalysis.ExtractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, gofmtName)
 				if err != nil {
 					return nil, errors.Wrapf(err, "can't extract issues from gofmt diff output %q", string(diff))
 				}

pkg/golinters/gofumpt.go

@@ -19,6 +19,14 @@ import (
 const gofumptName = "gofumpt"
 
 func NewGofumpt() *goanalysis.Linter {
+	goanalysis.SetTextModifier(gofumptName, func(lintCtx *linter.Context, _ string) string {
+		text := "File is not `gofumpt`-ed"
+		if lintCtx.Settings().Gofumpt.ExtraRules {
+			text += " with `-extra`"
+		}
+		return text
+	})
+
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue
 	differ := difflib.New()
@@ -73,7 +81,7 @@ func NewGofumpt() *goanalysis.Linter {
 					}
 
 					diff := out.String()
-					is, err := extractIssuesFromPatch(diff, lintCtx.Log, lintCtx, gofumptName)
+					is, err := goanalysis.ExtractIssuesFromPatch(diff, lintCtx.Log, lintCtx, gofumptName)
 					if err != nil {
 						return nil, errors.Wrapf(err, "can't extract issues from gofumpt diff output %q", diff)
 					}

pkg/golinters/goimports.go

@@ -15,6 +15,14 @@ import (
 const goimportsName = "goimports"
 
 func NewGoimports() *goanalysis.Linter {
+	goanalysis.SetTextModifier(goimportsName, func(lintCtx *linter.Context, _ string) string {
+		text := "File is not `goimports`-ed"
+		if lintCtx.Settings().Goimports.LocalPrefixes != "" {
+			text += " with -local " + lintCtx.Settings().Goimports.LocalPrefixes
+		}
+		return text
+	})
+
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue
 
@@ -47,7 +55,7 @@ func NewGoimports() *goanalysis.Linter {
 					continue
 				}
 
-				is, err := extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, goimportsName)
+				is, err := goanalysis.ExtractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, goimportsName)
 				if err != nil {
 					return nil, errors.Wrapf(err, "can't extract issues from gofmt diff output %q", string(diff))
 				}

pkg/result/issue.go

@@ -14,7 +14,7 @@ type Range struct {
 
 type Replacement struct {
 	NeedOnlyDelete bool     // need to delete all lines of the issue without replacement with new lines
-	NewLines       []string // if NeedDelete is false it's the replacement lines
+	NewLines       []string // if NeedOnlyDelete is false it's the replacement lines
 	Inline         *InlineFix
 }