diff options
22 files changed, 179 insertions, 150 deletions
diff --git a/internal/lsp/analysis/fillreturns/fillreturns.go b/internal/lsp/analysis/fillreturns/fillreturns.go index a75f64513..7ed965375 100644 --- a/internal/lsp/analysis/fillreturns/fillreturns.go +++ b/internal/lsp/analysis/fillreturns/fillreturns.go @@ -50,23 +50,9 @@ func run(pass *analysis.Pass) (interface{}, error) { errors := analysisinternal.GetTypeErrors(pass) // Filter out the errors that are not relevant to this analyzer. for _, typeErr := range errors { - matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(typeErr.Msg)) - if len(matches) < 3 { + if !FixesError(typeErr.Msg) { continue } - wantNum, err := strconv.Atoi(matches[1]) - if err != nil { - continue - } - gotNum, err := strconv.Atoi(matches[2]) - if err != nil { - continue - } - // Logic for handling more return values than expected is hard. - if wantNum < gotNum { - continue - } - var file *ast.File for _, f := range pass.Files { if f.Pos() <= typeErr.Pos && typeErr.Pos <= f.End() { @@ -203,3 +189,20 @@ func equalTypes(t1, t2 types.Type) bool { // TODO: Figure out if we want to check for types.AssignableTo(t1, t2) || types.ConvertibleTo(t1, t2) return false } + +func FixesError(msg string) bool { + matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(msg)) + if len(matches) < 3 { + return false + } + wantNum, err := strconv.Atoi(matches[1]) + if err != nil { + return false + } + gotNum, err := strconv.Atoi(matches[2]) + if err != nil { + return false + } + // Logic for handling more return values than expected is hard. + return wantNum >= gotNum +} diff --git a/internal/lsp/analysis/nonewvars/nonewvars.go b/internal/lsp/analysis/nonewvars/nonewvars.go index 31dcd25b9..e7fa430cc 100644 --- a/internal/lsp/analysis/nonewvars/nonewvars.go +++ b/internal/lsp/analysis/nonewvars/nonewvars.go @@ -37,8 +37,6 @@ var Analyzer = &analysis.Analyzer{ RunDespiteErrors: true, } -const noNewVarsMsg = "no new variables on left side of :=" - func run(pass *analysis.Pass) (interface{}, error) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) errors := analysisinternal.GetTypeErrors(pass) @@ -63,7 +61,7 @@ func run(pass *analysis.Pass) (interface{}, error) { } for _, err := range errors { - if err.Msg != noNewVarsMsg { + if !FixesError(err.Msg) { continue } if assignStmt.Pos() > err.Pos || err.Pos >= assignStmt.End() { @@ -89,3 +87,7 @@ func run(pass *analysis.Pass) (interface{}, error) { }) return nil, nil } + +func FixesError(msg string) bool { + return msg == "no new variables on left side of :=" +} diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues.go b/internal/lsp/analysis/noresultvalues/noresultvalues.go index 0259691ed..0e6b26f8b 100644 --- a/internal/lsp/analysis/noresultvalues/noresultvalues.go +++ b/internal/lsp/analysis/noresultvalues/noresultvalues.go @@ -34,8 +34,6 @@ var Analyzer = &analysis.Analyzer{ RunDespiteErrors: true, } -const noResultValuesMsg = "no result values expected" - func run(pass *analysis.Pass) (interface{}, error) { inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) errors := analysisinternal.GetTypeErrors(pass) @@ -56,7 +54,7 @@ func run(pass *analysis.Pass) (interface{}, error) { } for _, err := range errors { - if err.Msg != noResultValuesMsg { + if !FixesError(err.Msg) { continue } if retStmt.Pos() >= err.Pos || err.Pos >= retStmt.End() { @@ -83,3 +81,7 @@ func run(pass *analysis.Pass) (interface{}, error) { }) return nil, nil } + +func FixesError(msg string) bool { + return msg == "no result values expected" +} diff --git a/internal/lsp/analysis/undeclaredname/undeclared.go b/internal/lsp/analysis/undeclaredname/undeclared.go index 21059fa52..cf7c770a9 100644 --- a/internal/lsp/analysis/undeclaredname/undeclared.go +++ b/internal/lsp/analysis/undeclaredname/undeclared.go @@ -36,7 +36,7 @@ const undeclaredNamePrefix = "undeclared name: " func run(pass *analysis.Pass) (interface{}, error) { for _, err := range analysisinternal.GetTypeErrors(pass) { - if !strings.HasPrefix(err.Msg, undeclaredNamePrefix) { + if !FixesError(err.Msg) { continue } name := strings.TrimPrefix(err.Msg, undeclaredNamePrefix) @@ -184,3 +184,7 @@ func baseIfStmt(path []ast.Node, index int) ast.Stmt { } return stmt.(ast.Stmt) } + +func FixesError(msg string) bool { + return strings.HasPrefix(msg, undeclaredNamePrefix) +} diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go index ecedc31a9..563d3b53b 100644 --- a/internal/lsp/cache/analysis.go +++ b/internal/lsp/cache/analysis.go @@ -23,7 +23,7 @@ import ( errors "golang.org/x/xerrors" ) -func (s *snapshot) Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*source.Error, error) { +func (s *snapshot) Analyze(ctx context.Context, id string, analyzers ...*analysis.Analyzer) ([]*source.Error, error) { var roots []*actionHandle for _, a := range analyzers { diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go index 9fe7a02e9..bfda93a7e 100644 --- a/internal/lsp/cache/pkg.go +++ b/internal/lsp/cache/pkg.go @@ -5,11 +5,9 @@ package cache import ( - "context" "go/ast" "go/types" - "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/packagesinternal" "golang.org/x/tools/internal/span" @@ -125,46 +123,3 @@ func (p *pkg) Imports() []source.Package { func (p *pkg) Module() *packagesinternal.Module { return p.module } - -func (s *snapshot) FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*source.Error, *source.Analyzer, error) { - analyzer := findAnalyzer(s, analyzerName) - if analyzer.Analyzer == nil { - return nil, nil, errors.Errorf("unexpected analyzer: %s", analyzerName) - } - if !analyzer.Enabled(s) { - return nil, nil, errors.Errorf("disabled analyzer: %s", analyzerName) - } - act, err := s.actionHandle(ctx, packageID(pkgID), analyzer.Analyzer) - if err != nil { - return nil, nil, err - } - errs, _, err := act.analyze(ctx) - if err != nil { - return nil, nil, err - } - for _, err := range errs { - if err.Category != analyzer.Analyzer.Name { - continue - } - if err.Message != msg { - continue - } - if protocol.CompareRange(err.Range, rng) != 0 { - continue - } - return err, &analyzer, nil - } - return nil, nil, errors.Errorf("no matching diagnostic for %s:%v", pkgID, analyzerName) -} - -func findAnalyzer(s *snapshot, analyzerName string) source.Analyzer { - checked := s.View().Options().DefaultAnalyzers - if a, ok := checked[analyzerName]; ok { - return a - } - checked = s.View().Options().TypeErrorAnalyzers - if a, ok := checked[analyzerName]; ok { - return a - } - return source.Analyzer{} -} diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go index f21194f8b..24e916f00 100644 --- a/internal/lsp/cmd/test/check.go +++ b/internal/lsp/cmd/test/check.go @@ -14,7 +14,7 @@ import ( "golang.org/x/tools/internal/span" ) -func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) { +func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) { if len(want) == 1 && want[0].Message == "" { return } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index e12537556..4757a1ee1 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -259,10 +259,8 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.File return nil, nil, err } for _, diag := range diagnostics { - // This code assumes that the analyzer name is the Source of the diagnostic. - // If this ever changes, this will need to be addressed. - srcErr, analyzer, err := snapshot.FindAnalysisError(ctx, ph.ID(), diag.Source, diag.Message, diag.Range) - if err != nil { + srcErr, analyzer, ok := findSourceError(ctx, snapshot, ph.ID(), diag) + if !ok { continue } for _, fix := range srcErr.SuggestedFixes { @@ -289,6 +287,47 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.File return codeActions, sourceFixAllEdits, nil } +func findSourceError(ctx context.Context, snapshot source.Snapshot, pkgID string, diag protocol.Diagnostic) (*source.Error, source.Analyzer, bool) { + var analyzer *source.Analyzer + + // If the source is "compiler", we expect a type error analyzer. + if diag.Source == "compiler" { + for _, a := range snapshot.View().Options().TypeErrorAnalyzers { + if a.FixesError(diag.Message) { + analyzer = &a + break + } + } + } else { + // This code assumes that the analyzer name is the Source of the diagnostic. + // If this ever changes, this will need to be addressed. + if a, ok := snapshot.View().Options().DefaultAnalyzers[diag.Source]; ok { + analyzer = &a + } + } + if analyzer == nil || !analyzer.Enabled(snapshot) { + return nil, source.Analyzer{}, false + } + analysisErrors, err := snapshot.Analyze(ctx, pkgID, analyzer.Analyzer) + if err != nil { + return nil, source.Analyzer{}, false + } + for _, err := range analysisErrors { + if err.Message != diag.Message { + continue + } + if protocol.CompareRange(err.Range, diag.Range) != 0 { + continue + } + if err.Category != analyzer.Analyzer.Name { + continue + } + // The error matches. + return err, *analyzer, true + } + return nil, source.Analyzer{}, false +} + func documentChanges(fh source.FileHandle, edits []protocol.TextEdit) []protocol.TextDocumentEdit { return []protocol.TextDocumentEdit{ { diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 1519f322b..94592be3c 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -39,7 +39,7 @@ func (s *Server) diagnoseSnapshot(snapshot source.Snapshot) { // diagnose is a helper function for running diagnostics with a given context. // Do not call it directly. -func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysAnalyze bool) map[diagnosticKey][]source.Diagnostic { +func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysAnalyze bool) map[diagnosticKey][]*source.Diagnostic { ctx, done := event.StartSpan(ctx, "lsp:background-worker") defer done() @@ -51,7 +51,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA } defer func() { <-s.diagnosticsSema }() - allReports := make(map[diagnosticKey][]source.Diagnostic) + allReports := make(map[diagnosticKey][]*source.Diagnostic) var reportsMu sync.Mutex var wg sync.WaitGroup @@ -123,7 +123,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA return allReports } -func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[diagnosticKey][]source.Diagnostic) { +func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[diagnosticKey][]*source.Diagnostic) { // Check for context cancellation before publishing diagnostics. if ctx.Err() != nil { return @@ -189,7 +189,7 @@ func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, r // equalDiagnostics returns true if the 2 lists of diagnostics are equal. // It assumes that both a and b are already sorted. -func equalDiagnostics(a, b []source.Diagnostic) bool { +func equalDiagnostics(a, b []*source.Diagnostic) bool { if len(a) != len(b) { return false } @@ -201,7 +201,7 @@ func equalDiagnostics(a, b []source.Diagnostic) bool { return true } -func toProtocolDiagnostics(diagnostics []source.Diagnostic) []protocol.Diagnostic { +func toProtocolDiagnostics(diagnostics []*source.Diagnostic) []protocol.Diagnostic { reports := []protocol.Diagnostic{} for _, diag := range diagnostics { related := make([]protocol.DiagnosticRelatedInformation, 0, len(diag.Related)) diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 0b38d1da1..6e9d4b12f 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -38,7 +38,7 @@ func TestLSP(t *testing.T) { type runner struct { server *Server data *tests.Data - diagnostics map[span.URI][]source.Diagnostic + diagnostics map[span.URI][]*source.Diagnostic ctx context.Context } @@ -119,10 +119,10 @@ func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens) } } -func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) { +func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) { // Get the diagnostics for this view if we have not done it before. if r.diagnostics == nil { - r.diagnostics = make(map[span.URI][]source.Diagnostic) + r.diagnostics = make(map[span.URI][]*source.Diagnostic) v := r.server.session.View(r.data.Config.Dir) // Always run diagnostics with analysis. reports := r.server.diagnose(r.ctx, v.Snapshot(), true) @@ -383,7 +383,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) } // Get the diagnostics for this view if we have not done it before. if r.diagnostics == nil { - r.diagnostics = make(map[span.URI][]source.Diagnostic) + r.diagnostics = make(map[span.URI][]*source.Diagnostic) // Always run diagnostics with analysis. reports := r.server.diagnose(r.ctx, view.Snapshot(), true) for key, diags := range reports { @@ -396,7 +396,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) // some diagnostics have a range with the same start and end position (8:1-8:1). // The current marker functionality prevents us from having a range of 0 length. if protocol.ComparePosition(d.Range.Start, rng.Start) == 0 { - diag = &d + diag = d break } } @@ -413,7 +413,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) }, Context: protocol.CodeActionContext{ Only: codeActionKinds, - Diagnostics: toProtocolDiagnostics([]source.Diagnostic{*diag}), + Diagnostics: toProtocolDiagnostics([]*source.Diagnostic{diag}), }, }) if err != nil { diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index e36db7397..ba7730d4e 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -16,7 +16,7 @@ import ( "golang.org/x/tools/internal/telemetry/event" ) -func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]source.Diagnostic, map[string]*modfile.Require, error) { +func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]*source.Diagnostic, map[string]*modfile.Require, error) { // TODO: We will want to support diagnostics for go.mod files even when the -modfile flag is turned off. realURI, tempURI := snapshot.View().ModFiles() @@ -39,11 +39,11 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File if err != nil { return nil, nil, err } - reports := map[source.FileIdentity][]source.Diagnostic{ + reports := map[source.FileIdentity][]*source.Diagnostic{ realfh.Identity(): {}, } for _, e := range parseErrors { - diag := source.Diagnostic{ + diag := &source.Diagnostic{ Message: e.Message, Range: e.Range, SuggestedFixes: e.SuggestedFixes, diff --git a/internal/lsp/server.go b/internal/lsp/server.go index d8122a06a..44e441219 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -88,7 +88,7 @@ type Server struct { type sentDiagnostics struct { version float64 identifier string - sorted []source.Diagnostic + sorted []*source.Diagnostic withAnalysis bool snapshotID uint64 } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 2c97cba12..ec3e9d397 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -42,7 +42,7 @@ type RelatedInformation struct { Message string } -func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missingModules map[string]*modfile.Require, withAnalysis bool) (map[FileIdentity][]Diagnostic, bool, error) { +func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missingModules map[string]*modfile.Require, withAnalysis bool) (map[FileIdentity][]*Diagnostic, bool, error) { // If we are missing dependencies, it may because the user's workspace is // not correctly configured. Report errors, if possible. var warn bool @@ -83,7 +83,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi } // Prepare the reports we will send for the files in this package. - reports := make(map[FileIdentity][]Diagnostic) + reports := make(map[FileIdentity][]*Diagnostic) for _, fh := range pkg.CompiledGoFiles() { if err := clearReports(snapshot, reports, fh.File().Identity().URI); err != nil { return nil, warn, err @@ -139,7 +139,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi return reports, warn, nil } -func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (FileIdentity, []Diagnostic, error) { +func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (FileIdentity, []*Diagnostic, error) { fh, err := snapshot.GetFile(uri) if err != nil { return FileIdentity{}, nil, err @@ -167,7 +167,7 @@ type diagnosticSet struct { listErrors, parseErrors, typeErrors []*Diagnostic } -func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, pkg Package, hasMissingDeps bool) (bool, bool, error) { +func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, pkg Package, hasMissingDeps bool) (bool, bool, error) { ctx, done := event.StartSpan(ctx, "source.diagnostics", tag.Package.Of(pkg.ID())) _ = ctx // circumvent SA4006 defer done() @@ -217,7 +217,7 @@ func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentit return nonEmptyDiagnostics, hasTypeErrors, nil } -func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, missingModules map[string]*modfile.Require, uri span.URI) error { +func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, missingModules map[string]*modfile.Require, uri span.URI) error { if snapshot.View().Ignore(uri) || len(missingModules) == 0 { return nil } @@ -244,7 +244,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m return nil } if reports[fh.Identity()] == nil { - reports[fh.Identity()] = []Diagnostic{} + reports[fh.Identity()] = []*Diagnostic{} } for mod, req := range missingModules { if req.Syntax == nil { @@ -262,7 +262,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m if err != nil { return err } - reports[fh.Identity()] = append(reports[fh.Identity()], Diagnostic{ + reports[fh.Identity()] = append(reports[fh.Identity()], &Diagnostic{ Message: fmt.Sprintf("%s is not in your go.mod file.", req.Mod.Path), Range: rng, Source: "go mod tidy", @@ -272,7 +272,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m return nil } -func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, ph PackageHandle, analyses map[string]Analyzer) error { +func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, ph PackageHandle, analyses map[string]Analyzer) error { var analyzers []*analysis.Analyzer for _, a := range analyses { if !a.Enabled(snapshot) { @@ -280,8 +280,7 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][ } analyzers = append(analyzers, a.Analyzer) } - - analysisErrors, err := snapshot.Analyze(ctx, ph.ID(), analyzers) + analysisErrors, err := snapshot.Analyze(ctx, ph.ID(), analyzers...) if err != nil { return err } @@ -290,7 +289,7 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][ for _, e := range analysisErrors { // This is a bit of a hack, but clients > 3.15 will be able to grey out unnecessary code. // If we are deleting code as part of all of our suggested fixes, assume that this is dead code. - // TODO(golang/go/#34508): Return these codes from the diagnostics themselves. + // TODO(golang/go#34508): Return these codes from the diagnostics themselves. var tags []protocol.DiagnosticTag if onlyDeletions(e.SuggestedFixes) { tags = append(tags, protocol.Unnecessary) @@ -310,7 +309,7 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][ return nil } -func clearReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri span.URI) error { +func clearReports(snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, uri span.URI) error { if snapshot.View().Ignore(uri) { return nil } @@ -318,11 +317,11 @@ func clearReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri if err != nil { return err } - reports[fh.Identity()] = []Diagnostic{} + reports[fh.Identity()] = []*Diagnostic{} return nil } -func addReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri span.URI, diagnostics ...*Diagnostic) error { +func addReports(snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, uri span.URI, diagnostics ...*Diagnostic) error { if snapshot.View().Ignore(uri) { return nil } @@ -330,21 +329,29 @@ func addReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri sp if err != nil { return err } - if _, ok := reports[fh.Identity()]; !ok { + identity := fh.Identity() + existingDiagnostics, ok := reports[identity] + if !ok { return errors.Errorf("diagnostics for unexpected file %s", uri) } -Outer: - for _, diag := range diagnostics { - if diag == nil { - continue - } - for i, d := range reports[fh.Identity()] { - if diag.Message == d.Message && protocol.CompareRange(d.Range, diag.Range) == 0 { - reports[fh.Identity()][i].Source = diag.Source - continue Outer + if len(diagnostics) == 1 { + d1 := diagnostics[0] + if _, ok := snapshot.View().Options().TypeErrorAnalyzers[d1.Source]; ok { + for i, d2 := range existingDiagnostics { + if r := protocol.CompareRange(d1.Range, d2.Range); r != 0 { + continue + } + if d1.Message != d2.Message { + continue + } + reports[identity][i].SuggestedFixes = append(reports[identity][i].SuggestedFixes, d1.SuggestedFixes...) + reports[identity][i].Tags = append(reports[identity][i].Tags, d1.Tags...) } + return nil } - reports[fh.Identity()] = append(reports[fh.Identity()], *diag) + } + for _, diag := range diagnostics { + reports[fh.Identity()] = append(reports[fh.Identity()], diag) } return nil } diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 6b33d63b1..06b56b74d 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -497,10 +497,26 @@ func (r *OptionResult) setBool(b *bool) { func typeErrorAnalyzers() map[string]Analyzer { return map[string]Analyzer{ - fillreturns.Analyzer.Name: {Analyzer: fillreturns.Analyzer, enabled: false}, - nonewvars.Analyzer.Name: {Analyzer: nonewvars.Analyzer, enabled: false}, - noresultvalues.Analyzer.Name: {Analyzer: noresultvalues.Analyzer, enabled: false}, - undeclaredname.Analyzer.Name: {Analyzer: undeclaredname.Analyzer, enabled: false}, + fillreturns.Analyzer.Name: { + Analyzer: fillreturns.Analyzer, + enabled: false, + FixesError: fillreturns.FixesError, + }, + nonewvars.Analyzer.Name: { + Analyzer: nonewvars.Analyzer, + enabled: false, + FixesError: nonewvars.FixesError, + }, + noresultvalues.Analyzer.Name: { + Analyzer: noresultvalues.Analyzer, + enabled: false, + FixesError: noresultvalues.FixesError, + }, + undeclaredname.Analyzer.Name: { + Analyzer: undeclaredname.Analyzer, + enabled: false, + FixesError: undeclaredname.FixesError, + }, } } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index b9da1a0c6..619999ecb 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -88,7 +88,7 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { } } -func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) { +func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) { snapshot := r.view.Snapshot() fileID, got, err := source.FileDiagnostics(r.ctx, snapshot, uri) diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go index 627d7f482..5d275bddc 100644 --- a/internal/lsp/source/util.go +++ b/internal/lsp/source/util.go @@ -621,13 +621,13 @@ func formatFunction(params []string, results []string, writeResultParens bool) s return detail.String() } -func SortDiagnostics(d []Diagnostic) { +func SortDiagnostics(d []*Diagnostic) { sort.Slice(d, func(i int, j int) bool { return CompareDiagnostic(d[i], d[j]) < 0 }) } -func CompareDiagnostic(a, b Diagnostic) int { +func CompareDiagnostic(a, b *Diagnostic) int { if r := protocol.CompareRange(a.Range, b.Range); r != 0 { return r } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index eba54e609..cf6bc489e 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -42,11 +42,7 @@ type Snapshot interface { IsSaved(uri span.URI) bool // Analyze runs the analyses for the given package at this snapshot. - Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*Error, error) - - // FindAnalysisError returns the analysis error represented by the diagnostic. - // This is used to get the SuggestedFixes associated with that error. - FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*Error, *Analyzer, error) + Analyze(ctx context.Context, pkgID string, analyzers ...*analysis.Analyzer) ([]*Error, error) // ModTidyHandle returns a ModTidyHandle for the given go.mod file handle. // This function can have no data or error if there is no modfile detected. @@ -372,6 +368,11 @@ type Analyzer struct { // If this is true, then we can apply the suggested fixes // as part of a source.FixAll codeaction. HighConfidence bool + + // FixesError is only set for type-error analyzers. + // It reports true if the message provided indicates an error that could be + // fixed by the analyzer. + FixesError func(msg string) bool } func (a Analyzer) Enabled(snapshot Snapshot) bool { diff --git a/internal/lsp/testdata/lsp/primarymod/bad/bad1.go b/internal/lsp/testdata/lsp/primarymod/bad/bad1.go index 00451bff1..eb547f793 100644 --- a/internal/lsp/testdata/lsp/primarymod/bad/bad1.go +++ b/internal/lsp/testdata/lsp/primarymod/bad/bad1.go @@ -14,8 +14,8 @@ func random() int { //@item(random, "random", "func() int", "func") func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func"),item(bad_y_param, "y", "int", "var") x := 6 //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared but not used", "error") - var q blah //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "undeclaredname", "undeclared name: blah", "error") - var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "undeclaredname", "undeclared name: blob", "error") + var q blah //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "compiler", "undeclared name: blah", "error") + var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "compiler", "undeclared name: blob", "error") //@complete("", q, t, x, bad_y_param, global_a, bob, stateFunc, random, random2, random3, stuff) return y @@ -24,10 +24,10 @@ func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func") func random3(y ...int) { //@item(random3, "random3", "func(y ...int)", "func"),item(y_variadic_param, "y", "[]int", "var") //@complete("", y_variadic_param, global_a, bob, stateFunc, random, random2, random3, stuff) - var ch chan (favType1) //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "undeclaredname", "undeclared name: favType1", "error") - var m map[keyType]int //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "undeclaredname", "undeclared name: keyType", "error") - var arr []favType2 //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "undeclaredname", "undeclared name: favType2", "error") - var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "undeclaredname", "undeclared name: badResult", "error") - var fn2 func(badParam) //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "undeclaredname", "undeclared name: badParam", "error") + var ch chan (favType1) //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "compiler", "undeclared name: favType1", "error") + var m map[keyType]int //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "compiler", "undeclared name: keyType", "error") + var arr []favType2 //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "compiler", "undeclared name: favType2", "error") + var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "compiler", "undeclared name: badResult", "error") + var fn2 func(badParam) //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "compiler", "undeclared name: badParam", "error") //@complete("", arr, ch, fn1, fn2, m, y_variadic_param, global_a, bob, stateFunc, random, random2, random3, stuff) } diff --git a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go index 1a43292cf..b5f9287d4 100644 --- a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go +++ b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go @@ -1,13 +1,13 @@ package undeclared func m() int { - z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix") + z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix") if 100 < 90 { z = 1 - } else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix") + } else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix") z = 4 } - for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix") + for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix") } r() //@diag("r", "compiler", "undeclared name: r", "error") return z diff --git a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden index 63cf58ad6..68e900fd1 100644 --- a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden +++ b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden @@ -3,13 +3,13 @@ package undeclared func m() int { y := - z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix") + z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix") if 100 < 90 { z = 1 - } else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix") + } else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix") z = 4 } - for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix") + for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix") } r() //@diag("r", "compiler", "undeclared name: r", "error") return z @@ -19,14 +19,14 @@ func m() int { package undeclared func m() int { - z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix") + z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix") n := if 100 < 90 { z = 1 - } else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix") + } else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix") z = 4 } - for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix") + for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix") } r() //@diag("r", "compiler", "undeclared name: r", "error") return z @@ -36,14 +36,14 @@ func m() int { package undeclared func m() int { - z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix") + z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix") if 100 < 90 { z = 1 - } else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix") + } else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix") z = 4 } i := - for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix") + for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix") } r() //@diag("r", "compiler", "undeclared name: r", "error") return z diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index c4c483444..f843a5fac 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -44,7 +44,7 @@ const ( var UpdateGolden = flag.Bool("golden", false, "Update golden files") type CodeLens map[span.URI][]protocol.CodeLens -type Diagnostics map[span.URI][]source.Diagnostic +type Diagnostics map[span.URI][]*source.Diagnostic type CompletionItems map[token.Pos]*source.CompletionItem type Completions map[span.Span][]Completion type CompletionSnippets map[span.Span][]CompletionSnippet @@ -116,7 +116,7 @@ type Data struct { type Tests interface { CodeLens(*testing.T, span.URI, []protocol.CodeLens) - Diagnostics(*testing.T, span.URI, []source.Diagnostic) + Diagnostics(*testing.T, span.URI, []*source.Diagnostic) Completion(*testing.T, span.Span, Completion, CompletionItems) CompletionSnippet(*testing.T, span.Span, CompletionSnippet, bool, CompletionItems) UnimportedCompletion(*testing.T, span.Span, Completion, CompletionItems) @@ -907,7 +907,7 @@ func (data *Data) collectCodeLens(spn span.Span, title, cmd string) { func (data *Data) collectDiagnostics(spn span.Span, msgSource, msg, msgSeverity string) { if _, ok := data.Diagnostics[spn.URI()]; !ok { - data.Diagnostics[spn.URI()] = []source.Diagnostic{} + data.Diagnostics[spn.URI()] = []*source.Diagnostic{} } m, err := data.Mapper(spn.URI()) if err != nil { @@ -929,7 +929,7 @@ func (data *Data) collectDiagnostics(spn span.Span, msgSource, msg, msgSeverity severity = protocol.SeverityInformation } // This is not the correct way to do this, but it seems excessive to do the full conversion here. - want := source.Diagnostic{ + want := &source.Diagnostic{ Range: rng, Severity: severity, Source: msgSource, diff --git a/internal/lsp/tests/util.go b/internal/lsp/tests/util.go index e6de57f88..472b93c94 100644 --- a/internal/lsp/tests/util.go +++ b/internal/lsp/tests/util.go @@ -163,7 +163,7 @@ func summarizeWorkspaceSymbols(i int, want, got []protocol.SymbolInformation, re // DiffDiagnostics prints the diff between expected and actual diagnostics test // results. -func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string { +func DiffDiagnostics(uri span.URI, want, got []*source.Diagnostic) string { source.SortDiagnostics(want) source.SortDiagnostics(got) @@ -197,7 +197,7 @@ func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string { return "" } -func summarizeDiagnostics(i int, uri span.URI, want, got []source.Diagnostic, reason string, args ...interface{}) string { +func summarizeDiagnostics(i int, uri span.URI, want, got []*source.Diagnostic, reason string, args ...interface{}) string { msg := &bytes.Buffer{} fmt.Fprint(msg, "diagnostics failed") if i >= 0 { |