maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Remove LSP command-based #include insertion. 

Summary:
clangd will populate #include insertions as addtionalEdits in completion items.

The code completion tests in ClangdServerTest will be added back in D46497.

Reviewers: ilya-biryukov, sammccall

Reviewed By: ilya-biryukov

Subscribers: klimek, MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D46676

llvm-svn: 332362
This commit is contained in:
Eric Liu 2018-05-15 15:23:53 +00:00
parent a7c3c45267
commit 2c1905386c
10 changed files with 3 additions and 284 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
clang-tools-extra/clangd/ClangdLSPServer.cpp
View File

@ -115,9 +115,7 @@ void ClangdLSPServer::onInitialize(InitializeParams &Params) {
{"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 @@ void ClangdLSPServer::onCommand(ExecuteCommandParams &Params) {
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

60
clang-tools-extra/clangd/ClangdServer.cpp
View File

@ -272,66 +272,6 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
"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<HeaderFile> 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<tooling::Replacements>
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<void(std::string)> Callback) {
auto Action = [](decltype(Callback) Callback,

16
clang-tools-extra/clangd/ClangdServer.h
View File

@ -179,22 +179,6 @@ public:
void rename(PathRef File, Position Pos, llvm::StringRef NewName,
Callback<std::vector<tooling::Replacement>> 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<tooling::Replacements> 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.

21
clang-tools-extra/clangd/CodeComplete.cpp
View File

@ -289,27 +289,6 @@ struct CompletionCandidate {
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;

22
clang-tools-extra/clangd/Protocol.cpp
View File

@ -390,9 +390,6 @@ bool fromJSON(const json::Expr &Params, WorkspaceEdit &R) {
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 @@ bool fromJSON(const json::Expr &Params, ExecuteCommandParams &R) {
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 @@ json::Expr toJSON(const Command &C) {
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 @@ json::Expr toJSON(const WorkspaceEdit &WE) {
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 @@ json::Expr toJSON(const CompletionItem &CI) {
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);
}

25
clang-tools-extra/clangd/Protocol.h
View File

@ -538,26 +538,6 @@ struct WorkspaceEdit {
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 @@ json::Expr toJSON(const IncludeInsertion &II);
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> workspaceEdit;
llvm::Optional<IncludeInsertion> includeInsertion;
};
bool fromJSON(const json::Expr &, ExecuteCommandParams &);
@ -750,7 +726,6 @@ struct CompletionItem {
/// themselves.
std::vector<TextEdit> additionalTextEdits;
llvm::Optional<Command> command;
// TODO(krasimir): The following optional fields defined by the language
// server protocol are unsupported:
//

3
clang-tools-extra/test/clangd/initialize-params-invalid.test
View File

@ -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,

3
clang-tools-extra/test/clangd/initialize-params.test
View File

@ -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,

36
clang-tools-extra/test/clangd/insert-include.test
View File

@ -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":"<vector>","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\" (<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 <vector>\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"}

61
clang-tools-extra/unittests/clangd/ClangdTests.cpp
View File

@ -938,67 +938,6 @@ int d;
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<bool>(Replaces));
auto Changed = tooling::applyAllReplacements(Code, *Replaces);
EXPECT_TRUE(static_cast<bool>(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("<string>", /*Preferred=*/"", "<string>"));
EXPECT_TRUE(Inserted("<string>", /*Preferred=*/"", "<string>"));
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=*/"<Y.h>", "<Y.h>"));
EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\""));
EXPECT_TRUE(Inserted("<y.h>", PreferredHeader, "\"Y.h\""));
auto TestURIHeader =
URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str());
EXPECT_TRUE(static_cast<bool>(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;