[clang-tools-extra] 032727f - [clangd] Complete filenames after < / ".
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue May 19 04:32:34 PDT 2020
Author: Sam McCall
Date: 2020-05-19T13:32:26+02:00
New Revision: 032727f4f839a28ae449d2f38814857780c7453d
URL: https://github.com/llvm/llvm-project/commit/032727f4f839a28ae449d2f38814857780c7453d
DIFF: https://github.com/llvm/llvm-project/commit/032727f4f839a28ae449d2f38814857780c7453d.diff
LOG: [clangd] Complete filenames after < / ".
Summary:
Extract prefix filters to CodeComplete so it can be easily tested.
Fixes https://github.com/clangd/clangd/issues/366
Reviewers: adamcz
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D79456
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a89fa2f8104e..b3b2bdd976bf 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ClangdLSPServer.h"
+#include "CodeComplete.h"
#include "Diagnostics.h"
#include "DraftStore.h"
#include "GlobalCompilationDatabase.h"
@@ -593,9 +594,8 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
llvm::json::Object{
{"allCommitCharacters", " \t()[]{}<>:;,+-/*%^&#?.=\"'|"},
{"resolveProvider", false},
- // We do extra checks for '>' and ':' in completion to only
- // trigger on '->' and '::'.
- {"triggerCharacters", {".", ">", ":"}},
+ // We do extra checks, e.g. that > is part of ->.
+ {"triggerCharacters", {".", "<", ">", ":", "\"", "/"}},
}},
{"semanticTokensProvider",
llvm::json::Object{
@@ -1425,23 +1425,19 @@ std::vector<Fix> ClangdLSPServer::getFixes(llvm::StringRef File,
return FixItsIter->second;
}
+// A completion request is sent when the user types '>' or ':', but we only
+// want to trigger on '->' and '::'. We check the preceeding text to make
+// sure it matches what we expected.
+// Running the lexer here would be more robust (e.g. we can detect comments
+// and avoid triggering completion there), but we choose to err on the side
+// of simplicity here.
bool ClangdLSPServer::shouldRunCompletion(
const CompletionParams &Params) const {
- llvm::StringRef Trigger = Params.context.triggerCharacter;
- if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter ||
- (Trigger != ">" && Trigger != ":"))
+ if (Params.context.triggerKind != CompletionTriggerKind::TriggerCharacter)
return true;
-
auto Code = DraftMgr.getDraft(Params.textDocument.uri.file());
if (!Code)
return true; // completion code will log the error for untracked doc.
-
- // A completion request is sent when the user types '>' or ':', but we only
- // want to trigger on '->' and '::'. We check the preceeding character to make
- // sure it matches what we expected.
- // Running the lexer here would be more robust (e.g. we can detect comments
- // and avoid triggering completion there), but we choose to err on the side
- // of simplicity here.
auto Offset = positionToOffset(Code->Contents, Params.position,
/*AllowColumnsBeyondLineLength=*/false);
if (!Offset) {
@@ -1449,15 +1445,7 @@ bool ClangdLSPServer::shouldRunCompletion(
Params.position, Params.textDocument.uri.file());
return true;
}
- if (*Offset < 2)
- return false;
-
- if (Trigger == ">")
- return Code->Contents[*Offset - 2] == '-'; // trigger only on '->'.
- if (Trigger == ":")
- return Code->Contents[*Offset - 2] == ':'; // trigger only on '::'.
- assert(false && "unhandled trigger character");
- return true;
+ return allowImplicitCompletion(Code->Contents, *Offset);
}
void ClangdLSPServer::onHighlightingsReady(
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 0abe023c57a5..03c1cb1e2e13 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1912,5 +1912,43 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
return OS;
}
+// Heuristically detect whether the `Line` is an unterminated include filename.
+bool isIncludeFile(llvm::StringRef Line) {
+ Line = Line.ltrim();
+ if (!Line.consume_front("#"))
+ return false;
+ Line = Line.ltrim();
+ if (!(Line.consume_front("include_next") || Line.consume_front("include") ||
+ Line.consume_front("import")))
+ return false;
+ Line = Line.ltrim();
+ if (Line.consume_front("<"))
+ return Line.count('>') == 0;
+ if (Line.consume_front("\""))
+ return Line.count('"') == 0;
+ return false;
+}
+
+bool allowImplicitCompletion(llvm::StringRef Content, unsigned Offset) {
+ // Look at last line before completion point only.
+ Content = Content.take_front(Offset);
+ auto Pos = Content.rfind('\n');
+ if (Pos != llvm::StringRef::npos)
+ Content = Content.substr(Pos + 1);
+
+ // Complete after scope operators.
+ if (Content.endswith(".") || Content.endswith("->") || Content.endswith("::"))
+ return true;
+ // Complete after `#include <` and #include `<foo/`.
+ if ((Content.endswith("<") || Content.endswith("\"") ||
+ Content.endswith("/")) &&
+ isIncludeFile(Content))
+ return true;
+
+ // Complete words. Give non-ascii characters the benefit of the doubt.
+ return !Content.empty() &&
+ (isIdentifierBody(Content.back()) || !llvm::isASCII(Content.back()));
+}
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/CodeComplete.h b/clang-tools-extra/clangd/CodeComplete.h
index 7070aec79b79..01aad115db2e 100644
--- a/clang-tools-extra/clangd/CodeComplete.h
+++ b/clang-tools-extra/clangd/CodeComplete.h
@@ -314,6 +314,10 @@ struct CompletionPrefix {
CompletionPrefix guessCompletionPrefix(llvm::StringRef Content,
unsigned Offset);
+// Whether it makes sense to complete at the point based on typed characters.
+// For instance, we implicitly trigger at `a->^` but not at `a>^`.
+bool allowImplicitCompletion(llvm::StringRef Content, unsigned Offset);
+
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index 39bbc0fe01f5..f1f5312d1009 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -11,8 +11,11 @@
# CHECK-NEXT: "resolveProvider": false,
# CHECK-NEXT: "triggerCharacters": [
# CHECK-NEXT: ".",
+# CHECK-NEXT: "<",
# CHECK-NEXT: ">",
-# CHECK-NEXT: ":"
+# CHECK-NEXT: ":",
+# CHECK-NEXT: "\"",
+# CHECK-NEXT: "/"
# CHECK-NEXT: ]
# CHECK-NEXT: },
# CHECK-NEXT: "declarationProvider": true,
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 1f30d4314d78..77b5c5ac386c 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -25,6 +25,7 @@
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
+#include "llvm/Testing/Support/Annotations.h"
#include "llvm/Testing/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -2828,6 +2829,34 @@ TEST(NoCompileCompletionTest, WithIndex) {
ElementsAre(AllOf(Qualifier(""), Scope("a::"))));
}
+TEST(AllowImplicitCompletion, All) {
+ const char *Yes[] = {
+ "foo.^bar",
+ "foo->^bar",
+ "foo::^bar",
+ " # include <^foo.h>",
+ "#import <foo/^bar.h>",
+ "#include_next \"^",
+ };
+ const char *No[] = {
+ "foo>^bar",
+ "foo:^bar",
+ "foo\n^bar",
+ "#include <foo.h> //^",
+ "#include \"foo.h\"^",
+ "#error <^",
+ "#<^",
+ };
+ for (const char *Test : Yes) {
+ llvm::Annotations A(Test);
+ EXPECT_TRUE(allowImplicitCompletion(A.code(), A.point())) << Test;
+ }
+ for (const char *Test : No) {
+ llvm::Annotations A(Test);
+ EXPECT_FALSE(allowImplicitCompletion(A.code(), A.point())) << Test;
+ }
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list