Implement breaking lints in the LSP

.golangci.yml

@@ -115,6 +115,11 @@ issues:
  # G115 checks for use of truncating conversions.
  path: private/buf/buflsp/file.go
  text: "G115:"
+ - linters:
+ - gosec
+ # G115 checks for use of truncating conversions.
+ path: private/buf/buflsp/image.go
+ text: "G115:"
  - linters:
  - gosec
  # G115 checks for use of truncating conversions.

private/buf/buflsp/buflsp.go

@@ -27,6 +27,7 @@ import (
  "github.com/bufbuild/buf/private/buf/bufctl"
  "github.com/bufbuild/buf/private/bufpkg/bufcheck"
  "github.com/bufbuild/buf/private/pkg/app/appext"
+ "github.com/bufbuild/buf/private/pkg/command"
  "github.com/bufbuild/buf/private/pkg/slogext"
  "github.com/bufbuild/buf/private/pkg/storage"
  "github.com/bufbuild/buf/private/pkg/storage/storageos"
@@ -65,11 +66,13 @@ func Serve(
  &connWrapper{Conn: conn, logger: container.Logger()},
  zap.NewNop(), // The logging from protocol itself isn't very good, we've replaced it with connAdapter here.
  ),
+ container: container,
  logger: container.Logger(),
  controller: controller,
  checkClient: checkClient,
  rootBucket: bucket,
  wktBucket: wktBucket,
+ runner: command.NewRunner(),
  }
  lsp.fileManager = newFileManager(lsp)
  off := protocol.TraceOff
@@ -89,14 +92,16 @@ func Serve(
 // Its handler methods are not defined in buflsp.go; they are defined in other files, grouped
 // according to the groupings in
 type lsp struct {
- conn jsonrpc2.Conn
- client protocol.Client
+ conn jsonrpc2.Conn
+ client protocol.Client
+ container appext.Container
 
  logger *slog.Logger
  controller bufctl.Controller
  checkClient bufcheck.Client
  rootBucket storage.ReadBucket
  fileManager *fileManager
+ runner command.Runner
 
  wktBucket storage.ReadBucket
 
@@ -130,19 +135,47 @@ func (l *lsp) init(_ context.Context, params *protocol.InitializeParams) error {
 // to inject debug logging, tracing, and timeouts to requests.
 func (l *lsp) newHandler() jsonrpc2.Handler {
  actual := protocol.ServerHandler(newServer(l), nil)
- return func(ctx context.Context, reply jsonrpc2.Replier, req jsonrpc2.Request) (retErr error) {
- defer slogext.DebugProfile(l.logger, slog.String("method", req.Method()), slog.Any("params", req.Params()))()
-
- replier := l.wrapReplier(reply, req)
-
- // Verify that the server has been initialized if this isn't the initialization
- // request.
- if req.Method() != protocol.MethodInitialize && l.initParams.Load() == nil {
- return replier(ctx, nil, fmt.Errorf("the first call to the server must be the %q method", protocol.MethodInitialize))
- }
-
- l.lock.Lock()
- defer l.lock.Unlock()
- return actual(ctx, replier, req)
+ return func(ctx context.Context, reply jsonrpc2.Replier, req jsonrpc2.Request) error {
+ go func() {
+ // Although everything is behind one big lock, we need to run requests
+ // in their own goroutines; otherwise, if a request performs a call to the
+ // client, the single goroutine that actually handles sending to and
+ // from the client will deadlock.
+ //
+ // jsonrpc2 does not do a good job of documenting this limitation.
+
+ l.logger.Debug(
+ "handling request",
+ slog.String("method", req.Method()),
+ slog.Any("params", req.Params()),
+ )
+ defer slogext.DebugProfile(
+ l.logger,
+ slog.String("method", req.Method()),
+ slog.Any("params", req.Params()),
+ )()
+
+ replier := l.wrapReplier(reply, req)
+
+ var err error
+ if req.Method() != protocol.MethodInitialize && l.initParams.Load() == nil {
+ // Verify that the server has been initialized if this isn't the initialization
+ // request.
+ err = replier(ctx, nil, fmt.Errorf("the first call to the server must be the %q method", protocol.MethodInitialize))
+ } else {
+ l.lock.Lock()
+ err = actual(ctx, replier, req)
+ l.lock.Unlock()
+ }
+
+ if err != nil {
+ l.logger.Error(
+ "error while replying to request",
+ slog.String("method", req.Method()),
+ slogext.ErrorAttr(err),
+ )
+ }
+ }()
+ return nil
  }
 }