From 8ddc06776ea3bfd997f995105a4fa9a4e8a484d4 Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Tue, 12 May 2020 21:56:33 -0400 Subject: [PATCH] internal/lsp/source: don't link to packages matching GOPRIVATE in hover Currently, our hover text by default links point to public documentation sites (e.g. pkg.go.dev). This doesn't make sense for private repos, so hide the hovertext link when the import path matches GOPRIVATE. Implementing this was a little messy. To be optimal I had to thread the value of goprivate through cache.view, and to be correct I had to duplicate some code from cmd/go internal. Regtest will follow after https://golang.org/cl/232983 is submitted. Updates golang/go#36998 Change-Id: I1e556471bf919fea30132d9642426a08fdb7f434 Reviewed-on: https://go-review.googlesource.com/c/tools/+/233524 Run-TryBot: Robert Findley TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/cache/view.go | 66 ++++++++++++++++++++++++++++-- internal/lsp/source/hover.go | 11 +++-- internal/lsp/source/source_test.go | 2 +- internal/lsp/source/view.go | 4 ++ 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index be9e64aa26..673b1e72ea 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -13,6 +13,7 @@ import ( "io" "io/ioutil" "os" + "path" "path/filepath" "reflect" "strings" @@ -108,7 +109,7 @@ type view struct { goCommand bool // `go env` variables that need to be tracked. - gopath, gocache string + gopath, gocache, goprivate string // gocmdRunner guards go command calls from concurrency errors. gocmdRunner *gocommand.Runner @@ -709,10 +710,11 @@ func isSubdirectory(root, leaf string) bool { return err == nil && !strings.HasPrefix(rel, "..") } -// getGoEnv sets the view's build information's GOPATH, GOCACHE, and GOPACKAGESDRIVER values. -// It also returns the view's GOMOD value, which need not be cached. +// getGoEnv sets the view's build information's GOPATH, GOCACHE, GOPRIVATE, and +// GOPACKAGESDRIVER values. It also returns the view's GOMOD value, which need +// not be cached. func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) { - var gocache, gopath, gopackagesdriver bool + var gocache, gopath, gopackagesdriver, goprivate bool isGoCommand := func(gopackagesdriver string) bool { return gopackagesdriver == "" || gopackagesdriver == "off" } @@ -728,6 +730,9 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) { case "GOPATH": v.gopath = split[1] gopath = true + case "GOPRIVATE": + v.goprivate = split[1] + goprivate = true case "GOPACKAGESDRIVER": v.goCommand = isGoCommand(split[1]) gopackagesdriver = true @@ -762,6 +767,12 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) { return "", errors.New("unable to determine GOCACHE") } } + if !goprivate { + if goprivate, ok := envMap["GOPRIVATE"]; ok { + v.goprivate = goprivate + } + // No error here: GOPRIVATE is not essential. + } // The value of GOPACKAGESDRIVER is not returned through the go command. if !gopackagesdriver { v.goCommand = isGoCommand(os.Getenv("GOPACKAGESDRIVER")) @@ -770,6 +781,53 @@ func (v *view) getGoEnv(ctx context.Context, env []string) (string, error) { return gomod, nil } return "", nil + +} + +func (v *view) IsGoPrivatePath(target string) bool { + return globsMatchPath(v.goprivate, target) +} + +// Copied from +// https://cs.opensource.google/go/go/+/master:src/cmd/go/internal/str/path.go;l=58;drc=2910c5b4a01a573ebc97744890a07c1a3122c67a +func globsMatchPath(globs, target string) bool { + for globs != "" { + // Extract next non-empty glob in comma-separated list. + var glob string + if i := strings.Index(globs, ","); i >= 0 { + glob, globs = globs[:i], globs[i+1:] + } else { + glob, globs = globs, "" + } + if glob == "" { + continue + } + + // A glob with N+1 path elements (N slashes) needs to be matched + // against the first N+1 path elements of target, + // which end just before the N+1'th slash. + n := strings.Count(glob, "/") + prefix := target + // Walk target, counting slashes, truncating at the N+1'th slash. + for i := 0; i < len(target); i++ { + if target[i] == '/' { + if n == 0 { + prefix = target[:i] + break + } + n-- + } + } + if n > 0 { + // Not enough prefix elements. + continue + } + matched, _ := path.Match(glob, prefix) + if matched { + return true + } + } + return false } // This function will return the main go.mod file for this folder if it exists and whether the -modfile diff --git a/internal/lsp/source/hover.go b/internal/lsp/source/hover.go index 78f3721217..073aa3fa1e 100644 --- a/internal/lsp/source/hover.go +++ b/internal/lsp/source/hover.go @@ -58,7 +58,8 @@ func Hover(ctx context.Context, snapshot Snapshot, fh FileHandle, position proto if err != nil { return nil, err } - hover, err := FormatHover(h, snapshot.View().Options()) + isPrivate := h.Link != "" && snapshot.View().IsGoPrivatePath(h.Link) + hover, err := FormatHover(h, snapshot.View().Options(), isPrivate) if err != nil { return nil, err } @@ -330,7 +331,7 @@ func formatVar(node ast.Spec, obj types.Object, decl *ast.GenDecl) *HoverInforma return &HoverInformation{source: obj, comment: decl.Doc} } -func FormatHover(h *HoverInformation, options Options) (string, error) { +func FormatHover(h *HoverInformation, options Options, isPrivate bool) (string, error) { signature := h.Signature if signature != "" && options.PreferredContentFormat == protocol.Markdown { signature = fmt.Sprintf("```go\n%s\n```", signature) @@ -348,7 +349,10 @@ func FormatHover(h *HoverInformation, options Options) (string, error) { } return string(b), nil } - link := formatLink(h, options) + var link string + if !isPrivate { + link = formatLink(h, options) + } switch options.HoverKind { case SynopsisDocumentation: doc := formatDoc(h.Synopsis, options) @@ -374,7 +378,6 @@ func formatLink(h *HoverInformation, options Options) string { return plainLink } } - func formatDoc(doc string, options Options) string { if options.PreferredContentFormat == protocol.Markdown { return CommentToMarkdown(doc) diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 219388f156..4a0e9dde07 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -493,7 +493,7 @@ func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { if err != nil { t.Fatalf("failed for %v: %v", d.Src, err) } - hover, err := source.FormatHover(h, r.view.Options()) + hover, err := source.FormatHover(h, r.view.Options(), false) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index e204bd1027..24d692d366 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -152,6 +152,10 @@ type View interface { // user's workspace. In particular, if they are both outside of a module // and their GOPATH. ValidBuildConfiguration() bool + + // IsGoPrivatePath reports whether target is a private import path, as identified + // by the GOPRIVATE environment variable. + IsGoPrivatePath(path string) bool } // Session represents a single connection from a client.