Clang: Show function signature hint for constructors and functors

For "foo(|" [1] we requested a completion from libclang with the cursor
position just before "foo" and then filtered the function declarations
for functions matching the name "foo". This worked fine for ordinary
functions, but obviously not for constructors and functors.

Recent versions of libclang support proper function call completion with
XCursor_OverloadCandidate, so make use of that.

[1] '|' represents the cursor position

Task-number: QTCREATORBUG-14882
Task-number: QTCREATORBUG-14884
Change-Id: I9d31b3960ccff6a8b9440dbcb7ff9f5ca9f61266
Reviewed-by: Tim Jenssen <tim.jenssen@qt.io>

src/libs/clangbackendipc/codecompletion.cpp

@@ -37,6 +37,7 @@ static const char *completionKindToString(CodeCompletion::Kind kind)
         case CodeCompletion::Other: return "Other";
         case CodeCompletion::FunctionCompletionKind: return "Function";
         case CodeCompletion::TemplateFunctionCompletionKind: return "TemplateFunction";
+        case CodeCompletion::FunctionOverloadCompletionKind: return "FunctionOverload";
         case CodeCompletion::ConstructorCompletionKind: return "Constructor";
         case CodeCompletion::DestructorCompletionKind: return "Destructor";
         case CodeCompletion::VariableCompletionKind: return "Variable";

src/libs/clangbackendipc/codecompletion.h

@@ -44,6 +44,7 @@ public:
     enum Kind : quint32 {
         Other = 0,
         FunctionCompletionKind,
+        FunctionOverloadCompletionKind,
         TemplateFunctionCompletionKind,
         ConstructorCompletionKind,
         DestructorCompletionKind,

src/plugins/clangcodemodel/clangassistproposalitem.cpp

@@ -276,6 +276,8 @@ QIcon ClangAssistProposalItem::icon() const
             return snippetIcon;
         case CodeCompletion::Other:
             return Icons::iconForType(Icons::UnknownIconType);
+        default:
+            break;
     }
 
     return QIcon();

src/plugins/clangcodemodel/clangbackendipcintegration.cpp

@@ -175,11 +175,8 @@ void IpcReceiver::codeCompleted(const CodeCompletedMessage &message)
     const quint64 ticket = message.ticketNumber();
     QScopedPointer<ClangCompletionAssistProcessor> processor(m_assistProcessorsTable.take(ticket));
     if (processor) {
-        const bool finished = processor->handleAvailableAsyncCompletions(
-                                            message.codeCompletions(),
-                                            message.neededCorrection());
-        if (!finished)
-            processor.take();
+        processor->handleAvailableCompletions(message.codeCompletions(),
+                                              message.neededCorrection());
     }
 }
 

src/plugins/clangcodemodel/clangcompletionassistprocessor.cpp

@@ -51,6 +51,7 @@
 
 #include <clangbackendipc/filecontainer.h>
 
+#include <utils/algorithm.h>
 #include <utils/mimetypes/mimedatabase.h>
 #include <utils/qtcassert.h>
 
@@ -106,30 +107,6 @@ QList<AssistProposalItemInterface *> toAssistProposalItems(const CodeCompletions
     return results;
 }
 
-bool isFunctionHintLikeCompletion(CodeCompletion::Kind kind)
-{
-    return kind == CodeCompletion::FunctionCompletionKind
-        || kind == CodeCompletion::ConstructorCompletionKind
-        || kind == CodeCompletion::DestructorCompletionKind
-        || kind == CodeCompletion::SignalCompletionKind
-        || kind == CodeCompletion::SlotCompletionKind;
-}
-
-CodeCompletions matchingFunctionCompletions(const CodeCompletions completions,
-                                            const QString &functionName)
-{
-    CodeCompletions matching;
-
-    foreach (const CodeCompletion &completion, completions) {
-        if (isFunctionHintLikeCompletion(completion.completionKind())
-                && completion.text().toString() == functionName) {
-            matching.append(completion);
-        }
-    }
-
-    return matching;
-}
-
 } // Anonymous
 
 using namespace CPlusPlus;
@@ -157,25 +134,32 @@ IAssistProposal *ClangCompletionAssistProcessor::perform(const AssistInterface *
     return startCompletionHelper(); // == 0 if results are calculated asynchronously
 }
 
