qqmljs.g: insert empty identifiers when missing

For the completion in qmlls, users usually expect to see a list of
completions after typing in a T_DOT ("."). Sadly, this T_DOT usually
makes the QML code invalid or ambiguous, such that the parser aborts
parsing.

For qmlls, this is quite bad: no completions can be proposed if the AST
cannot be computed. Therefore, this commit tries to make the parser a
little bit more resistant to missing T_IDENTIFIER behind T_DOT.

Add a yyprevtoken field in the parser to keep track of what was the
last successfully parsed token, and update it with yytoken before yytoken
changes.

Extract the pushTokenWithEmptyLocation() logic from the automatic
semicolon inserting code, and reuse it for the automatic insertion of
identifiers after dots.

Add some tests in tst_qmlls_utils and adapt the qmlls completion code to
work with missing right hand sides (RHS) of dotted expression
`a.b`. Create a new file missingRHS.qml because yyy.qml does not seem to
stop growing.

The fix of this commit does not take care of all possible cases: when
T_DOT is followed by an T_IDENTIFIER, then no T_IDENTIFIER is inserted
even if the parsing fails afterwards. This happens when a JS
statement is behind a T_DOT without identifier, for example. Add tests
for that too, QEXPECT_FAIL them and put them in a separate file
missingRHS.parserfail.qml. They need to be in a separate file because no
completions can be obtained when the parser fails, and that affects all
the completions of the entire file. This special file
missingRHS.parserfail.qml also needs to be ignored by tst_qmlformat.

Task-number: QTBUG-115836
Change-Id: If307430131a7df25ae9bd4ea0393d47c0641c8d3
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

6 files changed, 101 insertions, 30 deletions

diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g
index 80225a87dc..6d9d11800c 100644
--- a/src/qml/parser/qqmljs.g
+++ b/src/qml/parser/qqmljs.g
@@ -328,6 +328,7 @@ protected:
     AST::UiQualifiedId *reparseAsQualifiedId(AST::ExpressionNode *expr);
 
     void pushToken(int token);
+    void pushTokenWithEmptyLocation(int token);
     int lookaheadToken(Lexer *lexer);
 
     static DiagnosticMessage compileError(const SourceLocation &location,
@@ -379,6 +380,7 @@ protected:
     QStringView yytokenraw;
     SourceLocation yylloc;
     SourceLocation yyprevlloc;
+    int yyprevtoken = -1;
 
     SavedToken token_buffer[TOKEN_BUFFER_SIZE];
     SavedToken *first_token = nullptr;
@@ -512,9 +514,19 @@ void Parser::pushToken(int token)
     yytoken = token;
 }
 
+void Parser::pushTokenWithEmptyLocation(int token)
+{
+    pushToken(token);
+    yylloc = yyprevlloc;
+    yylloc.offset += yylloc.length;
+    yylloc.startColumn += yylloc.length;
+    yylloc.length = 0;
+}
+
 int Parser::lookaheadToken(Lexer *lexer)
 {
     if (yytoken < 0) {
+        yyprevtoken = yytoken;
         yytoken = lexer->lex();
         yylval = lexer->tokenValue();
         yytokenspell = lexer->tokenSpell();
@@ -610,6 +622,7 @@ bool Parser::parse(int startToken)
 #endif
         if (action > 0) {
             if (action != ACCEPT_STATE) {
+                yyprevtoken = yytoken;
                 yytoken = -1;
                 sym(1).dval = yylval;
                 stringRef(1) = yytokenspell;
@@ -4790,35 +4803,26 @@ ExportSpecifier: IdentifierName T_AS IdentifierName;
     if (first_token == last_token) {
         const int errorState = state_stack[tos];
 
+        // automatic insertion of missing identifiers after dots
+        if (yytoken != -1 && m_enableIdentifierInsertion && t_action(errorState, T_IDENTIFIER) && yyprevtoken == T_DOT) {
+#ifdef PARSER_DEBUG
+            qDebug() << "Inserting missing identifier between" << spell[yyprevtoken] << "and"
+                     << spell[yytoken];
+#endif
+            pushTokenWithEmptyLocation(T_IDENTIFIER);
+            action = errorState;
+            goto _Lcheck_token;
+        }
+
+
         // automatic insertion of `;'
         if (yytoken != -1 && ((t_action(errorState, T_AUTOMATIC_SEMICOLON) && lexer->canInsertAutomaticSemicolon(yytoken))
                               || t_action(errorState, T_COMPATIBILITY_SEMICOLON))) {
 #ifdef PARSER_DEBUG
             qDebug() << "Inserting automatic semicolon.";
 #endif
-            SavedToken &tk = token_buffer[0];
-            tk.token = yytoken;
-            tk.dval = yylval;
-            tk.spell = yytokenspell;
-            tk.raw = yytokenraw;
-            tk.loc = yylloc;
-
-            yylloc = yyprevlloc;
-            yylloc.offset += yylloc.length;
-            yylloc.startColumn += yylloc.length;
-            yylloc.length = 0;
-
-            //const QString msg = QCoreApplication::translate("QQmlParser", "Missing `;'");
-            //diagnostic_messages.append(compileError(yyloc, msg, QtWarningMsg));
-
-            first_token = &token_buffer[0];
-            last_token = &token_buffer[1];
-
-            yytoken = T_SEMICOLON;
-            yylval = 0;
-
+            pushTokenWithEmptyLocation(T_SEMICOLON);
             action = errorState;
-
             goto _Lcheck_token;
         }
 
diff --git a/src/qmldom/qqmldomastcreator.cpp b/src/qmldom/qqmldomastcreator.cpp
index b51ac47a16..c348a460f1 100644
--- a/src/qmldom/qqmldomastcreator.cpp
+++ b/src/qmldom/qqmldomastcreator.cpp
@@ -1707,12 +1707,10 @@ void QQmlDomAstCreator::endVisit(AST::FieldMemberExpression *expression)
         removeCurrentScriptNode({});
     }
 
-    if (!expression->name.empty()) {
-        auto scriptIdentifier =
-                std::make_shared<ScriptElements::IdentifierExpression>(expression->identifierToken);
-        scriptIdentifier->setName(expression->name);
-        current->setRight(ScriptElementVariant::fromElement(scriptIdentifier));
-    }
+    auto scriptIdentifier =
+            std::make_shared<ScriptElements::IdentifierExpression>(expression->identifierToken);
+    scriptIdentifier->setName(expression->name);
+    current->setRight(ScriptElementVariant::fromElement(scriptIdentifier));
 
     pushScriptElement(current);
 }
diff --git a/tests/auto/qml/qmlformat/tst_qmlformat.cpp b/tests/auto/qml/qmlformat/tst_qmlformat.cpp
index 42eb4b880f..dbfbb84289 100644
--- a/tests/auto/qml/qmlformat/tst_qmlformat.cpp
+++ b/tests/auto/qml/qmlformat/tst_qmlformat.cpp
@@ -132,6 +132,8 @@ void TestQmlformat::initTestCase()
     m_invalidFiles << "tests/auto/qml/qqmllanguage/data/typeAnnotations.2.qml";
     m_invalidFiles << "tests/auto/qml/qqmlparser/data/disallowedtypeannotations/qmlnestedfunction.qml";
     m_invalidFiles << "tests/auto/qmlls/utils/data/emptyFile.qml";
+    m_invalidFiles << "tests/auto/qmlls/utils/data/completions/missingRHS.qml";
+    m_invalidFiles << "tests/auto/qmlls/utils/data/completions/missingRHS.parserfail.qml";
 
     // Files that get changed:
     // rewrite of import "bla/bla/.." to import "bla"
diff --git a/tests/auto/qmlls/utils/data/completions/missingRHS.parserfail.qml b/tests/auto/qmlls/utils/data/completions/missingRHS.parserfail.qml
new file mode 100644
index 0000000000..ce594d9f16
--- /dev/null
+++ b/tests/auto/qmlls/utils/data/completions/missingRHS.parserfail.qml
@@ -0,0 +1,9 @@
+import QtQuick
+
+Item {
+    function f() {
+        let x = root.
+        let y = root.
+        for(;;) {}
+    }
+}
diff --git a/tests/auto/qmlls/utils/data/completions/missingRHS.qml b/tests/auto/qmlls/utils/data/completions/missingRHS.qml
new file mode 100644
index 0000000000..3f55ecd993
--- /dev/null
+++ b/tests/auto/qmlls/utils/data/completions/missingRHS.qml
@@ -0,0 +1,19 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+ import QtQuick
+
+ Item {
+    id: root
+    property int good
+    Item {
+        property int bad
+        function f() {
+            return root.
+        }
+        property int boom: root.
+        Item {
+            property int helloSubItem
+        }
+    }
+}
diff --git a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
index 155481fe64..6d281b4de4 100644
--- a/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
+++ b/tests/auto/qmlls/utils/tst_qmlls_utils.cpp
@@ -2519,7 +2519,7 @@ void tst_qmlls_utils::completions_data()
             << QStringList{ propertyCompletion }
             << None;
 
-    QTest::newRow("partialBinaryExpressionRHS") << file << 138 << 17
+    QTest::newRow("binaryExpressionRHS") << file << 138 << 17
                                  << ExpectedCompletions{
                                         { u"log"_s, CompletionItemKind::Method },
                                         { u"error"_s, CompletionItemKind::Method },
@@ -2529,7 +2529,7 @@ void tst_qmlls_utils::completions_data()
                                                  u"height"_s,        u"layer"_s,
                                                  u"left"_s }
                                  << None;
-    QTest::newRow("partialBinaryExpressionLHS") << file << 138 << 12
+    QTest::newRow("binaryExpressionLHS") << file << 138 << 12
                                  << ExpectedCompletions{
                                         { u"qualifiedScriptIdentifiers"_s, CompletionItemKind::Method },
                                         { u"width"_s, CompletionItemKind::Property },
@@ -2537,6 +2537,38 @@ void tst_qmlls_utils::completions_data()
                                     }
                                  << QStringList{ u"log"_s, u"error"_s}
                                  << None;
+
+    const QString missingRHSFile = testFile(u"completions/missingRHS.qml"_s);
+    QTest::newRow("binaryExpressionMissingRHS") << missingRHSFile << 12 << 25
+                                 << ExpectedCompletions{
+                                        { u"good"_s, CompletionItemKind::Property },
+                                    }
+                                 << QStringList{ propertyCompletion, u"bad"_s }
+                                 << None;
+    QTest::newRow("binaryExpressionMissingRHSWithDefaultProperty") << missingRHSFile << 14 << 33
+                                 << ExpectedCompletions{
+                                        { u"good"_s, CompletionItemKind::Property },
+                                    }
+                                 << QStringList{ propertyCompletion, u"bad"_s, u"helloSubItem"_s }
+                                 << None;
+
+    QTest::newRow("binaryExpressionMissingRHSWithSemicolon")
+            << testFile(u"completions/missingRHS.parserfail.qml"_s)
+                                                             << 5 << 22
+                                 << ExpectedCompletions{
+                                        { u"good"_s, CompletionItemKind::Property },
+                                    }
+                                 << QStringList{ propertyCompletion, u"bad"_s, u"helloSubItem"_s }
+                                 << None;
+
+    QTest::newRow("binaryExpressionMissingRHSWithStatement") <<
+            testFile(u"completions/missingRHS.parserfail.qml"_s)
+                                                             << 6 << 22
+                                 << ExpectedCompletions{
+                                        { u"good"_s, CompletionItemKind::Property },
+                                    }
+                                 << QStringList{ propertyCompletion, u"bad"_s, u"helloSubItem"_s }
+                                 << None;
 }
 
 void tst_qmlls_utils::completions()
@@ -2558,6 +2590,10 @@ void tst_qmlls_utils::completions()
     auto locations = QQmlLSUtils::itemsFromTextLocation(
             file.field(QQmlJS::Dom::Fields::currentItem), line - 1, character - 1);
 
+    QEXPECT_FAIL("binaryExpressionMissingRHSWithSemicolon",
+                 "Current parser cannot recover from this error yet!", Abort);
+    QEXPECT_FAIL("binaryExpressionMissingRHSWithStatement",
+                 "Current parser cannot recover from this error yet!", Abort);
     QCOMPARE(locations.size(), 1);
 
     QString code;
@@ -2647,6 +2683,9 @@ void tst_qmlls_utils::completions()
         QEXPECT_FAIL("inMethodBody", "Completion for JS Statement/keywords not implemented yet",
                      Abort);
         QEXPECT_FAIL("letStatementAfterEqual", "Completion not implemented yet!", Abort);
+        QEXPECT_FAIL("binaryExpressionMissingRHSWithDefaultProperty",
+                     "Current parser cannot recover from this error yet!", Abort);
+
         QVERIFY2(labels.contains(exp.label),
                  u"no %1 in %2"_s
                          .arg(exp.label, QStringList(labels.begin(), labels.end()).join(u", "_s))