[clangd] Implement format on type

[krasimir]

Summary

This patch adds onTypeFormatting to clangd.

The trigger character is '}' and it works by scanning for the matching '{' and formatting the range in-between.

There are problems with ';' as a trigger character, the cursor position is before the |:

int main() {
  int i;|
}

becomes:

int main() {  int i;| }

which is not likely what the user intended.

Also formatting at semicolon in a non-properly closed scope puts the following tokens in the same unwrapped line, which doesn't reformat nicely.

[krasimir]

• removed newline

[bkramer]

clangd/ProtocolHandlers.cpp

117

This should be a std::string in trunk, StringRef gives you a use after free.

119

nit: use the character overload of this function (Code.find_last_of('{'...)

[krasimir]

• Address review comments

[bkramer]

lg

clangd/ClangDMain.cpp

int main(int argc, char *argv[]) {
   // FIXME: Implement textDocument/didClose.
   Dispatcher.registerHandler(
       "textDocument/didChange",
       llvm::make_unique<TextDocumentDidChangeHandler>(Out, Store));
   Dispatcher.registerHandler(
       "textDocument/rangeFormatting",
       llvm::make_unique<TextDocumentRangeFormattingHandler>(Out, Store));
   Dispatcher.registerHandler(
+      "textDocument/onTypeFormatting",
+      llvm::make_unique<TextDocumentOnTypeFormattingHandler>(Out, Store));
+  Dispatcher.registerHandler(
       "textDocument/formatting",
       llvm::make_unique<TextDocumentFormattingHandler>(Out, Store));
 
   while (std::cin.good()) {
     // A Language Server Protocol message starts with a HTTP header, delimited
     // by \r\n.
     std::string Line;
     std::getline(std::cin, Line);

clangd/Protocol.h

struct DocumentRangeFormattingParams {
 
   /// The format options
   FormattingOptions options;
 
   static llvm::Optional<DocumentRangeFormattingParams>
   parse(llvm::yaml::MappingNode *Params);
 };
 
+struct DocumentOnTypeFormattingParams {
+  /// The document to format.
+  TextDocumentIdentifier textDocument;
+
+  /// The position at which this request was sent.
+  Position position;
+
+  /// The character that has been typed.
+  std::string ch;
+
+  /// The format options.
+  FormattingOptions options;
+
+  static llvm::Optional<DocumentOnTypeFormattingParams>
+  parse(llvm::yaml::MappingNode *Params);
+};
+
 struct DocumentFormattingParams {
   /// The document to format.
   TextDocumentIdentifier textDocument;
 
   /// The format options
   FormattingOptions options;
 
   static llvm::Optional<DocumentFormattingParams>
   parse(llvm::yaml::MappingNode *Params);
 };
 
 } // namespace clangd
 } // namespace clang
 
 #endif

clangd/Protocol.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "Protocol.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
+
 using namespace clang::clangd;
 
 llvm::Optional<TextDocumentIdentifier>
 TextDocumentIdentifier::parse(llvm::yaml::MappingNode *Params) {
   TextDocumentIdentifier Result;
   for (auto &NextKeyValue : *Params) {
     auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
     if (!KeyString)
if (KeyValue == "textDocument") {
       Result.options = std::move(*Parsed);
     } else {
       return llvm::None;
     }
   }
   return Result;
 }
 
+llvm::Optional<DocumentOnTypeFormattingParams>
+DocumentOnTypeFormattingParams::parse(llvm::yaml::MappingNode *Params) {
+  DocumentOnTypeFormattingParams Result;
+  for (auto &NextKeyValue : *Params) {
+    auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
+    if (!KeyString)
+      return llvm::None;
+
+    llvm::SmallString<10> KeyStorage;
+    StringRef KeyValue = KeyString->getValue(KeyStorage);
+
+    if (KeyValue == "ch") {
+      auto *ScalarValue =
+          dyn_cast_or_null<llvm::yaml::ScalarNode>(NextKeyValue.getValue());
+      if (!ScalarValue)
+        return llvm::None;
+      llvm::SmallString<10> Storage;
+      Result.ch = ScalarValue->getValue(Storage);
+      continue;
+    }
+
+    auto *Value =
+        dyn_cast_or_null<llvm::yaml::MappingNode>(NextKeyValue.getValue());
+    if (!Value)
+      return llvm::None;
+    if (KeyValue == "textDocument") {
+      auto Parsed = TextDocumentIdentifier::parse(Value);
+      if (!Parsed)
+        return llvm::None;
+      Result.textDocument = std::move(*Parsed);
+    } else if (KeyValue == "position") {
+      auto Parsed = Position::parse(Value);
+      if (!Parsed)
+        return llvm::None;
+      Result.position = std::move(*Parsed);
+    } else if (KeyValue == "options") {
+      auto Parsed = FormattingOptions::parse(Value);
+      if (!Parsed)
+        return llvm::None;
+      Result.options = std::move(*Parsed);
+    } else {
+      return llvm::None;
+    }
+  }
+  return Result;
+}
+
 llvm::Optional<DocumentFormattingParams>
 DocumentFormattingParams::parse(llvm::yaml::MappingNode *Params) {
   DocumentFormattingParams Result;
   for (auto &NextKeyValue : *Params) {
     auto *KeyString = dyn_cast<llvm::yaml::ScalarNode>(NextKeyValue.getKey());
     if (!KeyString)
       return llvm::None;
 

clangd/ProtocolHandlers.h

struct InitializeHandler : Handler {
   InitializeHandler(JSONOutput &Output) : Handler(Output) {}
 
   void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override {
     writeMessage(
         R"({"jsonrpc":"2.0","id":)" + ID +
         R"(,"result":{"capabilities":{
           "textDocumentSync": 1,
           "documentFormattingProvider": true,
-          "documentRangeFormattingProvider": true
+          "documentRangeFormattingProvider": true,
+          "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}
         }}})");
   }
 };
 
 struct ShutdownHandler : Handler {
   ShutdownHandler(JSONOutput &Output) : Handler(Output) {}
 
   void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override {
TextDocumentDidChangeHandler(JSONOutput &Output, DocumentStore &Store)
       : Handler(Output), Store(Store) {}
 
   void handleNotification(llvm::yaml::MappingNode *Params) override;
 
 private:
   DocumentStore &Store;
 };
 
+struct TextDocumentOnTypeFormattingHandler : Handler {
+  TextDocumentOnTypeFormattingHandler(JSONOutput &Output, DocumentStore &Store)
+      : Handler(Output), Store(Store) {}
+
+  void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override;
+
+private:
+  DocumentStore &Store;
+};
+
 struct TextDocumentRangeFormattingHandler : Handler {
   TextDocumentRangeFormattingHandler(JSONOutput &Output, DocumentStore &Store)
       : Handler(Output), Store(Store) {}
 
   void handleMethod(llvm::yaml::MappingNode *Params, StringRef ID) override;
 
 private:
   DocumentStore &Store;

clangd/ProtocolHandlers.cpp

void TextDocumentRangeFormattingHandler::handleMethod(
 
   size_t Begin = positionToOffset(Code, DRFP->range.start);
   size_t Len = positionToOffset(Code, DRFP->range.end) - Begin;
 
   writeMessage(formatCode(Code, DRFP->textDocument.uri,
                           {clang::tooling::Range(Begin, Len)}, ID));
 }
 
+void TextDocumentOnTypeFormattingHandler::handleMethod(
+    llvm::yaml::MappingNode *Params, StringRef ID) {
+  auto DOTFP = DocumentOnTypeFormattingParams::parse(Params);
+  if (!DOTFP) {
+    Output.logs() << "Failed to decode DocumentOnTypeFormattingParams!\n";
+    return;
+  }
+
+  // Look for the previous opening brace from the character position and format
+  // starting from there.
+  StringRef Code = Store.getDocument(DOTFP->textDocument.uri);
+  size_t CursorPos = positionToOffset(Code, DOTFP->position);
+  size_t PreviousLBracePos = Code.find_last_of("{", CursorPos);
+  if (PreviousLBracePos == StringRef::npos)
+    PreviousLBracePos = CursorPos;
+  size_t Len = 1 + CursorPos - PreviousLBracePos;
+
+  writeMessage(formatCode(Code, DOTFP->textDocument.uri,
+                          {clang::tooling::Range(PreviousLBracePos, Len)}, ID));
+}
+
 void TextDocumentFormattingHandler::handleMethod(
     llvm::yaml::MappingNode *Params, StringRef ID) {
   auto DFP = DocumentFormattingParams::parse(Params);
   if (!DFP) {
     Output.logs() << "Failed to decode DocumentFormattingParams!\n";
     return;
   }
 
   // Format everything.
   StringRef Code = Store.getDocument(DFP->textDocument.uri);
   writeMessage(formatCode(Code, DFP->textDocument.uri,
                           {clang::tooling::Range(0, Code.size())}, ID));
 }

test/clangd/formatting.test

 # RUN: clangd < %s | FileCheck %s
 # It is absolutely vital that this file has CRLF line endings.
 #
 Content-Length: 125
 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-# CHECK: Content-Length: 191
+# CHECK: Content-Length: 294
 # CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{
 # CHECK:   "textDocumentSync": 1,
 # CHECK:   "documentFormattingProvider": true,
-# CHECK:   "documentRangeFormattingProvider": true
+# CHECK:   "documentRangeFormattingProvider": true,
+# CHECK:   "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}
 # CHECK: }}}
 #
 Content-Length: 193
 
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///foo.c","languageId":"c","version":1,"text":"int foo ( int x ) {\n    x = x+1;\n    return x;\n    }"}}}
 #
 #
 Content-Length: 233

 {"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///foo.c","version":9},"contentChanges":[{"text":"int foo(int x) {\n  x = x + 1;\n  return x;\n}"}]}}
 #
 #
 Content-Length: 153
 
 {"jsonrpc":"2.0","id":4,"method":"textDocument/formatting","params":{"textDocument":{"uri":"file:///foo.c"},"options":{"tabSize":4,"insertSpaces":true}}}
 # CHECK: {"jsonrpc":"2.0","id":4,"result":[]}
 #
+Content-Length: 193
+
+{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///foo.c","version":5},"contentChanges":[{"text":"int foo ( int x ) {\n  x = x + 1;\n  return x;\n}"}]}}
+#
+#
+Content-Length: 204
+
+{"jsonrpc":"2.0","id":5,"method":"textDocument/onTypeFormatting","params":{"textDocument":{"uri":"file:///foo.c"},"position":{"line":3,"character":1},"ch":"}","options":{"tabSize":4,"insertSpaces":true}}}
+# CHECK: {"jsonrpc":"2.0","id":5,"result":[{"range": {"start": {"line": 0, "character": 7}, "end": {"line": 0, "character": 8}}, "newText": ""},{"range": {"start": {"line": 0, "character": 9}, "end": {"line": 0, "character": 10}}, "newText": ""},{"range": {"start": {"line": 0, "character": 15}, "end": {"line": 0, "character": 16}}, "newText": ""}]}
+#
+
 Content-Length: 44
 
-{"jsonrpc":"2.0","id":5,"method":"shutdown"}
+{"jsonrpc":"2.0","id":6,"method":"shutdown"}