-bool ClangCompletionAssistProcessor::handleAvailableAsyncCompletions(
+static CodeCompletions filterFunctionSignatures(const CodeCompletions &completions)
+{
+    return ::Utils::filtered(completions, [](const CodeCompletion &completion) {
+        return completion.completionKind() == CodeCompletion::FunctionOverloadCompletionKind;
+    });
+}
+
+void ClangCompletionAssistProcessor::handleAvailableCompletions(
         const CodeCompletions &completions,
         CompletionCorrection neededCorrection)
 {
-    bool handled = true;
+    QTC_CHECK(m_completions.isEmpty());
 
-    switch (m_sentRequestType) {
-    case CompletionRequestType::NormalCompletion:
-        handleAvailableCompletions(completions, neededCorrection);
-        break;
-    case CompletionRequestType::FunctionHintCompletion:
-        handled = handleAvailableFunctionHintCompletions(completions);
-        break;
-    default:
-        QTC_CHECK(!"Unhandled ClangCompletionAssistProcessor::CompletionRequestType");
-        break;
-    }
+    if (m_sentRequestType == NormalCompletion) {
+        m_completions = toAssistProposalItems(completions);
+
+        if (m_addSnippets && !m_completions.isEmpty())
+            addSnippets();
 
-    return handled;
+        setAsyncProposalAvailable(createProposal(neededCorrection));
+    } else {
+        const CodeCompletions functionSignatures = filterFunctionSignatures(completions);
+        if (!functionSignatures.isEmpty())
+            setAsyncProposalAvailable(createFunctionHintProposal(functionSignatures));
+        // else: Not a function call, but e.g. a function declaration like "void f("
+    }
 }
 
 const TextEditorWidget *ClangCompletionAssistProcessor::textEditorWidget() const
@@ -268,7 +252,6 @@ IAssistProposal *ClangCompletionAssistProcessor::startCompletionHelper()
     }
     case ClangCompletionContextAnalyzer::PassThroughToLibClangAfterLeftParen: {
         m_sentRequestType = FunctionHintCompletion;
-        m_functionName = analyzer.functionName();
         const bool requestSent = sendCompletionRequest(analyzer.positionForClang(), QByteArray());
         setPerformWasApplicable(requestSent);
         break;
@@ -599,38 +582,12 @@ TextEditor::IAssistProposal *ClangCompletionAssistProcessor::createProposal(
     return new ClangAssistProposal(m_positionForProposal, model);
 }
 
-void ClangCompletionAssistProcessor::handleAvailableCompletions(
-        const CodeCompletions &completions,
-        CompletionCorrection neededCorrection)
+IAssistProposal *ClangCompletionAssistProcessor::createFunctionHintProposal(
+        const ClangBackEnd::CodeCompletions &completions) const
 {
-    QTC_CHECK(m_completions.isEmpty());
-
-    m_completions = toAssistProposalItems(completions);
-    if (m_addSnippets && !m_completions.isEmpty())
-        addSnippets();
-
-    setAsyncProposalAvailable(createProposal(neededCorrection));
-}
-
-bool ClangCompletionAssistProcessor::handleAvailableFunctionHintCompletions(
-        const CodeCompletions &completions)
-{
-    QTC_CHECK(!m_functionName.isEmpty());
-    const auto relevantCompletions = matchingFunctionCompletions(completions, m_functionName);
-
-    if (!relevantCompletions.isEmpty()) {
-        auto *model = new ClangFunctionHintModel(relevantCompletions);
-        auto *proposal = new FunctionHintProposal(m_positionForProposal, model);
-
-        setAsyncProposalAvailable(proposal);
-        return true;
-    } else {
-        m_addSnippets = false;
-        m_functionName.clear();
-        m_sentRequestType = NormalCompletion;
-        sendCompletionRequest(m_interface->position(), QByteArray());
-        return false; // We are not yet finished.
-    }
+    auto *model = new ClangFunctionHintModel(completions);
+    auto *proposal = new FunctionHintProposal(m_positionForProposal, model);
+    return proposal;
 }
 
 } // namespace Internal

src/plugins/clangcodemodel/clangcompletionassistprocessor.h

@@ -50,8 +50,8 @@ public:
 
     TextEditor::IAssistProposal *perform(const TextEditor::AssistInterface *interface) override;
 
-    bool handleAvailableAsyncCompletions(const CodeCompletions &completions,
-                                         CompletionCorrection neededCorrection);
+    void handleAvailableCompletions(const CodeCompletions &completions,
+                                    CompletionCorrection neededCorrection);
 
     const TextEditor::TextEditorWidget *textEditorWidget() const;
 
@@ -63,6 +63,8 @@ private:
 
     TextEditor::IAssistProposal *createProposal(
             CompletionCorrection neededCorrection = CompletionCorrection::NoCorrection) const;
+    TextEditor::IAssistProposal *createFunctionHintProposal(
+            const CodeCompletions &completions) const;
 
     bool completeInclude(const QTextCursor &cursor);
     bool completeInclude(int position);
@@ -82,15 +84,10 @@ private:
     void sendFileContent(const QByteArray &customFileContent);
     bool sendCompletionRequest(int position, const QByteArray &customFileContent);
 
-    void handleAvailableCompletions(const CodeCompletions &completions,
-                                    CompletionCorrection neededCorrection);
-    bool handleAvailableFunctionHintCompletions(const CodeCompletions &completions);
-
 private:
     QScopedPointer<const ClangCompletionAssistInterface> m_interface;
     unsigned m_completionOperator;
     enum CompletionRequestType { NormalCompletion, FunctionHintCompletion } m_sentRequestType;
-    QString m_functionName;     // For type == Type::FunctionHintCompletion
     bool m_addSnippets = false; // For type == Type::NormalCompletion
 };
 

src/plugins/clangcodemodel/clangcompletionchunkstotextconverter.cpp

@@ -167,7 +167,11 @@ void CompletionChunksToTextConverter::parse(
 
     switch (codeCompletionChunk.kind()) {
         case CodeCompletionChunk::ResultType: parseResultType(codeCompletionChunk.text()); break;
-        case CodeCompletionChunk::Placeholder: parsePlaceHolder(codeCompletionChunk); break;
+        // Do not rely on CurrentParameter because it might be wrong for
+        // invalid code. Instead, handle it as PlaceHolder.
+        case CodeCompletionChunk::CurrentParameter:
+        case CodeCompletionChunk::Placeholder:
+            parsePlaceHolder(codeCompletionChunk); break;
         case CodeCompletionChunk::LeftParen: parseLeftParen(codeCompletionChunk); break;
         case CodeCompletionChunk::LeftBrace: parseLeftBrace(codeCompletionChunk); break;
         default: parseText(codeCompletionChunk.text()); break;

src/plugins/clangcodemodel/clangcompletioncontextanalyzer.cpp

@@ -91,8 +91,7 @@ void ClangCompletionContextAnalyzer::analyze()
     }
 }
 
-ClangCompletionContextAnalyzer::FunctionInfo
-ClangCompletionContextAnalyzer::analyzeFunctionCall(int endOfOperator) const
+bool ClangCompletionContextAnalyzer::looksLikeAFunctionCall(int endOfOperator) const
 {
     int index = ActivationSequenceContextProcessor::skipPrecedingWhitespace(m_interface,
                                                                             endOfOperator);
@@ -104,15 +103,15 @@ ClangCompletionContextAnalyzer::analyzeFunctionCall(int endOfOperator) const
     index = ActivationSequenceContextProcessor::skipPrecedingWhitespace(m_interface, index);
     const int functionNameStart = ActivationSequenceContextProcessor::findStartOfName(m_interface,
                                                                                       index);
+    if (functionNameStart == -1)
+        return false;
 
-    QTextCursor textCursor2(m_interface->textDocument());
-    textCursor2.setPosition(functionNameStart);
-    textCursor2.setPosition(index, QTextCursor::KeepAnchor);
+    QTextCursor functionNameSelector(m_interface->textDocument());
+    functionNameSelector.setPosition(functionNameStart);
+    functionNameSelector.setPosition(index, QTextCursor::KeepAnchor);
+    const QString functionName = functionNameSelector.selectedText().trimmed();
 
-    FunctionInfo info;
-    info.functionNamePosition = functionNameStart;
-    info.functionName = textCursor2.selectedText().trimmed();
-    return info;
+    return !functionName.isEmpty();
 }
 
 void ClangCompletionContextAnalyzer::setActionAndClangPosition(CompletionAction action,
@@ -158,16 +157,15 @@ void ClangCompletionContextAnalyzer::handleFunctionCall(int afterOperatorPositio
             // No function completion if cursor is not after '(' or ','
             m_positionForProposal = afterOperatorPosition;
             setActionAndClangPosition(PassThroughToLibClang, afterOperatorPosition);
-        } else {
-            const FunctionInfo functionInfo = analyzeFunctionCall(afterOperatorPosition);
-            if (functionInfo.isValid()) {
-                m_functionName = functionInfo.functionName;
-                setActionAndClangPosition(PassThroughToLibClangAfterLeftParen,
-                                          functionInfo.functionNamePosition);
-            } else {
-                m_positionForProposal = afterOperatorPosition;
-                setActionAndClangPosition(PassThroughToLibClang, afterOperatorPosition);
-            }
+        } else if (looksLikeAFunctionCall(afterOperatorPosition)) {
+            // Always pass the position right after '(' to libclang because
+            // positions after the comma might be problematic if a preceding
+            // argument is invalid code.
+            setActionAndClangPosition(PassThroughToLibClangAfterLeftParen,
+                                      m_positionForProposal);
+        } else { // e.g. "(" without any function name in front
+            m_positionForProposal = afterOperatorPosition;
+            setActionAndClangPosition(PassThroughToLibClang, afterOperatorPosition);
         }
     }
 }

