feat(lsp): support configuration changes

The LSP server loads the quick-lint-js.tracing-directory configuration
option on startup. Allow the user to change this option any time after
startup.

Author: strager

plugin/vscode-lsp/extension.js

@@ -27,6 +27,9 @@ async function startOrRestartServerAsync() {
         { language: "javascriptreact" },
         { language: "json" },
       ],
+      synchronize: {
+        configurationSection: "quick-lint-js-lsp",
+      },
       middleware: {
         workspace: {
           async configuration(params, _token, _next) {
@@ -35,6 +38,11 @@ async function startOrRestartServerAsync() {
               getLSPWorkspaceConfig(vscodeConfig, item)
             );
           },
+          async didChangeConfiguration(sections) {
+            return client.sendNotification("workspace/didChangeConfiguration", {
+              settings: getLSPWorkspaceFullConfig(),
+            });
+          },
         },
       },
     }
@@ -57,6 +65,13 @@ async function startOrRestartServerAsync() {
         return null;
     }
   }
+
+  function getLSPWorkspaceFullConfig() {
+    let config = getLatestWorkspaceConfig();
+    return {
+      "quick-lint-js.tracing-directory": config.get("tracing-directory"),
+    };
+  }
 }
 
 async function stopServerIfStartedAsync() {

src/lsp-server.cpp

@@ -112,7 +112,18 @@ linting_lsp_server_handler::linting_lsp_server_handler(
   this->workspace_configuration_.add_item(
       u8"quick-lint-js.tracing-directory"sv,
       [this](std::string_view new_value) {
-        this->server_config_.tracing_directory = new_value;
+        bool changed = this->server_config_.tracing_directory != new_value;
+        if (changed) {
+          this->server_config_.tracing_directory = new_value;
+          if (this->tracer_) {
+            if (this->server_config_.tracing_directory.empty()) {
+              this->tracer_->disable();
+            } else {
+              this->tracer_->create_and_enable_in_child_directory(
+                  this->server_config_.tracing_directory);
+            }
+          }
+        }
       });
 }
 
@@ -160,6 +171,8 @@ void linting_lsp_server_handler::handle_notification(
     this->handle_text_document_did_open_notification(request);
   } else if (method == "textDocument/didClose") {
     this->handle_text_document_did_close_notification(request);
+  } else if (method == "workspace/didChangeConfiguration") {
+    this->handle_workspace_did_change_configuration_notification(request);
   } else if (method == "initialized") {
     this->handle_initialized_notification();
   } else if (method == "exit") {
@@ -239,11 +252,6 @@ void linting_lsp_server_handler::handle_workspace_configuration_response(
     // TODO(strager): Report an error.
     return;
   }
-
-  if (this->tracer_ && !this->server_config_.tracing_directory.empty()) {
-    this->tracer_->create_and_enable_in_child_directory(
-        this->server_config_.tracing_directory);
-  }
 }
 
 void linting_lsp_server_handler::handle_initialized_notification() {
@@ -435,6 +443,24 @@ void linting_lsp_server_handler::handle_text_document_did_open_notification(
   }
 }
 
+void linting_lsp_server_handler::
+    handle_workspace_did_change_configuration_notification(
+        ::simdjson::ondemand::object& request) {
+  ::simdjson::ondemand::object settings;
+  if (request["params"]["settings"].get(settings) != ::simdjson::SUCCESS) {
+    QLJS_DEBUG_LOG("failed to extract configuration notification settings\n");
+    // TODO(strager): Report an error.
+    return;
+  }
+
+  bool ok = this->workspace_configuration_.process_notification(settings);
+  if (!ok) {
+    QLJS_DEBUG_LOG("failed to process configuration notification\n");
+    // TODO(strager): Report an error.
+    return;
+  }
+}
+
 void linting_lsp_server_handler::handle_config_file_changes(
     const std::vector<configuration_change>& config_changes) {
   for (auto& entry : this->documents_) {

src/lsp-workspace-configuration.cpp

@@ -72,30 +72,70 @@ bool lsp_workspace_configuration::process_response(
     if ((*result_it).get(value) != ::simdjson::SUCCESS) {
       return false;
     }
+    if (!this->set_item(*spec_it, value)) {
+      return false;
+    }
+  }
+  return spec_it == spec_end && result_it == result_end;
+}
 
-    ::simdjson::ondemand::json_type type;
-    if (value.type().get(type) != ::simdjson::SUCCESS) {
+bool lsp_workspace_configuration::process_notification(
+    ::simdjson::ondemand::object settings) {
+  for (simdjson::simdjson_result< ::simdjson::ondemand::field> setting_field :
+       settings) {
+    std::string_view name;
+    if (setting_field.unescaped_key().get(name) != ::simdjson::SUCCESS) {
       return false;
     }
-    switch (type) {
-    case ::simdjson::ondemand::json_type::string: {
-      std::string_view string_value;
-      if (value.get(string_value) != ::simdjson::SUCCESS) {
-        return false;
-      }
-      spec_it->callback(string_value);
-      break;
+    ::simdjson::ondemand::value value;
+    if (setting_field.value().get(value) != ::simdjson::SUCCESS) {
+      return false;
+    }
+    item* i = this->find_item(to_string8_view(name));
+    if (!i) {
+      // Ignore unknown settings.
+      continue;
     }
+    if (!this->set_item(*i, value)) {
+      return false;
+    }
+  }
+  return true;
+}
 
-    case ::simdjson::ondemand::json_type::null:
-      spec_it->callback(std::string_view());
-      break;
+lsp_workspace_configuration::item* lsp_workspace_configuration::find_item(
+    string8_view name) {
+  for (item& i : this->items_) {
+    if (i.name == name) {
+      return &i;
+    }
+  }
+  return nullptr;
+}
 
-    default:
+bool lsp_workspace_configuration::set_item(const item& i,
+                                           ::simdjson::ondemand::value value) {
+  ::simdjson::ondemand::json_type type;
+  if (value.type().get(type) != ::simdjson::SUCCESS) {
+    return false;
+  }
+  switch (type) {
+  case ::simdjson::ondemand::json_type::string: {
+    std::string_view string_value;
+    if (value.get(string_value) != ::simdjson::SUCCESS) {
       return false;
     }
+    i.callback(string_value);
+    return true;
+  }
+
+  case ::simdjson::ondemand::json_type::null:
+    i.callback(std::string_view());
+    return true;
+
+  default:
+    return false;
   }
-  return spec_it == spec_end && result_it == result_end;
 }
 }
 

src/quick-lint-js/lsp-server.h

@@ -147,6 +147,8 @@ class linting_lsp_server_handler final : public lsp_endpoint_handler {
       ::simdjson::ondemand::object& request);
   void handle_text_document_did_open_notification(
       ::simdjson::ondemand::object& request);
+  void handle_workspace_did_change_configuration_notification(
+      ::simdjson::ondemand::object& request);
 
   void handle_config_file_changes(
       const std::vector<configuration_change>& config_changes);

src/quick-lint-js/lsp-workspace-configuration.h

@@ -24,7 +24,7 @@ class lsp_workspace_configuration {
  public:
   // Register a configuration setting.
   //
-  // callback is later called by process_response.
+  // callback is later called by process_response or process_notification.
   //
   // name must be have global lifetime (e.g. be a compile-time string).
   // name must be a JSON-encoded string (without surrounding quotation marks).
@@ -39,11 +39,19 @@ class lsp_workspace_configuration {
   // Handle a workspace/configuration JSON-RPC response sent by the LSP client.
   bool process_response(::simdjson::ondemand::value result);
 
+  // Handle a workspace/didChangeConfiguration JSON-RPC notification sent by the
+  // LSP client.
+  bool process_notification(::simdjson::ondemand::object settings);
+
  private:
   struct item {
     string8_view name;
     std::function<void(std::string_view)> callback;
   };
+
+  item* find_item(string8_view name);
+  bool set_item(const item&, ::simdjson::ondemand::value);
+
   std::vector<item> items_;
 };
 }

test/test-lsp-server.cpp

@@ -18,10 +18,12 @@
 #include <quick-lint-js/configuration.h>
 #include <quick-lint-js/fake-configuration-filesystem.h>
 #include <quick-lint-js/file-handle.h>
+#include <quick-lint-js/filesystem-test.h>
 #include <quick-lint-js/lsp-endpoint.h>
 #include <quick-lint-js/lsp-server.h>
 #include <quick-lint-js/padded-string.h>
 #include <quick-lint-js/spy-lsp-endpoint-remote.h>
+#include <quick-lint-js/trace-flusher.h>
 #include <quick-lint-js/version.h>
 #include <quick-lint-js/warning.h>
 #include <simdjson.h>
@@ -81,7 +83,7 @@ class mock_lsp_linter final : public lsp_linter {
   std::function<lint_and_get_diagnostics_notification_type> callback_;
 };
 
-class test_linting_lsp_server : public ::testing::Test {
+class test_linting_lsp_server : public ::testing::Test, public filesystem_test {
  public:
   explicit test_linting_lsp_server() { this->reset(); }
 
@@ -288,6 +290,149 @@ TEST_F(test_linting_lsp_server, stores_config_values_after_config_response) {
   EXPECT_EQ(config.tracing_directory, "/test/tracing/dir");
 }
 
+TEST_F(test_linting_lsp_server, did_change_configuration_notification) {
+  this->server->append(
+      make_message(u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": "/test/tracing/dir"
+          }
+        }
+      })"));
+  this->handler->flush_pending_notifications(*this->client);
+
+  EXPECT_THAT(this->client->messages, IsEmpty());
+  linting_lsp_server_config& config = this->handler->server_config();
+  EXPECT_EQ(config.tracing_directory, "/test/tracing/dir");
+}
+
+TEST_F(test_linting_lsp_server,
+       changing_config_to_same_tracing_dir_does_not_reset_tracing) {
+  std::string temp_dir = this->make_temporary_directory();
+  trace_flusher tracer;
+  fake_configuration_filesystem fs;
+  mock_lsp_linter linter;
+  linting_lsp_server_handler handler(&fs, &linter, &tracer);
+  spy_lsp_endpoint_remote client;
+  lsp_endpoint server(&handler, &client);
+
+  server.append(
+      make_message(u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": )" +
+                   json_to_string8(::boost::json::string(temp_dir)) + u8R"(
+          }
+        }
+      })"));
+
+  std::vector<std::string> original_children =
+      list_files_in_directory(temp_dir);
+  EXPECT_THAT(original_children, ElementsAre(::testing::_))
+      << "enabling tracing in " << temp_dir
+      << " should create a trace subdirectory";
+
+  server.append(
+      make_message(u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": )" +
+                   json_to_string8(::boost::json::string(temp_dir)) + u8R"(
+          }
+        }
+      })"));
+
+  std::vector<std::string> new_children = list_files_in_directory(temp_dir);
+  EXPECT_THAT(new_children, original_children)
+      << "enabling tracing in " << temp_dir
+      << " should not create another trace subdirectory";
+}
+
+TEST_F(test_linting_lsp_server,
+       changing_config_to_different_tracing_dir_resets_tracing) {
+  trace_flusher tracer;
+  fake_configuration_filesystem fs;
+  mock_lsp_linter linter;
+  linting_lsp_server_handler handler(&fs, &linter, &tracer);
+  spy_lsp_endpoint_remote client;
+  lsp_endpoint server(&handler, &client);
+
+  std::string original_tracing_dir = this->make_temporary_directory();
+  std::string new_tracing_dir = this->make_temporary_directory();
+
+  server.append(make_message(
+      u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": )" +
+      json_to_string8(::boost::json::string(original_tracing_dir)) +
+      u8R"(
+          }
+        }
+      })"));
+  server.append(
+      make_message(u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": )" +
+                   json_to_string8(::boost::json::string(new_tracing_dir)) +
+                   u8R"(
+          }
+        }
+      })"));
+
+  std::vector<std::string> new_children =
+      list_files_in_directory(new_tracing_dir);
+  EXPECT_THAT(new_children, ElementsAre(::testing::_))
+      << "changing tracing dir from " << original_tracing_dir << " to "
+      << new_tracing_dir << " should create a new trace subdirectory";
+}
+
+TEST_F(test_linting_lsp_server,
+       changing_tracing_dir_config_to_empty_disables_tracing) {
+  std::string temp_dir = this->make_temporary_directory();
+  trace_flusher tracer;
+  fake_configuration_filesystem fs;
+  mock_lsp_linter linter;
+  linting_lsp_server_handler handler(&fs, &linter, &tracer);
+  spy_lsp_endpoint_remote client;
+  lsp_endpoint server(&handler, &client);
+
+  server.append(
+      make_message(u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": )" +
+                   json_to_string8(::boost::json::string(temp_dir)) + u8R"(
+          }
+        }
+      })"));
+  EXPECT_TRUE(tracer.is_enabled());
+  server.append(
+      make_message(u8R"({
+        "jsonrpc": "2.0",
+        "method": "workspace/didChangeConfiguration",
+        "params": {
+          "settings": {
+            "quick-lint-js.tracing-directory": ""
+          }
+        }
+      })"));
+  EXPECT_FALSE(tracer.is_enabled());
+}
+
 // https://microsoft.github.io/language-server-protocol/specifications/specification-current/#shutdown
 TEST_F(test_linting_lsp_server, shutdown) {
   this->server->append(