internal/lsp: plumb suggested fixes through the LSP

Change-Id: Ia9e077e6b9cf8a817103d90481768ae99409c574
Reviewed-on: https://go-review.googlesource.com/c/tools/+/183264
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

go.mod

@@ -5,4 +5,5 @@ go 1.11
 require (
 	golang.org/x/net v0.0.0-20190311183353-d8887717615a
 	golang.org/x/sync v0.0.0-20190423024810-112230192c58
+	golang.org/x/tools/gopls v0.1.0 // indirect
 )

go.sum

@@ -5,3 +5,6 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEha
 golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+golang.org/x/tools v0.0.0-20190612231717-10539ce30318/go.mod h1:/rFqwRUd4F7ZHNgwSSTFct+R/Kf4OFW1sUzUTQQTgfc=
+golang.org/x/tools/gopls v0.1.0 h1:e5o2xK2HU//kzIRypLBw6/8pXdWuYDd8pliYpnQuNw8=
+golang.org/x/tools/gopls v0.1.0/go.mod h1:p8Q0IUu6EEeGxqmoN/g6Et3gReLCGA7PtNRdyOxcWJE=

internal/lsp/cache/pkg.go

@@ -37,7 +37,7 @@ type pkg struct {
 	analyses map[*analysis.Analyzer]*analysisEntry
 
 	diagMu      sync.Mutex
-	diagnostics []analysis.Diagnostic
+	diagnostics []source.Diagnostic
 }
 
 // packageID is a type that abstracts a package ID.
@@ -193,13 +193,13 @@ func (pkg *pkg) GetImport(pkgPath string) source.Package {
 	return nil
 }
 
-func (pkg *pkg) SetDiagnostics(diags []analysis.Diagnostic) {
+func (pkg *pkg) SetDiagnostics(diags []source.Diagnostic) {
 	pkg.diagMu.Lock()
 	defer pkg.diagMu.Unlock()
 	pkg.diagnostics = diags
 }
 
-func (pkg *pkg) GetDiagnostics() []analysis.Diagnostic {
+func (pkg *pkg) GetDiagnostics() []source.Diagnostic {
 	pkg.diagMu.Lock()
 	defer pkg.diagMu.Unlock()
 	return pkg.diagnostics

internal/lsp/code_action.go

@@ -16,7 +16,7 @@ import (
 func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) {
 	uri := span.NewURI(params.TextDocument.URI)
 	view := s.session.ViewOf(uri)
-	_, m, err := getSourceFile(ctx, view, uri)
+	gof, m, err := getGoFile(ctx, view, uri)
 	if err != nil {
 		return nil, err
 	}
@@ -57,6 +57,25 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 				},
 			})
 		}
+		diags := gof.GetPackage(ctx).GetDiagnostics()
+		for _, diag := range diags {
+			pdiag, err := toProtocolDiagnostic(ctx, view, diag)
+			if err != nil {
+				return nil, err
+			}
+			for _, ca := range diag.SuggestedFixes {
+				codeActions = append(codeActions, protocol.CodeAction{
+					Title: ca.Title,
+					Kind:  protocol.QuickFix, // TODO(matloob): Be more accurate about these?
+					Edit: &protocol.WorkspaceEdit{
+						Changes: &map[string][]protocol.TextEdit{
+							string(spn.URI()): edits,
+						},
+					},
+					Diagnostics: []protocol.Diagnostic{pdiag},
+				})
+			}
+		}
 	}
 	return codeActions, nil
 }

internal/lsp/diagnostics.go

@@ -71,27 +71,35 @@ func (s *Server) publishDiagnostics(ctx context.Context, view source.View, uri s
 func toProtocolDiagnostics(ctx context.Context, v source.View, diagnostics []source.Diagnostic) ([]protocol.Diagnostic, error) {
 	reports := []protocol.Diagnostic{}
 	for _, diag := range diagnostics {
-		_, m, err := getSourceFile(ctx, v, diag.Span.URI())
+		diagnostic, err := toProtocolDiagnostic(ctx, v, diag)
 		if err != nil {
 			return nil, err
 		}
-		var severity protocol.DiagnosticSeverity
+		reports = append(reports, diagnostic)
-		switch diag.Severity {
-		case source.SeverityError:
-			severity = protocol.SeverityError
-		case source.SeverityWarning:
-			severity = protocol.SeverityWarning
-		}
-		rng, err := m.Range(diag.Span)
-		if err != nil {
-			return nil, err
-		}
-		reports = append(reports, protocol.Diagnostic{
-			Message:  strings.TrimSpace(diag.Message), // go list returns errors prefixed by newline
-			Range:    rng,
-			Severity: severity,
-			Source:   diag.Source,
-		})
 	}
 	return reports, nil
 }
+
+func toProtocolDiagnostic(ctx context.Context, v source.View, diag source.Diagnostic) (protocol.Diagnostic, error) {
+	_, m, err := getSourceFile(ctx, v, diag.Span.URI())
+	if err != nil {
+		return protocol.Diagnostic{}, err
+	}
+	var severity protocol.DiagnosticSeverity
+	switch diag.Severity {
+	case source.SeverityError:
+		severity = protocol.SeverityError
+	case source.SeverityWarning:
+		severity = protocol.SeverityWarning
+	}
+	rng, err := m.Range(diag.Span)
+	if err != nil {
+		return protocol.Diagnostic{}, err
+	}
+	return protocol.Diagnostic{
+		Message:  strings.TrimSpace(diag.Message), // go list returns errors prefixed by newline
+		Range:    rng,
+		Severity: severity,
+		Source:   diag.Source,
+	}, nil
+}

internal/lsp/source/diagnostics.go

@@ -42,6 +42,13 @@ type Diagnostic struct {
 	Message  string
 	Source   string
 	Severity DiagnosticSeverity
+
+	SuggestedFixes []SuggestedFixes
+}
+
+type SuggestedFixes struct {
+	Title string
+	Edits []TextEdit
 }
 
 type DiagnosticSeverity int
@@ -59,7 +66,7 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[
 	// Prepare the reports we will send for the files in this package.
 	reports := make(map[span.URI][]Diagnostic)
 	for _, filename := range pkg.GetFilenames() {
-		addReport(view, reports, span.FileURI(filename), nil)
+		clearReports(view, reports, span.FileURI(filename))
 	}
 
 	// Prepare any additional reports for the errors in this package.
@@ -67,7 +74,7 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[
 		if err.Kind != packages.ListError {
 			continue
 		}
-		addReport(view, reports, packagesErrorSpan(err).URI(), nil)
+		clearReports(view, reports, packagesErrorSpan(err).URI())
 	}
 
 	// Run diagnostics for the package that this URI belongs to.
@@ -85,7 +92,7 @@ func Diagnostics(ctx context.Context, view View, f GoFile, disabledAnalyses map[
 			continue
 		}
 		for _, filename := range pkg.GetFilenames() {
-			addReport(view, reports, span.FileURI(filename), nil)
+			clearReports(view, reports, span.FileURI(filename))
 		}
 		diagnostics(ctx, view, pkg, reports)
 	}
@@ -146,22 +153,11 @@ func diagnostics(ctx context.Context, v View, pkg Package, reports map[span.URI]
 func analyses(ctx context.Context, v View, pkg Package, disabledAnalyses map[string]struct{}, reports map[span.URI][]Diagnostic) error {
 	// Type checking and parsing succeeded. Run analyses.
 	if err := runAnalyses(ctx, v, pkg, disabledAnalyses, func(a *analysis.Analyzer, diag analysis.Diagnostic) error {
-		r := span.NewRange(v.Session().Cache().FileSet(), diag.Pos, diag.End)
+		diagnostic, err := toDiagnostic(a, v, diag)
-		s, err := r.Span()
 		if err != nil {
-			// The diagnostic has an invalid position, so we don't have a valid span.
 			return err
 		}
-		category := a.Name
+		addReport(v, reports, diagnostic.Span.URI(), diagnostic)
-		if diag.Category != "" {
-			category += "." + category
-		}
-		addReport(v, reports, s.URI(), &Diagnostic{
-			Source:   category,
-			Span:     s,
-			Message:  diag.Message,
-			Severity: SeverityWarning,
-		})
 		return nil
 	}); err != nil {
 		return err
@@ -169,15 +165,42 @@ func analyses(ctx context.Context, v View, pkg Package, disabledAnalyses map[str
 	return nil
 }
 
-func addReport(v View, reports map[span.URI][]Diagnostic, uri span.URI, diagnostic *Diagnostic) {
+func toDiagnostic(a *analysis.Analyzer, v View, diag analysis.Diagnostic) (Diagnostic, error) {
+	r := span.NewRange(v.Session().Cache().FileSet(), diag.Pos, diag.End)
+	s, err := r.Span()
+	if err != nil {
+		// The diagnostic has an invalid position, so we don't have a valid span.
+		return Diagnostic{}, err
+	}
+	category := a.Name
+	if diag.Category != "" {
+		category += "." + category
+	}
+	ca, err := getCodeActions(v.Session().Cache().FileSet(), diag)
+	if err != nil {
+		return Diagnostic{}, err
+	}
+	return Diagnostic{
+		Source:         category,
+		Span:           s,
+		Message:        diag.Message,
+		Severity:       SeverityWarning,
+		SuggestedFixes: ca,
+	}, nil
+}
+
+func clearReports(v View, reports map[span.URI][]Diagnostic, uri span.URI) {
 	if v.Ignore(uri) {
 		return
 	}
-	if diagnostic == nil {
+	reports[uri] = []Diagnostic{}
-		reports[uri] = []Diagnostic{}
+}
-	} else {
+
-		reports[uri] = append(reports[uri], *diagnostic)
+func addReport(v View, reports map[span.URI][]Diagnostic, uri span.URI, diagnostic Diagnostic) {
+	if v.Ignore(uri) {
+		return
 	}
+	reports[uri] = append(reports[uri], diagnostic)
 }
 
 func packagesErrorSpan(err packages.Error) span.Span {
@@ -294,6 +317,7 @@ func runAnalyses(ctx context.Context, v View, pkg Package, disabledAnalyses map[
 
 	// Report diagnostics and errors from root analyzers.
 	for _, r := range roots {
+		var sdiags []Diagnostic
 		for _, diag := range r.diagnostics {
 			if r.err != nil {
 				// TODO(matloob): This isn't quite right: we might return a failed prerequisites error,
@@ -303,8 +327,13 @@ func runAnalyses(ctx context.Context, v View, pkg Package, disabledAnalyses map[
 			if err := report(r.Analyzer, diag); err != nil {
 				return err
 			}
+			sdiag, err := toDiagnostic(r.Analyzer, v, diag)
+			if err != nil {
+				return err
+			}
+			sdiags = append(sdiags, sdiag)
 		}
-		pkg.SetDiagnostics(r.diagnostics)
+		pkg.SetDiagnostics(sdiags)
 	}
 	return nil
 }

internal/lsp/source/suggested_fix.go

@@ -0,0 +1,10 @@
+// +build !experimental
+
+package source
+
+import "go/token"
+import "golang.org/x/tools/go/analysis"
+
+func getCodeActions(fset *token.FileSet, diag analysis.Diagnostic) ([]SuggestedFixes, error) {
+	return nil, nil
+}

internal/lsp/source/suggested_fix_experimental.go

@@ -0,0 +1,26 @@
+// +build experimental
+
+package source
+
+import (
+	"go/token"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/internal/span"
+)
+
+func getCodeActions(fset *token.FileSet, diag analysis.Diagnostic) ([]CodeAction, error) {
+	var cas []CodeAction
+	for _, fix := range diag.SuggestedFixes {
+		var ca CodeAction
+		ca.Title = fix.Message
+		for _, te := range fix.TextEdits {
+			span, err := span.NewRange(fset, te.Pos, te.End).Span()
+			if err != nil {
+				return nil, err
+			}
+			ca.Edits = append(ca.Edits, TextEdit{span, string(te.NewText)})
+		}
+		cas = append(cas, ca)
+	}
+	return cas, nil
+}

internal/lsp/source/view.go

@@ -264,8 +264,8 @@ type Package interface {
 	IsIllTyped() bool
 	GetActionGraph(ctx context.Context, a *analysis.Analyzer) (*Action, error)
 	GetImport(pkgPath string) Package
-	GetDiagnostics() []analysis.Diagnostic
+	GetDiagnostics() []Diagnostic
-	SetDiagnostics(diags []analysis.Diagnostic)
+	SetDiagnostics(diags []Diagnostic)
 }
 
 // TextEdit represents a change to a section of a document.