gopls/lsprpc: Adjust gopls to cuelsp and simplify

More replacing of gopls with cuelsp, but this time for rpc logging and
protocol.

Simplify the "fowarding" implementation: the logfile and debug options
are no longer supported so the forwarding handshake can be simplified.
The logfile is no longer supported because the lsp protocol itself
allows trace logging back to the editor which is perfectly sufficient.

Signed-off-by: Matthew Sackman <[email protected]>
Change-Id: I80302d363ea5e95c3b40ce7321cdbc092465e545
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1218088
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>

Author: cuematthew

internal/golangorgx/gopls/cmd/serve.go

@@ -24,15 +24,12 @@ import (
 // Serve is a struct that exposes the configurable parts of the LSP server as
 // flags, in the right form for tool.Main to consume.
 type Serve struct {
-	Logfile     string        `flag:"logfile" help:"filename to log to. if value is \"auto\", then logging to a default output file is enabled"`
 	Mode        string        `flag:"mode" help:"no effect"`
 	Port        int           `flag:"port" help:"port on which to run cuelsp for debugging purposes"`
 	Address     string        `flag:"listen" help:"address on which to listen for remote connections. If prefixed by 'unix;', the subsequent address is assumed to be a unix domain socket. Otherwise, TCP is used."`
 	IdleTimeout time.Duration `flag:"listen.timeout" help:"when used with -listen, shut down the server when there are no connected clients for this duration"`
 
 	RemoteListenTimeout time.Duration `flag:"remote.listen.timeout" help:"when used with -remote=auto, the -listen.timeout value used to start the daemon"`
-	RemoteDebug         string        `flag:"remote.debug" help:"when used with -remote=auto, the -debug value used to start the daemon"`
-	RemoteLogfile       string        `flag:"remote.logfile" help:"when used with -remote=auto, the -logfile value used to start the daemon"`
 
 	app *Application
 }
@@ -58,15 +55,9 @@ func (s *Serve) remoteArgs(network, address string) []string {
 	args := []string{"serve",
 		"-listen", fmt.Sprintf(`%s;%s`, network, address),
 	}
-	if s.RemoteDebug != "" {
-		args = append(args, "-debug", s.RemoteDebug)
-	}
 	if s.RemoteListenTimeout != 0 {
 		args = append(args, "-listen.timeout", s.RemoteListenTimeout.String())
 	}
-	if s.RemoteLogfile != "" {
-		args = append(args, "-logfile", s.RemoteLogfile)
-	}
 	return args
 }
 

internal/golangorgx/gopls/lsprpc/autostart_default.go

@@ -18,19 +18,19 @@ var (
 func runRemote(cmd *exec.Cmd) error {
 	daemonize(cmd)
 	if err := cmd.Start(); err != nil {
-		return fmt.Errorf("starting remote gopls: %w", err)
+		return fmt.Errorf("starting remote cuelsp: %w", err)
 	}
 	return nil
 }
 
 // autoNetworkAddressDefault returns the default network and address for the
-// automatically-started gopls remote. See autostart_posix.go for more
+// automatically-started cuelsp remote. See autostart_posix.go for more
 // information.
-func autoNetworkAddressDefault(goplsPath, id string) (network string, address string) {
+func autoNetworkAddressDefault(cuePath, id string) (network string, address string) {
 	if id != "" {
 		panic("identified remotes are not supported on windows")
 	}
-	return "tcp", "localhost:37374"
+	return "tcp", "localhost:37375"
 }
 
 func verifyRemoteOwnershipDefault(network, address string) (bool, error) {

internal/golangorgx/gopls/lsprpc/autostart_posix.go

@@ -34,30 +34,30 @@ func daemonizePosix(cmd *exec.Cmd) {
 
 // autoNetworkAddressPosix resolves an id on the 'auto' pseduo-network to a
 // real network and address. On unix, this uses unix domain sockets.
-func autoNetworkAddressPosix(goplsPath, id string) (network string, address string) {
+func autoNetworkAddressPosix(cuePath, id string) (network string, address string) {
 	// Especially when doing local development or testing, it's important that
-	// the remote gopls instance we connect to is running the same binary as our
+	// the remote cuelsp instance we connect to is running the same binary as our
 	// forwarder. So we encode a short hash of the binary path into the daemon
 	// socket name. If possible, we also include the buildid in this hash, to
 	// account for long-running processes where the binary has been subsequently
 	// rebuilt.
 	h := sha256.New()
-	cmd := exec.Command("go", "tool", "buildid", goplsPath)
+	cmd := exec.Command("go", "tool", "buildid", cuePath)
 	cmd.Stdout = h
 	var pathHash []byte
 	if err := cmd.Run(); err == nil {
 		pathHash = h.Sum(nil)
 	} else {
 		log.Printf("error getting current buildid: %v", err)
-		sum := sha256.Sum256([]byte(goplsPath))
+		sum := sha256.Sum256([]byte(cuePath))
 		pathHash = sum[:]
 	}
 	shortHash := fmt.Sprintf("%x", pathHash)[:6]
 	user := os.Getenv("USER")
 	if user == "" {
 		user = "shared"
 	}
-	basename := filepath.Base(goplsPath)
+	basename := filepath.Base(cuePath)
 	idComponent := ""
 	if id != "" {
 		idComponent = "-" + id

internal/golangorgx/gopls/lsprpc/dialer.go

@@ -16,7 +16,7 @@ import (
 	"cuelang.org/go/internal/golangorgx/tools/event"
 )
 
-// autoNetwork is the pseudo network type used to signal that gopls should use
+// autoNetwork is the pseudo network type used to signal that cuelsp should use
 // automatic discovery to resolve a remote address.
 const autoNetwork = "auto"
 
@@ -36,12 +36,12 @@ func newAutoDialer(rawAddr string, argFunc func(network, addr string) []string)
 	d.network, d.addr = ParseAddr(rawAddr)
 	if d.network == autoNetwork {
 		d.isAuto = true
-		bin, err := os.Executable()
+		cuePath, err := os.Executable()
 		if err != nil {
 			return nil, fmt.Errorf("getting executable: %w", err)
 		}
-		d.executable = bin
-		d.network, d.addr = autoNetworkAddress(bin, d.addr)
+		d.executable = cuePath
+		d.network, d.addr = autoNetworkAddress(cuePath, d.addr)
 	}
 	return &d, nil
 }

internal/golangorgx/gopls/lsprpc/lsprpc.go

@@ -46,9 +46,7 @@ type streamServer struct {
 	serverForTest protocol.Server
 }
 
-// NewStreamServer creates a StreamServer using the shared cache. If
-// withTelemetry is true, each session is instrumented with telemetry that
-// records RPC statistics.
+// NewStreamServer creates a StreamServer using the shared cache.
 func NewStreamServer(cache *cache.Cache, daemon bool, optionsFunc func(*settings.Options)) jsonrpc2.StreamServer {
 	return &streamServer{cache: cache, daemon: daemon, optionsOverrides: optionsFunc}
 }
@@ -70,17 +68,11 @@ func (s *streamServer) ServeStream(ctx context.Context, conn jsonrpc2.Conn) erro
 			event.Error(ctx, "error shutting down", err)
 		}
 	}()
-	executable, err := os.Executable()
-	if err != nil {
-		log.Printf("error getting gopls path: %v", err)
-		executable = ""
-	}
 	ctx = protocol.WithClient(ctx, client)
 	conn.Go(ctx,
 		protocol.Handlers(
-			handshaker("unknown", executable, s.daemon,
-				protocol.ServerHandler(svr,
-					jsonrpc2.MethodNotFound))))
+			handshaker("unknown", s.daemon,
+				protocol.ServerHandler(svr, jsonrpc2.MethodNotFound))))
 	if s.daemon {
 		log.Printf("Session %s: connected", "unknown")
 		defer log.Printf("Session %s: exited", "unknown")
@@ -89,12 +81,12 @@ func (s *streamServer) ServeStream(ctx context.Context, conn jsonrpc2.Conn) erro
 	return conn.Err()
 }
 
-// A forwarder is a jsonrpc2.StreamServer that handles an LSP stream by
-// forwarding it to a remote. This is used when the gopls process started by
-// the editor is in the `-remote` mode, which means it finds and connects to a
-// separate gopls daemon. In these cases, we still want the forwarder gopls to
-// be instrumented with telemetry, and want to be able to in some cases hijack
-// the jsonrpc2 connection with the daemon.
+// A forwarder is a jsonrpc2.StreamServer that handles an LSP stream
+// by forwarding it to a remote. This is used when the cuelsp process
+// started by the editor is in the `-remote` mode, which means it
+// finds and connects to a separate cuelsp daemon. In these cases, we
+// still want the forwarder cuelsp to in some cases hijack the
+// jsonrpc2 connection with the daemon.
 type forwarder struct {
 	dialer *autoDialer
 
@@ -128,23 +120,23 @@ func QueryServerState(ctx context.Context, addr string) (any, error) {
 		return nil, err
 	}
 	var state serverState
-	if err := protocol.Call(ctx, serverConn, sessionsMethod, nil, &state); err != nil {
+	if err := protocol.Call(ctx, serverConn, serversMethod, nil, &state); err != nil {
 		return nil, fmt.Errorf("querying server state: %w", err)
 	}
 	return &state, nil
 }
 
-// dialRemote is used for making calls into the gopls daemon. addr should be a
+// dialRemote is used for making calls into the cuelsp daemon. addr should be a
 // URL, possibly on the synthetic 'auto' network (e.g. tcp://..., unix://...,
 // or auto://...).
 func dialRemote(ctx context.Context, addr string) (jsonrpc2.Conn, error) {
 	network, address := ParseAddr(addr)
 	if network == autoNetwork {
-		gp, err := os.Executable()
+		cuePath, err := os.Executable()
 		if err != nil {
-			return nil, fmt.Errorf("getting gopls path: %w", err)
+			return nil, fmt.Errorf("getting cue path: %w", err)
 		}
-		network, address = autoNetworkAddress(gp, address)
+		network, address = autoNetworkAddress(cuePath, address)
 	}
 	netConn, err := net.DialTimeout(network, address, 5*time.Second)
 	if err != nil {
@@ -227,32 +219,16 @@ func (f *forwarder) ServeStream(ctx context.Context, clientConn jsonrpc2.Conn) e
 func (f *forwarder) handshake(ctx context.Context) {
 	// This call to os.Executable is redundant, and will be eliminated by the
 	// transition to the V2 API.
-	goplsPath, err := os.Executable()
-	if err != nil {
-		event.Error(ctx, "getting executable for handshake", err)
-		goplsPath = ""
-	}
-	var (
-		hreq = handshakeRequest{
-			ServerID:  f.serverID,
-			GoplsPath: goplsPath,
-		}
-		hresp handshakeResponse
-	)
+	hreq := handshakeRequest{ServerID: f.serverID}
+	var hresp handshakeResponse
 	if err := protocol.Call(ctx, f.serverConn, handshakeMethod, hreq, &hresp); err != nil {
 		// TODO(rfindley): at some point in the future we should return an error
 		// here.  Handshakes have become functional in nature.
-		event.Error(ctx, "forwarder: gopls handshake failed", err)
-	}
-	if hresp.GoplsPath != goplsPath {
-		event.Error(ctx, "", fmt.Errorf("forwarder: gopls path mismatch: forwarder is %q, remote is %q", goplsPath, hresp.GoplsPath))
+		event.Error(ctx, "forwarder: cuelsp handshake failed", err)
 	}
 	event.Log(ctx, "New server",
 		tag.NewServer.Of(f.serverID),
-		tag.Logfile.Of(hresp.Logfile),
-		tag.DebugAddress.Of(hresp.DebugAddr),
-		tag.GoplsPath.Of(hresp.GoplsPath),
-		tag.ClientID.Of(hresp.SessionID),
+		tag.ServerID.Of(hresp.ServerID),
 	)
 }
 
@@ -266,57 +242,35 @@ func ConnectToRemote(ctx context.Context, addr string) (net.Conn, error) {
 
 // A handshakeRequest identifies a client to the LSP server.
 type handshakeRequest struct {
-	// ServerID is the ID of the server on the client. This should usually be 0.
+	// ServerID is the ID of the server on the client. This should
+	// usually be 0.
 	ServerID string `json:"serverID"`
-	// Logfile is the location of the clients log file.
-	Logfile string `json:"logfile"`
-	// DebugAddr is the client debug address.
-	DebugAddr string `json:"debugAddr"`
-	// GoplsPath is the path to the Gopls binary running the current client
-	// process.
-	GoplsPath string `json:"goplsPath"`
 }
 
-// A handshakeResponse is returned by the LSP server to tell the LSP client
-// information about its session.
+// A handshakeResponse is returned by the LSP server to tell the LSP
+// client information about its server.
 type handshakeResponse struct {
-	// SessionID is the server session associated with the client.
-	SessionID string `json:"sessionID"`
-	// Logfile is the location of the server logs.
-	Logfile string `json:"logfile"`
-	// DebugAddr is the server debug address.
-	DebugAddr string `json:"debugAddr"`
-	// GoplsPath is the path to the Gopls binary running the current server
-	// process.
-	GoplsPath string `json:"goplsPath"`
+	// ServerID is the server server associated with the client.
+	ServerID string `json:"serverID"`
 }
 
-// clientSession identifies a current client LSP session on the server. Note
-// that it looks similar to handshakeResposne, but in fact 'Logfile' and
-// 'DebugAddr' now refer to the client.
-type clientSession struct {
-	SessionID string `json:"sessionID"`
-	Logfile   string `json:"logfile"`
-	DebugAddr string `json:"debugAddr"`
+// clientServer identifies a current client LSP server on the
+// server.
+type clientServer struct {
+	ServerID string `json:"serverID"`
 }
 
-// serverState holds information about the gopls daemon process, including its
-// debug information and debug information of all of its current connected
-// clients.
+// serverState holds information about the cuelsp daemon process.
 type serverState struct {
-	Logfile         string          `json:"logfile"`
-	DebugAddr       string          `json:"debugAddr"`
-	GoplsPath       string          `json:"goplsPath"`
-	CurrentClientID string          `json:"currentClientID"`
-	Clients         []clientSession `json:"clients"`
+	CurrentServerID string `json:"currentServerID"`
 }
 
 const (
-	handshakeMethod = "gopls/handshake"
-	sessionsMethod  = "gopls/sessions"
+	handshakeMethod = "cuelsp/handshake"
+	serversMethod   = "cuelsp/servers"
 )
 
-func handshaker(svrID string, goplsPath string, logHandshakes bool, handler jsonrpc2.Handler) jsonrpc2.Handler {
+func handshaker(svrID string, logHandshakes bool, handler jsonrpc2.Handler) jsonrpc2.Handler {
 	return func(ctx context.Context, reply jsonrpc2.Replier, r jsonrpc2.Request) error {
 		switch r.Method() {
 		case handshakeMethod:
@@ -326,30 +280,25 @@ func handshaker(svrID string, goplsPath string, logHandshakes bool, handler json
 			var req handshakeRequest
 			if err := json.Unmarshal(r.Params(), &req); err != nil {
 				if logHandshakes {
-					log.Printf("Error processing handshake for session %s: %v", svrID, err)
+					log.Printf("Error processing handshake for server %s: %v", svrID, err)
 				}
 				sendError(ctx, reply, err)
 				return nil
 			}
 			if logHandshakes {
-				log.Printf("Session %s: got handshake. Logfile: %q, Debug addr: %q", svrID, req.Logfile, req.DebugAddr)
+				log.Printf("Server %s: got handshake.", svrID)
 			}
-			event.Log(ctx, "Handshake session update",
-				tag.DebugAddress.Of(req.DebugAddr),
-				tag.Logfile.Of(req.Logfile),
+			event.Log(ctx, "Handshake server update",
 				tag.ServerID.Of(req.ServerID),
-				tag.GoplsPath.Of(req.GoplsPath),
 			)
 			resp := handshakeResponse{
-				SessionID: svrID,
-				GoplsPath: goplsPath,
+				ServerID: svrID,
 			}
 			return reply(ctx, resp, nil)
 
-		case sessionsMethod:
+		case serversMethod:
 			resp := serverState{
-				GoplsPath:       goplsPath,
-				CurrentClientID: svrID,
+				CurrentServerID: svrID,
 			}
 			return reply(ctx, resp, nil)
 		}
@@ -364,7 +313,7 @@ func sendError(ctx context.Context, reply jsonrpc2.Replier, err error) {
 	}
 }
 
-// ParseAddr parses the address of a gopls remote.
+// ParseAddr parses the address of a cuelsp remote.
 // TODO(rFindley): further document this syntax, and allow URI-style remote
 // addresses such as "auto://...".
 func ParseAddr(listen string) (network string, address string) {

internal/golangorgx/tools/event/tag/tag.go

@@ -36,11 +36,7 @@ var (
 	NewServer = keys.NewString("new_server", "A new server was added")
 	EndServer = keys.NewString("end_server", "A server was shut down")
 
-	ServerID     = keys.NewString("server", "The server ID an event is related to")
-	Logfile      = keys.NewString("logfile", "")
-	DebugAddress = keys.NewString("debug_address", "")
-	GoplsPath    = keys.NewString("gopls_path", "")
-	ClientID     = keys.NewString("client_id", "")
+	ServerID = keys.NewString("server", "The server ID an event is related to")
 
 	Level = keys.NewInt("level", "The logging level")
 )