Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for SuggestedFixes #5232

Merged
merged 57 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
6a296b0
chore: migrate dogsled
ldez Dec 11, 2024
b73ec12
chore: migrate gochecknoinits
ldez Dec 11, 2024
08d2476
chore: migrate gci
ldez Dec 13, 2024
d351aa8
chore: migrate gocritic
ldez Dec 13, 2024
36b2549
chore: migrate godot
ldez Dec 13, 2024
12d2c13
chore: migrate goheader
ldez Dec 13, 2024
30451ed
chore: migrate mirror
ldez Dec 13, 2024
6a40069
chore: migrate protogetter
ldez Dec 13, 2024
ebcff94
chore: migrate tagalign
ldez Dec 13, 2024
89dc119
chore: migrate whitespace
ldez Dec 13, 2024
0acc998
feat: add GetFilePositionFor
ldez Dec 13, 2024
19a66e6
chore: rewrite diff hunkChangesParser
ldez Dec 13, 2024
e7c65cb
chore: refactor nolintlint
ldez Dec 12, 2024
bf20023
chore: copy x/tools/internal/diff
ldez Dec 16, 2024
e5eefef
feat: new fixer
ldez Dec 16, 2024
632b805
chore: migrate nolintlint
ldez Dec 16, 2024
87f42b5
chore: migrate gofmt
ldez Dec 16, 2024
acfdecc
chore: migrate gofumpt
ldez Dec 16, 2024
473ab4e
chore: migrate goimports
ldez Dec 16, 2024
a85e53b
chore: migrate misspell
ldez Dec 16, 2024
75e4944
feat: support autofix for revive
ldez Dec 16, 2024
7b016e6
chore: tag all linters with autofix
ldez Dec 16, 2024
8444ec1
tests: autofix canonicalheader
ldez Dec 16, 2024
410281c
tests: autofix dupword
ldez Dec 16, 2024
e5c03da
tests: autofix fatcontext
ldez Dec 16, 2024
993ac5e
tests: autofix err113
ldez Dec 16, 2024
ddced09
tests: autofix errorlint
ldez Dec 16, 2024
4813ac7
tests: autofix iface
ldez Dec 16, 2024
ca2e133
tests: autofix importas
ldez Dec 16, 2024
54472f6
tests: autofix intrange
ldez Dec 16, 2024
46c6a38
tests: autofix nakedret
ldez Dec 16, 2024
ba516f4
tests: autofix nlreturn
ldez Dec 16, 2024
159bf02
tests: autofix perfsprint
ldez Dec 16, 2024
a642640
tests: autofix testifylint
ldez Dec 16, 2024
26c61a8
tests: autofix wsl
ldez Dec 16, 2024
65fd897
tests: autofix govet
ldez Dec 16, 2024
fe005f2
tests: autofix gosimple
ldez Dec 16, 2024
0cea44b
tests: autofix stylecheck
ldez Dec 16, 2024
6f36d37
tests: autofix staticcheck
ldez Dec 16, 2024
0dbb927
tests: autofix whitespace
ldez Dec 16, 2024
c4ba66d
tests: multiple fixes
ldez Dec 16, 2024
3c47cda
chore: migrate LLL
ldez Dec 16, 2024
66c31e6
chore: simplify godot
ldez Dec 16, 2024
b8d5d41
chore: migrate forbidigo
ldez Dec 16, 2024
30af54f
chore: migrate godox
ldez Dec 16, 2024
7d46339
chore: migrate makezero
ldez Dec 16, 2024
600e10e
chore: migrate nestif
ldez Dec 16, 2024
c7e3c0f
chore: migrate prealloc
ldez Dec 16, 2024
e1fbf35
chore: migrate unparam
ldez Dec 16, 2024
fe88914
tests: add cgo tests
ldez Dec 16, 2024
225f0f9
tests: ignore cgo tests for some linters
ldez Dec 16, 2024
94073d2
fix: dupl on Windows
ldez Dec 16, 2024
3763e1a
review
ldez Dec 16, 2024
f4b2e7a
tests: add missing whitespace tests
ldez Dec 16, 2024
898a5da
chore: add comments about TextEdits positions adjustements
ldez Dec 17, 2024
63b9001
feat: improve error handling
ldez Dec 17, 2024
936e333
fix: skip suggested changes for cgo
ldez Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/goanalysis/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type EncodingIssue struct {
Severity string
Pos token.Position
LineRange *result.Range
Replacement *result.Replacement
SuggestedFixes []analysis.SuggestedFix
ExpectNoLint bool
ExpectedNoLintLinter string
}
10 changes: 7 additions & 3 deletions pkg/goanalysis/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type Diagnostic struct {
Analyzer *analysis.Analyzer
Position token.Position
Pkg *packages.Package
File *token.File
}

