From f558378bf83116fb4c80b6acdc028aee0003eb1b Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Mon, 1 Apr 2019 20:08:14 -0400 Subject: [PATCH] internal/lsp: make definition use the lsp protocol instead of driving the source pacakge directly, it indirects through the lsp protocol (the same way check does) We are normalizing on all the command lines doing this, so that server mode is more viable in the future. Change-Id: Ib5f2a059a44a5c60a53129c554e3cc14ca72c4a8 Reviewed-on: https://go-review.googlesource.com/c/tools/+/170577 Run-TryBot: Ian Cottrell Reviewed-by: Rebecca Stambler --- internal/lsp/cmd/check.go | 23 +---- internal/lsp/cmd/cmd.go | 41 +++++++- internal/lsp/cmd/definition.go | 144 ++++++++++------------------- internal/lsp/source/diagnostics.go | 53 +++++++---- 4 files changed, 127 insertions(+), 134 deletions(-) diff --git a/internal/lsp/cmd/check.go b/internal/lsp/cmd/check.go index f4e29aaa..4fdcff6e 100644 --- a/internal/lsp/cmd/check.go +++ b/internal/lsp/cmd/check.go @@ -8,8 +8,6 @@ import ( "context" "flag" "fmt" - "go/token" - "io/ioutil" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" @@ -54,38 +52,27 @@ func (c *check) Run(ctx context.Context, args ...string) error { client := &checkClient{ diagnostics: make(chan entry), } - client.app = c.app - checking := map[span.URI][]byte{} + checking := map[span.URI]*protocol.ColumnMapper{} // now we ready to kick things off - server, err := c.app.connect(ctx, client) + _, err := c.app.connect(ctx, client) if err != nil { return err } for _, arg := range args { uri := span.FileURI(arg) - content, err := ioutil.ReadFile(arg) + m, err := client.AddFile(ctx, uri) if err != nil { return err } - checking[uri] = content - p := &protocol.DidOpenTextDocumentParams{} - p.TextDocument.URI = string(uri) - p.TextDocument.Text = string(content) - if err := server.DidOpen(ctx, p); err != nil { - return err - } + checking[uri] = m } // now wait for results for entry := range client.diagnostics { //TODO:timeout? - content, found := checking[entry.uri] + m, found := checking[entry.uri] if !found { continue } - fset := token.NewFileSet() - f := fset.AddFile(string(entry.uri), -1, len(content)) - f.SetLinesForContent(content) - m := protocol.NewColumnMapper(entry.uri, fset, f, content) for _, d := range entry.diagnostics { spn, err := m.RangeSpan(d.Range) if err != nil { diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go index 48e3ed6b..483ec8d7 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -14,6 +14,7 @@ import ( "go/ast" "go/parser" "go/token" + "io/ioutil" "log" "net" "os" @@ -118,7 +119,13 @@ func (app *Application) commands() []tool.Application { } } -func (app *Application) connect(ctx context.Context, client protocol.Client) (protocol.Server, error) { +type cmdClient interface { + protocol.Client + + prepare(app *Application, server protocol.Server) +} + +func (app *Application) connect(ctx context.Context, client cmdClient) (protocol.Server, error) { var server protocol.Server switch app.Remote { case "": @@ -150,6 +157,7 @@ func (app *Application) connect(ctx context.Context, client protocol.Client) (pr "configuration": true, }, } + client.prepare(app, server) if _, err := server.Initialize(ctx, params); err != nil { return nil, err } @@ -161,7 +169,9 @@ func (app *Application) connect(ctx context.Context, client protocol.Client) (pr type baseClient struct { protocol.Server - app *Application + app *Application + server protocol.Server + fset *token.FileSet } func (c *baseClient) ShowMessage(ctx context.Context, p *protocol.ShowMessageParams) error { return nil } @@ -217,3 +227,30 @@ func (c *baseClient) ApplyEdit(ctx context.Context, p *protocol.ApplyWorkspaceEd func (c *baseClient) PublishDiagnostics(ctx context.Context, p *protocol.PublishDiagnosticsParams) error { return nil } + +func (c *baseClient) prepare(app *Application, server protocol.Server) { + c.app = app + c.server = server + c.fset = token.NewFileSet() +} + +func (c *baseClient) AddFile(ctx context.Context, uri span.URI) (*protocol.ColumnMapper, error) { + fname, err := uri.Filename() + if err != nil { + return nil, fmt.Errorf("%v: %v", uri, err) + } + content, err := ioutil.ReadFile(fname) + if err != nil { + return nil, fmt.Errorf("%v: %v", uri, err) + } + f := c.fset.AddFile(fname, -1, len(content)) + f.SetLinesForContent(content) + m := protocol.NewColumnMapper(uri, c.fset, f, content) + p := &protocol.DidOpenTextDocumentParams{} + p.TextDocument.URI = string(uri) + p.TextDocument.Text = string(content) + if err := c.server.DidOpen(ctx, p); err != nil { + return nil, fmt.Errorf("%v: %v", uri, err) + } + return m, nil +} diff --git a/internal/lsp/cmd/definition.go b/internal/lsp/cmd/definition.go index e0f5f928..c906dc21 100644 --- a/internal/lsp/cmd/definition.go +++ b/internal/lsp/cmd/definition.go @@ -5,18 +5,15 @@ package cmd import ( - "bytes" "context" "encoding/json" "flag" "fmt" - "go/types" "os" + "strings" guru "golang.org/x/tools/cmd/guru/serial" - "golang.org/x/tools/internal/lsp/cache" - "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/lsp/xlog" + "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/tool" ) @@ -31,9 +28,9 @@ type Definition struct { // help is still valid. // They refer to "Set" in "flag.FlagSet" from the DetailedHelp method below. const ( - exampleLine = 47 + exampleLine = 44 exampleColumn = 47 - exampleOffset = 1359 + exampleOffset = 1270 ) // definition implements the definition noun for the query command. @@ -62,31 +59,67 @@ func (d *definition) Run(ctx context.Context, args ...string) error { if len(args) != 1 { return tool.CommandLineErrorf("definition expects 1 argument") } - log := xlog.New(xlog.StdSink{}) - view := cache.NewView(ctx, log, "definition_test", span.FileURI(d.query.app.Config.Dir), &d.query.app.Config) + client := &baseClient{} + server, err := d.query.app.connect(ctx, client) + if err != nil { + return err + } from := span.Parse(args[0]) - f, err := view.GetFile(ctx, from.URI()) + m, err := client.AddFile(ctx, from.URI()) if err != nil { return err } - converter := span.NewTokenConverter(view.FileSet(), f.GetToken(ctx)) - rng, err := from.Range(converter) + loc, err := m.Location(from) if err != nil { return err } - ident, err := source.Identifier(ctx, view, f, rng.Start) + p := protocol.TextDocumentPositionParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: loc.URI}, + Position: loc.Range.Start, + } + locs, err := server.Definition(ctx, &p) if err != nil { return fmt.Errorf("%v: %v", from, err) } - if ident == nil { + + if len(locs) == 0 { return fmt.Errorf("%v: not an identifier", from) } + hover, err := server.Hover(ctx, &p) + if err != nil { + return fmt.Errorf("%v: %v", from, err) + } + if hover == nil { + return fmt.Errorf("%v: not an identifier", from) + } + m, err = client.AddFile(ctx, span.NewURI(locs[0].URI)) + if err != nil { + return fmt.Errorf("%v: %v", from, err) + } + definition, err := m.Span(locs[0]) + if err != nil { + return fmt.Errorf("%v: %v", from, err) + } + //TODO: either work out how to request plain text, or + //use a less kludgy way of cleaning the markdown + description := hover.Contents.Value + if v := strings.TrimPrefix(description, "```go"); v != description { + description = strings.TrimSuffix(v, "```") + } + description = strings.TrimSpace(description) var result interface{} switch d.query.Emulate { case "": - result, err = buildDefinition(ctx, view, ident) + result = &Definition{ + Span: definition, + Description: description, + } case emulateGuru: - result, err = buildGuruDefinition(ctx, view, ident) + pos := span.New(definition.URI(), definition.Start(), definition.Start()) + result = &guru.Definition{ + ObjPos: fmt.Sprint(pos), + Desc: description, + } default: return fmt.Errorf("unknown emulation for definition: %s", d.query.Emulate) } @@ -108,82 +141,3 @@ func (d *definition) Run(ctx context.Context, args ...string) error { } return nil } - -func buildDefinition(ctx context.Context, view source.View, ident *source.IdentifierInfo) (*Definition, error) { - content, err := ident.Hover(ctx, nil) - if err != nil { - return nil, err - } - spn, err := ident.Declaration.Range.Span() - if err != nil { - return nil, err - } - return &Definition{ - Span: spn, - Description: content, - }, nil -} - -func buildGuruDefinition(ctx context.Context, view source.View, ident *source.IdentifierInfo) (*guru.Definition, error) { - spn, err := ident.Declaration.Range.Span() - if err != nil { - return nil, err - } - pkg := ident.File.GetPackage(ctx) - // guru does not support ranges - if !spn.IsPoint() { - spn = span.New(spn.URI(), spn.Start(), spn.Start()) - } - // Behavior that attempts to match the expected output for guru. For an example - // of the format, see the associated definition tests. - buf := &bytes.Buffer{} - q := types.RelativeTo(pkg.GetTypes()) - qualifyName := ident.Declaration.Object.Pkg() != pkg.GetTypes() - name := ident.Name - var suffix interface{} - switch obj := ident.Declaration.Object.(type) { - case *types.TypeName: - fmt.Fprint(buf, "type") - case *types.Var: - if obj.IsField() { - qualifyName = false - fmt.Fprint(buf, "field") - suffix = obj.Type() - } else { - fmt.Fprint(buf, "var") - } - case *types.Func: - fmt.Fprint(buf, "func") - typ := obj.Type() - if obj.Type() != nil { - if sig, ok := typ.(*types.Signature); ok { - buf := &bytes.Buffer{} - if recv := sig.Recv(); recv != nil { - if named, ok := recv.Type().(*types.Named); ok { - fmt.Fprintf(buf, "(%s).%s", named.Obj().Name(), name) - } - } - if buf.Len() == 0 { - buf.WriteString(name) - } - types.WriteSignature(buf, sig, q) - name = buf.String() - } - } - default: - fmt.Fprintf(buf, "unknown [%T]", obj) - } - fmt.Fprint(buf, " ") - if qualifyName { - fmt.Fprintf(buf, "%s.", ident.Declaration.Object.Pkg().Path()) - } - fmt.Fprint(buf, name) - if suffix != nil { - fmt.Fprint(buf, " ") - fmt.Fprint(buf, suffix) - } - return &guru.Definition{ - ObjPos: fmt.Sprint(spn), - Desc: buf.String(), - }, nil -} diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index cee64135..9e35cb39 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -85,25 +85,7 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag for _, diag := range diags { spn := span.Parse(diag.Pos) if spn.IsPoint() && diag.Kind == packages.TypeError { - // Don't set a range if it's anything other than a type error. - if diagFile, err := v.GetFile(ctx, spn.URI()); err == nil { - tok := diagFile.GetToken(ctx) - if tok == nil { - v.Logger().Errorf(ctx, "Could not matching tokens for diagnostic: %v", diagFile.URI()) - continue - } - content := diagFile.GetContent(ctx) - c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok) - s, err := spn.WithOffset(c) - //we just don't bother producing an error if this failed - if err == nil { - start := s.Start() - offset := start.Offset() - if l := bytes.IndexAny(content[offset:], " \n,():;[]"); l > 0 { - spn = span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+l, offset+l)) - } - } - } + spn = pointToSpan(ctx, v, spn) } diagnostic := Diagnostic{ Span: spn, @@ -142,6 +124,39 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag return reports, nil } +func pointToSpan(ctx context.Context, v View, spn span.Span) span.Span { + // Don't set a range if it's anything other than a type error. + diagFile, err := v.GetFile(ctx, spn.URI()) + if err != nil { + v.Logger().Errorf(ctx, "Could find file for diagnostic: %v", spn.URI()) + return spn + } + tok := diagFile.GetToken(ctx) + if tok == nil { + v.Logger().Errorf(ctx, "Could not find tokens for diagnostic: %v", spn.URI()) + return spn + } + content := diagFile.GetContent(ctx) + if content == nil { + v.Logger().Errorf(ctx, "Could not find content for diagnostic: %v", spn.URI()) + return spn + } + c := span.NewTokenConverter(diagFile.GetFileSet(ctx), tok) + s, err := spn.WithOffset(c) + //we just don't bother producing an error if this failed + if err != nil { + v.Logger().Errorf(ctx, "invalid span for diagnostic: %v: %v", spn.URI(), err) + return spn + } + start := s.Start() + offset := start.Offset() + width := bytes.IndexAny(content[offset:], " \n,():;[]") + if width <= 0 { + return spn + } + return span.New(spn.URI(), start, span.NewPoint(start.Line(), start.Column()+width, offset+width)) +} + func singleDiagnostic(uri span.URI, format string, a ...interface{}) map[span.URI][]Diagnostic { return map[span.URI][]Diagnostic{ uri: []Diagnostic{{