From 6b505debf4bcab2f8b753e693d7cb0dbfacdf987 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 24 Dec 2019 01:49:21 -0500 Subject: [PATCH] gopls: use mvdan.cc/xurls for textDocument/documentLink Our current implementation isn't robust, and it doesn't seem worth it to invest significant effort in improving it when this library exists. Also, make the protocol part of the default URL regex non-optional, as the alternative is that any string of the format "foo.bar" will appear to be a link. Updates golang/go#33505 Change-Id: Ia430a1c193eded394f8af12050bdd4dc2a9ccc94 Reviewed-on: https://go-review.googlesource.com/c/tools/+/212517 Reviewed-by: Heschi Kreinick Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot --- gopls/go.mod | 1 + gopls/go.sum | 4 ++-- gopls/internal/hooks/hooks.go | 2 ++ internal/lsp/link.go | 31 ++++--------------------------- internal/lsp/source/options.go | 3 +++ 5 files changed, 12 insertions(+), 29 deletions(-) diff --git a/gopls/go.mod b/gopls/go.mod index ceb6a44e37..afdeb6508d 100644 --- a/gopls/go.mod +++ b/gopls/go.mod @@ -7,6 +7,7 @@ require ( github.com/stretchr/testify v1.4.0 // indirect golang.org/x/tools v0.0.0-20191017151554-a3bc800455d5 honnef.co/go/tools v0.0.1-2019.2.3 + mvdan.cc/xurls/v2 v2.1.0 ) replace golang.org/x/tools => ../ diff --git a/gopls/go.sum b/gopls/go.sum index c076cc271f..f935705aac 100644 --- a/gopls/go.sum +++ b/gopls/go.sum @@ -30,8 +30,6 @@ golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc= -golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= @@ -42,3 +40,5 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= honnef.co/go/tools v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM= honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= +mvdan.cc/xurls/v2 v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA= +mvdan.cc/xurls/v2 v2.1.0/go.mod h1:5GrSd9rOnKOpZaji1OZLYL/yeAAtGDlo/cFe+8K5n8E= diff --git a/gopls/internal/hooks/hooks.go b/gopls/internal/hooks/hooks.go index 9037c3b345..6238da6051 100644 --- a/gopls/internal/hooks/hooks.go +++ b/gopls/internal/hooks/hooks.go @@ -9,11 +9,13 @@ package hooks // import "golang.org/x/tools/gopls/internal/hooks" import ( "golang.org/x/tools/internal/lsp/source" + "mvdan.cc/xurls/v2" ) func Options(options *source.Options) { if options.GoDiff { options.ComputeEdits = ComputeEdits } + options.URLRegexp = xurls.Relaxed() updateAnalyzers(options) } diff --git a/internal/lsp/link.go b/internal/lsp/link.go index 66ded42c19..ce748e52b5 100644 --- a/internal/lsp/link.go +++ b/internal/lsp/link.go @@ -9,16 +9,13 @@ import ( "fmt" "go/ast" "go/token" - "regexp" "strconv" - "sync" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" "golang.org/x/tools/internal/telemetry/log" "golang.org/x/tools/internal/telemetry/tag" - errors "golang.org/x/xerrors" ) func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLinkParams) ([]protocol.DocumentLink, error) { @@ -62,7 +59,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink if n.Kind != token.STRING { return false } - l, err := findLinksInString(n.Value, n.Pos(), view, m) + l, err := findLinksInString(view, n.Value, n.Pos(), m) if err != nil { log.Error(ctx, "cannot find links in string", err) return false @@ -75,7 +72,7 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink for _, commentGroup := range file.Comments { for _, comment := range commentGroup.List { - l, err := findLinksInString(comment.Text, comment.Pos(), view, m) + l, err := findLinksInString(view, comment.Text, comment.Pos(), m) if err != nil { log.Error(ctx, "cannot find links in comment", err) continue @@ -87,14 +84,9 @@ func (s *Server) documentLink(ctx context.Context, params *protocol.DocumentLink return links, nil } -func findLinksInString(src string, pos token.Pos, view source.View, mapper *protocol.ColumnMapper) ([]protocol.DocumentLink, error) { +func findLinksInString(view source.View, src string, pos token.Pos, mapper *protocol.ColumnMapper) ([]protocol.DocumentLink, error) { var links []protocol.DocumentLink - re, err := getURLRegexp() - if err != nil { - return nil, errors.Errorf("cannot create regexp for links: %s", err.Error()) - } - indexUrl := re.FindAllIndex([]byte(src), -1) - for _, urlIndex := range indexUrl { + for _, urlIndex := range view.Options().URLRegexp.FindAllIndex([]byte(src), -1) { var target string start := urlIndex[0] end := urlIndex[1] @@ -110,21 +102,6 @@ func findLinksInString(src string, pos token.Pos, view source.View, mapper *prot return links, nil } -const urlRegexpString = "((http|ftp|https)://)?([\\w_-]+(?:(?:\\.[\\w_-]+)+))([\\w.,@?^=%&:/~+#-]*[\\w@?^=%&/~+#-])?" - -var ( - urlRegexp *regexp.Regexp - regexpOnce sync.Once - regexpErr error -) - -func getURLRegexp() (*regexp.Regexp, error) { - regexpOnce.Do(func() { - urlRegexp, regexpErr = regexp.Compile(urlRegexpString) - }) - return urlRegexp, regexpErr -} - func toProtocolLink(view source.View, mapper *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 { diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 1ede059d5b..2d0aa7f49a 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -7,6 +7,7 @@ package source import ( "fmt" "os" + "regexp" "time" "golang.org/x/tools/go/analysis" @@ -68,6 +69,7 @@ var ( Budget: 100 * time.Millisecond, }, ComputeEdits: myers.ComputeEdits, + URLRegexp: regexp.MustCompile(`(http|ftp|https)://([\w_-]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:/~+#-]*[\w@?^=%&/~+#-])?`), Analyzers: defaultAnalyzers, GoDiff: true, LinkTarget: "pkg.go.dev", @@ -106,6 +108,7 @@ type Options struct { Completion CompletionOptions ComputeEdits diff.ComputeEdits + URLRegexp *regexp.Regexp Analyzers map[string]*analysis.Analyzer