QML: Consider the semicolon as part of expression statements

When asked for lastSourceLocation() we should always return the
semicolon token. In order for that to work, the semicolon token needs to
be valid in all cases. In the case of object literals as expressions for
properties we neither accepted nor synthesized a semicolon as delimiter.
Add an optional semicolon that we can then also use as end of the
expression statement.

Furthermore, this triggered a silent rule conflict for ImportSpecifier, which
for some reason did not arise before:
IdentifierReference could resolve to both ImpordBinding and IdentifierName,
causing ambiguity in the grammar, and ultimately caused parse failues
when parsing an import statement.
This is now resolved by explicitly telling the parser to prefer
shifting.

Initial-patch-by: Ulf Hermann <ulf.hermann@qt.io>
Change-Id: Iaec29c452b577312248a17cb48f005f4fc0bd8c4
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

3 files changed, 62 insertions, 5 deletions

diff --git a/src/qml/parser/qqmljs.g b/src/qml/parser/qqmljs.g
index e28899883f..f7b528ce13 100644
--- a/src/qml/parser/qqmljs.g
+++ b/src/qml/parser/qqmljs.g
@@ -125,6 +125,7 @@
 %nonassoc T_IDENTIFIER T_COLON T_SIGNAL T_PROPERTY T_READONLY T_ON T_SET T_GET T_OF T_STATIC T_FROM T_AS T_REQUIRED
 %nonassoc REDUCE_HERE
 %right T_THEN T_ELSE
+%right T_WITHOUTAS T_AS
 
 %start TopLevel
 
@@ -1011,15 +1012,27 @@ UiObjectMember: UiQualifiedId T_ON UiQualifiedId  UiObjectInitializer;
 ./
 
 
-UiObjectLiteral: T_LBRACE ExpressionStatementLookahead UiPropertyDefinitionList T_RBRACE;
-/.  case $rule_number: Q_FALLTHROUGH(); ./
-UiObjectLiteral: T_LBRACE ExpressionStatementLookahead UiPropertyDefinitionList T_COMMA T_RBRACE;
+UiObjectLiteral: T_LBRACE ExpressionStatementLookahead UiPropertyDefinitionList T_RBRACE Semicolon;
 /.
     case $rule_number: {
         AST::ObjectPattern *l = new (pool) AST::ObjectPattern(sym(3).PatternPropertyList->finish());
         l->lbraceToken = loc(1);
         l->rbraceToken = loc(4);
         AST::ExpressionStatement *node = new (pool) AST::ExpressionStatement(l);
+        node->semicolonToken = loc(5);
+        sym(1).Node = node;
+    } break;
+./
+
+
+UiObjectLiteral: T_LBRACE ExpressionStatementLookahead UiPropertyDefinitionList T_COMMA T_RBRACE Semicolon;
+/.
+    case $rule_number: {
+        AST::ObjectPattern *l = new (pool) AST::ObjectPattern(sym(3).PatternPropertyList->finish());
+        l->lbraceToken = loc(1);
+        l->rbraceToken = loc(5);
+        AST::ExpressionStatement *node = new (pool) AST::ExpressionStatement(l);
+        node->semicolonToken = loc(6);
         sym(1).Node = node;
     } break;
 ./
@@ -4356,7 +4369,10 @@ ImportsList: ImportsList T_COMMA ImportSpecifier;
     } break;
 ./
 
-ImportSpecifier: ImportedBinding;
+-- When enconutering an IdentifierReference it can resolve to both ImportedBinding and IdentifierName
+-- Using %right and %prec, we tell qlalr that it should not reduce immediately, but rather shift
+-- so that we have a chance of actually parsing the correct rule if there is an "as" identifier
+ImportSpecifier: ImportedBinding %prec T_WITHOUTAS;
 /.
     case $rule_number: {
         auto importSpecifier = new (pool) AST::ImportSpecifier(stringRef(1));
diff --git a/src/qml/parser/qqmljsast_p.h b/src/qml/parser/qqmljsast_p.h
index e436c4673d..fcf9933e67 100644
--- a/src/qml/parser/qqmljsast_p.h
+++ b/src/qml/parser/qqmljsast_p.h
@@ -1782,7 +1782,7 @@ public:
     { return expression->firstSourceLocation(); }
 
     SourceLocation lastSourceLocation() const override
-    { return expression->lastSourceLocation(); }
+    { return semicolonToken; }
 
 // attributes
     ExpressionNode *expression;
diff --git a/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp b/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp
index 9d8818d01e..4ba6a709df 100644
--- a/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp
+++ b/tests/auto/qml/qqmlparser/tst_qqmlparser.cpp
@@ -62,6 +62,7 @@ private slots:
     void typeAnnotations();
     void disallowedTypeAnnotations_data();
     void disallowedTypeAnnotations();
+    void semicolonPartOfExpressionStatement();
 
 private:
     QStringList excludedDirs;
@@ -141,6 +142,30 @@ struct TypeAnnotationObserver: public AST::Visitor
     }
 };
 
+struct ExpressionStatementObserver: public AST::Visitor
+{
+    int expressionsSeen = 0;
+    bool endsWithSemicolon = true;
+
+    void operator()(AST::Node *node)
+    {
+        AST::Node::accept(node, this);
+    }
+
+    virtual bool visit(AST::ExpressionStatement *statement)
+    {
+        ++expressionsSeen;
+        endsWithSemicolon = endsWithSemicolon
+                && (statement->lastSourceLocation().end() == statement->semicolonToken.end());
+        return true;
+    }
+
+    void throwRecursionDepthError() final
+    {
+        QFAIL("Maximum statement or expression depth exceeded");
+    }
+};
+
 }
 
 tst_qqmlparser::tst_qqmlparser()
@@ -438,6 +463,22 @@ void tst_qqmlparser::disallowedTypeAnnotations()
     QVERIFY2(parser.errorMessage().startsWith("Type annotations are not permitted "), qPrintable(parser.errorMessage()));
 }
 
+void tst_qqmlparser::semicolonPartOfExpressionStatement()
+{
+    QQmlJS::Engine engine;
+    QQmlJS::Lexer lexer(&engine);
+    lexer.setCode(QLatin1String("A { property int x: 1+1; property int y: 2+2 \n"
+                                "tt: {'a': 5, 'b': 6}; ff: {'c': 'rrr'}}"), 1);
+    QQmlJS::Parser parser(&engine);
+    QVERIFY(parser.parse());
+
+    check::ExpressionStatementObserver observer;
+    observer(parser.rootNode());
+
+    QCOMPARE(observer.expressionsSeen, 4);
+    QVERIFY(observer.endsWithSemicolon);
+}
+
 QTEST_MAIN(tst_qqmlparser)
 
 #include "tst_qqmlparser.moc"