Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp @@ -115,9 +115,7 @@ {"workspaceSymbolProvider", true}, {"executeCommandProvider", json::obj{ - {"commands", - {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND, - ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE}}, + {"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}}, }}, }}}}); } @@ -190,42 +188,6 @@ reply("Fix applied."); ApplyEdit(*Params.workspaceEdit); - } else if (Params.command == - ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) { - auto &FileURI = Params.includeInsertion->textDocument.uri; - auto Code = DraftMgr.getDraft(FileURI.file()); - if (!Code) - return replyError(ErrorCode::InvalidParams, - ("command " + - ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE + - " called on non-added file " + FileURI.file()) - .str()); - llvm::StringRef DeclaringHeader = Params.includeInsertion->declaringHeader; - if (DeclaringHeader.empty()) - return replyError( - ErrorCode::InvalidParams, - "declaringHeader must be provided for include insertion."); - llvm::StringRef PreferredHeader = Params.includeInsertion->preferredHeader; - auto Replaces = Server.insertInclude( - FileURI.file(), *Code, DeclaringHeader, - PreferredHeader.empty() ? DeclaringHeader : PreferredHeader); - if (!Replaces) { - std::string ErrMsg = - ("Failed to generate include insertion edits for adding " + - DeclaringHeader + " (" + PreferredHeader + ") into " + - FileURI.file()) - .str(); - log(ErrMsg + ":" + llvm::toString(Replaces.takeError())); - replyError(ErrorCode::InternalError, ErrMsg); - return; - } - auto Edits = replacementsToEdits(*Code, *Replaces); - WorkspaceEdit WE; - WE.changes = {{FileURI.uri(), Edits}}; - - reply(("Inserted header " + DeclaringHeader + " (" + PreferredHeader + ")") - .str()); - ApplyEdit(std::move(WE)); } else { // We should not get here because ExecuteCommandParams would not have // parsed in the first place and this handler should not be called. But if Index: clang-tools-extra/trunk/clangd/ClangdServer.h =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.h +++ clang-tools-extra/trunk/clangd/ClangdServer.h @@ -179,22 +179,6 @@ void rename(PathRef File, Position Pos, llvm::StringRef NewName, Callback> CB); - /// Inserts a new #include into \p File, if it's not present in \p Code. - /// - /// \p DeclaringHeader The original header corresponding to this insertion - /// e.g. the header that declared a symbol. This can be either a URI or a - /// literal string quoted with <> or "" that can be #included directly. - /// \p InsertedHeader The preferred header to be inserted. This may be - /// different from \p DeclaringHeader as a header file can have a different - /// canonical include. This can be either a URI or a literal string quoted - /// with <> or "" that can be #included directly. - /// - /// Both OriginalHeader and InsertedHeader will be considered to determine - /// whether an include needs to be added. - Expected insertInclude(PathRef File, StringRef Code, - StringRef DeclaringHeader, - StringRef InsertedHeader); - /// Only for testing purposes. /// 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. Index: clang-tools-extra/trunk/clangd/ClangdServer.cpp =================================================================== --- clang-tools-extra/trunk/clangd/ClangdServer.cpp +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp @@ -272,66 +272,6 @@ "Rename", File, Bind(Action, File.str(), NewName.str(), std::move(CB))); } -/// Creates a `HeaderFile` from \p Header which can be either a URI or a literal -/// include. -static llvm::Expected toHeaderFile(StringRef Header, - llvm::StringRef HintPath) { - if (isLiteralInclude(Header)) - return HeaderFile{Header.str(), /*Verbatim=*/true}; - auto U = URI::parse(Header); - if (!U) - return U.takeError(); - - auto IncludePath = URI::includeSpelling(*U); - if (!IncludePath) - return IncludePath.takeError(); - if (!IncludePath->empty()) - return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true}; - - auto Resolved = URI::resolve(*U, HintPath); - if (!Resolved) - return Resolved.takeError(); - return HeaderFile{std::move(*Resolved), /*Verbatim=*/false}; -} - -Expected -ClangdServer::insertInclude(PathRef File, StringRef Code, - StringRef DeclaringHeader, - StringRef InsertedHeader) { - assert(!DeclaringHeader.empty() && !InsertedHeader.empty()); - std::string ToInclude; - auto ResolvedOrginal = toHeaderFile(DeclaringHeader, File); - if (!ResolvedOrginal) - return ResolvedOrginal.takeError(); - auto ResolvedPreferred = toHeaderFile(InsertedHeader, File); - if (!ResolvedPreferred) - return ResolvedPreferred.takeError(); - tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File); - auto Include = - calculateIncludePath(File, Code, *ResolvedOrginal, *ResolvedPreferred, - CompileCommand, FSProvider.getFileSystem()); - if (!Include) - return Include.takeError(); - if (Include->empty()) - return tooling::Replacements(); - ToInclude = std::move(*Include); - - auto Style = format::getStyle("file", File, "llvm"); - if (!Style) { - llvm::consumeError(Style.takeError()); - // FIXME(ioeric): needs more consistent style support in clangd server. - Style = format::getLLVMStyle(); - } - // Replacement with offset UINT_MAX and length 0 will be treated as include - // insertion. - tooling::Replacement R(File, /*Offset=*/UINT_MAX, 0, "#include " + ToInclude); - auto Replaces = - format::cleanupAroundReplacements(Code, tooling::Replacements(R), *Style); - if (!Replaces) - return Replaces; - return formatReplacements(Code, *Replaces, *Style); -} - void ClangdServer::dumpAST(PathRef File, UniqueFunction Callback) { auto Action = [](decltype(Callback) Callback, Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -289,27 +289,6 @@ I.documentation = D->Documentation; if (I.detail.empty()) I.detail = D->CompletionDetail; - // FIXME: delay creating include insertion command to - // "completionItem/resolve", when it is supported - if (!D->IncludeHeader.empty()) { - // LSP favors additionalTextEdits over command. But we are still using - // command here because it would be expensive to calculate #include - // insertion edits for all candidates, and the include insertion edit - // is unlikely to conflict with the code completion edits. - Command Cmd; - // Command title is not added since this is not a user-facing command. - Cmd.command = ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE; - IncludeInsertion Insertion; - // Fallback to canonical header if declaration location is invalid. - Insertion.declaringHeader = - IndexResult->CanonicalDeclaration.FileURI.empty() - ? D->IncludeHeader - : IndexResult->CanonicalDeclaration.FileURI; - Insertion.preferredHeader = D->IncludeHeader; - Insertion.textDocument.uri = URIForFile(FileName); - Cmd.includeInsertion = std::move(Insertion); - I.command = std::move(Cmd); - } } } I.scoreInfo = Scores; Index: clang-tools-extra/trunk/clangd/Protocol.h =================================================================== --- clang-tools-extra/trunk/clangd/Protocol.h +++ clang-tools-extra/trunk/clangd/Protocol.h @@ -538,26 +538,6 @@ bool fromJSON(const json::Expr &, WorkspaceEdit &); json::Expr toJSON(const WorkspaceEdit &WE); -struct IncludeInsertion { - /// The document in which the command was invoked. - /// If either originalHeader or preferredHeader has been (directly) included - /// in the current file, no new include will be inserted. - TextDocumentIdentifier textDocument; - - /// The declaring header corresponding to this insertion e.g. the header that - /// declares a symbol. This could be either a URI or a literal string quoted - /// with <> or "" that can be #included directly. - std::string declaringHeader; - /// The preferred header to be inserted. This may be different from - /// originalHeader as a header file can have a different canonical include. - /// This could be either a URI or a literal string quoted with <> or "" that - /// can be #included directly. If empty, declaringHeader is used to calculate - /// the #include path. - std::string preferredHeader; -}; -bool fromJSON(const json::Expr &, IncludeInsertion &); -json::Expr toJSON(const IncludeInsertion &II); - /// Exact commands are not specified in the protocol so we define the /// ones supported by Clangd here. The protocol specifies the command arguments /// to be "any[]" but to make this safer and more manageable, each command we @@ -569,16 +549,12 @@ struct ExecuteCommandParams { // Command to apply fix-its. Uses WorkspaceEdit as argument. const static llvm::StringLiteral CLANGD_APPLY_FIX_COMMAND; - // Command to insert an #include into code. - const static llvm::StringLiteral CLANGD_INSERT_HEADER_INCLUDE; /// The command identifier, e.g. CLANGD_APPLY_FIX_COMMAND std::string command; // Arguments llvm::Optional workspaceEdit; - - llvm::Optional includeInsertion; }; bool fromJSON(const json::Expr &, ExecuteCommandParams &); @@ -750,7 +726,6 @@ /// themselves. std::vector additionalTextEdits; - llvm::Optional command; // TODO(krasimir): The following optional fields defined by the language // server protocol are unsupported: // Index: clang-tools-extra/trunk/clangd/Protocol.cpp =================================================================== --- clang-tools-extra/trunk/clangd/Protocol.cpp +++ clang-tools-extra/trunk/clangd/Protocol.cpp @@ -390,9 +390,6 @@ const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND = "clangd.applyFix"; -const llvm::StringLiteral ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE = - "clangd.insertInclude"; - bool fromJSON(const json::Expr &Params, ExecuteCommandParams &R) { json::ObjectMapper O(Params); if (!O || !O.map("command", R.command)) @@ -402,9 +399,6 @@ if (R.command == ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND) { return Args && Args->size() == 1 && fromJSON(Args->front(), R.workspaceEdit); - } else if (R.command == ExecuteCommandParams::CLANGD_INSERT_HEADER_INCLUDE) { - return Args && Args->size() == 1 && - fromJSON(Args->front(), R.includeInsertion); } return false; // Unrecognized command. } @@ -433,8 +427,6 @@ auto Cmd = json::obj{{"title", C.title}, {"command", C.command}}; if (C.workspaceEdit) Cmd["arguments"] = {*C.workspaceEdit}; - else if (C.includeInsertion) - Cmd["arguments"] = {*C.includeInsertion}; return std::move(Cmd); } @@ -447,18 +439,6 @@ return json::obj{{"changes", std::move(FileChanges)}}; } -bool fromJSON(const json::Expr &II, IncludeInsertion &R) { - json::ObjectMapper O(II); - return O && O.map("textDocument", R.textDocument) && - O.map("declaringHeader", R.declaringHeader) && - O.map("preferredHeader", R.preferredHeader); -} -json::Expr toJSON(const IncludeInsertion &II) { - return json::obj{{"textDocument", II.textDocument}, - {"declaringHeader", II.declaringHeader}, - {"preferredHeader", II.preferredHeader}}; -} - json::Expr toJSON(const ApplyWorkspaceEditParams &Params) { return json::obj{{"edit", Params.edit}}; } @@ -519,8 +499,6 @@ Result["textEdit"] = *CI.textEdit; if (!CI.additionalTextEdits.empty()) Result["additionalTextEdits"] = json::ary(CI.additionalTextEdits); - if (CI.command) - Result["command"] = *CI.command; return std::move(Result); } Index: clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test =================================================================== --- clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test +++ clang-tools-extra/trunk/test/clangd/initialize-params-invalid.test @@ -24,8 +24,7 @@ # CHECK-NEXT: "documentRangeFormattingProvider": true, # CHECK-NEXT: "executeCommandProvider": { # CHECK-NEXT: "commands": [ -# CHECK-NEXT: "clangd.applyFix", -# CHECK-NEXT: "clangd.insertInclude" +# CHECK-NEXT: "clangd.applyFix" # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, Index: clang-tools-extra/trunk/test/clangd/initialize-params.test =================================================================== --- clang-tools-extra/trunk/test/clangd/initialize-params.test +++ clang-tools-extra/trunk/test/clangd/initialize-params.test @@ -24,8 +24,7 @@ # CHECK-NEXT: "documentRangeFormattingProvider": true, # CHECK-NEXT: "executeCommandProvider": { # CHECK-NEXT: "commands": [ -# CHECK-NEXT: "clangd.applyFix", -# CHECK-NEXT: "clangd.insertInclude" +# CHECK-NEXT: "clangd.applyFix" # CHECK-NEXT: ] # CHECK-NEXT: }, # CHECK-NEXT: "hoverProvider": true, Index: clang-tools-extra/trunk/test/clangd/insert-include.test =================================================================== --- clang-tools-extra/trunk/test/clangd/insert-include.test +++ clang-tools-extra/trunk/test/clangd/insert-include.test @@ -1,36 +0,0 @@ -# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s -# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s -{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}} ---- -{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void f() {}"}}} ---- -{"jsonrpc":"2.0","id":3,"method":"workspace/executeCommand","params":{"arguments":[{"declaringHeader":"\"/usr/include/bits/vector\"", "preferredHeader":"","textDocument":{"uri":"test:///main.cpp"}}],"command":"clangd.insertInclude"}} -# CHECK: "id": 3, -# CHECK-NEXT: "jsonrpc": "2.0", -# CHECK-NEXT: "result": "Inserted header \"/usr/include/bits/vector\" ()" -# CHECK-NEXT: } -# CHECK: "method": "workspace/applyEdit", -# CHECK-NEXT: "params": { -# CHECK-NEXT: "edit": { -# CHECK-NEXT: "changes": { -# CHECK-NEXT: "file://{{.*}}/main.cpp": [ -# CHECK-NEXT: { -# CHECK-NEXT: "newText": "#include \n", -# CHECK-NEXT: "range": { -# CHECK-NEXT: "end": { -# CHECK-NEXT: "character": 0, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: }, -# CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 0, -# CHECK-NEXT: "line": 0 -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: ] -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: } -# CHECK-NEXT: } ---- -{"jsonrpc":"2.0","id":4,"method":"shutdown"} Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp @@ -938,67 +938,6 @@ ASSERT_EQ(DiagConsumer.Count, 2); // Sanity check - we actually ran both? } -TEST_F(ClangdVFSTest, InsertIncludes) { - MockFSProvider FS; - ErrorCheckingDiagConsumer DiagConsumer; - MockCompilationDatabase CDB; - std::string SearchDirArg = (llvm::Twine("-I") + testRoot()).str(); - CDB.ExtraClangFlags.insert(CDB.ExtraClangFlags.end(), {SearchDirArg.c_str()}); - ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); - - auto FooCpp = testPath("foo.cpp"); - const auto Code = R"cpp( -#include "x.h" -#include "z.h" - -void f() {} -)cpp"; - FS.Files[FooCpp] = Code; - runAddDocument(Server, FooCpp, Code); - - auto ChangedCode = [&](llvm::StringRef Original, llvm::StringRef Preferred) { - auto Replaces = Server.insertInclude( - FooCpp, Code, Original, Preferred.empty() ? Original : Preferred); - EXPECT_TRUE(static_cast(Replaces)); - auto Changed = tooling::applyAllReplacements(Code, *Replaces); - EXPECT_TRUE(static_cast(Changed)); - return *Changed; - }; - auto Inserted = [&](llvm::StringRef Original, llvm::StringRef Preferred, - llvm::StringRef Expected) { - return llvm::StringRef(ChangedCode(Original, Preferred)) - .contains((llvm::Twine("#include ") + Expected + "").str()); - }; - - EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"", "\"y.h\"")); - EXPECT_TRUE(Inserted("\"y.h\"", /*Preferred=*/"\"Y.h\"", "\"Y.h\"")); - EXPECT_TRUE(Inserted("", /*Preferred=*/"", "")); - EXPECT_TRUE(Inserted("", /*Preferred=*/"", "")); - - std::string OriginalHeader = URI::createFile(testPath("y.h")).toString(); - std::string PreferredHeader = URI::createFile(testPath("Y.h")).toString(); - EXPECT_TRUE(Inserted(OriginalHeader, - /*Preferred=*/"", "\"y.h\"")); - EXPECT_TRUE(Inserted(OriginalHeader, - /*Preferred=*/"", "")); - EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\"")); - EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\"")); - auto TestURIHeader = - URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str()); - EXPECT_TRUE(static_cast(TestURIHeader)); - EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\"")); - - // Check that includes are sorted. - const auto Expected = R"cpp( -#include "x.h" -#include "y.h" -#include "z.h" - -void f() {} -)cpp"; - EXPECT_EQ(Expected, ChangedCode("\"y.h\"", /*Preferred=*/"")); -} - TEST_F(ClangdVFSTest, FormatCode) { MockFSProvider FS; ErrorCheckingDiagConsumer DiagConsumer;