Clean up language server's output handling (#4855)

Handles verbose logging and printing errors as diagnostics to stderr
when there's no way to communicate them back on a request.

Author: jonmeow

toolchain/diagnostics/diagnostic_kind.def

@@ -30,6 +30,8 @@ CARBON_DIAGNOSTIC_KIND(CompileOutputFileOpenError)
 CARBON_DIAGNOSTIC_KIND(FormatMultipleFilesToOneOutput)
 CARBON_DIAGNOSTIC_KIND(LanguageServerMissingInputStream)
 CARBON_DIAGNOSTIC_KIND(LanguageServerTransportError)
+CARBON_DIAGNOSTIC_KIND(LanguageServerUnexpectedReply)
+CARBON_DIAGNOSTIC_KIND(LanguageServerUnsupportedNotification)
 
 // ============================================================================
 // SourceBuffer diagnostics

toolchain/driver/language_server_subcommand.cpp

@@ -27,9 +27,9 @@ auto LanguageServerSubcommand::Run(DriverEnv& driver_env) -> DriverResult {
     return {.success = false};
   }
 
-  bool success =
-      LanguageServer::Run(driver_env.input_stream, *driver_env.output_stream,
-                          *driver_env.error_stream, driver_env.consumer);
+  bool success = LanguageServer::Run(
+      driver_env.input_stream, *driver_env.output_stream,
+      *driver_env.error_stream, driver_env.vlog_stream, driver_env.consumer);
   return {.success = success};
 }
 

toolchain/language_server/BUILD

@@ -31,6 +31,7 @@ cc_library(
     hdrs = ["context.h"],
     deps = [
         "//common:map",
+        "//toolchain/diagnostics:diagnostic_emitter",
     ],
 )
 
@@ -60,6 +61,8 @@ cc_library(
         ":handle",
         "//common:check",
         "//common:map",
+        "//common:ostream",
+        "//common:raw_string_ostream",
         "@llvm-project//clang-tools-extra/clangd:ClangDaemon",
     ],
 )

toolchain/language_server/context.h

@@ -8,15 +8,24 @@
 #include <string>
 
 #include "common/map.h"
+#include "toolchain/diagnostics/diagnostic_consumer.h"
+#include "toolchain/diagnostics/diagnostic_emitter.h"
 
 namespace Carbon::LanguageServer {
 
 // Context for LSP call handling.
 class Context {
  public:
+  // `consumer` is required.
+  explicit Context(DiagnosticConsumer* consumer) : no_loc_emitter_(consumer) {}
+
+  auto no_loc_emitter() -> NoLocDiagnosticEmitter& { return no_loc_emitter_; }
+
   auto files() -> Map<std::string, std::string>& { return files_; }
 
  private:
+  NoLocDiagnosticEmitter no_loc_emitter_;
+
   // Content of files managed by the language client.
   Map<std::string, std::string> files_;
 };

toolchain/language_server/incoming_messages.cpp

@@ -4,6 +4,8 @@
 
 #include "toolchain/language_server/incoming_messages.h"
 
+#include "common/ostream.h"
+#include "common/raw_string_ostream.h"
 #include "toolchain/language_server/handle.h"
 
 namespace Carbon::LanguageServer {
@@ -83,7 +85,7 @@ auto IncomingMessages::onCall(llvm::StringRef name, llvm::json::Value params,
                      });
   } else {
     transport_->reply(id, llvm::make_error<clang::clangd::LSPError>(
-                              llvm::formatv("call `{0}` not found", name),
+                              llvm::formatv("unsupported call `{0}`", name),
                               clang::clangd::ErrorCode::MethodNotFound));
   }
 
@@ -98,10 +100,32 @@ auto IncomingMessages::onNotify(llvm::StringRef name, llvm::json::Value value)
   if (auto result = notification_handlers_.Lookup(name)) {
     (result.value())(*context_, std::move(value));
   } else {
-    clang::clangd::log("notification `{0}` not found", name);
+    CARBON_DIAGNOSTIC(LanguageServerUnsupportedNotification, Warning,
+                      "unsupported notification `{0}`", std::string);
+    context_->no_loc_emitter().Emit(LanguageServerUnsupportedNotification,
+                                    name.str());
   }
 
   return true;
 }
 
+auto IncomingMessages::onReply(llvm::json::Value id,
+                               llvm::Expected<llvm::json::Value> result)
+    -> bool {
+  RawStringOstream id_str;
+  id_str << id;
+  RawStringOstream result_str;
+  if (result) {
+    result_str << result.get();
+  } else {
+    result_str << result.takeError();
+  }
+  CARBON_DIAGNOSTIC(LanguageServerUnexpectedReply, Warning,
+                    "unexpected reply to request ID {0}: {1}", std::string,
+                    std::string);
+  context_->no_loc_emitter().Emit(LanguageServerUnexpectedReply,
+                                  id_str.TakeStr(), result_str.TakeStr());
+  return true;
+}
+
 }  // namespace Carbon::LanguageServer

toolchain/language_server/incoming_messages.h

