From 348364bffd379e291501dc49b192cdd2adf83811 Mon Sep 17 00:00:00 2001 From: Kirill Bobyrev Date: Tue, 9 Jun 2020 13:58:46 +0200 Subject: [PATCH] [clangd] Don't produce snippets when completion location is followed by parenthesis Summary: Prevent a second pair of parenthesis from being added when there already is one right after cursor. Related issue and more context: https://github.com/clangd/clangd/issues/387 Reviewers: sammccall Reviewed By: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D81380 --- clang-tools-extra/clangd/CodeComplete.cpp | 33 +++++++++--- .../clangd/unittests/CodeCompleteTests.cpp | 50 +++++++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp index 3cfdac1c3bc8..cf79673d821e 100644 --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -45,10 +45,12 @@ #include "clang/Basic/CharInfo.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Basic/TokenKinds.h" #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" #include "clang/Lex/ExternalPreprocessorSource.h" +#include "clang/Lex/Lexer.h" #include "clang/Lex/Preprocessor.h" #include "clang/Lex/PreprocessorOptions.h" #include "clang/Sema/CodeCompleteConsumer.h" @@ -255,10 +257,11 @@ struct CodeCompletionBuilder { const IncludeInserter &Includes, llvm::StringRef FileName, CodeCompletionContext::Kind ContextKind, - const CodeCompleteOptions &Opts, bool GenerateSnippets) + const CodeCompleteOptions &Opts, + bool IsUsingDeclaration, tok::TokenKind NextTokenKind) : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments), EnableFunctionArgSnippets(Opts.EnableFunctionArgSnippets), - GenerateSnippets(GenerateSnippets) { + IsUsingDeclaration(IsUsingDeclaration), NextTokenKind(NextTokenKind) { add(C, SemaCCS); if (C.SemaResult) { assert(ASTCtx); @@ -429,7 +432,13 @@ private: } std::string summarizeSnippet() const { - if (!GenerateSnippets) + if (IsUsingDeclaration) + return ""; + // Suppress function argument snippets if args are already present. + if ((Completion.Kind == CompletionItemKind::Function || + Completion.Kind == CompletionItemKind::Method || + Completion.Kind == CompletionItemKind::Constructor) && + NextTokenKind == tok::l_paren) return ""; auto *Snippet = onlyValue<&BundledEntry::SnippetSuffix>(); if (!Snippet) @@ -488,8 +497,10 @@ private: llvm::SmallVector Bundled; bool ExtractDocumentation; bool EnableFunctionArgSnippets; - /// When false, no snippets are generated argument lists. - bool GenerateSnippets; + // No snippets will be generated for using declarations and when the function + // arguments are already present. + bool IsUsingDeclaration; + tok::TokenKind NextTokenKind; }; // Determine the symbol ID for a Sema code completion result, if possible. @@ -1223,6 +1234,10 @@ class CodeCompleteFlow { CompletionRecorder *Recorder = nullptr; CodeCompletionContext::Kind CCContextKind = CodeCompletionContext::CCC_Other; bool IsUsingDeclaration = false; + // The snippets will not be generated if the token following completion + // location is an opening parenthesis (tok::l_paren) because this would add + // extra parenthesis. + tok::TokenKind NextTokenKind = tok::eof; // Counters for logging. int NSema = 0, NIndex = 0, NSemaAndIndex = 0, NIdent = 0; bool Incomplete = false; // Would more be available with a higher limit? @@ -1277,6 +1292,11 @@ public: auto Style = getFormatStyleForFile( SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, SemaCCInput.ParseInput.FSProvider->getFileSystem().get()); + const auto NextToken = Lexer::findNextToken( + Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(), + Recorder->CCSema->getSourceManager(), Recorder->CCSema->LangOpts); + if (NextToken) + NextTokenKind = NextToken->getKind(); // If preprocessor was run, inclusions from preprocessor callback should // already be added to Includes. Inserter.emplace( @@ -1694,8 +1714,7 @@ private: if (!Builder) Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr, Item, SemaCCS, QueryScopes, *Inserter, FileName, - CCContextKind, Opts, - /*GenerateSnippets=*/!IsUsingDeclaration); + CCContextKind, Opts, IsUsingDeclaration, NextTokenKind); else Builder->add(Item, SemaCCS); } diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 6114fac2dace..b060e67e848f 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -2875,6 +2875,56 @@ TEST(AllowImplicitCompletion, All) { } } +TEST(CompletionTest, FunctionArgsExist) { + clangd::CodeCompleteOptions Opts; + Opts.EnableSnippets = true; + std::string Context = R"cpp( + int foo(int A); + int bar(); + struct Object { + Object(int B) {} + }; + template + struct Container { + Container(int Size) {} + }; + )cpp"; + EXPECT_THAT(completions(Context + "int y = fo^", {}, Opts).Completions, + UnorderedElementsAre( + AllOf(Labeled("foo(int A)"), SnippetSuffix("(${1:int A})")))); + EXPECT_THAT( + completions(Context + "int y = fo^(42)", {}, Opts).Completions, + UnorderedElementsAre(AllOf(Labeled("foo(int A)"), SnippetSuffix("")))); + // FIXME(kirillbobyrev): No snippet should be produced here. + EXPECT_THAT(completions(Context + "int y = fo^o(42)", {}, Opts).Completions, + UnorderedElementsAre( + AllOf(Labeled("foo(int A)"), SnippetSuffix("(${1:int A})")))); + EXPECT_THAT( + completions(Context + "int y = ba^", {}, Opts).Completions, + UnorderedElementsAre(AllOf(Labeled("bar()"), SnippetSuffix("()")))); + EXPECT_THAT(completions(Context + "int y = ba^()", {}, Opts).Completions, + UnorderedElementsAre(AllOf(Labeled("bar()"), SnippetSuffix("")))); + EXPECT_THAT( + completions(Context + "Object o = Obj^", {}, Opts).Completions, + Contains(AllOf(Labeled("Object(int B)"), SnippetSuffix("(${1:int B})"), + Kind(CompletionItemKind::Constructor)))); + EXPECT_THAT(completions(Context + "Object o = Obj^()", {}, Opts).Completions, + Contains(AllOf(Labeled("Object(int B)"), SnippetSuffix(""), + Kind(CompletionItemKind::Constructor)))); + EXPECT_THAT( + completions(Context + "Container c = Cont^", {}, Opts).Completions, + Contains(AllOf(Labeled("Container(int Size)"), + SnippetSuffix("<${1:typename T}>(${2:int Size})"), + Kind(CompletionItemKind::Constructor)))); + // FIXME(kirillbobyrev): It would be nice to still produce the template + // snippet part: in this case it should be "<${1:typename T}>". + EXPECT_THAT( + completions(Context + "Container c = Cont^()", {}, Opts).Completions, + Contains(AllOf(Labeled("Container(int Size)"), + SnippetSuffix(""), + Kind(CompletionItemKind::Constructor)))); +} + } // namespace } // namespace clangd } // namespace clang