Author: krasimir Date: Fri Apr 7 06:03:26 2017 New Revision: 299758 URL: http://llvm.org/viewvc/llvm-project?rev=299758&view=rev Log: [clangd] Extract FsPath from file:// uri
Patch contributed by stanionascu! rfc8089#appendix-E.2 specifies that paths can begin with a drive letter e.g. as file:///c:/. In this case just consuming front file:// is not enough and the 3rd slash must be consumed to produce a valid path on windows. The patch introduce a generic way of converting an uri to a filesystem path and back. Differential Revision: https://reviews.llvm.org/D31401 Modified: clang-tools-extra/trunk/clangd/ASTManager.cpp clang-tools-extra/trunk/clangd/ASTManager.h clang-tools-extra/trunk/clangd/DocumentStore.h clang-tools-extra/trunk/clangd/Protocol.cpp clang-tools-extra/trunk/clangd/Protocol.h clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Modified: clang-tools-extra/trunk/clangd/ASTManager.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.cpp?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ASTManager.cpp (original) +++ clang-tools-extra/trunk/clangd/ASTManager.cpp Fri Apr 7 06:03:26 2017 @@ -28,7 +28,6 @@ getRemappedFiles(const DocumentStore &Do std::vector<ASTUnit::RemappedFile> RemappedFiles; for (const auto &P : Docs.getAllDocuments()) { StringRef FileName = P.first; - FileName.consume_front("file://"); RemappedFiles.push_back(ASTUnit::RemappedFile( FileName, llvm::MemoryBuffer::getMemBufferCopy(P.second, FileName).release())); @@ -142,7 +141,7 @@ void ASTManager::parseFileAndPublishDiag Diagnostics.pop_back(); // Drop trailing comma. Output.writeMessage( R"({"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":")" + - File + R"(","diagnostics":[)" + Diagnostics + R"(]}})"); + URI::fromFile(File).uri + R"(","diagnostics":[)" + Diagnostics + R"(]}})"); } ASTManager::~ASTManager() { @@ -155,42 +154,39 @@ ASTManager::~ASTManager() { ClangWorker.join(); } -void ASTManager::onDocumentAdd(StringRef Uri) { +void ASTManager::onDocumentAdd(StringRef File) { if (RunSynchronously) { - parseFileAndPublishDiagnostics(Uri); + parseFileAndPublishDiagnostics(File); return; } std::lock_guard<std::mutex> Guard(RequestLock); // Currently we discard all pending requests and just enqueue the latest one. RequestQueue.clear(); - RequestQueue.push_back(Uri); + RequestQueue.push_back(File); ClangRequestCV.notify_one(); } tooling::CompilationDatabase * -ASTManager::getOrCreateCompilationDatabaseForFile(StringRef Uri) { - auto &I = CompilationDatabases[Uri]; +ASTManager::getOrCreateCompilationDatabaseForFile(StringRef File) { + auto &I = CompilationDatabases[File]; if (I) return I.get(); - Uri.consume_front("file://"); - std::string Error; - I = tooling::CompilationDatabase::autoDetectFromSource(Uri, Error); + I = tooling::CompilationDatabase::autoDetectFromSource(File, Error); Output.log("Failed to load compilation database: " + Twine(Error) + "\n"); return I.get(); } std::unique_ptr<clang::ASTUnit> -ASTManager::createASTUnitForFile(StringRef Uri, const DocumentStore &Docs) { +ASTManager::createASTUnitForFile(StringRef File, const DocumentStore &Docs) { tooling::CompilationDatabase *CDB = - getOrCreateCompilationDatabaseForFile(Uri); + getOrCreateCompilationDatabaseForFile(File); - Uri.consume_front("file://"); std::vector<tooling::CompileCommand> Commands; if (CDB) { - Commands = CDB->getCompileCommands(Uri); + Commands = CDB->getCompileCommands(File); // chdir. This is thread hostile. if (!Commands.empty()) llvm::sys::fs::set_current_path(Commands.front().Directory); @@ -198,8 +194,8 @@ ASTManager::createASTUnitForFile(StringR if (Commands.empty()) { // Add a fake command line if we know nothing. Commands.push_back(tooling::CompileCommand( - llvm::sys::path::parent_path(Uri), llvm::sys::path::filename(Uri), - {"clang", "-fsyntax-only", Uri.str()}, "")); + llvm::sys::path::parent_path(File), llvm::sys::path::filename(File), + {"clang", "-fsyntax-only", File.str()}, "")); } // Inject the resource dir. @@ -278,7 +274,7 @@ public: } // namespace std::vector<CompletionItem> -ASTManager::codeComplete(StringRef Uri, unsigned Line, unsigned Column) { +ASTManager::codeComplete(StringRef File, unsigned Line, unsigned Column) { CodeCompleteOptions CCO; CCO.IncludeBriefComments = 1; // This is where code completion stores dirty buffers. Need to free after @@ -290,15 +286,13 @@ ASTManager::codeComplete(StringRef Uri, std::vector<CompletionItem> Items; CompletionItemsCollector Collector(&Items, CCO); std::lock_guard<std::mutex> Guard(ASTLock); - auto &Unit = ASTs[Uri]; + auto &Unit = ASTs[File]; if (!Unit) - Unit = createASTUnitForFile(Uri, this->Store); + Unit = createASTUnitForFile(File, this->Store); if (!Unit) return {}; IntrusiveRefCntPtr<SourceManager> SourceMgr( new SourceManager(*DiagEngine, Unit->getFileManager())); - StringRef File(Uri); - File.consume_front("file://"); // CodeComplete seems to require fresh LangOptions. LangOptions LangOpts = Unit->getLangOpts(); // The language server protocol uses zero-based line and column numbers. Modified: clang-tools-extra/trunk/clangd/ASTManager.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ASTManager.h?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ASTManager.h (original) +++ clang-tools-extra/trunk/clangd/ASTManager.h Fri Apr 7 06:03:26 2017 @@ -34,7 +34,7 @@ public: ASTManager(JSONOutput &Output, DocumentStore &Store, bool RunSynchronously); ~ASTManager() override; - void onDocumentAdd(StringRef Uri) override; + void onDocumentAdd(StringRef File) override; // FIXME: Implement onDocumentRemove /// Get code completions at a specified \p Line and \p Column in \p File. @@ -61,21 +61,21 @@ private: // asynchronously. bool RunSynchronously; - /// Loads a compilation database for URI. May return nullptr if it fails. The + /// Loads a compilation database for File. May return nullptr if it fails. The /// database is cached for subsequent accesses. clang::tooling::CompilationDatabase * - getOrCreateCompilationDatabaseForFile(StringRef Uri); - // Creates a new ASTUnit for the document at Uri. + getOrCreateCompilationDatabaseForFile(StringRef File); + // Creates a new ASTUnit for the document at File. // FIXME: This calls chdir internally, which is thread unsafe. std::unique_ptr<clang::ASTUnit> - createASTUnitForFile(StringRef Uri, const DocumentStore &Docs); + createASTUnitForFile(StringRef File, const DocumentStore &Docs); void runWorker(); void parseFileAndPublishDiagnostics(StringRef File); /// Clang objects. - /// A map from Uri-s to ASTUnit-s. Guarded by \c ASTLock. ASTUnit-s are used + /// A map from File-s to ASTUnit-s. Guarded by \c ASTLock. ASTUnit-s are used /// for generating diagnostics and fix-it-s asynchronously by the worker /// thread and synchronously for code completion. /// Modified: clang-tools-extra/trunk/clangd/DocumentStore.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/DocumentStore.h?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/DocumentStore.h (original) +++ clang-tools-extra/trunk/clangd/DocumentStore.h Fri Apr 7 06:03:26 2017 @@ -22,40 +22,40 @@ class DocumentStore; struct DocumentStoreListener { virtual ~DocumentStoreListener() = default; - virtual void onDocumentAdd(StringRef Uri) {} - virtual void onDocumentRemove(StringRef Uri) {} + virtual void onDocumentAdd(StringRef File) {} + virtual void onDocumentRemove(StringRef File) {} }; -/// A container for files opened in a workspace, addressed by URI. The contents +/// A container for files opened in a workspace, addressed by File. The contents /// are owned by the DocumentStore. class DocumentStore { public: /// Add a document to the store. Overwrites existing contents. - void addDocument(StringRef Uri, StringRef Text) { + void addDocument(StringRef File, StringRef Text) { { std::lock_guard<std::mutex> Guard(DocsMutex); - Docs[Uri] = Text; + Docs[File] = Text; } for (const auto &Listener : Listeners) - Listener->onDocumentAdd(Uri); + Listener->onDocumentAdd(File); } /// Delete a document from the store. - void removeDocument(StringRef Uri) { + void removeDocument(StringRef File) { { std::lock_guard<std::mutex> Guard(DocsMutex); - Docs.erase(Uri); + Docs.erase(File); } for (const auto &Listener : Listeners) - Listener->onDocumentRemove(Uri); + Listener->onDocumentRemove(File); } /// Retrieve a document from the store. Empty string if it's unknown. /// /// This function is thread-safe. It returns a copy to avoid handing out /// references to unguarded data. - std::string getDocument(StringRef Uri) const { + std::string getDocument(StringRef File) const { // FIXME: This could be a reader lock. std::lock_guard<std::mutex> Guard(DocsMutex); - return Docs.lookup(Uri); + return Docs.lookup(File); } /// Add a listener. Does not take ownership. Modified: clang-tools-extra/trunk/clangd/Protocol.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp (original) +++ clang-tools-extra/trunk/clangd/Protocol.cpp Fri Apr 7 06:03:26 2017 @@ -17,8 +17,44 @@ #include "llvm/ADT/SmallString.h" #include "llvm/Support/Format.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Support/Path.h" using namespace clang::clangd; + +URI URI::fromUri(llvm::StringRef uri) { + URI Result; + Result.uri = uri; + uri.consume_front("file://"); + // For Windows paths e.g. /X: + if (uri.size() > 2 && uri[0] == '/' && uri[2] == ':') + uri.consume_front("/"); + // Make sure that file paths are in native separators + Result.file = llvm::sys::path::convert_to_slash(uri); + return Result; +} + +URI URI::fromFile(llvm::StringRef file) { + using namespace llvm::sys; + URI Result; + Result.file = file; + Result.uri = "file://"; + // For Windows paths e.g. X: + if (file.size() > 1 && file[1] == ':') + Result.uri += "/"; + // Make sure that uri paths are with posix separators + Result.uri += path::convert_to_slash(file, path::Style::posix); + return Result; +} + +URI URI::parse(llvm::yaml::ScalarNode *Param) { + llvm::SmallString<10> Storage; + return URI::fromUri(Param->getValue(Storage)); +} + +std::string URI::unparse(const URI &U) { + return U.uri; +} + llvm::Optional<TextDocumentIdentifier> TextDocumentIdentifier::parse(llvm::yaml::MappingNode *Params) { TextDocumentIdentifier Result; @@ -34,9 +70,8 @@ TextDocumentIdentifier::parse(llvm::yaml if (!Value) return llvm::None; - llvm::SmallString<10> Storage; if (KeyValue == "uri") { - Result.uri = Value->getValue(Storage); + Result.uri = URI::parse(Value); } else if (KeyValue == "version") { // FIXME: parse version, but only for VersionedTextDocumentIdentifiers. } else { @@ -142,7 +177,7 @@ TextDocumentItem::parse(llvm::yaml::Mapp llvm::SmallString<10> Storage; if (KeyValue == "uri") { - Result.uri = Value->getValue(Storage); + Result.uri = URI::parse(Value); } else if (KeyValue == "languageId") { Result.languageId = Value->getValue(Storage); } else if (KeyValue == "version") { Modified: clang-tools-extra/trunk/clangd/Protocol.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h (original) +++ clang-tools-extra/trunk/clangd/Protocol.h Fri Apr 7 06:03:26 2017 @@ -29,9 +29,20 @@ namespace clang { namespace clangd { +struct URI { + std::string uri; + std::string file; + + static URI fromUri(llvm::StringRef uri); + static URI fromFile(llvm::StringRef file); + + static URI parse(llvm::yaml::ScalarNode *Param); + static std::string unparse(const URI &U); +}; + struct TextDocumentIdentifier { /// The text document's URI. - std::string uri; + URI uri; static llvm::Optional<TextDocumentIdentifier> parse(llvm::yaml::MappingNode *Params); @@ -90,7 +101,7 @@ struct TextEdit { struct TextDocumentItem { /// The text document's URI. - std::string uri; + URI uri; /// The text document's language identifier. std::string languageId; @@ -328,7 +339,7 @@ struct CompletionItem { /// this completion. Edits must not overlap with the main edit nor with /// themselves. std::vector<TextEdit> additionalTextEdits; - + // TODO(krasimir): The following optional fields defined by the language // server protocol are unsupported: // Modified: clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp (original) +++ clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp Fri Apr 7 06:03:26 2017 @@ -21,7 +21,7 @@ void TextDocumentDidOpenHandler::handleN Output.log("Failed to decode DidOpenTextDocumentParams!\n"); return; } - Store.addDocument(DOTDP->textDocument.uri, DOTDP->textDocument.text); + Store.addDocument(DOTDP->textDocument.uri.file, DOTDP->textDocument.text); } void TextDocumentDidChangeHandler::handleNotification( @@ -32,7 +32,7 @@ void TextDocumentDidChangeHandler::handl return; } // We only support full syncing right now. - Store.addDocument(DCTDP->textDocument.uri, DCTDP->contentChanges[0].text); + Store.addDocument(DCTDP->textDocument.uri.file, DCTDP->contentChanges[0].text); } /// Turn a [line, column] pair into an offset in Code. @@ -83,9 +83,6 @@ static std::string formatCode(StringRef // Call clang-format. // FIXME: Don't ignore style. format::FormatStyle Style = format::getLLVMStyle(); - // On windows FileManager doesn't like file://. Just strip it, clang-format - // doesn't need it. - Filename.consume_front("file://"); tooling::Replacements Replacements = format::reformat(Style, Code, Ranges, Filename); @@ -102,12 +99,12 @@ void TextDocumentRangeFormattingHandler: return; } - std::string Code = Store.getDocument(DRFP->textDocument.uri); + std::string Code = Store.getDocument(DRFP->textDocument.uri.file); size_t Begin = positionToOffset(Code, DRFP->range.start); size_t Len = positionToOffset(Code, DRFP->range.end) - Begin; - writeMessage(formatCode(Code, DRFP->textDocument.uri, + writeMessage(formatCode(Code, DRFP->textDocument.uri.file, {clang::tooling::Range(Begin, Len)}, ID)); } @@ -121,14 +118,14 @@ void TextDocumentOnTypeFormattingHandler // Look for the previous opening brace from the character position and format // starting from there. - std::string Code = Store.getDocument(DOTFP->textDocument.uri); + std::string Code = Store.getDocument(DOTFP->textDocument.uri.file); size_t CursorPos = positionToOffset(Code, DOTFP->position); size_t PreviousLBracePos = StringRef(Code).find_last_of('{', CursorPos); if (PreviousLBracePos == StringRef::npos) PreviousLBracePos = CursorPos; size_t Len = 1 + CursorPos - PreviousLBracePos; - writeMessage(formatCode(Code, DOTFP->textDocument.uri, + writeMessage(formatCode(Code, DOTFP->textDocument.uri.file, {clang::tooling::Range(PreviousLBracePos, Len)}, ID)); } @@ -141,8 +138,8 @@ void TextDocumentFormattingHandler::hand } // Format everything. - std::string Code = Store.getDocument(DFP->textDocument.uri); - writeMessage(formatCode(Code, DFP->textDocument.uri, + std::string Code = Store.getDocument(DFP->textDocument.uri.file); + writeMessage(formatCode(Code, DFP->textDocument.uri.file, {clang::tooling::Range(0, Code.size())}, ID)); } @@ -156,7 +153,7 @@ void CodeActionHandler::handleMethod(llv // We provide a code action for each diagnostic at the requested location // which has FixIts available. - std::string Code = AST.getStore().getDocument(CAP->textDocument.uri); + std::string Code = AST.getStore().getDocument(CAP->textDocument.uri.file); std::string Commands; for (Diagnostic &D : CAP->context.diagnostics) { std::vector<clang::tooling::Replacement> Fixes = AST.getFixIts(D); @@ -166,7 +163,7 @@ void CodeActionHandler::handleMethod(llv Commands += R"({"title":"Apply FixIt ')" + llvm::yaml::escape(D.message) + R"('", "command": "clangd.applyFix", "arguments": [")" + - llvm::yaml::escape(CAP->textDocument.uri) + + llvm::yaml::escape(CAP->textDocument.uri.uri) + R"(", [)" + Edits + R"(]]},)"; } @@ -187,7 +184,7 @@ void CompletionHandler::handleMethod(llv return; } - auto Items = AST.codeComplete(TDPP->textDocument.uri, TDPP->position.line, + auto Items = AST.codeComplete(TDPP->textDocument.uri.file, TDPP->position.line, TDPP->position.character); std::string Completions; for (const auto &Item : Items) { Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=299758&r1=299757&r2=299758&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts (original) +++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Fri Apr 7 06:03:26 2017 @@ -23,7 +23,14 @@ export function activate(context: vscode const clientOptions: vscodelc.LanguageClientOptions = { // Register the server for C/C++ files - documentSelector: ['c', 'cc', 'cpp', 'h', 'hh', 'hpp'] + documentSelector: ['c', 'cc', 'cpp', 'h', 'hh', 'hpp'], + 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 => undefined + } }; const clangdClient = new vscodelc.LanguageClient('Clang Language Server', serverOptions, clientOptions); @@ -31,7 +38,8 @@ export function activate(context: vscode function applyTextEdits(uri: string, edits: vscodelc.TextEdit[]) { let textEditor = vscode.window.activeTextEditor; - if (textEditor && textEditor.document.uri.toString() === uri) { + // FIXME: vscode expects that uri will be percent encoded + if (textEditor && textEditor.document.uri.toString(true) === uri) { textEditor.edit(mutator => { for (const edit of edits) { mutator.replace(vscodelc.Protocol2Code.asRange(edit.range), edit.newText); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits