From 91429aa7521dff2006451577514c8954fcc3f793 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Thu, 23 Jul 2015 16:06:48 +0200 Subject: [PATCH] Clang: Fix completion position for clang and proposal * Rename some members/functions to clarify their meaning. * Ensure that the position for the proposal widget is at start of the identifer, so that the filter prefix will be found correctly in the GenericProposalWidget. For certain cases the completion were calculated but the widget was never shown: Case 1: void f() { } Case 2: void f() { st } Case 3: if (true) Case 4: foo. mem Change-Id: Ie79e01e8a22f8ec306136ec4ccbfffd544edd573 Reviewed-by: Erik Verbruggen --- .../activationsequencecontextprocessor.cpp | 61 +++++++++++-------- .../activationsequencecontextprocessor.h | 17 +++--- .../activationsequenceprocessor.cpp | 2 +- .../activationsequenceprocessor.h | 2 +- .../clangcompletionassistprocessor.cpp | 2 +- .../clangcompletioncontextanalyzer.cpp | 6 +- ...activationsequencecontextprocessortest.cpp | 2 +- .../activationsequenceprocessortest.cpp | 6 +- .../clangcompletioncontextanalyzertest.cpp | 60 +++++++++++++++--- 9 files changed, 108 insertions(+), 50 deletions(-) diff --git a/src/plugins/clangcodemodel/activationsequencecontextprocessor.cpp b/src/plugins/clangcodemodel/activationsequencecontextprocessor.cpp index 6edb842613d..ce9c5f0471e 100644 --- a/src/plugins/clangcodemodel/activationsequencecontextprocessor.cpp +++ b/src/plugins/clangcodemodel/activationsequencecontextprocessor.cpp @@ -46,8 +46,8 @@ ActivationSequenceContextProcessor::ActivationSequenceContextProcessor(const Cla : m_textCursor(assistInterface->textDocument()), m_assistInterface(assistInterface), m_positionInDocument(assistInterface->position()), - m_positionAfterOperator(m_positionInDocument), - m_positionBeforeOperator(m_positionAfterOperator) + m_startOfNamePosition(m_positionInDocument), + m_operatorStartPosition(m_positionInDocument) { m_textCursor.setPosition(m_positionInDocument); @@ -65,19 +65,19 @@ const QTextCursor &ActivationSequenceContextProcessor::textCursor_forTestOnly() return m_textCursor; } -int ActivationSequenceContextProcessor::positionAfterOperator() const +int ActivationSequenceContextProcessor::startOfNamePosition() const { - return m_positionAfterOperator; + return m_startOfNamePosition; } -int ActivationSequenceContextProcessor::positionBeforeOperator() const +int ActivationSequenceContextProcessor::operatorStartPosition() const { - return m_positionBeforeOperator; + return m_operatorStartPosition; } void ActivationSequenceContextProcessor::process() { - skipeWhiteSpacesAndIdentifierBeforeCursor(); + goBackToStartOfName(); processActivationSequence(); if (m_completionKind != CPlusPlus::T_EOF_SYMBOL) { @@ -90,20 +90,21 @@ void ActivationSequenceContextProcessor::process() processSlashOutsideOfAString(); processLeftParen(); processPreprocessorInclude(); - resetPositionForEOFCompletionKind(); } + + resetPositionsForEOFCompletionKind(); } void ActivationSequenceContextProcessor::processActivationSequence() { - const auto activationSequence = m_assistInterface->textAt(m_positionInDocument - 3, 3); + const int nonSpacePosition = findStartOfNonSpaceChar(m_startOfNamePosition); + const auto activationSequence = m_assistInterface->textAt(nonSpacePosition - 3, 3); ActivationSequenceProcessor activationSequenceProcessor(activationSequence, - m_positionInDocument, + nonSpacePosition, true); m_completionKind = activationSequenceProcessor.completionKind(); - m_positionBeforeOperator = activationSequenceProcessor.position(); - + m_operatorStartPosition = activationSequenceProcessor.operatorStartPosition(); } void ActivationSequenceContextProcessor::processStringLiteral() @@ -221,27 +222,37 @@ void ActivationSequenceContextProcessor::processPreprocessorInclude() } } -void ActivationSequenceContextProcessor::resetPositionForEOFCompletionKind() +void ActivationSequenceContextProcessor::resetPositionsForEOFCompletionKind() { if (m_completionKind == CPlusPlus::T_EOF_SYMBOL) - m_positionBeforeOperator = m_positionInDocument; + m_operatorStartPosition = m_positionInDocument; } -void ActivationSequenceContextProcessor::skipeWhiteSpacesAndIdentifierBeforeCursor() +int ActivationSequenceContextProcessor::findStartOfNonSpaceChar(int startPosition) { - QTextDocument *document = m_assistInterface->textDocument(); + int position = startPosition; + while (m_assistInterface->characterAt(position - 1).isSpace()) + --position; + return position; +} - const QRegExp findNonWhiteSpaceRegularExpression(QStringLiteral("[^\\s\\w]")); +int ActivationSequenceContextProcessor::findStartOfName(int startPosition) +{ + int position = startPosition; + QChar character; + do { + character = m_assistInterface->characterAt(--position); + } while (character.isLetterOrNumber() || character == QLatin1Char('_')); - auto nonWhiteSpaceTextCursor = document->find(findNonWhiteSpaceRegularExpression, - m_positionInDocument, - QTextDocument::FindBackward); + return position + 1; +} - if (!nonWhiteSpaceTextCursor.isNull()) { - m_positionInDocument = nonWhiteSpaceTextCursor.position(); - m_positionAfterOperator = m_positionInDocument; - m_textCursor.setPosition(m_positionInDocument); - } +void ActivationSequenceContextProcessor::goBackToStartOfName() +{ + m_startOfNamePosition = findStartOfName(m_positionInDocument); + + if (m_startOfNamePosition != m_positionInDocument) + m_textCursor.setPosition(m_startOfNamePosition); } } // namespace Internal diff --git a/src/plugins/clangcodemodel/activationsequencecontextprocessor.h b/src/plugins/clangcodemodel/activationsequencecontextprocessor.h index 268c8112d7a..d85a44090ea 100644 --- a/src/plugins/clangcodemodel/activationsequencecontextprocessor.h +++ b/src/plugins/clangcodemodel/activationsequencecontextprocessor.h @@ -50,15 +50,16 @@ public: ActivationSequenceContextProcessor(const ClangCompletionAssistInterface *assistInterface); CPlusPlus::Kind completionKind() const; + int startOfNamePosition() const; // e.g. points to 'b' in "foo.bar" + int operatorStartPosition() const; // e.g. points to '.' for "foo.bar" const QTextCursor &textCursor_forTestOnly() const; - int positionAfterOperator() const; - int positionBeforeOperator() const; - protected: void process(); - void skipeWhiteSpacesAndIdentifierBeforeCursor(); + int findStartOfNonSpaceChar(int startPosition); + int findStartOfName(int startPosition); + void goBackToStartOfName(); void processActivationSequence(); void processStringLiteral(); void processComma(); @@ -69,7 +70,7 @@ protected: void processSlashOutsideOfAString(); void processLeftParen(); void processPreprocessorInclude(); - void resetPositionForEOFCompletionKind(); + void resetPositionsForEOFCompletionKind(); bool isCompletionKindStringLiteralOrSlash() const; bool isProbablyPreprocessorIncludeDirective() const; @@ -80,9 +81,9 @@ private: CPlusPlus::Token m_token; const ClangCompletionAssistInterface *m_assistInterface; int m_tokenIndex; - int m_positionInDocument; - int m_positionAfterOperator; - int m_positionBeforeOperator; + const int m_positionInDocument; + int m_startOfNamePosition; + int m_operatorStartPosition; CPlusPlus::Kind m_completionKind; }; diff --git a/src/plugins/clangcodemodel/activationsequenceprocessor.cpp b/src/plugins/clangcodemodel/activationsequenceprocessor.cpp index a6a5076a31c..a7f4ea948d0 100644 --- a/src/plugins/clangcodemodel/activationsequenceprocessor.cpp +++ b/src/plugins/clangcodemodel/activationsequenceprocessor.cpp @@ -71,7 +71,7 @@ int ActivationSequenceProcessor::offset() const return m_offset; } -int ActivationSequenceProcessor::position() const +int ActivationSequenceProcessor::operatorStartPosition() const { return m_positionInDocument - m_offset; } diff --git a/src/plugins/clangcodemodel/activationsequenceprocessor.h b/src/plugins/clangcodemodel/activationsequenceprocessor.h index eacc81af203..2c6350dbafc 100644 --- a/src/plugins/clangcodemodel/activationsequenceprocessor.h +++ b/src/plugins/clangcodemodel/activationsequenceprocessor.h @@ -47,7 +47,7 @@ public: CPlusPlus::Kind completionKind() const; int offset() const; - int position() const; + int operatorStartPosition() const; // e.g. points to '.' for "foo.bar" private: void extractCharactersBeforePosition(const QString &activationString); diff --git a/src/plugins/clangcodemodel/clangcompletionassistprocessor.cpp b/src/plugins/clangcodemodel/clangcompletionassistprocessor.cpp index 126c0201afe..4c1304b3512 100644 --- a/src/plugins/clangcodemodel/clangcompletionassistprocessor.cpp +++ b/src/plugins/clangcodemodel/clangcompletionassistprocessor.cpp @@ -363,7 +363,7 @@ int ClangCompletionAssistProcessor::startOfOperator(int positionInDocument, *kind = activationSequenceProcessor.completionKind(); - int start = activationSequenceProcessor.position(); + int start = activationSequenceProcessor.operatorStartPosition(); if (start != positionInDocument) { QTextCursor tc(m_interface->textDocument()); tc.setPosition(positionInDocument); diff --git a/src/plugins/clangcodemodel/clangcompletioncontextanalyzer.cpp b/src/plugins/clangcodemodel/clangcompletioncontextanalyzer.cpp index 99a75f9b84b..63fd8408780 100644 --- a/src/plugins/clangcodemodel/clangcompletioncontextanalyzer.cpp +++ b/src/plugins/clangcodemodel/clangcompletioncontextanalyzer.cpp @@ -86,9 +86,9 @@ void ClangCompletionContextAnalyzer::analyze() ActivationSequenceContextProcessor activationSequenceContextProcessor(m_interface); m_completionOperator = activationSequenceContextProcessor.completionKind(); - int afterOperatorPosition = activationSequenceContextProcessor.positionAfterOperator(); - m_positionEndOfExpression = activationSequenceContextProcessor.positionBeforeOperator(); - m_positionForProposal = activationSequenceContextProcessor.positionAfterOperator(); + int afterOperatorPosition = activationSequenceContextProcessor.startOfNamePosition(); + m_positionEndOfExpression = activationSequenceContextProcessor.operatorStartPosition(); + m_positionForProposal = activationSequenceContextProcessor.startOfNamePosition(); const bool actionIsSet = handleNonFunctionCall(afterOperatorPosition); if (!actionIsSet) { diff --git a/tests/unit/unittest/activationsequencecontextprocessortest.cpp b/tests/unit/unittest/activationsequencecontextprocessortest.cpp index 443a8aeacb0..e69f8b97f13 100644 --- a/tests/unit/unittest/activationsequencecontextprocessortest.cpp +++ b/tests/unit/unittest/activationsequencecontextprocessortest.cpp @@ -54,7 +54,7 @@ TEST(ActivationSequeneContextProcessor, TextCursorPosition) ClangCompletionAssistInterface interface("foobar", 4); ContextProcessor processor{&interface}; - ASSERT_THAT(processor.textCursor_forTestOnly().position(), 4); + ASSERT_THAT(processor.textCursor_forTestOnly().position(), 0); } TEST(ActivationSequeneContextProcessor, StringLiteral) diff --git a/tests/unit/unittest/activationsequenceprocessortest.cpp b/tests/unit/unittest/activationsequenceprocessortest.cpp index 88736f243ba..d5962cdc5a0 100644 --- a/tests/unit/unittest/activationsequenceprocessortest.cpp +++ b/tests/unit/unittest/activationsequenceprocessortest.cpp @@ -47,14 +47,14 @@ MATCHER_P3(HasResult, completionKind, offset, newPosition, std::string(negation ? "hasn't" : "has") + " result of completion kind " + PrintToString(Token::name(completionKind)) + ", offset " + PrintToString(offset) - + " and new position in document" + PrintToString(newPosition)) + + " and new operator start position" + PrintToString(newPosition)) { if (arg.completionKind() != completionKind || arg.offset() != offset - || arg.position() != newPosition) { + || arg.operatorStartPosition() != newPosition) { *result_listener << "completion kind is " << PrintToString(Token::name(arg.completionKind())) << ", offset is " << PrintToString(arg.offset()) - << " and new position in document is " << PrintToString(arg.position()); + << " and new operator start position is " << PrintToString(arg.operatorStartPosition()); return false; } diff --git a/tests/unit/unittest/clangcompletioncontextanalyzertest.cpp b/tests/unit/unittest/clangcompletioncontextanalyzertest.cpp index 8603b05c5ef..08e92ee2f94 100644 --- a/tests/unit/unittest/clangcompletioncontextanalyzertest.cpp +++ b/tests/unit/unittest/clangcompletioncontextanalyzertest.cpp @@ -172,6 +172,20 @@ CCA ClangCompletionContextAnalyzer::runAnalyzer(const char *text) return analyzer; } +TEST_F(ClangCompletionContextAnalyzer, WordsBeforeCursor) +{ + auto analyzer = runAnalyzer("foo bar@"); + + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -3, -3, positionInText)); +} + +TEST_F(ClangCompletionContextAnalyzer, AfterSpace) +{ + auto analyzer = runAnalyzer("foo @"); + + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); +} + TEST_F(ClangCompletionContextAnalyzer, AtEndOfDotMember) { auto analyzer = runAnalyzer("o.mem@"); @@ -183,7 +197,7 @@ TEST_F(ClangCompletionContextAnalyzer, AtEndOfDotMemberWithSpaceInside) { auto analyzer = runAnalyzer("o. mem@"); - ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -4, -4, positionInText)); + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -3, -3, positionInText)); } TEST_F(ClangCompletionContextAnalyzer, AtBeginOfDotMember) @@ -197,7 +211,7 @@ TEST_F(ClangCompletionContextAnalyzer, AtBeginOfDotMemberWithSpaceInside) { auto analyzer = runAnalyzer("o. @mem"); - ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -1, -1, positionInText)); + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); } TEST_F(ClangCompletionContextAnalyzer, AtEndOfArrow) @@ -211,7 +225,7 @@ TEST_F(ClangCompletionContextAnalyzer, AtEndOfArrowWithSpaceInside) { auto analyzer = runAnalyzer("o-> mem@"); - ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -4, -4, positionInText)); + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -3, -3, positionInText)); } TEST_F(ClangCompletionContextAnalyzer, AtBeginOfArrow) @@ -225,7 +239,7 @@ TEST_F(ClangCompletionContextAnalyzer, AtBeginOfArrowWithSpaceInside) { auto analyzer = runAnalyzer("o-> @mem"); - ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -1, -1, positionInText)); + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); } TEST_F(ClangCompletionContextAnalyzer, ParameteOneAtCall) @@ -246,7 +260,7 @@ TEST_F(ClangCompletionContextAnalyzer, ParameteTwoWithSpaceAtCall) { auto analyzer = runAnalyzer("f(1, @"); - ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, -1, -1, positionInText)); + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClangAfterLeftParen, -5, -3, positionInText)); } TEST_F(ClangCompletionContextAnalyzer, ParameteOneAtSignal) @@ -396,13 +410,45 @@ TEST_F(ClangCompletionContextAnalyzer, DoxygenMarkerInNonDoxygenComment2) ASSERT_THAT(analyzer, IsPassThroughToClang()); } -TEST_F(ClangCompletionContextAnalyzer, OneLineComment) +TEST_F(ClangCompletionContextAnalyzer, AtEndOfOneLineComment) { - auto analyzer = runAnalyzer("// text@"); + auto analyzer = runAnalyzer("// comment@"); ASSERT_THAT(analyzer, IsPassThroughToClang()); } +TEST_F(ClangCompletionContextAnalyzer, AfterOneLineCommentLine) +{ + auto analyzer = runAnalyzer("// comment\n" + "@"); + + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); +} + +TEST_F(ClangCompletionContextAnalyzer, AfterEmptyOneLineComment) +{ + auto analyzer = runAnalyzer("//\n" + "@"); + + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); +} + +TEST_F(ClangCompletionContextAnalyzer, AfterOneLineDoxygenComment1) +{ + auto analyzer = runAnalyzer("/// comment\n" + "@"); + + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); +} + +TEST_F(ClangCompletionContextAnalyzer, AfterOneLineDoxygenComment2) +{ + auto analyzer = runAnalyzer("//! comment \n" + "@"); + + ASSERT_THAT(analyzer, HasResult(CCA::PassThroughToLibClang, 0, 0, positionInText)); +} + TEST_F(ClangCompletionContextAnalyzer, BeginEndComment) { auto analyzer = runAnalyzer("/* text@ */");