Skip to content

Commit

Permalink
Allow custom linters to auto-fix
Browse files Browse the repository at this point in the history
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 <[email protected]>

Test plan:
go test ./...
  • Loading branch information
StevenACoffman authored and csilvers committed Aug 29, 2023
1 parent 547bca2 commit 5cb8e96
Show file tree
Hide file tree
Showing 2 changed files with 298 additions and 13 deletions.
68 changes: 55 additions & 13 deletions pkg/golinters/goanalysis/runners.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package goanalysis

import (
"fmt"
"go/token"
"runtime"
"sort"
"strings"
Expand Down Expand Up @@ -91,19 +92,7 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st
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, linterName))

if len(diag.Related) > 0 {
for _, info := range diag.Related {
Expand All @@ -119,6 +108,59 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st
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 getIssuesCacheKey(analyzers []*analysis.Analyzer) string {
return "lint/result:" + analyzersHashID(analyzers)
}
Expand Down
243 changes: 243 additions & 0 deletions pkg/golinters/goanalysis/runners_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,243 @@
package goanalysis

import (
"go/token"
"reflect"
"testing"

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"

"github.com/golangci/golangci-lint/pkg/result"
)

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
}

0 comments on commit 5cb8e96

Please sign in to comment.