diff --git a/go.mod b/go.mod index 975e144b1935..a654acd1a48c 100644 --- a/go.mod +++ b/go.mod @@ -80,7 +80,7 @@ require ( github.com/spf13/pflag v1.0.5 github.com/spf13/viper v1.10.1 github.com/ssgreg/nlreturn/v2 v2.2.1 - github.com/stretchr/testify v1.7.0 + github.com/stretchr/testify v1.8.2 github.com/sylvia7788/contextcheck v1.0.4 github.com/tdakkota/asciicheck v0.1.1 github.com/tetafro/godot v1.4.11 @@ -95,7 +95,7 @@ require ( github.com/yeya24/promlinter v0.1.1-0.20210918184747-d757024714a1 gitlab.com/bosi/decorder v0.2.1 golang.org/x/tools v0.1.10 - gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b + gopkg.in/yaml.v3 v3.0.1 honnef.co/go/tools v0.2.2 mvdan.cc/gofumpt v0.3.0 mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed @@ -150,7 +150,7 @@ require ( github.com/spf13/afero v1.6.0 // indirect github.com/spf13/cast v1.4.1 // indirect github.com/spf13/jwalterweatherman v1.1.0 // indirect - github.com/stretchr/objx v0.1.1 // indirect + github.com/stretchr/objx v0.5.0 // indirect github.com/subosito/gotenv v1.2.0 // indirect github.com/tklauser/go-sysconf v0.3.9 // indirect github.com/tklauser/numcpus v0.3.0 // indirect diff --git a/go.sum b/go.sum index b0feaff6dc6b..ffa73d5d501e 100644 --- a/go.sum +++ b/go.sum @@ -693,8 +693,10 @@ github.com/spf13/viper v1.10.1/go.mod h1:IGlFPqhNAPKRxohIzWpI5QEy4kuI7tcl5WvR+8q github.com/ssgreg/nlreturn/v2 v2.2.1 h1:X4XDI7jstt3ySqGU86YGAURbxw3oTDPK9sPEi6YEwQ0= github.com/ssgreg/nlreturn/v2 v2.2.1/go.mod h1:E/iiPB78hV7Szg2YfRgyIrk1AD6JVMTRkkxBiELzh2I= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= +github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c= +github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v0.0.0-20170130113145-4d4bfba8f1d1/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.1.4/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= @@ -702,8 +704,11 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= +github.com/stretchr/testify v1.8.2 h1:+h33VjcLVPDHtOdpUCuF+7gSuG3yGIftsP1YvFihtJ8= +github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= github.com/sylvia7788/contextcheck v1.0.4 h1:MsiVqROAdr0efZc/fOCt0c235qm9XJqHtWwM+2h2B04= @@ -711,16 +716,10 @@ github.com/sylvia7788/contextcheck v1.0.4/go.mod h1:vuPKJMQ7MQ91ZTqfdyreNKwZjyUg github.com/tdakkota/asciicheck v0.1.1 h1:PKzG7JUTUmVspQTDqtkX9eSiLGossXTybutHwTXuO0A= github.com/tdakkota/asciicheck v0.1.1/go.mod h1:yHp0ai0Z9gUljN3o0xMhYJnH/IcvkdTBOX2fmJ93JEM= github.com/tenntenn/modver v1.0.1 h1:2klLppGhDgzJrScMpkj9Ujy3rXPUspSjAcev9tSEBgA= -github.com/tenntenn/modver v1.0.1 h1:2klLppGhDgzJrScMpkj9Ujy3rXPUspSjAcev9tSEBgA= -github.com/tenntenn/modver v1.0.1/go.mod h1:bePIyQPb7UeioSRkw3Q0XeMhYZSMx9B8ePqg6SAMGH0= github.com/tenntenn/modver v1.0.1/go.mod h1:bePIyQPb7UeioSRkw3Q0XeMhYZSMx9B8ePqg6SAMGH0= github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3 h1:f+jULpRQGxTSkNYKJ51yaw6ChIqO+Je8UqsTKN/cDag= -github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3 h1:f+jULpRQGxTSkNYKJ51yaw6ChIqO+Je8UqsTKN/cDag= -github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3/go.mod h1:ON8b8w4BN/kE1EOhwT0o+d62W65a6aPw1nouo9LMgyY= github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3/go.mod h1:ON8b8w4BN/kE1EOhwT0o+d62W65a6aPw1nouo9LMgyY= github.com/tetafro/godot v1.4.11 h1:BVoBIqAf/2QdbFmSwAWnaIqDivZdOV0ZRwEm6jivLKw= -github.com/tetafro/godot v1.4.11 h1:BVoBIqAf/2QdbFmSwAWnaIqDivZdOV0ZRwEm6jivLKw= -github.com/tetafro/godot v1.4.11/go.mod h1:LR3CJpxDVGlYOWn3ZZg1PgNZdTUvzsZWu8xaEohUpn8= github.com/tetafro/godot v1.4.11/go.mod h1:LR3CJpxDVGlYOWn3ZZg1PgNZdTUvzsZWu8xaEohUpn8= github.com/timakin/bodyclose v0.0.0-20210704033933-f49887972144 h1:kl4KhGNsJIbDHS9/4U9yQo1UcPQM0kOMJHn29EoH/Ro= github.com/timakin/bodyclose v0.0.0-20210704033933-f49887972144/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk= @@ -1118,10 +1117,8 @@ golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.1-0.20210205202024-ef80cdb6ec6d/go.mod h1:9bzcO0MWcOuT0tm1iBGzDVPshzfwoVvREIui8C+MHqU= -golang.org/x/tools v0.1.1-0.20210205202024-ef80cdb6ec6d/go.mod h1:9bzcO0MWcOuT0tm1iBGzDVPshzfwoVvREIui8C+MHqU= golang.org/x/tools v0.1.1-0.20210302220138-2ac05c832e1a/go.mod h1:9bzcO0MWcOuT0tm1iBGzDVPshzfwoVvREIui8C+MHqU= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= -golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.3/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.4/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= @@ -1304,8 +1301,9 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/pkg/golinters/goanalysis/linter.go b/pkg/golinters/goanalysis/linter.go index ef49e4284aa3..36b755452ff7 100644 --- a/pkg/golinters/goanalysis/linter.go +++ b/pkg/golinters/goanalysis/linter.go @@ -10,6 +10,7 @@ import ( "golang.org/x/tools/go/analysis" "github.com/golangci/golangci-lint/pkg/lint/linter" + libpackages "github.com/golangci/golangci-lint/pkg/packages" "github.com/golangci/golangci-lint/pkg/result" ) @@ -144,6 +145,31 @@ func (lnt *Linter) configure() error { return nil } +func buildIssuesFromErrorsForTypecheckMode(errs []error, lintCtx *linter.Context) ([]result.Issue, error) { + var issues []result.Issue + uniqReportedIssues := map[string]bool{} + for _, err := range errs { + itErr, ok := errors.Cause(err).(*IllTypedError) + if !ok { + return nil, err + } + for _, err := range libpackages.ExtractErrors(itErr.Pkg) { + i, perr := parseError(err) + if perr != nil { // failed to parse + if uniqReportedIssues[err.Msg] { + continue + } + uniqReportedIssues[err.Msg] = true + lintCtx.Log.Errorf("typechecking error: %s", err.Msg) + } else { + i.Pkg = itErr.Pkg // to save to cache later + issues = append(issues, *i) + } + } + } + return issues, nil +} + func (lnt *Linter) preRun(lintCtx *linter.Context) error { if err := analysis.Validate(lnt.analyzers); err != nil { return errors.Wrap(err, "failed to validate analyzers") diff --git a/pkg/golinters/goanalysis/linter_test.go b/pkg/golinters/goanalysis/linter_test.go index 44ded60043c0..a56e0be12997 100644 --- a/pkg/golinters/goanalysis/linter_test.go +++ b/pkg/golinters/goanalysis/linter_test.go @@ -2,9 +2,14 @@ package goanalysis import ( "fmt" + "go/token" + "reflect" "testing" + "github.com/golangci/golangci-lint/pkg/result" + "github.com/stretchr/testify/assert" + "golang.org/x/tools/go/analysis" "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 +} diff --git a/pkg/golinters/goanalysis/runners.go b/pkg/golinters/goanalysis/runners.go index 7e4cf902e79c..8277fd5c9298 100644 --- a/pkg/golinters/goanalysis/runners.go +++ b/pkg/golinters/goanalysis/runners.go @@ -2,6 +2,7 @@ package goanalysis import ( "fmt" + "go/token" "runtime" "sort" "strings" @@ -88,34 +89,63 @@ func buildIssues(diags []Diagnostic, linterNameBuilder func(diag *Diagnostic) st var issues []result.Issue for i := range diags { diag := &diags[i] - linterName := linterNameBuilder(diag) + issues = append(issues, buildSingleIssue(diag, linterNameBuilder(diag))) + } + return issues +} - var text string - if diag.Analyzer.Name == linterName { - text = diag.Message - } else { - text = fmt.Sprintf("%s: %s", diag.Analyzer.Name, diag.Message) - } +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") - 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, - }) + // 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 issues + + 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 {