Allow custom linters to auto-fix

This allows custom linters hook into the `--fix` functionality.
Custom linters specify the fixes using the Go analysis structures,
which allow for arbitrary char offsets for fixes; they get converted
into golangci structures, which are line-based. If the conversion is
not possible, the fix is dropped on the floor.

Signed-off-by: Steve Coffman <steve@khanacademy.org>

pkg/golinters/goanalysis/linter.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"go/token"
 	"runtime"
 	"sort"
 	"strings"
@@ -219,23 +220,65 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st
 	var issues []result.Issue
 	for i := range diags {
 		diag := &diags[i]
-		linterName := linterNameBuilder(diag)
-		var text string
-		if diag.Analyzer.Name == linterName {
-			text = diag.Message
-		} else {
-			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,
-		})
+		issues = append(issues, buildSingleIssue(diag, linterNameBuilder(diag)))
 	}
 	return issues
 }
 
+func buildSingleIssue(diag *Diagnostic, linterName string) result.Issue {
+	text := generateIssueText(diag, linterName)
+	issue := result.Issue{
+		FromLinter: linterName,
+		Text:       text,
+		Pos:        diag.Position,
+		Pkg:        diag.Pkg,
+	}
+
+	if len(diag.SuggestedFixes) > 0 {
+		// Don't really have a better way of picking a best fix right now
+		chosenFix := diag.SuggestedFixes[0]
+
+		// It could be confusing to return more than one issue per single diagnostic,
+		// but if we return a subset it might be a partial application of a fix. Don't
+		// apply a fix unless there is only one for now
+		if len(chosenFix.TextEdits) == 1 {
+			edit := chosenFix.TextEdits[0]
+
+			pos := diag.Pkg.Fset.Position(edit.Pos)
+			end := diag.Pkg.Fset.Position(edit.End)
+
+			newLines := strings.Split(string(edit.NewText), "\n")
+
+			// This only works if we're only replacing whole lines with brand new lines
+			if onlyReplacesWholeLines(pos, end, newLines) {
+
+				// both original and new content ends with newline, omit to avoid partial line replacement
+				newLines = newLines[:len(newLines)-1]
+
+				issue.Replacement = &result.Replacement{NewLines: newLines}
+				issue.LineRange = &result.Range{From: pos.Line, To: end.Line - 1}
+
+				return issue
+			}
+		}
+	}
+
+	return issue
+}
+
+func onlyReplacesWholeLines(oPos token.Position, oEnd token.Position, newLines []string) bool {
+	return oPos.Column == 1 && oEnd.Column == 1 &&
+		oPos.Line < oEnd.Line && // must be replacing at least one line
+		newLines[len(newLines)-1] == "" // edit.NewText ended with '\n'
+}
+
+func generateIssueText(diag *Diagnostic, linterName string) string {
+	if diag.Analyzer.Name == linterName {
+		return diag.Message
+	}
+	return fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message)
+}
+
 func (lnt *Linter) preRun(lintCtx *linter.Context) error {
 	if err := analysis.Validate(lnt.analyzers); err != nil {
 		return errors.Wrap(err, "failed to validate analyzers")

pkg/golinters/goanalysis/linter_test.go

@@ -2,8 +2,13 @@ package goanalysis
 
 import (
 	"fmt"
+	"go/token"
+	"reflect"
 	"testing"
 
+	"github.com/golangci/golangci-lint/pkg/result"
+	"golang.org/x/tools/go/analysis"
+
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/tools/go/packages"
 )
@@ -46,3 +51,234 @@ func TestParseError(t *testing.T) {
 		assert.Equal(t, "msg", i.Text)
 	}
 }
+
+func Test_buildIssues(t *testing.T) {
+	type args struct {
+		diags             []Diagnostic
+		linterNameBuilder func(diag *Diagnostic) string
+	}
+	tests := []struct {
+		name string
+		args args
+		want []result.Issue
+	}{
+		{
+			name: "No Diagnostics",
+			args: args{
+				diags: []Diagnostic{},
+				linterNameBuilder: func(*Diagnostic) string {
+					return "some-linter"
+				},
+			},
+			want: []result.Issue(nil),
+		},
+		{
+			name: "Linter Name is Analyzer Name",
+			args: args{
+				diags: []Diagnostic{
+					{
+						Diagnostic: analysis.Diagnostic{
+							Message: "failure message",
+						},
+						Analyzer: &analysis.Analyzer{
+							Name: "some-linter",
+						},
+						Position: token.Position{},
+						Pkg:      nil,
+					},
+				},
+				linterNameBuilder: func(*Diagnostic) string {
+					return "some-linter"
+				},
+			},
+			want: []result.Issue{
+				{
+					FromLinter: "some-linter",
+					Text:       "failure message",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := buildIssues(tt.args.diags, tt.args.linterNameBuilder); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("buildIssues() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func Test_buildSingleIssue(t *testing.T) {
+	type args struct {
+		diag       *Diagnostic
+		linterName string
+	}
+	fakePkg := packages.Package{
+		Fset: makeFakeFileSet(),
+	}
+	tests := []struct {
+		name      string
+		args      args
+		wantIssue result.Issue
+	}{
+		{
+			name: "Linter Name is Analyzer Name",
+			args: args{
+				diag: &Diagnostic{
+					Diagnostic: analysis.Diagnostic{
+						Message: "failure message",
+					},
+					Analyzer: &analysis.Analyzer{
+						Name: "some-linter",
+					},
+					Position: token.Position{},
+					Pkg:      nil,
+				},
+
+				linterName: "some-linter",
+			},
+			wantIssue: result.Issue{
+				FromLinter: "some-linter",
+				Text:       "failure message",
+			},
+		},
+		{
+			name: "Linter Name is NOT Analyzer Name",
+			args: args{
+				diag: &Diagnostic{
+					Diagnostic: analysis.Diagnostic{
+						Message: "failure message",
+					},
+					Analyzer: &analysis.Analyzer{
+						Name: "some-analyzer",
+					},
+					Position: token.Position{},
+					Pkg:      nil,
+				},
+				linterName: "some-linter",
+			},
+			wantIssue: result.Issue{
+				FromLinter: "some-linter",
+				Text:       "some-analyzer: failure message",
+			},
+		},
+		{
+			name: "Shows issue when suggested edits exist but has no TextEdits",
+			args: args{
+				diag: &Diagnostic{
+					Diagnostic: analysis.Diagnostic{
+						Message: "failure message",
+						SuggestedFixes: []analysis.SuggestedFix{
+							{
+								Message:   "fix something",
+								TextEdits: []analysis.TextEdit{},
+							},
+						},
+					},
+					Analyzer: &analysis.Analyzer{
+						Name: "some-analyzer",
+					},
+					Position: token.Position{},
+					Pkg:      nil,
+				},
+				linterName: "some-linter",
+			},
+			wantIssue: result.Issue{
+				FromLinter: "some-linter",
+				Text:       "some-analyzer: failure message",
+			},
+		},
+		{
+			name: "Replace Whole Line",
+			args: args{
+				diag: &Diagnostic{
+					Diagnostic: analysis.Diagnostic{
+						Message: "failure message",
+						SuggestedFixes: []analysis.SuggestedFix{
+							{
+								Message: "fix something",
+								TextEdits: []analysis.TextEdit{
+									{
+										Pos:     101,
+										End:     201,
+										NewText: []byte("// Some comment to fix\n"),
+									},
+								},
+							},
+						},
+					},
+					Analyzer: &analysis.Analyzer{
+						Name: "some-analyzer",
+					},
+					Position: token.Position{},
+					Pkg:      &fakePkg,
+				},
+				linterName: "some-linter",
+			},
+			wantIssue: result.Issue{
+				FromLinter: "some-linter",
+				Text:       "some-analyzer: failure message",
+				LineRange: &result.Range{
+					From: 2,
+					To:   2,
+				},
+				Replacement: &result.Replacement{
+					NeedOnlyDelete: false,
+					NewLines: []string{
+						"// Some comment to fix",
+					},
+				},
+				Pkg: &fakePkg,
+			},
+		},
+		{
+			name: "Excludes Replacement if TextEdit doesn't modify only whole lines",
+			args: args{
+				diag: &Diagnostic{
+					Diagnostic: analysis.Diagnostic{
+						Message: "failure message",
+						SuggestedFixes: []analysis.SuggestedFix{
+							{
+								Message: "fix something",
+								TextEdits: []analysis.TextEdit{
+									{
+										Pos:     101,
+										End:     151,
+										NewText: []byte("// Some comment to fix\n"),
+									},
+								},
+							},
+						},
+					},
+					Analyzer: &analysis.Analyzer{
+						Name: "some-analyzer",
+					},
+					Position: token.Position{},
+					Pkg:      &fakePkg,
+				},
+				linterName: "some-linter",
+			},
+			wantIssue: result.Issue{
+				FromLinter: "some-linter",
+				Text:       "some-analyzer: failure message",
+				Pkg:        &fakePkg,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if gotIssues := buildSingleIssue(tt.args.diag, tt.args.linterName); !reflect.DeepEqual(gotIssues, tt.wantIssue) {
+				t.Errorf("buildSingleIssue() = %v, want %v", gotIssues, tt.wantIssue)
+			}
+		})
+	}
+}
+
+func makeFakeFileSet() *token.FileSet {
+	fSet := token.NewFileSet()
+	file := fSet.AddFile("fake.go", 1, 1000)
+	for i := 100; i < 1000; i += 100 {
+		file.AddLine(i)
+	}
+	return fSet
+}