Support --fix for gofumpt (#1239)

Author: Sergey Vilgelm

go.mod

@@ -39,6 +39,7 @@ require (
 	github.com/ryancurrah/gomodguard v1.1.0
 	github.com/ryanrolds/sqlclosecheck v0.3.0
 	github.com/securego/gosec/v2 v2.3.0
+	github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c
 	github.com/shirou/gopsutil v0.0.0-20190901111213-e4ec7b275ada // v2.19.8
 	github.com/sirupsen/logrus v1.6.0
 	github.com/sonatard/noctx v0.0.1

go.sum

@@ -307,6 +307,8 @@ github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb
 github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
 github.com/securego/gosec/v2 v2.3.0 h1:y/9mCF2WPDbSDpL3QDWZD3HHGrSYw0QSHnCqTfs4JPE=
 github.com/securego/gosec/v2 v2.3.0/go.mod h1:UzeVyUXbxukhLeHKV3VVqo7HdoQR9MrRfFmZYotn8ME=
+github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c h1:W65qqJCIOVP4jpqPQ0YvHYKwcMEMVWIzWC5iNQQfBTU=
+github.com/shazow/go-diff v0.0.0-20160112020656-b6b7b6733b8c/go.mod h1:/PevMnwAxekIXwN8qQyfc5gl2NlkB3CQlkizAbOkeBs=
 github.com/shirou/gopsutil v0.0.0-20190901111213-e4ec7b275ada h1:WokF3GuxBeL+n4Lk4Fa8v9mbdjlrl7bHuneF4N1bk2I=
 github.com/shirou/gopsutil v0.0.0-20190901111213-e4ec7b275ada/go.mod h1:WWnYX4lzhCH5h/3YBfyVA3VbLYjlMZZAQcW9ojMexNc=
 github.com/shirou/w32 v0.0.0-20160930032740-bb4de0191aa4 h1:udFKJ0aHUL60LboW/A+DfgoHVedieIzIXE8uylPue0U=

pkg/golinters/gofmt.go

@@ -46,7 +46,7 @@ func NewGofmt() *goanalysis.Linter {
 					continue
 				}
 
-				is, err := extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, false)
+				is, err := 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/gofmt_common.go

@@ -207,7 +207,29 @@ func (p *hunkChangesParser) parse(h *diffpkg.Hunk) []Change {
 	return p.ret
 }
 
-func extractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Context, isGoimports bool) ([]result.Issue, error) {
+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
+		}
+	}
+	return text
+}
+
+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")
@@ -225,35 +247,19 @@ func extractIssuesFromPatch(patch string, log logutils.Log, lintCtx *linter.Cont
 		}
 
 		for _, hunk := range d.Hunks {
-			var text string
-			if isGoimports {
-				text = "File is not `goimports`-ed"
-				if lintCtx.Settings().Goimports.LocalPrefixes != "" {
-					text += " with -local " + lintCtx.Settings().Goimports.LocalPrefixes
-				}
-			} else {
-				text = "File is not `gofmt`-ed"
-				if lintCtx.Settings().Gofmt.Simplify {
-					text += " with `-s`"
-				}
-			}
 			p := hunkChangesParser{
 				log: log,
 			}
 			changes := p.parse(hunk)
 			for _, change := range changes {
 				change := change // fix scope
-				linterName := gofmtName
-				if isGoimports {
-					linterName = goimportsName
-				}
 				i := result.Issue{
 					FromLinter: linterName,
 					Pos: token.Position{
 						Filename: d.NewName,
 						Line:     change.LineRange.From,
 					},
-					Text:        text,
+					Text:        getErrorTextForLinter(lintCtx, linterName),
 					Replacement: &change.Replacement,
 				}
 				if change.LineRange.From != change.LineRange.To {

pkg/golinters/gofumpt.go

@@ -3,24 +3,24 @@ package golinters
 import (
 	"bytes"
 	"fmt"
-	"go/token"
 	"io/ioutil"
-	"strings"
 	"sync"
 
+	"github.com/pkg/errors"
+	"github.com/shazow/go-diff/difflib"
 	"golang.org/x/tools/go/analysis"
 	"mvdan.cc/gofumpt/format"
 
 	"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
 	"github.com/golangci/golangci-lint/pkg/lint/linter"
-	"github.com/golangci/golangci-lint/pkg/result"
 )
 
 const gofumptName = "gofumpt"
 
 func NewGofumpt() *goanalysis.Linter {
 	var mu sync.Mutex
 	var resIssues []goanalysis.Issue
+	differ := difflib.New()
 
 	analyzer := &analysis.Analyzer{
 		Name: gofumptName,
@@ -53,14 +53,26 @@ func NewGofumpt() *goanalysis.Linter {
 					return nil, fmt.Errorf("error while running gofumpt: %w", err)
 				}
 				if !bytes.Equal(input, output) {
-					issues = append(issues, goanalysis.NewIssue(&result.Issue{
-						FromLinter: gofumptName,
-						Text:       "File is not `gofumpt`-ed",
-						Pos: token.Position{
-							Filename: f,
-							Line:     strings.Count(string(input), "\n"),
-						},
-					}, pass))
+					out := bytes.Buffer{}
+					_, err = out.WriteString(fmt.Sprintf("--- %[1]s\n+++ %[1]s\n", f))
+					if err != nil {
+						return nil, fmt.Errorf("error while running gofumpt: %w", err)
+					}
+
+					err = differ.Diff(&out, bytes.NewReader(input), bytes.NewReader(output))
+					if err != nil {
+						return nil, fmt.Errorf("error while running gofumpt: %w", err)
+					}
+
+					diff := out.String()
+					is, err := extractIssuesFromPatch(diff, lintCtx.Log, lintCtx, gofumptName)
+					if err != nil {
+						return nil, errors.Wrapf(err, "can't extract issues from gofumpt diff output %q", diff)
+					}
+
+					for i := range is {
+						issues = append(issues, goanalysis.NewIssue(&is[i], pass))
+					}
 				}
 			}
 

pkg/golinters/goimports.go

@@ -47,7 +47,7 @@ func NewGoimports() *goanalysis.Linter {
 					continue
 				}
 
-				is, err := extractIssuesFromPatch(string(diff), lintCtx.Log, lintCtx, true)
+				is, err := 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))
 				}

test/testdata/fix/in/gofumpt.go

@@ -0,0 +1,9 @@
+//args: -Egofumpt
+//config: linters-settings.gofumpt.extra-rules=true
+package testdata
+
+import "fmt"
+
+func GofmtNotExtra(bar string, baz string) {
+	fmt.Print(bar, baz)
+}

test/testdata/fix/out/gofumpt.go

@@ -0,0 +1,9 @@
+//args: -Egofumpt
+//config: linters-settings.gofumpt.extra-rules=true
+package testdata
+
+import "fmt"
+
+func GofmtNotExtra(bar, baz string) {
+	fmt.Print(bar, baz)
+}

test/testdata/gofumpt.go

@@ -4,8 +4,5 @@ package testdata
 import "fmt"
 
 func GofumptNewLine() {
-
-	fmt.Println("foo")
+	fmt.Println( "foo" ) // ERROR "File is not `gofumpt`-ed"
 }
-
-// ERROR "File is not `gofumpt`-ed"

test/testdata/gofumpt_with_extra.go

@@ -0,0 +1,9 @@
+//args: -Egofumpt
+//config: linters-settings.gofumpt.extra-rules=true
+package testdata
+
+import "fmt"
+
+func GofmtNotExtra(bar string, baz string) { // ERROR "File is not `gofumpt`-ed with `-extra`"
+	fmt.Print("foo")
+}