From 05c1efd8ef359a8c6b45d24abf6f2eb780fe73db Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Thu, 23 Jun 2016 16:34:03 +0200 Subject: [PATCH] Clang: Fix dot-arrow-correction for not-yet-parsed unsaved content The issue was re-producible with e.g.: void g() { // Type 'foo.' as fast as possible in the next line } This led to "foo->" with completion results as if there was no "foo." at all in that line. We relied on a correct position for translationUnit.sourceLocationAtWithoutReparsing(), but the just typed characters were not yet reparsed. And we do not want to reparse at that point since takes too long. We already determine the utf8 position for the dot character, so simply use that instead. This completes commit 17c1325cc45eb563333d28725a51c04e33cd01f2. Change-Id: I669888b5c17ee63b2aec7b16c9921f9d79e281f9 Reviewed-by: David Schulz --- .../clangbackend/ipcsource/codecompleter.cpp | 31 ++++++----- .../clangbackend/ipcsource/codecompleter.h | 12 ++-- .../clangbackend/ipcsource/unsavedfile.cpp | 17 ++++-- .../clangbackend/ipcsource/unsavedfile.h | 1 + tests/unit/unittest/codecompletiontest.cpp | 55 +++++++++++-------- ...tArrowCorrectionForPointer_afterTyping.cpp | 6 ++ ...ArrowCorrectionForPointer_beforeTyping.cpp | 6 ++ tests/unit/unittest/unsavedfiletest.cpp | 39 +++++++++++++ 8 files changed, 121 insertions(+), 46 deletions(-) create mode 100644 tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_afterTyping.cpp create mode 100644 tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_beforeTyping.cpp diff --git a/src/tools/clangbackend/ipcsource/codecompleter.cpp b/src/tools/clangbackend/ipcsource/codecompleter.cpp index 436c6680524..4344a22aa93 100644 --- a/src/tools/clangbackend/ipcsource/codecompleter.cpp +++ b/src/tools/clangbackend/ipcsource/codecompleter.cpp @@ -68,8 +68,7 @@ CodeCompletions CodeCompleter::complete(uint line, uint column) translationUnit.cxUnsavedFiles(), translationUnit.unsavedFilesCount()); - if (results.hasNoResultsForDotCompletion() && hasDotAt(line, column - 1)) - results = completeWithArrowInsteadOfDot(line, column); + tryDotArrowCorrectionIfNoResults(results, line, column); return toCodeCompletions(results); } @@ -93,13 +92,6 @@ ClangCodeCompleteResults CodeCompleter::complete(uint line, defaultOptions()); } -bool CodeCompleter::hasDotAt(uint line, uint column) const -{ - const UnsavedFile &unsavedFile = translationUnit.unsavedFile(); - - return unsavedFile.hasCharacterAt(line, column, '.'); -} - uint CodeCompleter::defaultOptions() const { uint options = CXCodeComplete_IncludeMacros @@ -111,12 +103,25 @@ uint CodeCompleter::defaultOptions() const return options; } -ClangCodeCompleteResults CodeCompleter::completeWithArrowInsteadOfDot(uint line, uint column) +void CodeCompleter::tryDotArrowCorrectionIfNoResults(ClangCodeCompleteResults &results, + uint line, + uint column) +{ + if (results.hasNoResultsForDotCompletion()) { + const UnsavedFile &unsavedFile = translationUnit.unsavedFile(); + bool positionIsOk = false; + const uint dotPosition = unsavedFile.toUtf8Position(line, column - 1, &positionIsOk); + if (positionIsOk && unsavedFile.hasCharacterAt(dotPosition, '.')) + results = completeWithArrowInsteadOfDot(line, column, dotPosition); + } +} + +ClangCodeCompleteResults CodeCompleter::completeWithArrowInsteadOfDot(uint line, + uint column, + uint dotPosition) { ClangCodeCompleteResults results; - - const SourceLocation location = translationUnit.sourceLocationAtWithoutReparsing(line, column - 1); - const bool replaced = translationUnit.unsavedFile().replaceAt(location.offset(), + const bool replaced = translationUnit.unsavedFile().replaceAt(dotPosition, 1, Utf8StringLiteral("->")); diff --git a/src/tools/clangbackend/ipcsource/codecompleter.h b/src/tools/clangbackend/ipcsource/codecompleter.h index 260ac4d581c..c785ae5939d 100644 --- a/src/tools/clangbackend/ipcsource/codecompleter.h +++ b/src/tools/clangbackend/ipcsource/codecompleter.h @@ -46,18 +46,20 @@ public: CompletionCorrection neededCorrection() const; -public: // for tests - bool hasDotAt(uint line, uint column) const; - private: uint defaultOptions() const; + void tryDotArrowCorrectionIfNoResults(ClangCodeCompleteResults &results, + uint line, + uint column); + ClangCodeCompleteResults complete(uint line, uint column, CXUnsavedFile *unsavedFiles, unsigned unsavedFileCount); - - ClangCodeCompleteResults completeWithArrowInsteadOfDot(uint line, uint column); + ClangCodeCompleteResults completeWithArrowInsteadOfDot(uint line, + uint column, + uint dotPosition); Utf8String filePath() const; static void checkCodeCompleteResult(CXCodeCompleteResults *completeResults); diff --git a/src/tools/clangbackend/ipcsource/unsavedfile.cpp b/src/tools/clangbackend/ipcsource/unsavedfile.cpp index a4d6f0fb82d..dffbdfd1ff0 100644 --- a/src/tools/clangbackend/ipcsource/unsavedfile.cpp +++ b/src/tools/clangbackend/ipcsource/unsavedfile.cpp @@ -71,15 +71,24 @@ const char *UnsavedFile::filePath() const return cxUnsavedFile.Filename; } -bool UnsavedFile::hasCharacterAt(uint line, uint column, char character) const +uint UnsavedFile::toUtf8Position(uint line, uint column, bool *ok) const { Utf8PositionFromLineColumn converter(cxUnsavedFile.Contents); if (converter.find(line, column)) { - const uint utf8Position = converter.position(); - return hasCharacterAt(utf8Position, character); + *ok = true; + return converter.position(); } - return false; + *ok = false; + return 0; +} + +bool UnsavedFile::hasCharacterAt(uint line, uint column, char character) const +{ + bool positionIsOk = false; + const uint utf8Position = toUtf8Position(line, column, &positionIsOk); + + return positionIsOk && hasCharacterAt(utf8Position, character); } bool UnsavedFile::hasCharacterAt(uint position, char character) const diff --git a/src/tools/clangbackend/ipcsource/unsavedfile.h b/src/tools/clangbackend/ipcsource/unsavedfile.h index 3433093ae15..dcd4fd026b1 100644 --- a/src/tools/clangbackend/ipcsource/unsavedfile.h +++ b/src/tools/clangbackend/ipcsource/unsavedfile.h @@ -55,6 +55,7 @@ public: const char *filePath() const; // 1-based line and column + uint toUtf8Position(uint line, uint column, bool *ok) const; bool hasCharacterAt(uint line, uint column, char character) const; bool hasCharacterAt(uint position, char character) const; bool replaceAt(uint position, uint length, const Utf8String &replacement); diff --git a/tests/unit/unittest/codecompletiontest.cpp b/tests/unit/unittest/codecompletiontest.cpp index a370f935867..c25b03a492a 100644 --- a/tests/unit/unittest/codecompletiontest.cpp +++ b/tests/unit/unittest/codecompletiontest.cpp @@ -112,6 +112,19 @@ protected: readFileContent(QStringLiteral("/complete_withDotArrowCorrectionForPointer.cpp")), true }; + ClangBackEnd::FileContainer dotArrowCorrectionForPointerFileContainerBeforeTyping{ + Utf8StringLiteral(TESTDATA_DIR"/complete_withDotArrowCorrectionForPointer.cpp"), + projectPart.projectPartId(), + readFileContent(QStringLiteral("/complete_withDotArrowCorrectionForPointer_beforeTyping.cpp")), + true + }; + ClangBackEnd::FileContainer dotArrowCorrectionForPointerFileContainerAfterTyping{ + Utf8StringLiteral(TESTDATA_DIR"/complete_withDotArrowCorrectionForPointer.cpp"), + projectPart.projectPartId(), + readFileContent(QStringLiteral("/complete_withDotArrowCorrectionForPointer_afterTyping.cpp")), + true + }; + ClangBackEnd::FileContainer dotArrowCorrectionForPointerFileContainerInitial{ Utf8StringLiteral(TESTDATA_DIR"/complete_withDotArrowCorrectionForPointer.cpp"), projectPart.projectPartId(), @@ -313,30 +326,6 @@ TEST_F(CodeCompleter, ArrowCompletion) ClangBackEnd::CompletionCorrection::NoCorrection); } -TEST_F(CodeCompleter, HasDotAt) -{ - auto myCompleter = setupCompleter(dotArrowCorrectionForPointerFileContainer); - - ASSERT_TRUE(myCompleter.hasDotAt(5, 8)); -} - -TEST_F(CodeCompleter, HasDotAtWithUpdatedUnsavedFile) -{ - auto myCompleter = setupCompleter(dotArrowCorrectionForPointerFileContainerInitial); - unsavedFiles.createOrUpdate({dotArrowCorrectionForPointerFileContainerUpdated}); - - ASSERT_TRUE(myCompleter.hasDotAt(5, 8)); -} - -TEST_F(CodeCompleter, HasNoDotAtDueToMissingUnsavedFile) -{ - const ClangBackEnd::FileContainer fileContainer = dotArrowCorrectionForPointerFileContainer; - translationUnits.create({fileContainer}); - ClangBackEnd::CodeCompleter myCompleter(translationUnits.translationUnit(fileContainer)); - - ASSERT_FALSE(myCompleter.hasDotAt(5, 8)); -} - TEST_F(CodeCompleter, DotToArrowCompletionForPointer) { auto myCompleter = setupCompleter(dotArrowCorrectionForPointerFileContainer); @@ -350,6 +339,24 @@ TEST_F(CodeCompleter, DotToArrowCompletionForPointer) ClangBackEnd::CompletionCorrection::DotToArrowCorrection); } +TEST_F(CodeCompleter, DotToArrowCompletionForPointerInOutdatedTranslationUnit) +{ + auto fileContainerBeforeTyping = dotArrowCorrectionForPointerFileContainerBeforeTyping; + auto myCompleter = setupCompleter(fileContainerBeforeTyping); + auto translationUnit = translationUnits.translationUnit(fileContainerBeforeTyping.filePath(), + fileContainerBeforeTyping.projectPartId()); + translationUnit.cxTranslationUnit(); // Parse + unsavedFiles.createOrUpdate({dotArrowCorrectionForPointerFileContainerAfterTyping}); + + const ClangBackEnd::CodeCompletions completions = myCompleter.complete(5, 9); + + ASSERT_THAT(completions, + Contains(IsCodeCompletion(Utf8StringLiteral("member"), + CodeCompletion::VariableCompletionKind))); + ASSERT_THAT(myCompleter.neededCorrection(), + ClangBackEnd::CompletionCorrection::DotToArrowCorrection); +} + TEST_F(CodeCompleter, NoDotToArrowCompletionForObject) { auto myCompleter = setupCompleter(noDotArrowCorrectionForObjectFileContainer); diff --git a/tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_afterTyping.cpp b/tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_afterTyping.cpp new file mode 100644 index 00000000000..9f8d3645b2e --- /dev/null +++ b/tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_afterTyping.cpp @@ -0,0 +1,6 @@ +struct Foo { int member; }; + +void g(Foo *foo) +{ + foo. +} diff --git a/tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_beforeTyping.cpp b/tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_beforeTyping.cpp new file mode 100644 index 00000000000..0472cfacaa4 --- /dev/null +++ b/tests/unit/unittest/data/complete_withDotArrowCorrectionForPointer_beforeTyping.cpp @@ -0,0 +1,6 @@ +struct Foo { int member; }; + +void g(Foo *foo) +{ + +} diff --git a/tests/unit/unittest/unsavedfiletest.cpp b/tests/unit/unittest/unsavedfiletest.cpp index 3f0ae6d90b1..463c29be8b3 100644 --- a/tests/unit/unittest/unsavedfiletest.cpp +++ b/tests/unit/unittest/unsavedfiletest.cpp @@ -34,6 +34,7 @@ using ClangBackEnd::UnsavedFile; using ClangBackEnd::UnsavedFiles; +using ::testing::Eq; using ::testing::PrintToString; namespace { @@ -150,6 +151,44 @@ TEST_F(UnsavedFile, Replace) ASSERT_THAT(unsavedFile, IsUnsavedFile(filePath, expectedContent, expectedContent.byteSize())); } +TEST_F(UnsavedFile, ToUtf8PositionForValidLineColumn) +{ + ::UnsavedFile unsavedFile(filePath, fileContent); + bool ok = false; + + const uint position = unsavedFile.toUtf8Position(1, 1, &ok); + + ASSERT_TRUE(ok); + ASSERT_THAT(position, Eq(0)); +} + +TEST_F(UnsavedFile, ToUtf8PositionForInValidLineColumn) +{ + ::UnsavedFile unsavedFile(filePath, fileContent); + bool ok = false; + + unsavedFile.toUtf8Position(2, 1, &ok); + + ASSERT_FALSE(ok); +} + +TEST_F(UnsavedFile, ToUtf8PositionForDefaultConstructedUnsavedFile) +{ + ::UnsavedFile unsavedFile; + bool ok = false; + + unsavedFile.toUtf8Position(1, 1, &ok); + + ASSERT_FALSE(ok); +} + +TEST_F(UnsavedFile, HasNoCharacterForDefaultConstructedUnsavedFile) +{ + ::UnsavedFile unsavedFile; + + ASSERT_FALSE(unsavedFile.hasCharacterAt(0, 'x')); +} + TEST_F(UnsavedFile, HasNoCharacterForTooBigOffset) { ::UnsavedFile unsavedFile(filePath, fileContent);