C++: Fine-tune auto insertion of '}'

Do not insert for these cases:
 * <Cursor>{
 * namespace X <Cursor>
 * if the next block is indented, like e.g.:
     if (e) <Cursor>
        g();
 * on empty line if text before looks like a finished statement or
   scope opening/end

Change-Id: Id9decc1e964a775724a929c2a3e79b5283105560
Reviewed-by: David Schulz <david.schulz@qt.io>

src/libs/cplusplus/MatchingText.cpp

@@ -34,6 +34,8 @@
 #include <QChar>
 #include <QDebug>
 
+#include <utils/algorithm.h>
+
 using namespace CPlusPlus;
 
 enum { MAX_NUM_LINES = 20 };
@@ -135,16 +137,215 @@ static const Token tokenAtPosition(const Tokens &tokens, const unsigned pos)
     return Token();
 }
 
+static int tokenIndexBeforePosition(const Tokens &tokens, unsigned pos)
+{
+    for (int i = tokens.size() - 1; i >= 0; --i) {
+        if (tokens[i].utf16charsBegin() < pos)
+            return i;
+    }
+    return -1;
+}
+
+static bool isCursorAtEndOfLineButMaybeBeforeComment(const Tokens &tokens, int pos)
+{
+    int index = tokenIndexBeforePosition(tokens, uint(pos));
+    if (index == -1 || index >= tokens.size())
+        return false;
+
+    do {
+        ++index;
+    } while (index < tokens.size() && tokens[index].isComment());
+
+    return index >= tokens.size();
+}
+
+// 10.6.1 Attribute syntax and semantics
+// This does not handle alignas() since it is not needed for the namespace case.
+static int skipAttributeSpecifierSequence(const Tokens &tokens, int index)
+{
+    // [[ attribute-using-prefixopt attribute-list ]]
+    if (index >= 1 && tokens[index].is(T_RBRACKET) && tokens[index - 1].is(T_RBRACKET)) {
+        // Skip everything within [[ ]]
+        for (int i = index - 2; i >= 0; --i) {
+            if (i >= 1 && tokens[i].is(T_LBRACKET) && tokens[i - 1].is(T_LBRACKET))
+                return i - 2;
+        }
+
+        return -1;
+    }
+
+    return index;
+}
+
+static int skipNamespaceName(const Tokens &tokens, int index)
+{
+    if (index >= tokens.size())
+        return -1;
+
+    if (!tokens[index].is(T_IDENTIFIER))
+        return index;
+
+    // Accept
+    //  SomeName
+    //  Some::Nested::Name
+    bool expectIdentifier = false;
+    for (int i = index - 1; i >= 0; --i) {
+        if (expectIdentifier) {
+            if (tokens[i].is(T_IDENTIFIER))
+                expectIdentifier = false;
+            else
+                return -1;
+        } else if (tokens[i].is(T_COLON_COLON)) {
+            expectIdentifier = true;
+        } else {
+            return i;
+        }
+    }
+
+    return index;
+}
+
+// 10.3.1 Namespace definition
+static bool isAfterNamespaceDefinition(const Tokens &tokens, int position)
+{
+    int index = tokenIndexBeforePosition(tokens, uint(position));
+    if (index == -1)
+        return false;
+
+    // Handle optional name
+    index = skipNamespaceName(tokens, index);
+    if (index == -1)
+        return false;
+
+    // Handle optional attribute specifier sequence
+    index = skipAttributeSpecifierSequence(tokens, index);
+    if (index == -1)
+        return false;
+
+    return index >= 0 && tokens[index].is(T_NAMESPACE);
+}
+
+static int isEmptyOrWhitespace(const QString &text)
+{
+    return Utils::allOf(text, [](const QChar &c) {return c.isSpace(); });
+}
+
+static QTextBlock previousNonEmptyBlock(const QTextBlock &currentBlock)
+{
+    QTextBlock block = currentBlock.previous();
+    forever {
+        if (!block.isValid() || !isEmptyOrWhitespace(block.text()))
+            return block;
+        block = block.previous();
+    }
+}
+
+static QTextBlock nextNonEmptyBlock(const QTextBlock &currentBlock)
+{
+    QTextBlock block = currentBlock.next();
+    forever {
+        if (!block.isValid() || !isEmptyOrWhitespace(block.text()))
+            return block;
+        block = block.next();
+    }
+}
+
+static bool allowAutoClosingBraceAtEmptyLine(
+        const QTextBlock &block,
+        MatchingText::IsNextBlockDeeperIndented isNextDeeperIndented)
+{
+    QTextBlock previousBlock = previousNonEmptyBlock(block);
+    if (!previousBlock.isValid())
+        return false; // Nothing before
+
+    QTextBlock nextBlock = nextNonEmptyBlock(block);
+    if (!nextBlock.isValid())
+        return true; // Nothing behind
+
+    if (isNextDeeperIndented && isNextDeeperIndented(previousBlock))
+        return false; // Before indented
+
+    const QString trimmedText = previousBlock.text().trimmed();
+    return !trimmedText.endsWith(';')
+        && !trimmedText.endsWith('{')
+        && !trimmedText.endsWith('}');
+}
+
+static Tokens getTokens(const QTextCursor &cursor, int &prevState)
+{
+    LanguageFeatures features;
+    features.qtEnabled = false;
+    features.qtKeywordsEnabled = false;
+    features.qtMocRunEnabled = false;
+    features.cxx11Enabled = true;
+    features.cxxEnabled = true;
+    features.c99Enabled = true;
+    features.objCEnabled = true;
+
+    SimpleLexer tokenize;
+    tokenize.setLanguageFeatures(features);
+
+    prevState = BackwardsScanner::previousBlockState(cursor.block()) & 0xFF;
+    return tokenize(cursor.block().text(), prevState);
+}
+
+static QChar firstNonSpace(const QTextCursor &cursor)
+{
+    int position = cursor.position();
+    QChar ch = cursor.document()->characterAt(position);
+    while (ch.isSpace())
+        ch = cursor.document()->characterAt(++position);
+
+    return ch;
+}
+
+static bool allowAutoClosingBraceByLookahead(const QTextCursor &cursor)
+{
+    const QChar lookAhead = firstNonSpace(cursor);
+    if (lookAhead.isNull())
+        return true;
+
+    switch (lookAhead.unicode()) {
+    case ';': case ',':
+    case ')': case '}': case ']':
+        return true;
+    }
+
+    return false;
+}
+
+static bool allowAutoClosingBrace(const QTextCursor &cursor,
+                                  MatchingText::IsNextBlockDeeperIndented isNextIndented)
+{
+    if (MatchingText::isInCommentHelper(cursor))
+        return false;
+
+    const QTextBlock block = cursor.block();
+    if (isEmptyOrWhitespace(block.text()))
+        return allowAutoClosingBraceAtEmptyLine(cursor.block(), isNextIndented);
+
+    int prevState;
+    const Tokens tokens = getTokens(cursor, prevState);
+    if (isAfterNamespaceDefinition(tokens, cursor.positionInBlock()))
+        return false;
+
+    if (isCursorAtEndOfLineButMaybeBeforeComment(tokens, cursor.positionInBlock()))
+        return !(isNextIndented && isNextIndented(block));
+
+    return allowAutoClosingBraceByLookahead(cursor);
+}
+
 bool MatchingText::contextAllowsAutoParentheses(const QTextCursor &cursor,
-                                                const QString &textToInsert)
+                                                const QString &textToInsert,
+                                                IsNextBlockDeeperIndented isNextIndented)
 {
     QChar ch;
 
     if (!textToInsert.isEmpty())
         ch = textToInsert.at(0);
 
-    if (ch == QLatin1Char('{') && cursor.block().text().trimmed().isEmpty())
+    if (ch == QLatin1Char('{'))
-        return false; // User just might want to wrap up some lines.
+        return allowAutoClosingBrace(cursor, isNextIndented);
 
     if (!shouldInsertMatchingText(cursor) && ch != QLatin1Char('\'') && ch != QLatin1Char('"'))
         return false;
@@ -198,24 +399,6 @@ bool MatchingText::shouldInsertMatchingText(QChar lookAhead)
     } // switch
 }
 
-static Tokens getTokens(const QTextCursor &cursor, int &prevState)
-{
-    LanguageFeatures features;
-    features.qtEnabled = false;
-    features.qtKeywordsEnabled = false;
-    features.qtMocRunEnabled = false;
-    features.cxx11Enabled = true;
-    features.cxxEnabled = true;
-    features.c99Enabled = true;
-    features.objCEnabled = true;
-
-    SimpleLexer tokenize;
-    tokenize.setLanguageFeatures(features);
-
-    prevState = BackwardsScanner::previousBlockState(cursor.block()) & 0xFF;
-    return tokenize(cursor.block().text(), prevState);
-}
-
 bool MatchingText::isInCommentHelper(const QTextCursor &cursor, Token *retToken)
 {
     int prevState = 0;

src/libs/cplusplus/MatchingText.h

@@ -30,7 +30,10 @@
 #include <cplusplus/Token.h>
 #include <cplusplus/CPlusPlusForwardDeclarations.h>
 
+#include <functional>
+
 QT_FORWARD_DECLARE_CLASS(QTextCursor)
+QT_FORWARD_DECLARE_CLASS(QTextBlock)
 QT_FORWARD_DECLARE_CLASS(QChar)
 
 namespace CPlusPlus {
@@ -38,8 +41,11 @@ namespace CPlusPlus {
 class CPLUSPLUS_EXPORT MatchingText
 {
 public:
+    using IsNextBlockDeeperIndented = std::function<bool(const QTextBlock &textBlock)>;
     static bool contextAllowsAutoParentheses(const QTextCursor &cursor,
-                                             const QString &textToInsert);
+                                             const QString &textToInsert,
+                                             IsNextBlockDeeperIndented isNextIndented
+                                                = IsNextBlockDeeperIndented());
     static bool contextAllowsAutoQuotes(const QTextCursor &cursor,
                                         const QString &textToInsert);
     static bool contextAllowsElectricCharacters(const QTextCursor &cursor);

src/plugins/cppeditor/cppautocompleter.cpp

@@ -27,15 +27,20 @@
 
 #include <cplusplus/MatchingText.h>
 
+#include <texteditor/tabsettings.h>
+
+#include <QTextBlock>
 #include <QTextCursor>
 
 using namespace CppEditor;
 using namespace Internal;
 
 bool CppAutoCompleter::contextAllowsAutoBrackets(const QTextCursor &cursor,
-                                                    const QString &textToInsert) const
+                                                 const QString &textToInsert) const
 {
-    return CPlusPlus::MatchingText::contextAllowsAutoParentheses(cursor, textToInsert);
+    const CPlusPlus::MatchingText::IsNextBlockDeeperIndented isIndented
+            = [this](const QTextBlock &b) { return isNextBlockIndented(b); };
+    return CPlusPlus::MatchingText::contextAllowsAutoParentheses(cursor, textToInsert, isIndented);
 }
 
 bool CppAutoCompleter::contextAllowsAutoQuotes(const QTextCursor &cursor,

src/plugins/texteditor/autocompleter.cpp

@@ -150,6 +150,24 @@ bool AutoCompleter::isQuote(const QString &text)
     return text == QLatin1String("\"") || text == QLatin1String("'");
 }
 
+bool AutoCompleter::isNextBlockIndented(const QTextBlock &currentBlock) const
+{
+    QTextBlock block = currentBlock;
+    int indentation = m_tabSettings.indentationColumn(block.text());
+
+    if (block.next().isValid()) { // not the last block
+        block = block.next();
+        //skip all empty blocks
+        while (block.isValid() && m_tabSettings.onlySpace(block.text()))
+            block = block.next();
+        if (block.isValid()
+                && m_tabSettings.indentationColumn(block.text()) > indentation)
+            return true;
+    }
+
+    return false;
+}
+
 QString AutoCompleter::replaceSelection(QTextCursor &cursor, const QString &textToInsert) const
 {
     if (!cursor.hasSelection())
@@ -301,17 +319,8 @@ int AutoCompleter::paragraphSeparatorAboutToBeInserted(QTextCursor &cursor)
             if (condition) {|
                 statement;
     */
-    int indentation = m_tabSettings.indentationColumn(block.text());
+    if (isNextBlockIndented(block))
-
+        return 0;
-    if (block.next().isValid()) { // not the last block
-        block = block.next();
-        //skip all empty blocks
-        while (block.isValid() && m_tabSettings.onlySpace(block.text()))
-            block = block.next();
-        if (block.isValid()
-                && m_tabSettings.indentationColumn(block.text()) > indentation)
-            return 0;
-    }
 
     const QString &textToInsert = insertParagraphSeparator(cursor);
     int pos = cursor.position();

src/plugins/texteditor/autocompleter.h

@@ -31,6 +31,7 @@
 #include <QString>
 
 QT_BEGIN_NAMESPACE
+class QTextBlock;
 class QTextCursor;
 QT_END_NAMESPACE
 
@@ -90,6 +91,7 @@ public:
     virtual QString insertParagraphSeparator(const QTextCursor &cursor) const;
 
     static bool isQuote(const QString &text);
+    bool isNextBlockIndented(const QTextBlock &currentBlock) const;
 
 private:
     QString replaceSelection(QTextCursor &cursor, const QString &textToInsert) const;

tests/unit/unittest/matchingtext-test.cpp

@@ -57,7 +57,12 @@ public:
 };
 
 using MT = CPlusPlus::MatchingText;
-class MatchingText : public testing::Test {};
+using IsNextBlockDeeperIndented = MT::IsNextBlockDeeperIndented;
+
+class MatchingText : public testing::Test {
+protected:
+    const IsNextBlockDeeperIndented nextBlockIsIndented = [](const QTextBlock &) { return true; };
+};
 
 TEST_F(MatchingText, ContextAllowsAutoParentheses_ForNoInput)
 {
@@ -87,14 +92,22 @@ TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_AfterFunctionDeclar
     ASSERT_TRUE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
 }
 
-// TODO: Fix!
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_AfterFunctionDeclaratorNewLine)
-//TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_AfterFunctionDeclaratorNewLine)
+{
-//{
+    const Document document("void g()\n"
-//    const Document document("void g()\n"
+                            "@");
-//                            "@");
 
-//    ASSERT_TRUE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+    ASSERT_TRUE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
-//}
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_AfterFunctionDeclaratorNewLineAndMore)
+{
+    const Document document("void g()\n"
+                            "@\n"
+                            "1+1;");
+
+    ASSERT_TRUE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
 
 TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_AfterLambdaDeclarator)
 {
@@ -103,12 +116,11 @@ TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_AfterLambdaDeclarat
     ASSERT_TRUE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
 }
 
-// TODO: This does not seem useful, remove.
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeOpeningCurlyBrace)
-TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_BeforeOpeningCurlyBrace)
 {
     const Document document("@{");
 
-    ASSERT_TRUE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
 }
 
 TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_BeforeClosingCurlyBrace)
@@ -160,21 +172,88 @@ TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotInCppDoxygenComm
     ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
 }
 
-// TODO: Fix!
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeIndented)
-//TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeIndented)
+{
-//{
+    const Document document("@\n"
-//    const Document document("if (true) @\n"
+                            "    1+1;");
-//                            "    1+1;");
 
-//    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{", nextBlockIsIndented));
-//}
+}
 
-// TODO: Fix!
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeIndentedWithFollowingComment)
-//TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeNamespace)
+{
-//{
+    const Document document("@\n // comment"
-//    const Document document("namespace X @");
+                            "    1+1;");
 
-//    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{", nextBlockIsIndented));
-//}
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeIndentedWithTextInFront)
+{
+    const Document document("if (true) @\n"
+                            "    1+1;");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{", nextBlockIsIndented));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeIndentedOnEmptyLine1)
+{
+    const Document document("if (true)\n"
+                            "@\n"
+                            "    1+1;");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{", nextBlockIsIndented));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeIndentedOnEmptyLine2)
+{
+    const Document document("if (true)\n"
+                            "    @\n"
+                            "    1+1;");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{", nextBlockIsIndented));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotInTheMiddle)
+{
+    const Document document("if (true) @ true;");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeNamedNamespace)
+{
+    const Document document("namespace X @");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeNamedNamespaceWithAttributeSpecifier)
+{
+    const Document document("namespace [[xyz]] X @");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeUnnamedNamespace)
+{
+    const Document document("namespace @");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeUnnamedNamespaceWithAttributeSpecifier)
+{
+    const Document document("namespace [[xyz]] @");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
+
+TEST_F(MatchingText, ContextAllowsAutoParentheses_CurlyBrace_NotBeforeNestedNamespace)
+{
+    const Document document("namespace X::Y::Z @");
+
+    ASSERT_FALSE(MT::contextAllowsAutoParentheses(document.cursor, "{"));
+}
 
 } // anonymous