Properly signal errors when accessing lowercase enum values

Task-number: QTBUG-46758
Change-Id: I14e394021c231bda5552c8d1c98f20c903a62f12
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

12 files changed, 167 insertions, 27 deletions

diff --git a/src/qml/compiler/qqmltypecompiler.cpp b/src/qml/compiler/qqmltypecompiler.cpp
index 6fd15d1eb8..fcc0ca8d14 100644
--- a/src/qml/compiler/qqmltypecompiler.cpp
+++ b/src/qml/compiler/qqmltypecompiler.cpp
@@ -1157,6 +1157,17 @@ struct StaticQtMetaObject : public QObject
         { return &staticQtMetaObject; }
 };
 
+bool QQmlEnumTypeResolver::assignEnumToBinding(QmlIR::Binding *binding, const QString &enumName, int enumValue, bool isQtObject)
+{
+    if (enumName.length() > 0 && enumName[0].isLower() && !isQtObject) {
+        COMPILE_EXCEPTION(binding, tr("Invalid property assignment: Enum value \"%1\" cannot start with a lowercase letter").arg(enumName));
+    }
+    binding->type = QV4::CompiledData::Binding::Type_Number;
+    binding->value.d = (double)enumValue;
+    binding->flags |= QV4::CompiledData::Binding::IsResolvedEnum;
+    return true;
+}
+
 bool QQmlEnumTypeResolver::tryQualifiedEnumAssignment(const QmlIR::Object *obj, const QQmlPropertyCache *propertyCache, const QQmlPropertyData *prop, QmlIR::Binding *binding)
 {
     bool isIntProp = (prop->propType == QMetaType::Int) && !prop->isEnum();
@@ -1179,6 +1190,7 @@ bool QQmlEnumTypeResolver::tryQualifiedEnumAssignment(const QmlIR::Object *obj,
         return true;
 
     QHashedStringRef typeName(string.constData(), dot);
+    const bool isQtObject = (typeName == QLatin1String("Qt"));
     QString enumValue = string.mid(dot+1);
 
     if (isIntProp) {
@@ -1186,16 +1198,15 @@ bool QQmlEnumTypeResolver::tryQualifiedEnumAssignment(const QmlIR::Object *obj,
         bool ok;
         int enumval = evaluateEnum(typeName.toString(), enumValue.toUtf8(), &ok);
         if (ok) {
-            binding->type = QV4::CompiledData::Binding::Type_Number;
-            binding->value.d = (double)enumval;
-            binding->flags |= QV4::CompiledData::Binding::IsResolvedEnum;
+            if (!assignEnumToBinding(binding, enumValue, enumval, isQtObject))
+                return false;
         }
         return true;
     }
     QQmlType *type = 0;
     imports->resolveType(typeName, &type, 0, 0, 0);
 
-    if (!type && typeName != QLatin1String("Qt"))
+    if (!type && !isQtObject)
         return true;
 
     int value = 0;
@@ -1228,10 +1239,7 @@ bool QQmlEnumTypeResolver::tryQualifiedEnumAssignment(const QmlIR::Object *obj,
     if (!ok)
         return true;
 
-    binding->type = QV4::CompiledData::Binding::Type_Number;
-    binding->value.d = (double)value;
-    binding->flags |= QV4::CompiledData::Binding::IsResolvedEnum;
-    return true;
+    return assignEnumToBinding(binding, enumValue, value, isQtObject);
 }
 
 int QQmlEnumTypeResolver::evaluateEnum(const QString &scope, const QByteArray &enumValue, bool *ok) const
diff --git a/src/qml/compiler/qqmltypecompiler_p.h b/src/qml/compiler/qqmltypecompiler_p.h
index c5be92d256..091007bf25 100644
--- a/src/qml/compiler/qqmltypecompiler_p.h
+++ b/src/qml/compiler/qqmltypecompiler_p.h
@@ -190,6 +190,7 @@ public:
     bool resolveEnumBindings();
 
 private:
+    bool assignEnumToBinding(QmlIR::Binding *binding, const QString &enumName, int enumValue, bool isQtObject);
     bool tryQualifiedEnumAssignment(const QmlIR::Object *obj, const QQmlPropertyCache *propertyCache,
                                     const QQmlPropertyData *prop,
                                     QmlIR::Binding *binding);
diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp
index 6c29f2fbb5..7ae8332ec4 100644
--- a/src/qml/qml/qqmltypewrapper.cpp
+++ b/src/qml/qml/qqmltypewrapper.cpp
@@ -119,6 +119,28 @@ ReturnedValue QmlTypeWrapper::create(QV4::ExecutionEngine *engine, QObject *o, Q
     return w.asReturnedValue();
 }
 
+static int enumForSingleton(String *name, QObject *qobjectSingleton)
+{
+    // ### Optimize
+    QByteArray enumName = name->toQString().toUtf8();
+    const QMetaObject *metaObject = qobjectSingleton->metaObject();
+    for (int ii = metaObject->enumeratorCount() - 1; ii >= 0; --ii) {
+        QMetaEnum e = metaObject->enumerator(ii);
+        bool ok;
+        int value = e.keyToValue(enumName.constData(), &ok);
+        if (ok)
+            return value;
+    }
+    return -1;
+}
+
+static ReturnedValue throwLowercaseEnumError(QV4::ExecutionEngine *v4, String *name, QQmlType *type)
+{
+    const QString message =
+            QStringLiteral("Cannot access enum value '%1' of '%2', enum values need to start with an uppercase letter.")
+                .arg(name->toQString()).arg(QLatin1String(type->typeName()));
+    return v4->throwTypeError(message);
+}
 
 ReturnedValue QmlTypeWrapper::get(const Managed *m, String *name, bool *hasProperty)
 {
@@ -135,9 +157,9 @@ ReturnedValue QmlTypeWrapper::get(const Managed *m, String *name, bool *hasPrope
     QQmlContextData *context = v4->callingQmlContext();
 
     QObject *object = w->d()->object;
+    QQmlType *type = w->d()->type;
 
-    if (w->d()->type) {
-        QQmlType *type = w->d()->type;
+    if (type) {
 
         // singleton types are handled differently to other types.
         if (type->isSingleton()) {
@@ -147,24 +169,29 @@ ReturnedValue QmlTypeWrapper::get(const Managed *m, String *name, bool *hasPrope
 
             QObject *qobjectSingleton = siinfo->qobjectApi(e);
             if (qobjectSingleton) {
+
                 // check for enum value
-                if (name->startsWithUpper()) {
-                    if (w->d()->mode == Heap::QmlTypeWrapper::IncludeEnums) {
-                        // ### Optimize
-                        QByteArray enumName = name->toQString().toUtf8();
-                        const QMetaObject *metaObject = qobjectSingleton->metaObject();
-                        for (int ii = metaObject->enumeratorCount() - 1; ii >= 0; --ii) {
-                            QMetaEnum e = metaObject->enumerator(ii);
-                            bool ok;
-                            int value = e.keyToValue(enumName.constData(), &ok);
-                            if (ok)
-                                return QV4::Primitive::fromInt32(value).asReturnedValue();
-                        }
-                    }
+                const bool includeEnums = w->d()->mode == Heap::QmlTypeWrapper::IncludeEnums;
+                if (includeEnums && name->startsWithUpper()) {
+                    const int value = enumForSingleton(name, qobjectSingleton);
+                    if (value != -1)
+                        return QV4::Primitive::fromInt32(value).asReturnedValue();
                 }
 
                 // check for property.
-                return QV4::QObjectWrapper::getQmlProperty(v4, context, qobjectSingleton, name, QV4::QObjectWrapper::IgnoreRevision, hasProperty);
+                bool ok;
+                const ReturnedValue result = QV4::QObjectWrapper::getQmlProperty(v4, context, qobjectSingleton, name, QV4::QObjectWrapper::IgnoreRevision, &ok);
+                if (hasProperty)
+                    *hasProperty = ok;
+
+                // Warn when attempting to access a lowercased enum value, singleton case
+                if (!ok && includeEnums && !name->startsWithUpper()) {
+                    const int value = enumForSingleton(name, qobjectSingleton);
+                    if (value != -1)
+                        return throwLowercaseEnumError(v4, name, type);
+                }
+
+                return result;
             } else if (!siinfo->scriptApi(e).isUndefined()) {
                 // NOTE: if used in a binding, changes will not trigger re-evaluation since non-NOTIFYable.
                 QV4::ScopedObject o(scope, QJSValuePrivate::convertedToValue(v4, siinfo->scriptApi(e)));
@@ -221,9 +248,20 @@ ReturnedValue QmlTypeWrapper::get(const Managed *m, String *name, bool *hasPrope
         Q_ASSERT(!"Unreachable");
     }
 
+    bool ok = false;
+    const ReturnedValue result = Object::get(m, name, &ok);
     if (hasProperty)
-        *hasProperty = false;
-    return Object::get(m, name, hasProperty);
+        *hasProperty = ok;
+
+    // Warn when attempting to access a lowercased enum value, non-singleton case
+    if (!ok && type && !type->isSingleton() && !name->startsWithUpper()) {
+        bool enumOk = false;
+        type->enumValue(QQmlEnginePrivate::get(v4->qmlEngine()), name, &enumOk);
+        if (enumOk)
+            return throwLowercaseEnumError(v4, name, type);
+    }
+
+    return result;
 }
 
 
diff --git a/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.1.errors.txt b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.1.errors.txt
new file mode 100644
index 0000000000..33360e96cf
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.1.errors.txt
@@ -0,0 +1 @@
+5:5:Invalid property assignment: Enum value "lowercaseEnumVal" cannot start with a lowercase letter
diff --git a/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.1.qml b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.1.qml
new file mode 100644
index 0000000000..f6c3e9b404
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.1.qml
@@ -0,0 +1,6 @@
+import QtQuick 2.0
+import Test 1.0
+
+MyTypeObject {
+    intProperty: MyTypeObject.lowercaseEnumVal
+}
diff --git a/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.2.errors.txt b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.2.errors.txt
new file mode 100644
index 0000000000..33360e96cf
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.2.errors.txt
@@ -0,0 +1 @@
+5:5:Invalid property assignment: Enum value "lowercaseEnumVal" cannot start with a lowercase letter
diff --git a/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.2.qml b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.2.qml
new file mode 100644
index 0000000000..0dfe26c71d
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/lowercaseEnumCompileTime.2.qml
@@ -0,0 +1,6 @@
+import QtQuick 2.0
+import Test 1.0
+
+MyTypeObject {
+    enumProperty: MyTypeObjectSingleton.lowercaseEnumVal
+}
diff --git a/tests/auto/qml/qqmllanguage/data/lowercaseEnumRuntime.1.qml b/tests/auto/qml/qqmllanguage/data/lowercaseEnumRuntime.1.qml
new file mode 100644
index 0000000000..866b49e1d5
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/lowercaseEnumRuntime.1.qml
@@ -0,0 +1,10 @@
+import QtQuick 2.0
+import Test 1.0
+
+MyTypeObject {
+    enumProperty: MyTypeObject.EnumVal1
+    Component.onCompleted: {
+        var a = MyTypeObject.EnumVal1;
+        var b = MyTypeObject.lowercaseEnumVal
+    }
+}
diff --git a/tests/auto/qml/qqmllanguage/data/lowercaseEnumRuntime.2.qml b/tests/auto/qml/qqmllanguage/data/lowercaseEnumRuntime.2.qml
new file mode 100644
index 0000000000..686977a11a
--- /dev/null
+++ b/tests/auto/qml/qqmllanguage/data/lowercaseEnumRuntime.2.qml
@@ -0,0 +1,10 @@
+import QtQuick 2.0
+import Test 1.0
+
+MyTypeObject {
+    intProperty: MyTypeObjectSingleton.EnumVal1
+    Component.onCompleted: {
+        var a = MyTypeObjectSingleton.EnumVal1;
+        var b = MyTypeObjectSingleton.lowercaseEnumVal;
+    }
+}
diff --git a/tests/auto/qml/qqmllanguage/testtypes.cpp b/tests/auto/qml/qqmllanguage/testtypes.cpp
index 95a98788c3..5a8190756d 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.cpp
+++ b/tests/auto/qml/qqmllanguage/testtypes.cpp
@@ -34,6 +34,14 @@
 
 #include <private/qqmlcompiler_p.h>
 
+static QObject *myTypeObjectSingleton(QQmlEngine *engine, QJSEngine *scriptEngine)
+{
+    Q_UNUSED(engine)
+    Q_UNUSED(scriptEngine)
+
+    return new MyTypeObject();
+}
+
 void registerTypes()
 {
     qmlRegisterInterface<MyInterface>("MyInterface");
@@ -93,6 +101,8 @@ void registerTypes()
     qmlRegisterType<RootObjectInCreationTester>("Test", 1, 0, "RootObjectInCreationTester");
 
     qmlRegisterType<MyCompositeBaseType>("Test", 1, 0, "MyCompositeBaseType");
+
+    qmlRegisterSingletonType<MyTypeObjectSingleton>("Test", 1, 0, "MyTypeObjectSingleton", myTypeObjectSingleton);
 }
 
 QVariant myCustomVariantTypeConverter(const QString &data)
diff --git a/tests/auto/qml/qqmllanguage/testtypes.h b/tests/auto/qml/qqmllanguage/testtypes.h
index c64fda5ea1..c6c956cf36 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.h
+++ b/tests/auto/qml/qqmllanguage/testtypes.h
@@ -297,7 +297,7 @@ public:
         emit flagPropertyChanged();
     }
 
-    enum MyEnum { EnumVal1, EnumVal2 };
+    enum MyEnum { EnumVal1, EnumVal2, lowercaseEnumVal };
     MyEnum enumPropertyValue;
     MyEnum enumProperty() const {
         return enumPropertyValue;
@@ -597,6 +597,12 @@ signals:
 };
 Q_DECLARE_OPERATORS_FOR_FLAGS(MyTypeObject::MyFlags)
 
+// FIXME: If no subclass is used for the singleton registration with qmlRegisterSingletonType(),
+//        the valueTypes() test will fail.
+class MyTypeObjectSingleton : public MyTypeObject
+{
+    Q_OBJECT
+};
 
 class MyContainer : public QObject
 {
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 71f206ed8f..f66caa31f1 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -203,6 +203,10 @@ private slots:
     void crash2();
 
     void globalEnums();
+    void lowercaseEnumRuntime_data();
+    void lowercaseEnumRuntime();
+    void lowercaseEnumCompileTime_data();
+    void lowercaseEnumCompileTime();
     void literals_data();
     void literals();
 
@@ -3502,6 +3506,45 @@ void tst_qqmllanguage::globalEnums()
     delete o;
 }
 
+void tst_qqmllanguage::lowercaseEnumRuntime_data()
+{
+    QTest::addColumn<QString>("file");
+    QTest::addColumn<QString>("errorMessage");
+
+    QTest::newRow("enum from normal type") << "lowercaseEnumRuntime.1.qml" << ":8: TypeError: Cannot access enum value 'lowercaseEnumVal' of 'MyTypeObject', enum values need to start with an uppercase letter.";
+    QTest::newRow("enum from singleton type") << "lowercaseEnumRuntime.2.qml" << ":8: TypeError: Cannot access enum value 'lowercaseEnumVal' of 'MyTypeObjectSingleton', enum values need to start with an uppercase letter.";
+}
+
+void tst_qqmllanguage::lowercaseEnumRuntime()
+{
+    QFETCH(QString, file);
+    QFETCH(QString, errorMessage);
+
+    QQmlComponent component(&engine, testFileUrl(file));
+    VERIFY_ERRORS(0);
+    QString warning = component.url().toString() + errorMessage;
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(warning));
+    delete component.create();
+}
+
+void tst_qqmllanguage::lowercaseEnumCompileTime_data()
+{
+    QTest::addColumn<QString>("file");
+    QTest::addColumn<QString>("errorFile");
+
+    QTest::newRow("assignment to int property") << "lowercaseEnumCompileTime.1.qml" << "lowercaseEnumCompileTime.1.errors.txt";
+    QTest::newRow("assignment to enum property") << "lowercaseEnumCompileTime.2.qml" << "lowercaseEnumCompileTime.2.errors.txt";
+}
+
+void tst_qqmllanguage::lowercaseEnumCompileTime()
+{
+    QFETCH(QString, file);
+    QFETCH(QString, errorFile);
+
+    QQmlComponent component(&engine, testFileUrl(file));
+    VERIFY_ERRORS(qPrintable(errorFile));
+}
+
 void tst_qqmllanguage::literals_data()
 {
     QTest::addColumn<QString>("property");