From bf11424b7fcabf997c2d43c7d0d9949351588b45 Mon Sep 17 00:00:00 2001 From: Marc-Andre Laperle Date: Mon, 2 Oct 2017 18:00:37 +0000 Subject: [PATCH] [clangd] Handle workspace/didChangeWatchedFiles Summary: The client can send notifications when it detects watched files have changed. This patch adds the protocol handling for this type of notification. For now, the notification will be passed down to the ClangdServer, but it will not be acted upon. However, this will become useful for the indexer to react to file changes. The events could also potentially be used to invalidate other caches (compilation database, etc). This change also updates the VSCode extension so that it sends the events. Signed-off-by: Marc-Andre Laperle Reviewers: ilya-biryukov, Nebiroth Subscribers: ilya-biryukov Tags: #clang-tools-extra Differential Revision: https://reviews.llvm.org/D38422 llvm-svn: 314693 --- clang-tools-extra/clangd/ClangdLSPServer.cpp | 4 ++ clang-tools-extra/clangd/ClangdLSPServer.h | 1 + clang-tools-extra/clangd/ClangdServer.cpp | 5 ++ clang-tools-extra/clangd/ClangdServer.h | 2 + clang-tools-extra/clangd/Protocol.cpp | 78 ++++++++++++++++++++++ clang-tools-extra/clangd/Protocol.h | 27 ++++++++ clang-tools-extra/clangd/ProtocolHandlers.cpp | 24 ++++++- clang-tools-extra/clangd/ProtocolHandlers.h | 1 + .../clangd/clients/clangd-vscode/package.json | 5 ++ .../clangd/clients/clangd-vscode/src/extension.ts | 8 ++- .../test/clangd/did-change-watch-files.test | 61 +++++++++++++++++ 11 files changed, 214 insertions(+), 2 deletions(-) create mode 100644 clang-tools-extra/test/clangd/did-change-watch-files.test diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp index 19a8935..0384423 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.cpp +++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp @@ -73,6 +73,10 @@ void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams Params, Params.contentChanges[0].text); } +void ClangdLSPServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { + Server.onFileEvent(Params); +} + void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams Params, JSONOutput &Out) { Server.removeDocument(Params.textDocument.uri.file); diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h index 608ff9b..656252f 100644 --- a/clang-tools-extra/clangd/ClangdLSPServer.h +++ b/clang-tools-extra/clangd/ClangdLSPServer.h @@ -71,6 +71,7 @@ private: JSONOutput &Out) override; void onSwitchSourceHeader(TextDocumentIdentifier Params, StringRef ID, JSONOutput &Out) override; + void onFileEvent(const DidChangeWatchedFilesParams &Params) override; std::vector getFixIts(StringRef File, const clangd::Diagnostic &D); diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index 2cf2cb7..f5d0325 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -424,3 +424,8 @@ ClangdServer::scheduleCancelRebuild(std::shared_ptr Resources) { std::move(DeferredCancel)); return DoneFuture; } + +void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) { + // FIXME: Do nothing for now. This will be used for indexing and potentially + // invalidating other caches. +} diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h index b819264..b42d2d6 100644 --- a/clang-tools-extra/clangd/ClangdServer.h +++ b/clang-tools-extra/clangd/ClangdServer.h @@ -268,6 +268,8 @@ public: /// Waits until all requests to worker thread are finished and dumps AST for /// \p File. \p File must be in the list of added documents. std::string dumpAST(PathRef File); + /// Called when an event occurs for a watched file in the workspace. + void onFileEvent(const DidChangeWatchedFilesParams &Params); private: std::future diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp index e68add6..f45e07d1 100644 --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -447,6 +447,84 @@ DidChangeTextDocumentParams::parse(llvm::yaml::MappingNode *Params, return Result; } +llvm::Optional FileEvent::parse(llvm::yaml::MappingNode *Params, + clangd::Logger &Logger) { + llvm::Optional Result = FileEvent(); + for (auto &NextKeyValue : *Params) { + // We have to consume the whole MappingNode because it doesn't support + // skipping and we want to be able to parse further valid events. + if (!Result) + continue; + + auto *KeyString = dyn_cast(NextKeyValue.getKey()); + if (!KeyString) { + Result.reset(); + continue; + } + + llvm::SmallString<10> KeyStorage; + StringRef KeyValue = KeyString->getValue(KeyStorage); + auto *Value = + dyn_cast_or_null(NextKeyValue.getValue()); + if (!Value) { + Result.reset(); + continue; + } + llvm::SmallString<10> Storage; + if (KeyValue == "uri") { + Result->uri = URI::parse(Value); + } else if (KeyValue == "type") { + long long Val; + if (llvm::getAsSignedInteger(Value->getValue(Storage), 0, Val)) { + Result.reset(); + continue; + } + Result->type = static_cast(Val); + if (Result->type < FileChangeType::Created || + Result->type > FileChangeType::Deleted) + Result.reset(); + } else { + logIgnoredField(KeyValue, Logger); + } + } + return Result; +} + +llvm::Optional +DidChangeWatchedFilesParams::parse(llvm::yaml::MappingNode *Params, + clangd::Logger &Logger) { + DidChangeWatchedFilesParams Result; + for (auto &NextKeyValue : *Params) { + auto *KeyString = dyn_cast(NextKeyValue.getKey()); + if (!KeyString) + return llvm::None; + + llvm::SmallString<10> KeyStorage; + StringRef KeyValue = KeyString->getValue(KeyStorage); + auto *Value = NextKeyValue.getValue(); + + llvm::SmallString<10> Storage; + if (KeyValue == "changes") { + auto *Seq = dyn_cast(Value); + if (!Seq) + return llvm::None; + for (auto &Item : *Seq) { + auto *I = dyn_cast(&Item); + if (!I) + return llvm::None; + auto Parsed = FileEvent::parse(I, Logger); + if (Parsed) + Result.changes.push_back(std::move(*Parsed)); + else + Logger.log("Failed to decode a FileEvent.\n"); + } + } else { + logIgnoredField(KeyValue, Logger); + } + } + return Result; +} + llvm::Optional TextDocumentContentChangeEvent::parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger) { diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h index 71ab957..a6fcca6 100644 --- a/clang-tools-extra/clangd/Protocol.h +++ b/clang-tools-extra/clangd/Protocol.h @@ -237,6 +237,33 @@ struct DidChangeTextDocumentParams { parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger); }; +enum class FileChangeType { + /// The file got created. + Created = 1, + /// The file got changed. + Changed = 2, + /// The file got deleted. + Deleted = 3 +}; + +struct FileEvent { + /// The file's URI. + URI uri; + /// The change type. + FileChangeType type; + + static llvm::Optional parse(llvm::yaml::MappingNode *Params, + clangd::Logger &Logger); +}; + +struct DidChangeWatchedFilesParams { + /// The actual file events. + std::vector changes; + + static llvm::Optional + parse(llvm::yaml::MappingNode *Params, clangd::Logger &Logger); +}; + struct FormattingOptions { /// Size of a tab in spaces. int tabSize; diff --git a/clang-tools-extra/clangd/ProtocolHandlers.cpp b/clang-tools-extra/clangd/ProtocolHandlers.cpp index 544c44c..936c673 100644 --- a/clang-tools-extra/clangd/ProtocolHandlers.cpp +++ b/clang-tools-extra/clangd/ProtocolHandlers.cpp @@ -226,6 +226,25 @@ private: ProtocolCallbacks &Callbacks; }; +struct WorkspaceDidChangeWatchedFilesHandler : Handler { + WorkspaceDidChangeWatchedFilesHandler(JSONOutput &Output, + ProtocolCallbacks &Callbacks) + : Handler(Output), Callbacks(Callbacks) {} + + void handleNotification(llvm::yaml::MappingNode *Params) { + auto DCWFP = DidChangeWatchedFilesParams::parse(Params, Output); + if (!DCWFP) { + Output.log("Failed to decode DidChangeWatchedFilesParams.\n"); + return; + } + + Callbacks.onFileEvent(*DCWFP); + } + +private: + ProtocolCallbacks &Callbacks; +}; + } // namespace void clangd::registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, @@ -264,5 +283,8 @@ void clangd::registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, llvm::make_unique(Out, Callbacks)); Dispatcher.registerHandler( "textDocument/switchSourceHeader", - llvm::make_unique(Out, Callbacks)); + llvm::make_unique(Out, Callbacks)); + Dispatcher.registerHandler( + "workspace/didChangeWatchedFiles", + llvm::make_unique(Out, Callbacks)); } diff --git a/clang-tools-extra/clangd/ProtocolHandlers.h b/clang-tools-extra/clangd/ProtocolHandlers.h index 70796d3..8592e57 100644 --- a/clang-tools-extra/clangd/ProtocolHandlers.h +++ b/clang-tools-extra/clangd/ProtocolHandlers.h @@ -51,6 +51,7 @@ public: JSONOutput &Out) = 0; virtual void onSwitchSourceHeader(TextDocumentIdentifier Params, StringRef ID, JSONOutput &Out) = 0; + virtual void onFileEvent(const DidChangeWatchedFilesParams &Params) = 0; }; void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out, diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/package.json b/clang-tools-extra/clangd/clients/clangd-vscode/package.json index 729297a..c048d96 100644 --- a/clang-tools-extra/clangd/clients/clangd-vscode/package.json +++ b/clang-tools-extra/clangd/clients/clangd-vscode/package.json @@ -51,6 +51,11 @@ "type": "string" }, "description": "Arguments for clangd server" + }, + "clangd.syncFileEvents": { + "type": "boolean", + "default": true, + "description": "Whether or not to send file events to clangd (File created, changed or deleted). This can be disabled for performance consideration." } } } diff --git a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts index c76e0a0..f89ddc9 100644 --- a/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts +++ b/clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts @@ -18,18 +18,24 @@ function getConfig(option: string, defaultValue?: any) : T { export function activate(context: vscode.ExtensionContext) { const clangdPath = getConfig('path'); const clangdArgs = getConfig('arguments'); + const syncFileEvents = getConfig('syncFileEvents', true); const serverOptions: vscodelc.ServerOptions = { command: clangdPath, args: clangdArgs }; + const cppFileExtensions: string[] = ['cpp', 'c', 'cc', 'cxx', 'c++', 'm', 'mm', 'h', 'hh', 'hpp', 'hxx', 'inc']; + const cppFileExtensionsPattern = cppFileExtensions.join(); const clientOptions: vscodelc.LanguageClientOptions = { // Register the server for C/C++ files - documentSelector: ['c', 'cc', 'cpp', 'h', 'hh', 'hpp'], + documentSelector: cppFileExtensions, uriConverters: { // FIXME: by default the URI sent over the protocol will be percent encoded (see rfc3986#section-2.1) // the "workaround" below disables temporarily the encoding until decoding // is implemented properly in clangd code2Protocol: (uri: vscode.Uri) : string => uri.toString(true), protocol2Code: (uri: string) : vscode.Uri => vscode.Uri.parse(uri) + }, + synchronize: !syncFileEvents ? undefined : { + fileEvents: vscode.workspace.createFileSystemWatcher('**/*.{' + cppFileExtensionsPattern + '}') } }; diff --git a/clang-tools-extra/test/clangd/did-change-watch-files.test b/clang-tools-extra/test/clangd/did-change-watch-files.test new file mode 100644 index 0000000..d9ef6b4 --- /dev/null +++ b/clang-tools-extra/test/clangd/did-change-watch-files.test @@ -0,0 +1,61 @@ +# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s +# It is absolutely vital that this file has CRLF line endings. +# +# Test initialize request parameters with rootUri +Content-Length: 143 + +{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootUri":"file:///path/to/workspace","capabilities":{},"trace":"off"}} +# CHECK: Content-Length: 466 +# CHECK: {"jsonrpc":"2.0","id":0,"result":{"capabilities":{ +# CHECK: "textDocumentSync": 1, +# CHECK: "documentFormattingProvider": true, +# CHECK: "documentRangeFormattingProvider": true, +# CHECK: "documentOnTypeFormattingProvider": {"firstTriggerCharacter":"}","moreTriggerCharacter":[]}, +# CHECK: "codeActionProvider": true, +# CHECK: "completionProvider": {"resolveProvider": false, "triggerCharacters": [".",">",":"]}, +# CHECK: "definitionProvider": true +# CHECK: }}} +# +#Normal case +Content-Length: 217 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file.cpp","type":1},{"uri":"file:///path/to/file2.cpp","type":2},{"uri":"file:///path/to/file3.cpp","type":3}]}} + +# Wrong event type, integer +Content-Length: 173 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":0},{"uri":"file:///path/to/file3.cpp","type":4}]}} +# STDERR: Failed to decode a FileEvent. +# Wrong event type, string +Content-Length: 132 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":"foo"}]}} +# STDERR: Failed to decode a FileEvent. +#Custom event field +Content-Length: 143 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":1,"custom":"foo"}]}} +# STDERR: Failed to decode a FileEvent. +#Event field with object +Content-Length: 140 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[{"uri":"file:///path/to/file2.cpp","type":{"foo":"bar"}}]}} +# STDERR: Failed to decode a FileEvent. +# Changes field with sequence but no object +Content-Length: 86 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":[""]}} +# STDERR: Failed to decode DidChangeWatchedFilesParams. +# Changes field with no sequence +Content-Length: 84 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"changes":""}} +# STDERR: Failed to decode DidChangeWatchedFilesParams. +# Custom field +Content-Length: 86 + +{"jsonrpc":"2.0","method":"workspace/didChangeWatchedFiles","params":{"custom":"foo"}} +# STDERR: Ignored unknown field "custom" +Content-Length: 44 + +{"jsonrpc":"2.0","id":3,"method":"shutdown"} -- 2.7.4