diff --git a/buildserver/bench_test.go b/buildserver/bench_test.go index 9f664666..c643e66d 100644 --- a/buildserver/bench_test.go +++ b/buildserver/bench_test.go @@ -126,7 +126,7 @@ func BenchmarkIntegration(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { b.StopTimer() - c, done := connectionToNewBuildServer(string(rootURI), b) + c, done := connectionToNewBuildServer(string(rootURI), b, true) b.StartTimer() if err := c.Call(ctx, "initialize", lspext.ClientProxyInitializeParams{ @@ -161,7 +161,7 @@ func BenchmarkIntegration(b *testing.B) { for i := 0; i < b.N; i++ { b.StopTimer() - c, done := connectionToNewBuildServer(string(rootURI), b) + c, done := connectionToNewBuildServer(string(rootURI), b, true) b.StartTimer() if err := c.Call(ctx, "initialize", lspext.ClientProxyInitializeParams{ @@ -270,10 +270,10 @@ func BenchmarkIntegrationShared(b *testing.B) { for i := 0; i < b.N; i++ { b.StopTimer() - c, done1 := connectionToNewBuildServer(test.oldRootURI, b) + c, done1 := connectionToNewBuildServer(test.oldRootURI, b, true) defer done1() // TODO ensure we close between each loop do(c, test.oldRootURI) - c, done2 := connectionToNewBuildServer(test.rootURI, b) + c, done2 := connectionToNewBuildServer(test.rootURI, b, true) defer done2() b.StartTimer() diff --git a/buildserver/build_server.go b/buildserver/build_server.go index aea08c83..c3363aef 100644 --- a/buildserver/build_server.go +++ b/buildserver/build_server.go @@ -77,10 +77,16 @@ type BuildHandler struct { findPkg map[findPkgKey]*findPkgValue langserver.HandlerCommon *langserver.HandlerShared - init *lspext.InitializeParams // set by "initialize" request - rootImportPath string // root import path of the workspace (e.g., "github.com/foo/bar") - cachingClient *http.Client // http.Client with a cache backed by an in-memory LRU cache - closers []io.Closer // values to dispose of when Close() is called + init *lspext.InitializeParams // set by "initialize" request + originalRootURI *url.URL // derived from InitializeParams.OriginalRootURI + rootImportPath string // root import path of the workspace (e.g., "github.com/foo/bar") + cachingClient *http.Client // http.Client with a cache backed by an in-memory LRU cache + closers []io.Closer // values to dispose of when Close() is called + // Whether URIs in the same workspace begin with: + // - `file://` (true) + // - `git://` (false) + // This affects URI rewriting between the client and server. + clientUsesFileSchemeWithinWorkspace bool } // reset clears all internal state in h. @@ -96,6 +102,11 @@ func (h *BuildHandler) reset(init *lspext.InitializeParams, conn *jsonrpc2.Conn, return err } h.init = init + var err error + h.originalRootURI, err = url.Parse(string(h.init.OriginalRootURI)) + if h.init.OriginalRootURI == "" || err != nil { + h.originalRootURI = nil + } // 100 MiB cache, no age-based eviction h.cachingClient = &http.Client{Transport: httpcache.NewTransport(lrucache.New(100*1024*1024, 0))} h.depURLMutex = newKeyMutex() @@ -178,6 +189,23 @@ func (h *BuildHandler) Handle(ctx context.Context, conn *jsonrpc2.Conn, req *jso return nil, err } + // In the `rootUri`, clients can send either: + // + // - A `file://` URI, which indicates that: + // - Same-workspace file paths will also be `file://` URIs + // - Out-of-workspace file paths will be `git://` URIs + // - `originalRootUri` is present + // - A `git://` URI, which indicates that: + // - Both same-workspace and out-of-workspace file paths will be non-`file://` URIs + // - `originalRootUri` is absent and `rootUri` contains the original root URI + if strings.HasPrefix(string(params.RootURI), "file://") { + h.clientUsesFileSchemeWithinWorkspace = true + } else { + params.OriginalRootURI = params.RootURI + params.RootURI = "file:///" + h.clientUsesFileSchemeWithinWorkspace = false + } + if Debug { var b []byte if req.Params != nil { @@ -327,10 +355,26 @@ func (h *BuildHandler) Handle(ctx context.Context, conn *jsonrpc2.Conn, req *jso } } rewriteURIFromClient := func(uri lsp.DocumentURI) lsp.DocumentURI { - if !strings.HasPrefix(string(uri), "file:///") { - return uri // refers to a resource outside of this workspace + var path string + if h.clientUsesFileSchemeWithinWorkspace { + if !strings.HasPrefix(string(uri), "file:///") { + return uri // refers to a resource outside of this workspace + } + path = strings.TrimPrefix(string(uri), "file://") + } else { + currentURL, err := url.Parse(string(uri)) + if err != nil { + return uri + } + if h.originalRootURI == nil { + return uri + } + path = currentURL.Fragment + currentURL.Fragment = "" + if *currentURL != *h.originalRootURI { + return uri // refers to a resource outside of this workspace + } } - path := strings.TrimPrefix(string(uri), "file://") path = pathpkg.Join(h.RootFSPath, path) if !util.PathHasPrefix(path, h.RootFSPath) { panic(fmt.Sprintf("file path %q must have prefix %q (file URI is %q, root URI is %q)", path, h.RootFSPath, uri, h.init.RootPath)) @@ -472,21 +516,37 @@ func (h *BuildHandler) rewriteURIFromLangServer(uri lsp.DocumentURI) (lsp.Docume if util.PathHasPrefix(u.Path, goroot) { fileInGoStdlib := util.PathTrimPrefix(u.Path, goroot) if h.rootImportPath == "" { - // The workspace is the Go stdlib and this refers to - // something in the Go stdlib, so let's use file:/// - // so that the client adds our current rev, instead - // of using runtime.Version() (which is not - // necessarily the commit of the Go stdlib we're - // analyzing). - return lsp.DocumentURI("file:///" + fileInGoStdlib), nil + if h.clientUsesFileSchemeWithinWorkspace { + // The workspace is the Go stdlib and this refers to + // something in the Go stdlib, so let's use file:/// + // so that the client adds our current rev, instead + // of using runtime.Version() (which is not + // necessarily the commit of the Go stdlib we're + // analyzing). + return lsp.DocumentURI("file:///" + fileInGoStdlib), nil + } + if h.originalRootURI == nil { + return uri, nil + } + newURI, _ := url.Parse(h.originalRootURI.String()) + newURI.Fragment = fileInGoStdlib + return lsp.DocumentURI(newURI.String()), nil } return lsp.DocumentURI("git://github.com/golang/go?" + gosrc.RuntimeVersion + "#" + fileInGoStdlib), nil } // Refers to a file in the same workspace? if util.PathHasPrefix(u.Path, h.RootFSPath) { - pathInThisWorkspace := util.PathTrimPrefix(u.Path, h.RootFSPath) - return lsp.DocumentURI("file:///" + pathInThisWorkspace), nil + if h.clientUsesFileSchemeWithinWorkspace { + pathInThisWorkspace := util.PathTrimPrefix(u.Path, h.RootFSPath) + return lsp.DocumentURI("file:///" + pathInThisWorkspace), nil + } + if h.originalRootURI == nil { + return uri, nil + } + newURI, _ := url.Parse(h.originalRootURI.String()) + newURI.Fragment = util.PathTrimPrefix(u.Path, h.RootFSPath) + return lsp.DocumentURI(newURI.String()), nil } // Refers to a file in the GOPATH (that's from another repo)? diff --git a/buildserver/integration_test.go b/buildserver/integration_test.go index 26db4b67..79524597 100644 --- a/buildserver/integration_test.go +++ b/buildserver/integration_test.go @@ -237,7 +237,7 @@ func TestIntegration(t *testing.T) { ctx := context.Background() - c, done := connectionToNewBuildServer(string(rootURI), t) + c, done := connectionToNewBuildServer(string(rootURI), t, true) defer done() // Prepare the connection. diff --git a/buildserver/proxy_test.go b/buildserver/proxy_test.go index 0e22776e..8314bcd7 100644 --- a/buildserver/proxy_test.go +++ b/buildserver/proxy_test.go @@ -131,92 +131,6 @@ func TestProxy(t *testing.T) { "a_test.go:1:46": "A int", }, }, - "go subdirectory in repo": { - rootURI: "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d", - mode: "go", - fs: map[string]string{ - "a.go": "package d; func A() { A() }", - "d2/b.go": `package d2; import "test/pkg/d"; func B() { d.A(); B() }`, - }, - wantHover: map[string]string{ - "a.go:1:17": "func A()", - "a.go:1:23": "func A()", - "d2/b.go:1:39": "func B()", - "d2/b.go:1:47": "func A()", - "d2/b.go:1:52": "func B()", - }, - wantDefinition: map[string]string{ - "a.go:1:17": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:1:17", - "a.go:1:23": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:1:17", - "d2/b.go:1:39": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:39", - "d2/b.go:1:47": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:1:17", - "d2/b.go:1:52": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:39", - }, - wantXDefinition: map[string]string{ - "a.go:1:17": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:1:17 id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - "a.go:1:23": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:1:17 id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - "d2/b.go:1:39": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:39 id:test/pkg/d/d2/-/B name:B package:test/pkg/d/d2 packageName:d2 recv: vendor:false", - "d2/b.go:1:47": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:1:17 id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - "d2/b.go:1:52": "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:39 id:test/pkg/d/d2/-/B name:B package:test/pkg/d/d2 packageName:d2 recv: vendor:false", - }, - wantSymbols: map[string][]string{ - "": []string{"git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:function:A:0:16", "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:function:B:0:38"}, - "is:exported": []string{"git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/a.go:function:A:0:16", "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:function:B:0:38"}, - }, - wantXReferences: map[*lsext.WorkspaceReferencesParams][]string{ - // Non-matching name query. - {Query: lsext.SymbolDescriptor{"name": "nope"}}: []string{}, - - // Matching against invalid field name. - {Query: lsext.SymbolDescriptor{"nope": "A"}}: []string{}, - - // Matching against an invalid dirs hint. - {Query: lsext.SymbolDescriptor{"package": "test/pkg/d"}, Hints: map[string]interface{}{"dirs": []string{"file:///src/test/pkg/d/d3"}}}: []string{}, - - // Matching against a dirs hint with multiple dirs. - {Query: lsext.SymbolDescriptor{"package": "test/pkg/d"}, Hints: map[string]interface{}{"dirs": []string{"file:///d2", "file:///invalid"}}}: []string{ - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:20-1:32 -> id:test/pkg/d name: package:test/pkg/d packageName:d recv: vendor:false", - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:47-1:48 -> id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - }, - - // Matching against a dirs hint. - {Query: lsext.SymbolDescriptor{"package": "test/pkg/d"}, Hints: map[string]interface{}{"dirs": []string{"file:///d2"}}}: []string{ - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:20-1:32 -> id:test/pkg/d name: package:test/pkg/d packageName:d recv: vendor:false", - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:47-1:48 -> id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - }, - - // Matching against single field. - {Query: lsext.SymbolDescriptor{"package": "test/pkg/d"}}: []string{ - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:20-1:32 -> id:test/pkg/d name: package:test/pkg/d packageName:d recv: vendor:false", - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:47-1:48 -> id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - }, - - // Matching against no fields. - {Query: lsext.SymbolDescriptor{}}: []string{ - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:20-1:32 -> id:test/pkg/d name: package:test/pkg/d packageName:d recv: vendor:false", - "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:47-1:48 -> id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false", - }, - { - Query: lsext.SymbolDescriptor{ - "name": "", - "package": "test/pkg/d", - "packageName": "d", - "recv": "", - "vendor": false, - }, - }: []string{"git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:20-1:32 -> id:test/pkg/d name: package:test/pkg/d packageName:d recv: vendor:false"}, - { - Query: lsext.SymbolDescriptor{ - "name": "A", - "package": "test/pkg/d", - "packageName": "d", - "recv": "", - "vendor": false, - }, - }: []string{"git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef#d/d2/b.go:1:47-1:48 -> id:test/pkg/d/-/A name:A package:test/pkg/d packageName:d recv: vendor:false"}, - }, - wantXPackages: []string{"test/pkg/d", "test/pkg/d/d2"}, - }, "go multiple packages in dir": { rootURI: "git://test/pkg?deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", mode: "go", @@ -606,18 +520,32 @@ func yza() {} t.Fatal(err) } - c, done := connectionToNewBuildServer(string(test.rootURI), t) - defer done() + r := func(clientUsesFileSchemeWithinWorkspace bool) { + c, done := connectionToNewBuildServer(string(test.rootURI), t, clientUsesFileSchemeWithinWorkspace) + defer done() + + // Prepare the connection. + var initializeParams lspext.InitializeParams + if clientUsesFileSchemeWithinWorkspace { + initializeParams = lspext.InitializeParams{ + InitializeParams: lsp.InitializeParams{RootURI: "file:///"}, + OriginalRootURI: test.rootURI, + } + } else { + initializeParams = lspext.InitializeParams{ + InitializeParams: lsp.InitializeParams{RootURI: test.rootURI}, + OriginalRootURI: "", + } + } + if err := c.Call(ctx, "initialize", initializeParams, nil); err != nil { + t.Fatal("initialize:", err) + } - // Prepare the connection. - if err := c.Call(ctx, "initialize", lspext.InitializeParams{ - InitializeParams: lsp.InitializeParams{RootURI: "file:///"}, - OriginalRootURI: test.rootURI, - }, nil); err != nil { - t.Fatal("initialize:", err) + lspTests(t, ctx, c, root, test.wantHover, test.wantDefinition, test.wantXDefinition, test.wantReferences, test.wantSymbols, test.wantXDependencies, test.wantXReferences, test.wantXPackages) } - lspTests(t, ctx, c, root, test.wantHover, test.wantDefinition, test.wantXDefinition, test.wantReferences, test.wantSymbols, test.wantXDependencies, test.wantXReferences, test.wantXPackages) + r(true) + r(false) }) } } @@ -649,7 +577,7 @@ func (c *pipeReadWriteCloser) Close() error { return err2 } -func connectionToNewBuildServer(root string, t testing.TB) (*jsonrpc2.Conn, func()) { +func connectionToNewBuildServer(root string, t testing.TB, rewriteURIs bool) (*jsonrpc2.Conn, func()) { rootURI, err := gituri.Parse(root) if err != nil { t.Fatal(err) @@ -684,18 +612,22 @@ func connectionToNewBuildServer(root string, t testing.TB) (*jsonrpc2.Conn, func onSend := func(req *jsonrpc2.Request, res *jsonrpc2.Response) { if res == nil { - err := convertURIs(req.Params, RelWorkspaceURI) - if err != nil { - t.Fatal(err) + if rewriteURIs { + err := convertURIs(req.Params, RelWorkspaceURI) + if err != nil { + t.Fatal(err) + } } } } onRecv := func(req *jsonrpc2.Request, res *jsonrpc2.Response) { if res != nil && res.Result != nil { - err := convertURIs(res.Result, AbsWorkspaceURI) - if err != nil { - t.Fatal(err) + if rewriteURIs { + err := convertURIs(res.Result, AbsWorkspaceURI) + if err != nil { + t.Fatal(err) + } } } } diff --git a/buildserver/stress_test.go b/buildserver/stress_test.go index 7e71024f..62d8c407 100644 --- a/buildserver/stress_test.go +++ b/buildserver/stress_test.go @@ -118,7 +118,7 @@ func BenchmarkStress(b *testing.B) { // Don't measure the time it takes to dial and // initialize, because this is amortized over each // operation we do. - c, done := connectionToNewBuildServer(string(rootURI), b) + c, done := connectionToNewBuildServer(string(rootURI), b, true) if err := c.Call(ctx, "initialize", lspext.ClientProxyInitializeParams{ InitializeParams: lsp.InitializeParams{RootURI: lsp.DocumentURI(root.String())}, InitializationOptions: lspext.ClientProxyInitializationOptions{Mode: test.mode}, diff --git a/buildserver/xlang_test.go b/buildserver/xlang_test.go index e0a241ab..cbde9941 100644 --- a/buildserver/xlang_test.go +++ b/buildserver/xlang_test.go @@ -19,11 +19,11 @@ import ( "time" "github.com/sourcegraph/ctxvfs" + "github.com/sourcegraph/go-langserver/gituri" "github.com/sourcegraph/go-langserver/pkg/lsp" lsext "github.com/sourcegraph/go-langserver/pkg/lspext" "github.com/sourcegraph/go-lsp/lspext" "github.com/sourcegraph/jsonrpc2" - "github.com/sourcegraph/go-langserver/gituri" ) var update = flag.Bool("update", false, "update golden files on disk")