src/plugins/clangcodemodel/clangcompletioncontextanalyzer.h

@@ -58,18 +58,10 @@ public:
     int positionForClang() const { return m_positionForClang; }
     int positionEndOfExpression() const { return m_positionEndOfExpression; }
 
-    QString functionName() const { return m_functionName; }
-
 private:
     ClangCompletionContextAnalyzer();
 
-    struct FunctionInfo {
-        bool isValid() const { return functionNamePosition != -1 && !functionName.isEmpty(); }
-
-        int functionNamePosition = -1;
-        QString functionName;
-    };
-    FunctionInfo analyzeFunctionCall(int endOfExpression) const;
+    bool looksLikeAFunctionCall(int endOfExpression) const;
 
     void setActionAndClangPosition(CompletionAction action, int position);
     void setAction(CompletionAction action);
@@ -88,7 +80,6 @@ private:
     int m_positionForProposal = -1;
     int m_positionForClang = -1;
     int m_positionEndOfExpression = -1;
-    QString m_functionName;
 };
 
 } // namespace Internal

src/plugins/clangcodemodel/test/clangcodecompletion_test.cpp

@@ -819,13 +819,14 @@ void ClangCodeCompletionTest::testCompleteFunctions()
     QVERIFY(hasItem(t.proposal, "TType<QString> f(bool)"));
 }
 
-void ClangCodeCompletionTest::testCompleteConstructorAndFallbackToGlobalCompletion()
+void ClangCodeCompletionTest::testCompleteConstructor()
 {
     ProjectLessCompletionTest t("constructorCompletion.cpp");
 
-    QVERIFY(hasItem(t.proposal, "globalVariable"));
-    QVERIFY(hasItem(t.proposal, "GlobalClassWithCustomConstructor"));
-    QVERIFY(!hasSnippet(t.proposal, "class"));
+    QVERIFY(!hasItem(t.proposal, "globalVariable"));
+    QVERIFY(!hasItem(t.proposal, "class"));
+    QVERIFY(hasItem(t.proposal, "Foo(int)"));
+    QVERIFY(hasItem(t.proposal, "Foo(int, double)"));
 }
 
 // Explicitly Inserting The Dot

src/plugins/clangcodemodel/test/clangcodecompletion_test.h

@@ -45,7 +45,7 @@ private slots:
     void testCompleteGlobals();
     void testCompleteMembers();
     void testCompleteFunctions();
-    void testCompleteConstructorAndFallbackToGlobalCompletion();
+    void testCompleteConstructor();
 
     void testCompleteWithDotToArrowCorrection();
     void testDontCompleteWithDotToArrowCorrectionForFloats();

src/plugins/clangcodemodel/test/data/constructorCompletion.cpp

 int globalVariable;
 
-struct GlobalClassWithCustomConstructor {
-    GlobalClassWithCustomConstructor(int) {}
+struct Foo {
+    Foo(int) {}
+    Foo(int, double) {}
 };
 
 void f() {
-    GlobalClassWithCustomConstructor foo( /* COMPLETE HERE */
+    Foo foo( /* COMPLETE HERE */
 }

src/tools/clangbackend/ipcsource/codecompletionsextractor.cpp

@@ -152,6 +152,9 @@ void CodeCompletionsExtractor::extractCompletionKind()
         case CXCursor_NotImplemented:
             currentCodeCompletion_.setCompletionKind(CodeCompletion::KeywordCompletionKind);
             break;
+        case CXCursor_OverloadCandidate:
+            currentCodeCompletion_.setCompletionKind(CodeCompletion::FunctionOverloadCompletionKind);
+            break;
         default:
             currentCodeCompletion_.setCompletionKind(CodeCompletion::Other);
     }

tests/unit/unittest/clangcompletioncontextanalyzer-test.cpp

@@ -238,28 +238,28 @@ TEST_F(ClangCompletionContextAnalyzer, ArgumentOneAtCall)
 {
     auto analyzer = runAnalyzer("f(@");
 
-    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -2, 0, positionInText));
+    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, 0, 0, positionInText));
 }
 
 TEST_F(ClangCompletionContextAnalyzer, ArgumentTwoAtCall)
 {
     auto analyzer = runAnalyzer("f(1,@");
 
-    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -4, -2, positionInText));
+    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -2, -2, positionInText));
 }
 
 TEST_F(ClangCompletionContextAnalyzer, ArgumentTwoWithSpaceAtCall)
 {
     auto analyzer = runAnalyzer("f(1, @");
 
-    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -5, -3, positionInText));
+    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -3, -3, positionInText));
 }
 
 TEST_F(ClangCompletionContextAnalyzer, WhitespaceAfterFunctionName)
 {
     auto analyzer = runAnalyzer("foo (@");
 
-    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -5, 0, positionInText));
+    ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, 0, 0, positionInText));
 }
 
 TEST_F(ClangCompletionContextAnalyzer, AfterOpeningParenthesis)

tests/unit/unittest/codecompletionsextractor-test.cpp

@@ -153,6 +153,7 @@ protected:
     ClangBackEnd::UnsavedFiles unsavedFiles;
     ClangBackEnd::Documents documents{projects, unsavedFiles};
     Document functionDocument{Utf8StringLiteral(TESTDATA_DIR"/complete_extractor_function.cpp"), project, Utf8StringVector(), documents};
+    Document functionOverloadDocument{Utf8StringLiteral(TESTDATA_DIR"/complete_extractor_functionoverload.cpp"), project, Utf8StringVector(), documents};
     Document variableDocument{Utf8StringLiteral(TESTDATA_DIR"/complete_extractor_variable.cpp"), project, Utf8StringVector(), documents};
     Document classDocument{Utf8StringLiteral(TESTDATA_DIR"/complete_extractor_class.cpp"), project, Utf8StringVector(), documents};
     Document namespaceDocument{Utf8StringLiteral(TESTDATA_DIR"/complete_extractor_namespace.cpp"), project, Utf8StringVector(), documents};
@@ -678,6 +679,21 @@ TEST_F(CodeCompletionsExtractorSlowTest, BriefComment)
     ASSERT_THAT(extractor, HasBriefComment(Utf8StringLiteral("BriefComment"), Utf8StringLiteral("A brief comment")));
 }
 
+TEST_F(CodeCompletionsExtractorSlowTest, OverloadCandidate)
+{
+    ClangCodeCompleteResults completeResults(getResults(functionOverloadDocument, 8, 13));
+
+    ::CodeCompletionsExtractor extractor(completeResults.data());
+
+    ASSERT_THAT(extractor, HasCompletionChunks(Utf8String(),
+                                               CodeCompletionChunks({
+                                                    {CodeCompletionChunk::Text, Utf8StringLiteral("Foo")},
+                                                    {CodeCompletionChunk::LeftParen, Utf8StringLiteral("(")},
+                                                    {CodeCompletionChunk::CurrentParameter, Utf8StringLiteral("const Foo &foo")},
+                                                    {CodeCompletionChunk::RightParen, Utf8StringLiteral(")")},
+                                               })));
+}
+
 ClangCodeCompleteResults CodeCompletionsExtractor::getResults(const Document &document,
                                                               uint line,
                                                               uint column,