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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