type runner struct {
Expand Down Expand Up @@ -312,17 +313,20 @@ func extractDiagnostics(roots []*action) (retDiags []Diagnostic, retErrors []err
// We don't display a.Name/f.Category
// as most users don't care.

posn := act.Package.Fset.Position(diag.Pos)
k := key{posn, act.Analyzer, diag.Message}
position := GetFilePositionFor(act.Package.Fset, diag.Pos)
file := act.Package.Fset.File(diag.Pos)

k := key{Position: position, Analyzer: act.Analyzer, message: diag.Message}
if seen[k] {
continue // duplicate
}
seen[k] = true

retDiag := Diagnostic{
File: file,
Diagnostic: diag,
Analyzer: act.Analyzer,
Position: posn,
Position: position,
Pkg: act.Package,
}
retDiags = append(retDiags, retDiag)
Expand Down
33 changes: 29 additions & 4 deletions pkg/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"

"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/packages"
Expand Down Expand Up @@ -81,6 +82,7 @@ func runAnalyzers(cfg runAnalyzersConfig, lintCtx *linter.Context) ([]result.Iss

func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) string) []result.Issue {
var issues []result.Issue

for i := range diags {
diag := &diags[i]
linterName := linterNameBuilder(diag)
Expand All @@ -92,11 +94,34 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st
text = fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message)
}

var suggestedFixes []analysis.SuggestedFix

for _, sf := range diag.SuggestedFixes {
nsf := analysis.SuggestedFix{Message: sf.Message}

for _, edit := range sf.TextEdits {
end := edit.End

if !end.IsValid() {
end = edit.Pos
}

nsf.TextEdits = append(nsf.TextEdits, analysis.TextEdit{
Pos: token.Pos(diag.File.Offset(edit.Pos)),
End: token.Pos(diag.File.Offset(end)),
NewText: edit.NewText,
})
ldez marked this conversation as resolved.
Show resolved Hide resolved
}

suggestedFixes = append(suggestedFixes, nsf)
}

issues = append(issues, result.Issue{
FromLinter: linterName,
Text: text,
Pos: diag.Position,
Pkg: diag.Pkg,
FromLinter: linterName,
Text: text,
Pos: diag.Position,
Pkg: diag.Pkg,
SuggestedFixes: suggestedFixes,
})

if len(diag.Related) > 0 {
Expand Down
4 changes: 2 additions & 2 deletions pkg/goanalysis/runners_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func saveIssuesToCache(allPkgs []*packages.Package, pkgsFromCache map[*packages.
Severity: i.Severity,
Pos: i.Pos,
LineRange: i.LineRange,
Replacement: i.Replacement,
SuggestedFixes: i.SuggestedFixes,
ExpectNoLint: i.ExpectNoLint,
ExpectedNoLintLinter: i.ExpectedNoLintLinter,
})
Expand Down Expand Up @@ -123,7 +123,7 @@ func loadIssuesFromCache(pkgs []*packages.Package, lintCtx *linter.Context,
Severity: issue.Severity,
Pos: issue.Pos,
LineRange: issue.LineRange,
Replacement: issue.Replacement,
SuggestedFixes: issue.SuggestedFixes,
Pkg: pkg,
ExpectNoLint: issue.ExpectNoLint,
ExpectedNoLintLinter: issue.ExpectedNoLintLinter,
Expand Down
2 changes: 1 addition & 1 deletion pkg/printers/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestJSON_Print(t *testing.T) {
err := printer.Print(issues)
require.NoError(t, err)

expected := `{"Issues":[{"FromLinter":"linter-a","Text":"some issue","Severity":"warning","SourceLines":null,"Replacement":null,"Pos":{"Filename":"path/to/filea.go","Offset":2,"Line":10,"Column":4},"ExpectNoLint":false,"ExpectedNoLintLinter":""},{"FromLinter":"linter-b","Text":"another issue","Severity":"error","SourceLines":["func foo() {","\tfmt.Println(\"bar\")","}"],"Replacement":null,"Pos":{"Filename":"path/to/fileb.go","Offset":5,"Line":300,"Column":9},"ExpectNoLint":false,"ExpectedNoLintLinter":""}],"Report":null}
expected := `{"Issues":[{"FromLinter":"linter-a","Text":"some issue","Severity":"warning","SourceLines":null,"Pos":{"Filename":"path/to/filea.go","Offset":2,"Line":10,"Column":4},"ExpectNoLint":false,"ExpectedNoLintLinter":""},{"FromLinter":"linter-b","Text":"another issue","Severity":"error","SourceLines":["func foo() {","\tfmt.Println(\"bar\")","}"],"Pos":{"Filename":"path/to/fileb.go","Offset":5,"Line":300,"Column":9},"ExpectNoLint":false,"ExpectedNoLintLinter":""}],"Report":null}
`

assert.Equal(t, expected, buf.String())
Expand Down
2 changes: 1 addition & 1 deletion pkg/printers/testdata/golden-json.json

Large diffs are not rendered by default.

42 changes: 12 additions & 30 deletions pkg/printers/testdata/in-issues.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
"SourceLines": [
"func init() {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/experimental/myplugin/myplugin.go",
"Offset": 162,
Expand All @@ -23,7 +22,6 @@
"SourceLines": [
"func (b *PluginBuilder) loadConfig(cfg *config.Config, name string, settings config.CustomLinterSettings) (*linter.Config, error) {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/lint/lintersdb/builder_plugin.go",
"Offset": 1480,
Expand All @@ -40,15 +38,18 @@
"SourceLines": [
""
],
"Replacement": {
"NeedOnlyDelete": false,
"NewLines": [
"",
"\t\"github.com/stretchr/testify/require\"",
""
],
"Inline": null
},
"SuggestedFixes": [
{
"Message": "",
"TextEdits": [
{
"Pos": 13,
"End": 32,
"NewText": null
}
]
}
],
"Pos": {
"Filename": "pkg/printers/printer_test.go",
"Offset": 0,
Expand All @@ -65,7 +66,6 @@
"SourceLines": [
"type Issues struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/issues.go",
"Offset": 3338,
Expand All @@ -82,7 +82,6 @@
"SourceLines": [
"type LintersSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 4576,
Expand All @@ -99,7 +98,6 @@
"SourceLines": [
"type ExhaustiveSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 10829,
Expand All @@ -116,7 +114,6 @@
"SourceLines": [
"type GoConstSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 14399,
Expand All @@ -133,7 +130,6 @@
"SourceLines": [
"type GoCriticSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 14934,
Expand All @@ -150,7 +146,6 @@
"SourceLines": [
"type GosmopolitanSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 18601,
Expand All @@ -167,7 +162,6 @@
"SourceLines": [
"type GovetSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 18867,
Expand All @@ -184,7 +178,6 @@
"SourceLines": [
"type NoLintLintSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 22337,
Expand All @@ -201,7 +194,6 @@
"SourceLines": [
"type ReviveSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 24019,
Expand All @@ -218,7 +210,6 @@
"SourceLines": [
"type SlogLintSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 24648,
Expand All @@ -235,7 +226,6 @@
"SourceLines": [
"type TagAlignSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 25936,
Expand All @@ -252,7 +242,6 @@
"SourceLines": [
"type VarnamelenSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 28758,
Expand All @@ -269,7 +258,6 @@
"SourceLines": [
"type WSLSettings struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/linters_settings.go",
"Offset": 29898,
Expand All @@ -286,7 +274,6 @@
"SourceLines": [
"type Run struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/config/run.go",
"Offset": 112,
Expand All @@ -303,7 +290,6 @@
"SourceLines": [
"type Config struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/lint/linter/config.go",
"Offset": 1329,
Expand All @@ -320,7 +306,6 @@
"SourceLines": [
"\tfor _, tc := range []struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/golinters/govet_test.go",
"Offset": 1804,
Expand All @@ -337,7 +322,6 @@
"SourceLines": [
"type Diff struct {"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/result/processors/diff.go",
"Offset": 233,
Expand All @@ -354,7 +338,6 @@
"SourceLines": [
"\t\t\tRun: func(pass *analysis.Pass) (any, error) {"
],
"Replacement": null,
"LineRange": {
"From": 49,
"To": 49
Expand All @@ -375,7 +358,6 @@
"SourceLines": [
"const defaultFileMode = 0644"
],
"Replacement": null,
"Pos": {
"Filename": "pkg/commands/run.go",
"Offset": 1209,
Expand Down
23 changes: 6 additions & 17 deletions pkg/result/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,14 @@ import (
"fmt"
"go/token"

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

type Range struct {
From, To int
}

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
Inline *InlineFix
}

type InlineFix struct {
StartCol int // zero-based
Length int // length of chunk to be replaced
NewString string
}

type Issue struct {
FromLinter string
Text string
Expand All @@ -33,19 +22,19 @@ type Issue struct {
// Source lines of a code with the issue to show
SourceLines []string

// If we know how to fix the issue we can provide replacement lines
Replacement *Replacement

// Pkg is needed for proper caching of linting results
Pkg *packages.Package `json:"-"`

LineRange *Range `json:",omitempty"`

Pos token.Position

LineRange *Range `json:",omitempty"`

// HunkPos is used only when golangci-lint is run over a diff
HunkPos int `json:",omitempty"`

// If we know how to fix the issue we can provide replacement lines
SuggestedFixes []analysis.SuggestedFix `json:",omitempty"`

// If we are expecting a nolint (because this is from nolintlint), record the expected linter
ExpectNoLint bool
ExpectedNoLintLinter string
Expand Down
Loading