Improved error messages for malformed .import statements

Report errors in .import statements, rather than pass them through
to V8 to yield 'Syntax error'.

Task-number: QTBUG-24867
Change-Id: I111b3bd3d198e97f42b29591f61753e86295aeb2
Reviewed-by: Glenn Watson <glenn.watson@nokia.com>

src/qml/qml/qqmlscript.cpp

@@ -1476,8 +1476,10 @@ static inline bool isUriToken(int token)
     return false;
 }
 
-QQmlScript::Parser::JavaScriptMetaData QQmlScript::Parser::extractMetaData(QString &script)
+QQmlScript::Parser::JavaScriptMetaData QQmlScript::Parser::extractMetaData(QString &script, QQmlError *error)
 {
+    Q_ASSERT(error);
+
     JavaScriptMetaData rv;
 
     QQmlScript::Object::ScriptBlock::Pragmas &pragmas = rv.pragmas;
@@ -1499,6 +1501,10 @@ QQmlScript::Parser::JavaScriptMetaData QQmlScript::Parser::extractMetaData(QStri
         int startLine = l.tokenStartLine();
         int startColumn = l.tokenStartColumn();
 
+        QQmlError importError;
+        importError.setLine(startLine);
+        importError.setColumn(startColumn);
+
         token = l.lex();
 
         CHECK_LINE;
@@ -1516,32 +1522,46 @@ QQmlScript::Parser::JavaScriptMetaData QQmlScript::Parser::extractMetaData(QStri
 
                 QString file = l.tokenText();
 
-                if (!file.endsWith(js))
+                if (!file.endsWith(js)) {
+                    importError.setDescription(QCoreApplication::translate("QQmlParser","Imported file must be a script"));
+                    *error = importError;
                     return rv;
+                }
+
+                bool invalidImport = false;
 
                 token = l.lex();
 
-                CHECK_TOKEN(T_AS);
-                CHECK_LINE;
+                if ((token != QQmlJSGrammar::T_AS) || (l.tokenStartLine() != startLine)) {
+                    invalidImport = true;
+                } else {
+                    token = l.lex();
 
-                token = l.lex();
+                    if ((token != QQmlJSGrammar::T_IDENTIFIER) || (l.tokenStartLine() != startLine))
+                        invalidImport = true;
+                }
 
-                CHECK_TOKEN(T_IDENTIFIER);
-                CHECK_LINE;
+
+                if (invalidImport) {
+                    importError.setDescription(QCoreApplication::translate("QQmlParser","File import requires a qualifier"));
+                    *error = importError;
+                    return rv;
+                }
 
                 int endOffset = l.tokenLength() + l.tokenOffset();
 
                 QString importId = script.mid(l.tokenOffset(), l.tokenLength());
 
-                if (!importId.at(0).isUpper())
-                    return rv;
-
                 QQmlScript::LocationSpan location =
                     locationFromLexer(l, startLine, startColumn, startOffset);
 
                 token = l.lex();
-                if (l.tokenStartLine() == startLine)
+
+                if (!importId.at(0).isUpper() || (l.tokenStartLine() == startLine)) {
+                    importError.setDescription(QCoreApplication::translate("QQmlParser","Invalid import qualifier"));
+                    *error = importError;
                     return rv;
+                }
 
                 replaceWithSpace(script, startOffset, endOffset - startOffset);
 
@@ -1557,8 +1577,11 @@ QQmlScript::Parser::JavaScriptMetaData QQmlScript::Parser::extractMetaData(QStri
                 QString uri;
 
                 while (true) {
-                    if (!isUriToken(token))
+                    if (!isUriToken(token)) {
+                        importError.setDescription(QCoreApplication::translate("QQmlParser","Invalid module URI"));
+                        *error = importError;
                         return rv;
+                    }
 
                     uri.append(l.tokenText());
 
@@ -1573,34 +1596,50 @@ QQmlScript::Parser::JavaScriptMetaData QQmlScript::Parser::extractMetaData(QStri
                     CHECK_LINE;
                 }
 
-                CHECK_TOKEN(T_NUMERIC_LITERAL);
+                if (token != QQmlJSGrammar::T_NUMERIC_LITERAL) {
+                    importError.setDescription(QCoreApplication::translate("QQmlParser","Module import requires a version"));
+                    *error = importError;
+                    return rv;
+                }
+
                 int vmaj, vmin;
                 ProcessAST::extractVersion(QStringRef(&script, l.tokenOffset(), l.tokenLength()),
                                            &vmaj, &vmin);
 
-                token = l.lex();
-
-                CHECK_TOKEN(T_AS);
-                CHECK_LINE;
+                bool invalidImport = false;
 
                 token = l.lex();
 
-                CHECK_TOKEN(T_IDENTIFIER);
-                CHECK_LINE;
+                if ((token != QQmlJSGrammar::T_AS) || (l.tokenStartLine() != startLine)) {
+                    invalidImport = true;
+                } else {
+                    token = l.lex();
+
+                    if ((token != QQmlJSGrammar::T_IDENTIFIER) || (l.tokenStartLine() != startLine))
+                        invalidImport = true;
+                }
+
+
+                if (invalidImport) {
+                    importError.setDescription(QCoreApplication::translate("QQmlParser","Module import requires a qualifier"));
+                    *error = importError;
+                    return rv;
+                }
 
                 int endOffset = l.tokenLength() + l.tokenOffset();
 
                 QString importId = script.mid(l.tokenOffset(), l.tokenLength());
 
-                if (!importId.at(0).isUpper())
-                    return rv;
-
                 QQmlScript::LocationSpan location =
                     locationFromLexer(l, startLine, startColumn, startOffset);
 
                 token = l.lex();
-                if (l.tokenStartLine() == startLine)
+
+                if (!importId.at(0).isUpper() || (l.tokenStartLine() == startLine)) {
+                    importError.setDescription(QCoreApplication::translate("QQmlParser","Invalid import qualifier"));
+                    *error = importError;
                     return rv;
+                }
 
                 replaceWithSpace(script, startOffset, endOffset - startOffset);
 

src/qml/qml/qqmlscript_p.h

@@ -490,7 +490,7 @@ public:
     };
 
     static QQmlScript::Object::ScriptBlock::Pragmas extractPragmas(QString &);
-    static JavaScriptMetaData extractMetaData(QString &);
+    static JavaScriptMetaData extractMetaData(QString &, QQmlError *error);
 
 
 // ### private:

src/qml/qml/qqmltypeloader.cpp

@@ -1969,8 +1969,17 @@ void QQmlScriptBlob::dataReceived(const Data &data)
 
     m_source = QString::fromUtf8(data.data(), data.size());
 
+    m_scriptData = new QQmlScriptData();
+    m_scriptData->url = finalUrl();
+    m_scriptData->urlString = finalUrlString();
+
+    QQmlError metaDataError;
     QQmlScript::Parser::JavaScriptMetaData metadata =
-        QQmlScript::Parser::extractMetaData(m_source);
+        QQmlScript::Parser::extractMetaData(m_source, &metaDataError);
+    if (metaDataError.isValid()) {
+        metaDataError.setUrl(finalUrl());
+        m_scriptData->setError(metaDataError);
+    }
 
     m_imports.setBaseUrl(finalUrl(), finalUrlString());
 
@@ -2052,9 +2061,7 @@ void QQmlScriptBlob::done()
         return;
 
     QQmlEngine *engine = typeLoader()->engine();
-    m_scriptData = new QQmlScriptData();
-    m_scriptData->url = finalUrl();
-    m_scriptData->urlString = finalUrlString();
+
     m_scriptData->importCache = new QQmlTypeNameCache();
 
     QSet<QString> ns;

src/qml/qml/qqmltypeloader_p.h

@@ -437,6 +437,10 @@ public:
     bool isInitialized() const { return hasEngine(); }
     void initialize(QQmlEngine *);
 
+    bool hasError() const { return m_error.isValid(); }
+    void setError(const QQmlError &error) { m_error = error; }
+    QQmlError error() const { return m_error; }
+
 protected:
     virtual void clear(); // From QQmlCleanup
 
@@ -448,6 +452,7 @@ private:
     QByteArray m_programSource;
     v8::Persistent<v8::Script> m_program;
     v8::Persistent<v8::Object> m_value;
+    QQmlError m_error;
 };
 
 class Q_AUTOTEST_EXPORT QQmlScriptBlob : public QQmlDataBlob

src/qml/qml/qqmlvme.cpp

@@ -1160,10 +1160,17 @@ v8::Persistent<v8::Object> QQmlVME::run(QQmlContextData *parentCtxt, QQmlScriptD
     if (script->m_loaded)
         return qPersistentNew<v8::Object>(script->m_value);
 
+    v8::Persistent<v8::Object> rv;
+
     Q_ASSERT(parentCtxt && parentCtxt->engine);
     QQmlEnginePrivate *ep = QQmlEnginePrivate::get(parentCtxt->engine);
     QV8Engine *v8engine = ep->v8engine();
 
+    if (script->hasError()) {
+        ep->warning(script->error());
+        return rv;
+    }
+
     bool shared = script->pragmas & QQmlScript::Object::ScriptBlock::Shared;
 
     QQmlContextData *effectiveCtxt = parentCtxt;
@@ -1222,8 +1229,6 @@ v8::Persistent<v8::Object> QQmlVME::run(QQmlContextData *parentCtxt, QQmlScriptD
         Q_ASSERT(try_catch.HasCaught());
     }
 
-    v8::Persistent<v8::Object> rv;
-    
     if (try_catch.HasCaught()) {
         v8::Local<v8::Message> message = try_catch.Message();
         if (!message.IsEmpty()) {

tests/auto/qml/qmlmin/tst_qmlmin.cpp

@@ -110,6 +110,14 @@ void tst_qmlmin::initTestCase()
     invalidFiles << "tests/auto/qml/qqmlecmascript/data/qtbug_22843.library.js";
     invalidFiles << "tests/auto/qml/qquickworkerscript/data/script_error_onLoad.js";
     invalidFiles << "tests/auto/qml/parserstress/tests/ecma_3/Unicode/regress-352044-02-n.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFileQualifier.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedImport.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModule.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleQualifier.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleVersion.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/missingFileQualifier.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/missingModuleQualifier.js";
+    invalidFiles << "tests/auto/qml/qqmlecmascript/data/jsimportfail/missingModuleVersion.js";
 }
 
 QStringList tst_qmlmin::findFiles(const QDir &d)

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFile.js

@@ -0,0 +1,3 @@
+.import "other.qml" as Other
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFile.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedFile.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFileQualifier.2.js

@@ -0,0 +1,3 @@
+.import "other.js" as other
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFileQualifier.2.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedFileQualifier.2.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFileQualifier.js

@@ -0,0 +1,3 @@
+.import "other.js" Other
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedFileQualifier.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedFileQualifier.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedImport.js

@@ -0,0 +1,3 @@
+.impooooort QtQuick 2.0 as QQ
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedImport.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedImport.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModule.js

@@ -0,0 +1,3 @@
+.import QtQuick.++ 2.0 as QQ
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModule.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedModule.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleQualifier.2.js

@@ -0,0 +1,3 @@
+.import QtQuick 2.0 as qq
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleQualifier.2.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedModuleQualifier.2.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleQualifier.js

@@ -0,0 +1,3 @@
+.import QtQuick 2.0 QQ
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleQualifier.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedModuleQualifier.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleVersion.js

@@ -0,0 +1,3 @@
+.import QtQuick latest as QQ
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/malformedModuleVersion.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "malformedModuleVersion.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/missingFileQualifier.js

@@ -0,0 +1,3 @@
+.import "other.js"
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/missingFileQualifier.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "missingFileQualifier.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/missingModuleQualifier.js

@@ -0,0 +1,3 @@
+.import QtQuick 2.0
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/missingModuleQualifier.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "missingModuleQualifier.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/data/jsimportfail/missingModuleVersion.js

@@ -0,0 +1,3 @@
+.import QtQuick as QQ
+
+function foo() { return 'bar' }

tests/auto/qml/qqmlecmascript/data/jsimportfail/missingModuleVersion.qml

@@ -0,0 +1,5 @@
+import QtQuick 2.0
+import "missingModuleVersion.js" as JS
+
+Item {
+}

tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp

@@ -3783,6 +3783,83 @@ void tst_qqmlecmascript::importScripts_data()
             << (QVariantList() << QVariant(QString("Hello"))
                                << QVariant(QString("Hello"))
                                << QVariant(QString("Hello")));
+
+    QTest::newRow("malformed import statement")
+            << testFileUrl("jsimportfail/malformedImport.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedImport.js").toString() + QLatin1String(":1: SyntaxError: Unexpected token ."))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed file name")
+            << testFileUrl("jsimportfail/malformedFile.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedFile.js").toString() + QLatin1String(":0:1: Imported file must be a script"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("missing file qualifier")
+            << testFileUrl("jsimportfail/missingFileQualifier.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/missingFileQualifier.js").toString() + QLatin1String(":0:1: File import requires a qualifier"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed file qualifier")
+            << testFileUrl("jsimportfail/malformedFileQualifier.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedFileQualifier.js").toString() + QLatin1String(":0:1: File import requires a qualifier"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed module qualifier 2")
+            << testFileUrl("jsimportfail/malformedFileQualifier.2.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedFileQualifier.2.js").toString() + QLatin1String(":0:1: Invalid import qualifier"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed module uri")
+            << testFileUrl("jsimportfail/malformedModule.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedModule.js").toString() + QLatin1String(":0:1: Invalid module URI"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("missing module version")
+            << testFileUrl("jsimportfail/missingModuleVersion.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/missingModuleVersion.js").toString() + QLatin1String(":0:1: Module import requires a version"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed module version")
+            << testFileUrl("jsimportfail/malformedModuleVersion.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedModuleVersion.js").toString() + QLatin1String(":0:1: Module import requires a version"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("missing module qualifier")
+            << testFileUrl("jsimportfail/missingModuleQualifier.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/missingModuleQualifier.js").toString() + QLatin1String(":0:1: Module import requires a qualifier"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed module qualifier")
+            << testFileUrl("jsimportfail/malformedModuleQualifier.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedModuleQualifier.js").toString() + QLatin1String(":0:1: Module import requires a qualifier"))
+            << QStringList()
+            << QVariantList();
+
+    QTest::newRow("malformed module qualifier 2")
+            << testFileUrl("jsimportfail/malformedModuleQualifier.2.qml")
+            << QString()
+            << (QStringList() << testFileUrl("jsimportfail/malformedModuleQualifier.2.js").toString() + QLatin1String(":0:1: Invalid import qualifier"))
+            << QStringList()
+            << QVariantList();
 }
 
 void tst_qqmlecmascript::importScripts()