@@ -25,20 +25,18 @@ class IncomingMessages : public clang::clangd::Transport::MessageHandler {
   explicit IncomingMessages(clang::clangd::Transport* transport,
                             Context* context);
 
-  // Dispatches calls to the appropriate entry in `call_handlers_`.
+  // Dispatches calls to the appropriate entry in `call_handlers_`. Always
+  // returns true.
   auto onCall(llvm::StringRef name, llvm::json::Value params,
               llvm::json::Value id) -> bool override;
 
   // Dispatches notifications to the appropriate entry in
   // `notification_handlers_`, except for `exit` which directly returns false.
   auto onNotify(llvm::StringRef name, llvm::json::Value value) -> bool override;
 
-  // Handles replies.
-  // TODO: Implement when needed.
+  // Handles replies. Always returns true.
   auto onReply(llvm::json::Value /*id*/,
-               llvm::Expected<llvm::json::Value> /*result*/) -> bool override {
-    return true;
-  }
+               llvm::Expected<llvm::json::Value> /*result*/) -> bool override;
 
  private:
   // These are the signatures expected for handlers.

toolchain/language_server/language_server.cpp

@@ -15,20 +15,47 @@
 
 namespace Carbon::LanguageServer {
 
+// An adapter for clangd's logging that sends most messages to `error_stream`.
+// Verbose logging is only printed if `vlog_stream` is provided, and will be
+// sent there.
+class Logger : public clang::clangd::Logger {
+ public:
+  explicit Logger(llvm::raw_ostream* error_stream,
+                  llvm::raw_ostream* vlog_stream)
+      : error_logger_(*error_stream, clang::clangd::Logger::Info),
+        vlog_logger_(vlog_stream
+                         ? std::make_unique<clang::clangd::StreamLogger>(
+                               *vlog_stream, clang::clangd::Logger::Verbose)
+                         : nullptr) {}
+
+  auto log(Level level, const char* format,
+           const llvm::formatv_object_base& message) -> void override {
+    if (level != clang::clangd::Logger::Verbose) {
+      error_logger_.log(level, format, message);
+    } else if (vlog_logger_) {
+      vlog_logger_->log(level, format, message);
+    }
+  }
+
+ private:
+  clang::clangd::StreamLogger error_logger_;
+  std::unique_ptr<clang::clangd::StreamLogger> vlog_logger_;
+};
+
 auto Run(FILE* input_stream, llvm::raw_ostream& output_stream,
-         llvm::raw_ostream& error_stream, DiagnosticConsumer& consumer)
-    -> bool {
-  // TODO: Consider implementing a custom logger that splits vlog to
-  // vlog_stream when provided. For now, this disables verbose logging.
-  clang::clangd::StreamLogger logger(error_stream, clang::clangd::Logger::Info);
+         llvm::raw_ostream& error_stream, llvm::raw_ostream* vlog_stream,
+         DiagnosticConsumer& consumer) -> bool {
+  // The language server internally uses diagnostics for logging issues, but the
+  // clangd parts have their own logging system. We intercept that here.
+  Logger logger(&error_stream, vlog_stream);
   clang::clangd::LoggingSession logging_session(logger);
 
   // Set up the connection.
   std::unique_ptr<clang::clangd::Transport> transport(
       clang::clangd::newJSONTransport(input_stream, output_stream,
                                       /*InMirror=*/nullptr,
                                       /*Pretty=*/true));
-  Context context;
+  Context context(&consumer);
   // TODO: Use error_stream in IncomingMessages to report dropped errors.
   IncomingMessages incoming(transport.get(), &context);
   OutgoingMessages outgoing(transport.get());

toolchain/language_server/language_server.h

@@ -13,8 +13,11 @@ namespace Carbon::LanguageServer {
 // Start the language server. input_stream and output_stream are used by LSP;
 // error_stream is primarily for errors that don't fit into LSP. Returns true if
 // the server cleanly exits.
+//
+// This is thread-hostile because `clangd::LoggingSession` relies on a global.
 auto Run(FILE* input_stream, llvm::raw_ostream& output_stream,
-         llvm::raw_ostream& error_stream, DiagnosticConsumer& consumer) -> bool;
+         llvm::raw_ostream& error_stream, llvm::raw_ostream* vlog_stream,
+         DiagnosticConsumer& consumer) -> bool;
 
 }  // namespace Carbon::LanguageServer
 

toolchain/language_server/testdata/parse_error.carbon

@@ -0,0 +1,23 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/language_server/testdata/parse_error.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/language_server/testdata/parse_error.carbon
+
+
+// --- STDIN
+[[@LSP-REPLY:1]]
+[[@LSP-REPLY:2:"result": 3]]
+[[@LSP-NOTIFY:exit]]
+
+// --- AUTOUPDATE-SPLIT
+
+// CHECK:STDERR: warning: unexpected reply to request ID "1": null [LanguageServerUnexpectedReply]
+// CHECK:STDERR:
+// CHECK:STDERR: warning: unexpected reply to request ID "2": 3 [LanguageServerUnexpectedReply]
+// CHECK:STDERR:
+// CHECK:STDOUT:

toolchain/language_server/testdata/unexpected_reply.carbon

@@ -0,0 +1,23 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/language_server/testdata/unexpected_reply.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/language_server/testdata/unexpected_reply.carbon
+
+
+// --- STDIN
+[[@LSP-REPLY:1]]
+[[@LSP-REPLY:2:"value": 3]]
+[[@LSP-NOTIFY:exit]]
+
+// --- AUTOUPDATE-SPLIT
+
+// CHECK:STDERR: warning: unexpected reply to request ID "1": null [LanguageServerUnexpectedReply]
+// CHECK:STDERR:
+// CHECK:STDERR: warning: unexpected reply to request ID "2": null [LanguageServerUnexpectedReply]
+// CHECK:STDERR:
+// CHECK:STDOUT: