From 48cfad2f5e0033b567e917e68582d9053bb571dc Mon Sep 17 00:00:00 2001 From: Rohan Challa Date: Tue, 18 Feb 2020 15:47:38 -0500 Subject: [PATCH] internal/lsp: support textDocument/documentLink for .mod extension This change implements support for textDocument/documentLink when it comes to go.mod files. Updates golang/go#36501 Change-Id: Ic0974e3e858dd1c8df54b7d7abee085bbcb6d4ee Reviewed-on: https://go-review.googlesource.com/c/tools/+/219938 Run-TryBot: Rohan Challa TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cache/mod.go | 205 +++++++++--------- internal/lsp/cache/session.go | 1 + internal/lsp/cache/snapshot.go | 15 ++ internal/lsp/link.go | 108 +++++++-- internal/lsp/mod/code_lens.go | 5 +- internal/lsp/source/view.go | 20 +- .../lsp/testdata/upgradedep/primarymod/go.mod | 8 +- .../testdata/upgradedep/summary.txt.golden | 2 +- internal/lsp/tests/tests.go | 8 +- 9 files changed, 243 insertions(+), 129 deletions(-) diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index 54a0749b23..447cf0c883 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -31,32 +31,26 @@ const ( SyntaxError = "syntax" ) -type parseModKey struct { - view string - gomod string +type modKey struct { cfg string -} - -type parseModHandle struct { - handle *memoize.Handle - file source.FileHandle - cfg *packages.Config + gomod string + view string } type modTidyKey struct { - view string - imports string - gomod string cfg string + gomod string + imports string + view string } -type modTidyHandle struct { +type modHandle struct { handle *memoize.Handle file source.FileHandle cfg *packages.Config } -type modTidyData struct { +type modData struct { memoize.NoCopy // origfh is the file handle for the original go.mod file. @@ -92,53 +86,77 @@ type modTidyData struct { err error } -func (pmh *parseModHandle) String() string { - return pmh.File().Identity().URI.Filename() +func (mh *modHandle) String() string { + return mh.File().Identity().URI.Filename() } -func (pmh *parseModHandle) File() source.FileHandle { - return pmh.file +func (mh *modHandle) File() source.FileHandle { + return mh.file } -func (pmh *parseModHandle) Upgrades(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, map[string]string, error) { - v := pmh.handle.Get(ctx) +func (mh *modHandle) Parse(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, error) { + v := mh.handle.Get(ctx) if v == nil { - return nil, nil, nil, errors.Errorf("no parsed file for %s", pmh.File().Identity().URI) + return nil, nil, errors.Errorf("no parsed file for %s", mh.File().Identity().URI) } - data := v.(*modTidyData) + data := v.(*modData) + return data.origParsedFile, data.origMapper, data.err +} + +func (mh *modHandle) Upgrades(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, map[string]string, error) { + v := mh.handle.Get(ctx) + if v == nil { + return nil, nil, nil, errors.Errorf("no parsed file for %s", mh.File().Identity().URI) + } + data := v.(*modData) return data.origParsedFile, data.origMapper, data.upgrades, data.err } -func (s *snapshot) ParseModHandle(ctx context.Context) (source.ParseModHandle, error) { - cfg := s.Config(ctx) - folder := s.View().Folder().Filename() +func (s *snapshot) ModHandle(ctx context.Context, fh source.FileHandle) source.ModHandle { + uri := fh.Identity().URI + if handle := s.getModHandle(uri); handle != nil { + return handle + } realURI, tempURI := s.view.ModFiles() - fh, err := s.GetFile(realURI) - if err != nil { - return nil, err - } + folder := s.View().Folder().Filename() + cfg := s.Config(ctx) - key := parseModKey{ - view: folder, - gomod: fh.Identity().String(), + key := modKey{ cfg: hashConfig(cfg), + gomod: fh.Identity().String(), + view: folder, } h := s.view.session.cache.store.Bind(key, func(ctx context.Context) interface{} { - ctx, done := trace.StartSpan(ctx, "cache.ParseModHandle", telemetry.File.Of(realURI)) + ctx, done := trace.StartSpan(ctx, "cache.ModHandle", telemetry.File.Of(uri)) defer done() - data := &modTidyData{} contents, _, err := fh.Read(ctx) if err != nil { - data.err = err - return data + return &modData{ + err: err, + } } - parsedFile, err := modfile.Parse(realURI.Filename(), contents, nil) + parsedFile, err := modfile.Parse(uri.Filename(), contents, nil) if err != nil { - data.err = err + return &modData{ + err: err, + } + } + data := &modData{ + origfh: fh, + origParsedFile: parsedFile, + origMapper: &protocol.ColumnMapper{ + URI: uri, + Converter: span.NewContentConverter(uri.Filename(), contents), + Content: contents, + }, + } + // If the go.mod file is not the view's go.mod file, then we just want to parse. + if uri != realURI { return data } + // If we have a tempModfile, copy the real go.mod file content into the temp go.mod file. if tempURI != "" { if err := ioutil.WriteFile(tempURI.Filename(), contents, os.ModePerm); err != nil { @@ -146,27 +164,22 @@ func (s *snapshot) ParseModHandle(ctx context.Context) (source.ParseModHandle, e return data } } - data = &modTidyData{ - origfh: fh, - origParsedFile: parsedFile, - origMapper: &protocol.ColumnMapper{ - URI: realURI, - Converter: span.NewContentConverter(realURI.Filename(), contents), - Content: contents, - }, - } + // Only get dependency upgrades if the go.mod file is the same as the view's. data.upgrades, data.err = dependencyUpgrades(ctx, cfg, folder, data) return data }) - return &parseModHandle{ + s.mu.Lock() + defer s.mu.Unlock() + s.modHandles[uri] = &modHandle{ handle: h, file: fh, cfg: cfg, - }, nil + } + return s.modHandles[uri] } -func dependencyUpgrades(ctx context.Context, cfg *packages.Config, folder string, modData *modTidyData) (map[string]string, error) { - if len(modData.origParsedFile.Require) == 0 { +func dependencyUpgrades(ctx context.Context, cfg *packages.Config, folder string, data *modData) (map[string]string, error) { + if len(data.origParsedFile.Require) == 0 { return nil, nil } // Run "go list -u -m all" to be able to see which deps can be upgraded. @@ -200,20 +213,12 @@ func dependencyUpgrades(ctx context.Context, cfg *packages.Config, folder string return upgrades, nil } -func (mth *modTidyHandle) String() string { - return mth.File().Identity().URI.Filename() -} - -func (mth *modTidyHandle) File() source.FileHandle { - return mth.file -} - -func (mth *modTidyHandle) Tidy(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, map[string]*modfile.Require, []source.Error, error) { - v := mth.handle.Get(ctx) +func (mh *modHandle) Tidy(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, map[string]*modfile.Require, []source.Error, error) { + v := mh.handle.Get(ctx) if v == nil { - return nil, nil, nil, nil, errors.Errorf("no parsed file for %s", mth.File().Identity().URI) + return nil, nil, nil, nil, errors.Errorf("no parsed file for %s", mh.File().Identity().URI) } - data := v.(*modTidyData) + data := v.(*modData) return data.origParsedFile, data.origMapper, data.missingDeps, data.parseErrors, data.err } @@ -241,7 +246,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) cfg: hashConfig(cfg), } h := s.view.session.cache.store.Bind(key, func(ctx context.Context) interface{} { - data := &modTidyData{} + data := &modData{} // Check the case when the tempModfile flag is turned off. if realURI == "" || tempURI == "" { @@ -306,7 +311,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) return data } - data = &modTidyData{ + data = &modData{ origfh: realfh, origParsedFile: origParsedFile, origMapper: realMapper, @@ -335,7 +340,7 @@ func (s *snapshot) ModTidyHandle(ctx context.Context, realfh source.FileHandle) } return data }) - return &modTidyHandle{ + return &modHandle{ handle: h, file: realfh, cfg: cfg, @@ -385,27 +390,27 @@ func extractModParseErrors(ctx context.Context, uri span.URI, m *protocol.Column // modRequireErrors extracts the errors that occur on the require directives. // It checks for directness issues and unused dependencies. -func modRequireErrors(ctx context.Context, options source.Options, modData *modTidyData) ([]source.Error, error) { +func modRequireErrors(ctx context.Context, options source.Options, data *modData) ([]source.Error, error) { var errors []source.Error - for dep, req := range modData.unusedDeps { + for dep, req := range data.unusedDeps { if req.Syntax == nil { continue } // Handle dependencies that are incorrectly labeled indirect and vice versa. - if modData.missingDeps[dep] != nil && req.Indirect != modData.missingDeps[dep].Indirect { - directErr, err := modDirectnessErrors(ctx, options, modData, req) + if data.missingDeps[dep] != nil && req.Indirect != data.missingDeps[dep].Indirect { + directErr, err := modDirectnessErrors(ctx, options, data, req) if err != nil { return nil, err } errors = append(errors, directErr) } // Handle unused dependencies. - if modData.missingDeps[dep] == nil { - rng, err := rangeFromPositions(modData.origfh.Identity().URI, modData.origMapper, req.Syntax.Start, req.Syntax.End) + if data.missingDeps[dep] == nil { + rng, err := rangeFromPositions(data.origfh.Identity().URI, data.origMapper, req.Syntax.Start, req.Syntax.End) if err != nil { return nil, err } - edits, err := dropDependencyEdits(ctx, options, modData, req) + edits, err := dropDependencyEdits(ctx, options, data, req) if err != nil { return nil, err } @@ -413,10 +418,10 @@ func modRequireErrors(ctx context.Context, options source.Options, modData *modT Category: ModTidyError, Message: fmt.Sprintf("%s is not used in this module.", dep), Range: rng, - URI: modData.origfh.Identity().URI, + URI: data.origfh.Identity().URI, SuggestedFixes: []source.SuggestedFix{{ Title: fmt.Sprintf("Remove dependency: %s", dep), - Edits: map[span.URI][]protocol.TextEdit{modData.origfh.Identity().URI: edits}, + Edits: map[span.URI][]protocol.TextEdit{data.origfh.Identity().URI: edits}, }}, }) } @@ -425,8 +430,8 @@ func modRequireErrors(ctx context.Context, options source.Options, modData *modT } // modDirectnessErrors extracts errors when a dependency is labeled indirect when it should be direct and vice versa. -func modDirectnessErrors(ctx context.Context, options source.Options, modData *modTidyData, req *modfile.Require) (source.Error, error) { - rng, err := rangeFromPositions(modData.origfh.Identity().URI, modData.origMapper, req.Syntax.Start, req.Syntax.End) +func modDirectnessErrors(ctx context.Context, options source.Options, data *modData, req *modfile.Require) (source.Error, error) { + rng, err := rangeFromPositions(data.origfh.Identity().URI, data.origMapper, req.Syntax.Start, req.Syntax.End) if err != nil { return source.Error{}, err } @@ -436,12 +441,12 @@ func modDirectnessErrors(ctx context.Context, options source.Options, modData *m end := comments.Suffix[0].Start end.LineRune += len(comments.Suffix[0].Token) end.Byte += len([]byte(comments.Suffix[0].Token)) - rng, err = rangeFromPositions(modData.origfh.Identity().URI, modData.origMapper, comments.Suffix[0].Start, end) + rng, err = rangeFromPositions(data.origfh.Identity().URI, data.origMapper, comments.Suffix[0].Start, end) if err != nil { return source.Error{}, err } } - edits, err := changeDirectnessEdits(ctx, options, modData, req, false) + edits, err := changeDirectnessEdits(ctx, options, data, req, false) if err != nil { return source.Error{}, err } @@ -449,15 +454,15 @@ func modDirectnessErrors(ctx context.Context, options source.Options, modData *m Category: ModTidyError, Message: fmt.Sprintf("%s should be a direct dependency.", req.Mod.Path), Range: rng, - URI: modData.origfh.Identity().URI, + URI: data.origfh.Identity().URI, SuggestedFixes: []source.SuggestedFix{{ Title: fmt.Sprintf("Make %s direct", req.Mod.Path), - Edits: map[span.URI][]protocol.TextEdit{modData.origfh.Identity().URI: edits}, + Edits: map[span.URI][]protocol.TextEdit{data.origfh.Identity().URI: edits}, }}, }, nil } // If the dependency should be indirect, add the // indirect. - edits, err := changeDirectnessEdits(ctx, options, modData, req, true) + edits, err := changeDirectnessEdits(ctx, options, data, req, true) if err != nil { return source.Error{}, err } @@ -465,10 +470,10 @@ func modDirectnessErrors(ctx context.Context, options source.Options, modData *m Category: ModTidyError, Message: fmt.Sprintf("%s should be an indirect dependency.", req.Mod.Path), Range: rng, - URI: modData.origfh.Identity().URI, + URI: data.origfh.Identity().URI, SuggestedFixes: []source.SuggestedFix{{ Title: fmt.Sprintf("Make %s indirect", req.Mod.Path), - Edits: map[span.URI][]protocol.TextEdit{modData.origfh.Identity().URI: edits}, + Edits: map[span.URI][]protocol.TextEdit{data.origfh.Identity().URI: edits}, }}, }, nil } @@ -485,20 +490,20 @@ func modDirectnessErrors(ctx context.Context, options source.Options, modData *m // module t // // go 1.11 -func dropDependencyEdits(ctx context.Context, options source.Options, modData *modTidyData, req *modfile.Require) ([]protocol.TextEdit, error) { - if err := modData.origParsedFile.DropRequire(req.Mod.Path); err != nil { +func dropDependencyEdits(ctx context.Context, options source.Options, data *modData, req *modfile.Require) ([]protocol.TextEdit, error) { + if err := data.origParsedFile.DropRequire(req.Mod.Path); err != nil { return nil, err } - modData.origParsedFile.Cleanup() - newContents, err := modData.origParsedFile.Format() + data.origParsedFile.Cleanup() + newContents, err := data.origParsedFile.Format() if err != nil { return nil, err } // Reset the *modfile.File back to before we dropped the dependency. - modData.origParsedFile.AddNewRequire(req.Mod.Path, req.Mod.Version, req.Indirect) + data.origParsedFile.AddNewRequire(req.Mod.Path, req.Mod.Version, req.Indirect) // Calculate the edits to be made due to the change. - diff := options.ComputeEdits(modData.origfh.Identity().URI, string(modData.origMapper.Content), string(newContents)) - edits, err := source.ToProtocolEdits(modData.origMapper, diff) + diff := options.ComputeEdits(data.origfh.Identity().URI, string(data.origMapper.Content), string(newContents)) + edits, err := source.ToProtocolEdits(data.origMapper, diff) if err != nil { return nil, err } @@ -519,34 +524,34 @@ func dropDependencyEdits(ctx context.Context, options source.Options, modData *m // go 1.11 // // require golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee // indirect -func changeDirectnessEdits(ctx context.Context, options source.Options, modData *modTidyData, req *modfile.Require, indirect bool) ([]protocol.TextEdit, error) { +func changeDirectnessEdits(ctx context.Context, options source.Options, data *modData, req *modfile.Require, indirect bool) ([]protocol.TextEdit, error) { var newReq []*modfile.Require prevIndirect := false // Change the directness in the matching require statement. - for _, r := range modData.origParsedFile.Require { + for _, r := range data.origParsedFile.Require { if req.Mod.Path == r.Mod.Path { prevIndirect = req.Indirect req.Indirect = indirect } newReq = append(newReq, r) } - modData.origParsedFile.SetRequire(newReq) - modData.origParsedFile.Cleanup() - newContents, err := modData.origParsedFile.Format() + data.origParsedFile.SetRequire(newReq) + data.origParsedFile.Cleanup() + newContents, err := data.origParsedFile.Format() if err != nil { return nil, err } // Change the dependency back to the way it was before we got the newContents. - for _, r := range modData.origParsedFile.Require { + for _, r := range data.origParsedFile.Require { if req.Mod.Path == r.Mod.Path { req.Indirect = prevIndirect } newReq = append(newReq, r) } - modData.origParsedFile.SetRequire(newReq) + data.origParsedFile.SetRequire(newReq) // Calculate the edits to be made due to the change. - diff := options.ComputeEdits(modData.origfh.Identity().URI, string(modData.origMapper.Content), string(newContents)) - edits, err := source.ToProtocolEdits(modData.origMapper, diff) + diff := options.ComputeEdits(data.origfh.Identity().URI, string(data.origMapper.Content), string(newContents)) + edits, err := source.ToProtocolEdits(data.origMapper, diff) if err != nil { return nil, err } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 9df76aa34e..eceb84d675 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -125,6 +125,7 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, actions: make(map[actionKey]*actionHandle), workspacePackages: make(map[packageID]packagePath), unloadableFiles: make(map[span.URI]struct{}), + modHandles: make(map[span.URI]*modHandle), }, ignoredURIs: make(map[span.URI]struct{}), } diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index af8bbbdeac..4fc51093b3 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -57,6 +57,9 @@ type snapshot struct { // unloadableFiles keeps track of files that we've failed to load. unloadableFiles map[span.URI]struct{} + + // modHandles keeps track of any ParseModHandles for this snapshot. + modHandles map[span.URI]*modHandle } type packageKey struct { @@ -218,6 +221,12 @@ func (s *snapshot) transitiveReverseDependencies(id packageID, ids map[packageID } } +func (s *snapshot) getModHandle(uri span.URI) *modHandle { + s.mu.Lock() + defer s.mu.Unlock() + return s.modHandles[uri] +} + func (s *snapshot) getImportedBy(id packageID) []packageID { s.mu.Lock() defer s.mu.Unlock() @@ -612,6 +621,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi files: make(map[span.URI]source.FileHandle), workspacePackages: make(map[packageID]packagePath), unloadableFiles: make(map[span.URI]struct{}), + modHandles: make(map[span.URI]*modHandle), } // Copy all of the FileHandles. @@ -622,6 +632,10 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi for k, v := range s.unloadableFiles { result.unloadableFiles[k] = v } + // Copy all of the modHandles. + for k, v := range s.modHandles { + result.modHandles[k] = v + } // transitiveIDs keeps track of transitive reverse dependencies. // If an ID is present in the map, invalidate its types. @@ -649,6 +663,7 @@ func (s *snapshot) clone(ctx context.Context, withoutURIs map[span.URI]source.Fi for id := range s.workspacePackages { directIDs[id] = struct{}{} } + delete(result.modHandles, withoutURI) } // If this is a file we don't yet know about, diff --git a/internal/lsp/link.go b/internal/lsp/link.go index ed14a20897..ae7a0a51d1 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -5,6 +5,7 @@ package lsp import ( + "bytes" "context" "fmt" "go/ast" @@ -23,11 +24,68 @@ import ( func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) { // TODO(golang/go#36501): Support document links for go.mod files. - snapshot, fh, ok, err := s.beginFileRequest(params.TextDocument.URI, source.Go) + snapshot, fh, ok, err := s.beginFileRequest(params.TextDocument.URI, source.UnknownKind) if !ok { return nil, err } + switch fh.Identity().Kind { + case source.Mod: + return modLinks(ctx, snapshot, fh) + case source.Go: + return goLinks(ctx, snapshot.View(), fh) + } + return nil, nil +} + +func modLinks(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]protocol.DocumentLink, error) { view := snapshot.View() + + file, m, err := snapshot.ModHandle(ctx, fh).Parse(ctx) + if err != nil { + return nil, err + } + var links []protocol.DocumentLink + for _, req := range file.Require { + dep := []byte(req.Mod.Path) + s, e := req.Syntax.Start.Byte, req.Syntax.End.Byte + i := bytes.Index(m.Content[s:e], dep) + if i == -1 { + continue + } + // Shift the start position to the location of the + // dependency within the require statement. + start, end := token.Pos(s+i), token.Pos(s+i+len(dep)) + target := fmt.Sprintf("https://%s/mod/%s", view.Options().LinkTarget, req.Mod.String()) + if l, err := toProtocolLink(view, m, target, start, end, source.Mod); err == nil { + links = append(links, l) + } else { + log.Error(ctx, "failed to create protocol link", err) + } + } + // TODO(ridersofrohan): handle links for replace and exclude directives + if syntax := file.Syntax; syntax == nil { + return links, nil + } + // Get all the links that are contained in the comments of the file. + for _, expr := range file.Syntax.Stmt { + comments := expr.Comment() + if comments == nil { + continue + } + for _, cmt := range comments.Before { + links = append(links, findLinksInString(ctx, view, cmt.Token, token.Pos(cmt.Start.Byte), m, source.Mod)...) + } + for _, cmt := range comments.Suffix { + links = append(links, findLinksInString(ctx, view, cmt.Token, token.Pos(cmt.Start.Byte), m, source.Mod)...) + } + for _, cmt := range comments.After { + links = append(links, findLinksInString(ctx, view, cmt.Token, token.Pos(cmt.Start.Byte), m, source.Mod)...) + } + } + return links, nil +} + +func goLinks(ctx context.Context, view source.View, fh source.FileHandle) ([]protocol.DocumentLink, error) { phs, err := view.Snapshot().PackageHandles(ctx, fh) if err != nil { return nil, err @@ -52,7 +110,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink target = fmt.Sprintf("https://%s/%s", view.Options().LinkTarget, target) // Account for the quotation marks in the positions. start, end := n.Path.Pos()+1, n.Path.End()-1 - if l, err := toProtocolLink(view, m, target, start, end); err == nil { + if l, err := toProtocolLink(view, m, target, start, end, source.Go); err == nil { links = append(links, l) } else { log.Error(ctx, "failed to create protocol link", err) @@ -62,7 +120,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink case *ast.BasicLit: // Look for links in string literals. if n.Kind == token.STRING { - links = append(links, findLinksInString(ctx, view, n.Value, n.Pos(), m)...) + links = append(links, findLinksInString(ctx, view, n.Value, n.Pos(), m, source.Go)...) } return false } @@ -71,7 +129,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink // Look for links in comments. for _, commentGroup := range file.Comments { for _, comment := range commentGroup.List { - links = append(links, findLinksInString(ctx, view, comment.Text, comment.Pos(), m)...) + links = append(links, findLinksInString(ctx, view, comment.Text, comment.Pos(), m, source.Go)...) } } return links, nil @@ -96,7 +154,7 @@ func moduleAtVersion(ctx context.Context, target string, ph source.PackageHandle return modpath, version, true } -func findLinksInString(ctx context.Context, view source.View, src string, pos token.Pos, m *protocol.ColumnMapper) []protocol.DocumentLink { +func findLinksInString(ctx context.Context, view source.View, src string, pos token.Pos, m *protocol.ColumnMapper, fileKind source.FileKind) []protocol.DocumentLink { var links []protocol.DocumentLink for _, index := range view.Options().URLRegexp.FindAllIndex([]byte(src), -1) { start, end := index[0], index[1] @@ -111,7 +169,7 @@ func findLinksInString(ctx context.Context, view source.View, src string, pos to if url.Scheme == "" { url.Scheme = "https" } - l, err := toProtocolLink(view, m, url.String(), startPos, endPos) + l, err := toProtocolLink(view, m, url.String(), startPos, endPos, fileKind) if err != nil { log.Error(ctx, "failed to create protocol link", err) continue @@ -130,7 +188,7 @@ func findLinksInString(ctx context.Context, view source.View, src string, pos to } org, repo, number := matches[1], matches[2], matches[3] target := fmt.Sprintf("https://github.com/%s/%s/issues/%s", org, repo, number) - l, err := toProtocolLink(view, m, target, startPos, endPos) + l, err := toProtocolLink(view, m, target, startPos, endPos, fileKind) if err != nil { log.Error(ctx, "failed to create protocol link", err) continue @@ -152,14 +210,34 @@ var ( issueRegexp *regexp.Regexp ) -func toProtocolLink(view source.View, m *protocol.ColumnMapper, target string, start, end token.Pos) (protocol.DocumentLink, error) { - spn, err := span.NewRange(view.Session().Cache().FileSet(), start, end).Span() - if err != nil { - return protocol.DocumentLink{}, err - } - rng, err := m.Range(spn) - if err != nil { - return protocol.DocumentLink{}, err +func toProtocolLink(view source.View, m *protocol.ColumnMapper, target string, start, end token.Pos, fileKind source.FileKind) (protocol.DocumentLink, error) { + var rng protocol.Range + switch fileKind { + case source.Go: + spn, err := span.NewRange(view.Session().Cache().FileSet(), start, end).Span() + if err != nil { + return protocol.DocumentLink{}, err + } + rng, err = m.Range(spn) + if err != nil { + return protocol.DocumentLink{}, err + } + case source.Mod: + s, e := int(start), int(end) + line, col, err := m.Converter.ToPosition(s) + if err != nil { + return protocol.DocumentLink{}, err + } + start := span.NewPoint(line, col, s) + line, col, err = m.Converter.ToPosition(e) + if err != nil { + return protocol.DocumentLink{}, err + } + end := span.NewPoint(line, col, e) + rng, err = m.Range(span.New(m.URI, start, end)) + if err != nil { + return protocol.DocumentLink{}, err + } } return protocol.DocumentLink{ Range: rng, diff --git a/internal/lsp/mod/code_lens.go b/internal/lsp/mod/code_lens.go index 4217f82014..bb087882ac 100644 --- a/internal/lsp/mod/code_lens.go +++ b/internal/lsp/mod/code_lens.go @@ -13,7 +13,6 @@ import ( func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]protocol.CodeLens, error) { realURI, _ := snapshot.View().ModFiles() - // Check the case when the tempModfile flag is turned off. if realURI == "" { return nil, nil } @@ -24,11 +23,11 @@ func CodeLens(ctx context.Context, snapshot source.Snapshot, uri span.URI) ([]pr ctx, done := trace.StartSpan(ctx, "mod.CodeLens", telemetry.File.Of(realURI)) defer done() - pmh, err := snapshot.ParseModHandle(ctx) + fh, err := snapshot.GetFile(realURI) if err != nil { return nil, err } - f, m, upgrades, err := pmh.Upgrades(ctx) + f, m, upgrades, err := snapshot.ModHandle(ctx, fh).Upgrades(ctx) if err != nil { return nil, err } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 23e5872183..ede4665a90 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -51,9 +51,9 @@ type Snapshot interface { // This function can have no data or error if there is no modfile detected. ModTidyHandle(ctx context.Context, fh FileHandle) (ModTidyHandle, error) - // ParseModHandle returns a ParseModHandle for the view's go.mod file handle. - // This function can have no data or error if there is no modfile detected. - ParseModHandle(ctx context.Context) (ParseModHandle, error) + // ModHandle returns a ModHandle for the passed in go.mod file handle. + // This function can have no data if there is no modfile detected. + ModHandle(ctx context.Context, fh FileHandle) ModHandle // PackageHandles returns the PackageHandles for the packages that this file // belongs to. @@ -258,17 +258,23 @@ type ParseGoHandle interface { Cached() (file *ast.File, src []byte, m *protocol.ColumnMapper, parseErr error, err error) } -// ParseModHandle represents a handle to the modfile for a go.mod. -type ParseModHandle interface { +// ModHandle represents a handle to the modfile for a go.mod. +type ModHandle interface { // File returns a file handle for which to get the modfile. File() FileHandle + // Parse returns the parsed modfile and a mapper for the go.mod file. + // If the file is not available, returns nil and an error. + Parse(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, error) + // Upgrades returns the parsed modfile, a mapper, and any dependency upgrades - // for the go.mod file. If the file is not available, returns nil and an error. + // for the go.mod file. Note that this will only work if the go.mod is the view's go.mod. + // If the file is not available, returns nil and an error. Upgrades(ctx context.Context) (*modfile.File, *protocol.ColumnMapper, map[string]string, error) } -// ModTidyHandle represents a handle to the modfile for a go.mod. +// ModTidyHandle represents a handle to the modfile for the view. +// Specifically for the purpose of getting diagnostics by running "go mod tidy". type ModTidyHandle interface { // File returns a file handle for which to get the modfile. File() FileHandle diff --git a/internal/lsp/testdata/upgradedep/primarymod/go.mod b/internal/lsp/testdata/upgradedep/primarymod/go.mod index 7b1239f4b7..ac1ed82303 100644 --- a/internal/lsp/testdata/upgradedep/primarymod/go.mod +++ b/internal/lsp/testdata/upgradedep/primarymod/go.mod @@ -1,5 +1,11 @@ module upgradedep +// TODO(microsoft/vscode-go#12): Another issue. //@link(`microsoft/vscode-go#12`, `https://github.com/microsoft/vscode-go/issues/12`) + go 1.12 -require example.com/extramodule v1.0.0 //@codelens("require example.com/extramodule v1.0.0", "Upgrade dependency to v1.1.0", "upgrade.dependency") +// TODO(golang/go#1234): Link the relevant issue. //@link(`golang/go#1234`, `https://github.com/golang/go/issues/1234`) + +require example.com/extramodule v1.0.0 //@link(`example.com/extramodule`, `https://pkg.go.dev/mod/example.com/extramodule@v1.0.0`),codelens("require example.com/extramodule v1.0.0", "Upgrade dependency to v1.1.0", "upgrade.dependency") + +// https://example.com/comment: Another issue. //@link(`https://example.com/comment`,`https://example.com/comment`) diff --git a/internal/lsp/testdata/upgradedep/summary.txt.golden b/internal/lsp/testdata/upgradedep/summary.txt.golden index e3c0e4d173..7ae33ebc61 100644 --- a/internal/lsp/testdata/upgradedep/summary.txt.golden +++ b/internal/lsp/testdata/upgradedep/summary.txt.golden @@ -23,6 +23,6 @@ WorkspaceSymbolsCount = 0 FuzzyWorkspaceSymbolsCount = 0 CaseSensitiveWorkspaceSymbolsCount = 0 SignaturesCount = 0 -LinksCount = 0 +LinksCount = 4 ImplementationsCount = 0 diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 741df0fb08..6e863a7f34 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -702,8 +702,12 @@ func Run(t *testing.T, tests Tests, data *Data) { t.Helper() for uri, wantLinks := range data.Links { // If we are testing GOPATH, then we do not want links with - // the versions attached (pkg.go.dev/repoa/moda@v1.1.0/pkg). + // the versions attached (pkg.go.dev/repoa/moda@v1.1.0/pkg), + // unless the file is a go.mod, then we can skip it alltogether. if data.Exported.Exporter == packagestest.GOPATH { + if strings.HasSuffix(uri.Filename(), ".mod") { + continue + } re := regexp.MustCompile(`@v\d+\.\d+\.[\w-]+`) for i, link := range wantLinks { wantLinks[i].Target = re.ReplaceAllString(link.Target, "") @@ -1223,7 +1227,7 @@ func shouldSkip(data *Data, uri span.URI) bool { } // If the -modfile flag is not available, then we do not want to run // any tests on the go.mod file. - if strings.Contains(uri.Filename(), ".mod") { + if strings.HasSuffix(uri.Filename(), ".mod") { return true } // If the -modfile flag is not available, then we do not want to test any