[PATCH] D95229: [clangd] Treat optional field type mismatches as soft failures

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Clangd currently throws away any protocol messages whenever an optional
field has an unexpected type. This patch changes the behaviour to treat such
fields as missing.

This enables clangd to be more tolerant against small violations to the LSP
spec.

Fixes https://github.com/clangd/vscode-clangd/issues/134


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D95229

Files:
  clang-tools-extra/clangd/Protocol.cpp


Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -27,6 +27,19 @@
 
 namespace clang {
 namespace clangd {
+namespace {
+// Helper to ignore type mismatches on optional fields.
+template <typename T>
+bool mapOptionalOrLog(llvm::json::ObjectMapper &O, llvm::StringLiteral Prop,
+                      T &Out) {
+  if (!O.mapOptional(Prop, Out)) {
+    elog("Unexpected value for optional field '{0}', ignoring", Prop);
+    Out = T{}; // Default-initialize Out to ensure we don't have a
+               // half-constructed object.
+  }
+  return true;
+}
+} // namespace
 
 char LSPError::ID;
 
@@ -490,7 +503,7 @@
   return O && O.map("textDocument", R.textDocument) &&
          O.map("contentChanges", R.contentChanges) &&
          O.map("wantDiagnostics", R.wantDiagnostics) &&
-         O.mapOptional("forceRebuild", R.forceRebuild);
+         mapOptionalOrLog(O, "forceRebuild", R.forceRebuild);
 }
 
 bool fromJSON(const llvm::json::Value &E, FileChangeType &Out,
@@ -580,10 +593,10 @@
               llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
   return O && O.map("range", R.range) && O.map("message", R.message) &&
-         O.mapOptional("severity", R.severity) &&
-         O.mapOptional("category", R.category) &&
-         O.mapOptional("code", R.code) && O.mapOptional("source", R.source);
-  return true;
+         mapOptionalOrLog(O, "severity", R.severity) &&
+         mapOptionalOrLog(O, "category", R.category) &&
+         mapOptionalOrLog(O, "code", R.code) &&
+         mapOptionalOrLog(O, "source", R.source);
 }
 
 llvm::json::Value toJSON(const PublishDiagnosticsParams &PDP) {
@@ -817,9 +830,9 @@
               llvm::json::Path P) {
   llvm::json::ObjectMapper O(Params, P);
   int TriggerKind;
-  if (!O || !O.map("triggerKind", TriggerKind) ||
-      !O.mapOptional("triggerCharacter", R.triggerCharacter))
+  if (!O || !O.map("triggerKind", TriggerKind))
     return false;
+  mapOptionalOrLog(O, "triggerCharacter", R.triggerCharacter);
   R.triggerKind = static_cast<CompletionTriggerKind>(TriggerKind);
   return true;
 }
@@ -1121,8 +1134,9 @@
   llvm::json::ObjectMapper O(Params, P);
   if (!O)
     return true; // 'any' type in LSP.
-  return O.mapOptional("compilationDatabaseChanges",
-                       S.compilationDatabaseChanges);
+  mapOptionalOrLog(O, "compilationDatabaseChanges",
+                   S.compilationDatabaseChanges);
+  return true;
 }
 
 bool fromJSON(const llvm::json::Value &Params, InitializationOptions &Opts,
@@ -1133,8 +1147,8 @@
 
   return fromJSON(Params, Opts.ConfigSettings, P) &&
          O.map("compilationDatabasePath", Opts.compilationDatabasePath) &&
-         O.mapOptional("fallbackFlags", Opts.fallbackFlags) &&
-         O.mapOptional("clangdFileStatus", Opts.FileStatus);
+         mapOptionalOrLog(O, "fallbackFlags", Opts.fallbackFlags) &&
+         mapOptionalOrLog(O, "clangdFileStatus", Opts.FileStatus);
 }
 
 bool fromJSON(const llvm::json::Value &E, TypeHierarchyDirection &Out,
@@ -1190,10 +1204,11 @@
   return O && O.map("name", I.name) && O.map("kind", I.kind) &&
          O.map("uri", I.uri) && O.map("range", I.range) &&
          O.map("selectionRange", I.selectionRange) &&
-         O.mapOptional("detail", I.detail) &&
-         O.mapOptional("deprecated", I.deprecated) &&
-         O.mapOptional("parents", I.parents) &&
-         O.mapOptional("children", I.children) && O.mapOptional("data", I.data);
+         mapOptionalOrLog(O, "detail", I.detail) &&
+         mapOptionalOrLog(O, "deprecated", I.deprecated) &&
+         mapOptionalOrLog(O, "parents", I.parents) &&
+         mapOptionalOrLog(O, "children", I.children) &&
+         mapOptionalOrLog(O, "data", I.data);
 }
 
 bool fromJSON(const llvm::json::Value &Params,
@@ -1238,7 +1253,7 @@
   return O && O.map("name", I.name) && O.map("kind", I.kind) &&
          O.map("uri", I.uri) && O.map("range", I.range) &&
          O.map("selectionRange", I.selectionRange) &&
-         O.mapOptional("data", I.data);
+         mapOptionalOrLog(O, "data", I.data);
 }
 
 bool fromJSON(const llvm::json::Value &Params,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits