refactor(lsp): use callbacks instead of string* for config items

We need to run code only if a config item changes. This isn't possible
with the current interface of lsp_workspace_configuration, which only
tells us the latest value of configs.

Refactor lsp_workspace_configuration's interface to take callback
functions instead output string pointers.

Author: strager

src/lsp-server.cpp

@@ -111,7 +111,9 @@ linting_lsp_server_handler::linting_lsp_server_handler(
       tracer_(tracer) {
   this->workspace_configuration_.add_item(
       u8"quick-lint-js.tracing-directory"sv,
-      &this->server_config_.tracing_directory);
+      [this](std::string_view new_value) {
+        this->server_config_.tracing_directory = new_value;
+      });
 }
 
 void linting_lsp_server_handler::handle_request(

src/lsp-workspace-configuration.cpp

@@ -16,11 +16,11 @@
 using namespace std::literals::string_view_literals;
 
 namespace quick_lint_js {
-void lsp_workspace_configuration::add_item(string8_view name,
-                                           std::string* out_value) {
+void lsp_workspace_configuration::add_item(
+    string8_view name, std::function<void(std::string_view)>&& callback) {
   this->items_.push_back(item{
       .name = name,
-      .value = out_value,
+      .callback = std::move(callback),
   });
 }
 
@@ -83,12 +83,12 @@ bool lsp_workspace_configuration::process_response(
       if (value.get(string_value) != ::simdjson::SUCCESS) {
         return false;
       }
-      *spec_it->value = string_value;
+      spec_it->callback(string_value);
       break;
     }
 
     case ::simdjson::ondemand::json_type::null:
-      spec_it->value->clear();
+      spec_it->callback(std::string_view());
       break;
 
     default:

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

@@ -8,6 +8,7 @@
 // No LSP on the web.
 #else
 
+#include <functional>
 #include <quick-lint-js/char8.h>
 #include <quick-lint-js/lsp-endpoint.h>
 #include <simdjson.h>
@@ -23,11 +24,12 @@ class lsp_workspace_configuration {
  public:
   // Register a configuration setting.
   //
-  // out_value is stored; *out_value will be modified by process_response.
+  // callback is later called by process_response.
   //
   // name must be have global lifetime (e.g. be a compile-time string).
   // name must be a JSON-encoded string (without surrounding quotation marks).
-  void add_item(string8_view name, std::string* out_value);
+  void add_item(string8_view name,
+                std::function<void(std::string_view)>&& callback);
 
   // Create a workspace/configuration JSON-RPC request to send to the LSP
   // client.
@@ -40,7 +42,7 @@ class lsp_workspace_configuration {
  private:
   struct item {
     string8_view name;
-    std::string* value;
+    std::function<void(std::string_view)> callback;
   };
   std::vector<item> items_;
 };

test/test-lsp-workspace-configuration.cpp

@@ -55,11 +55,10 @@ TEST(test_lsp_workspace_configuration, empty_config_request) {
 }
 
 TEST(test_lsp_workspace_configuration, config_request_with_three_items) {
-  std::string items[3];
   lsp_workspace_configuration config;
-  config.add_item(u8"first"sv, &items[0]);
-  config.add_item(u8"second"sv, &items[1]);
-  config.add_item(u8"third"sv, &items[2]);
+  config.add_item(u8"first"sv, [](std::string_view) {});
+  config.add_item(u8"second"sv, [](std::string_view) {});
+  config.add_item(u8"third"sv, [](std::string_view) {});
 
   byte_buffer request_json;
   config.build_request(77, request_json);
@@ -85,9 +84,15 @@ TEST(test_lsp_workspace_configuration, empty_config_response) {
 TEST(test_lsp_workspace_configuration, config_response_with_strings) {
   std::string items[3];
   lsp_workspace_configuration config;
-  config.add_item(u8"first"sv, &items[0]);
-  config.add_item(u8"second"sv, &items[1]);
-  config.add_item(u8"third"sv, &items[2]);
+  config.add_item(u8"first"sv, [&items](std::string_view new_value) {
+    items[0] = new_value;
+  });
+  config.add_item(u8"second"sv, [&items](std::string_view new_value) {
+    items[1] = new_value;
+  });
+  config.add_item(u8"third"sv, [&items](std::string_view new_value) {
+    items[2] = new_value;
+  });
 
   easy_simdjson_parser result(
       R"(["firstval", "secondval", "thirdval"])"_padded);
@@ -102,43 +107,56 @@ TEST(test_lsp_workspace_configuration, config_response_with_strings) {
 
 TEST(test_lsp_workspace_configuration,
      empty_config_response_with_added_items_fails) {
-  std::string myitem = "originalvalue";
   lsp_workspace_configuration config;
-  config.add_item(u8"myitem"sv, &myitem);
+  bool myitem_callback_called = false;
+  config.add_item(u8"myitem"sv, [&myitem_callback_called](
+                                    std::string_view new_value) {
+    myitem_callback_called = true;
+    ADD_FAILURE() << "myitem callback should not have been called; new_value="
+                  << new_value;
+  });
 
   easy_simdjson_parser result("[]"_padded);
   ASSERT_EQ(result.error, ::simdjson::SUCCESS);
   bool ok = config.process_response(result.value);
   ASSERT_FALSE(ok);
 
-  EXPECT_EQ(myitem, "originalvalue") << "item should not change value";
+  EXPECT_FALSE(myitem_callback_called);
 }
 
 TEST(test_lsp_workspace_configuration,
-     more_values_than_config_fails_but_sets_values_anyway) {
-  std::string myitem;
+     more_values_than_config_fails_but_calls_callback_anyway) {
   lsp_workspace_configuration config;
-  config.add_item(u8"myitem"sv, &myitem);
+  bool myitem_callback_called = false;
+  config.add_item(u8"myitem"sv,
+                  [&myitem_callback_called](std::string_view new_value) {
+                    myitem_callback_called = true;
+                    EXPECT_EQ(new_value, "val");
+                  });
 
   easy_simdjson_parser result(R"(["val", "otherval"])"_padded);
   ASSERT_EQ(result.error, ::simdjson::SUCCESS);
   bool ok = config.process_response(result.value);
   ASSERT_FALSE(ok);
 
-  EXPECT_EQ(myitem, "val");
+  EXPECT_TRUE(myitem_callback_called);
 }
 
-TEST(test_lsp_workspace_configuration, null_clears_string) {
-  std::string myitem = "hello";
+TEST(test_lsp_workspace_configuration, null_is_coerced_to_empty_string) {
   lsp_workspace_configuration config;
-  config.add_item(u8"myitem"sv, &myitem);
+  bool myitem_callback_called = false;
+  config.add_item(u8"myitem"sv,
+                  [&myitem_callback_called](std::string_view new_value) {
+                    myitem_callback_called = true;
+                    EXPECT_EQ(new_value, "");
+                  });
 
   easy_simdjson_parser result(R"([null])"_padded);
   ASSERT_EQ(result.error, ::simdjson::SUCCESS);
   bool ok = config.process_response(result.value);
   EXPECT_TRUE(ok);
 
-  EXPECT_EQ(myitem, "");
+  EXPECT_TRUE(myitem_callback_called);
 }
 
 TEST(test_lsp_workspace_configuration, non_array_config_response_fails) {