qmljs: fix qmljscheck

* fix ASTVariableReference::value: correctly get reference value type by using either initialiser of bindingTarget (broken since a codemodel update in 2018) * disable warning for casting in bool to null comparison (it does not cast, is always false) * fix property checks (where skipped without default of readonly) * remove non relevant checks (ErrInvalidPropertyType for lowercase now that custom value types are supported, and for properties called data) * updated import version Change-Id: I38407acf327d0f773b38dda4c02fb4d95a420851 Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
2021-01-13 02:02:49 +01:00
parent 4f96e397d2
commit cc00af8334
14 changed files with 77 additions and 88 deletions
--- a/src/libs/qmljs/qmljscheck.cpp
+++ b/src/libs/qmljs/qmljscheck.cpp
@@ -1124,52 +1124,39 @@ bool Check::visit(UiArrayBinding *ast)
 bool Check::visit(UiPublicMember *ast)
 {
    if (ast->type == UiPublicMember::Property) {
-        if (ast->defaultToken.isValid() || ast->readonlyToken.isValid()) {
-            const QStringView typeName = ast->memberType->name;
-            if (!typeName.isEmpty() && typeName.at(0).isLower()) {
-                const QString typeNameS = typeName.toString();
-                if (!isValidBuiltinPropertyType(typeNameS))
-                    addMessage(ErrInvalidPropertyType, ast->typeToken, typeNameS);
-            }
+        const QStringView typeName = ast->memberType->name;
+        // warn about dubious use of var/variant
+        if (typeName == QLatin1String("variant") || typeName == QLatin1String("var")) {
+            Evaluate evaluator(&_scopeChain);
+            const Value *init = evaluator(ast->statement);
+            QString preferredType;
+            if (init->asNumberValue())
+                preferredType = tr("'int' or 'real'");
+            else if (init->asStringValue())
+                preferredType = "'string'";
+            else if (init->asBooleanValue())
+                preferredType = "'bool'";
+            else if (init->asColorValue())
+                preferredType = "'color'";
+            else if (init == _context->valueOwner()->qmlPointObject())
+                preferredType = "'point'";
+            else if (init == _context->valueOwner()->qmlRectObject())
+                preferredType = "'rect'";
+            else if (init == _context->valueOwner()->qmlSizeObject())
+                preferredType = "'size'";
+            else if (init == _context->valueOwner()->qmlVector2DObject())
+                preferredType = "'vector2d'";
+            else if (init == _context->valueOwner()->qmlVector3DObject())
+                preferredType = "'vector3d'";
+            else if (init == _context->valueOwner()->qmlVector4DObject())
+                preferredType = "'vector4d'";
+            else if (init == _context->valueOwner()->qmlQuaternionObject())
+                preferredType = "'quaternion'";
+            else if (init == _context->valueOwner()->qmlMatrix4x4Object())
+                preferredType = "'matrix4x4'";

-            const QStringView name = ast->name;
-
-            if (name == QLatin1String("data"))
-                addMessage(ErrInvalidPropertyName, ast->identifierToken, name.toString());
-
-            // warn about dubious use of var/variant
-            if (typeName == QLatin1String("variant") || typeName == QLatin1String("var")) {
-                Evaluate evaluator(&_scopeChain);
-                const Value *init = evaluator(ast->statement);
-                QString preferredType;
-                if (init->asNumberValue())
-                    preferredType = tr("'int' or 'real'");
-                else if (init->asStringValue())
-                    preferredType = "'string'";
-                else if (init->asBooleanValue())
-                    preferredType = "'bool'";
-                else if (init->asColorValue())
-                    preferredType = "'color'";
-                else if (init == _context->valueOwner()->qmlPointObject())
-                    preferredType = "'point'";
-                else if (init == _context->valueOwner()->qmlRectObject())
-                    preferredType = "'rect'";
-                else if (init == _context->valueOwner()->qmlSizeObject())
-                    preferredType = "'size'";
-                else if (init == _context->valueOwner()->qmlVector2DObject())
-                    preferredType = "'vector2d'";
-                else if (init == _context->valueOwner()->qmlVector3DObject())
-                    preferredType = "'vector3d'";
-                else if (init == _context->valueOwner()->qmlVector4DObject())
-                    preferredType = "'vector4d'";
-                else if (init == _context->valueOwner()->qmlQuaternionObject())
-                    preferredType = "'quaternion'";
-                else if (init == _context->valueOwner()->qmlMatrix4x4Object())
-                    preferredType = "'matrix4x4'";
-
-                if (!preferredType.isEmpty())
-                    addMessage(HintPreferNonVarPropertyType, ast->typeToken, preferredType);
-            }
+            if (!preferredType.isEmpty())
+                addMessage(HintPreferNonVarPropertyType, ast->typeToken, preferredType);
        }

        checkBindingRhs(ast->statement);
@@ -1281,7 +1268,8 @@ static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs
        return true; // coerces object to primitive

    if (lhs->asBooleanValue() && (!rhs->asBooleanValue()
-                                  && !rhs->asUndefinedValue()))
+                                  && !rhs->asUndefinedValue()
+                                  && !rhs->asNullValue()))
        return true; // coerces bool to number

    return false;
--- a/src/libs/qmljs/qmljsinterpreter.cpp
+++ b/src/libs/qmljs/qmljsinterpreter.cpp
@@ -1940,16 +1940,18 @@ const PatternElement *ASTVariableReference::ast() const
 const Value *ASTVariableReference::value(ReferenceContext *referenceContext) const
 {
    // may be assigned to later
-    if (!m_ast->expressionCast())
+    ExpressionNode *exp = ((m_ast->initializer) ? m_ast->initializer : m_ast->bindingTarget);
+    if (!exp)
        return valueOwner()->unknownValue();

    Document::Ptr doc = m_doc->ptr();
    ScopeChain scopeChain(doc, referenceContext->context());
    ScopeBuilder builder(&scopeChain);
-    builder.push(ScopeAstPath(doc)(m_ast->expressionCast()->firstSourceLocation().begin()));
+    builder.push(ScopeAstPath(doc)(exp->firstSourceLocation().begin()));

    Evaluate evaluator(&scopeChain, referenceContext);
-    return evaluator(m_ast->expressionCast());
+    const Value *res = evaluator(exp);
+    return res;
 }

 bool ASTVariableReference::getSourceLocation(QString *fileName, int *line, int *column) const
--- a/tests/auto/qml/codemodel/check/avoid-eval.qml
+++ b/tests/auto/qml/codemodel/check/avoid-eval.qml
@@ -1,4 +1,4 @@
-import QtQuick 1.0
+import QtQuick 2.0

 Item {
    function foo() {
--- a/tests/auto/qml/codemodel/check/avoid-some-constructors.qml
+++ b/tests/auto/qml/codemodel/check/avoid-some-constructors.qml
@@ -1,4 +1,4 @@
-import QtQuick 1.0
+import QtQuick 2.0

 Item {
    function foo() {
--- a/tests/auto/qml/codemodel/check/avoid-var.qml
+++ b/tests/auto/qml/codemodel/check/avoid-var.qml
@@ -1,9 +1,9 @@
-import QtQuick 1.0
+import QtQuick 2.0

 Item {
    property int x: 10
    property var x: 10 // 311 14 16
-    property string x: "abc"
+    readonly property string x: "abc"
    property var x: "abc" // 311 14 16
    property string x: true
    property var x: true // 311 14 16
@@ -17,4 +17,6 @@ Item {
    property var x: Qt.size(1, 1) // 311 14 16
    property vector3d x: Qt.vector3d(1, 1, 1)
    property var x: Qt.vector3d(1, 1, 1) // 311 14 16
+    default property int x
+    required property var x
 }
--- a/tests/auto/qml/codemodel/check/confusing-plus-minus.qml
+++ b/tests/auto/qml/codemodel/check/confusing-plus-minus.qml
@@ -1,4 +1,4 @@
-import QtQuick 1.0
+import QtQuick 2.0

 Item {
    function foo() {
--- a/tests/auto/qml/codemodel/check/equality-checks.qml
+++ b/tests/auto/qml/codemodel/check/equality-checks.qml
@@ -1,4 +1,4 @@
-import QtQuick 1.0
+import QtQuick 2.0

 Rectangle {
    function foo(k) {
@@ -10,54 +10,54 @@ Rectangle {
        var b = true
        var o = {}

-        if (s == s) {} // -1 15 16 # false positive
+        if (s == s) {}
        if (s == n) {} // 126 15 16
-        if (s == N) {} // -2 15 16 # wrong warning (always false)
-        if (s == u) {} // -2 15 16 # wrong warning (always false)
+        if (s == N) {}
+        if (s == u) {}
        if (s == b) {} // 126 15 16
        if (s == o) {} // 126 15 16
        if (s == k) {} // 126 15 16

        if (n == s) {} // 126 15 16
-        if (n == n) {} // -1 15 16 # false positive
-        if (n == N) {} // -2 15 16 # wrong warning (always false)
-        if (n == u) {} // -2 15 16 # wrong warning (always false)
+        if (n == n) {}
+        if (n == N) {}
+        if (n == u) {}
        if (n == b) {} // 126 15 16
        if (n == o) {} // 126 15 16
        if (n == k) {} // 126 15 16

-        if (N == s) {} // -2 15 16 # wrong warning (always false)
-        if (N == n) {} // -2 15 16 # wrong warning (always false)
-        if (N == N) {} // -1 15 16 # false positive
-        if (N == u) {} // -2 15 16 # wrong warning (always true)
+        if (N == s) {}
+        if (N == n) {}
+        if (N == N) {}
+        if (N == u) {}

-        if (N == b) {} // -2 15 16 # wrong warning (always false)
-        if (N == o) {} // -2 15 16 # wrong warning (always false)
+        if (N == b) {}
+        if (N == o) {}
        if (N == k) {} // 126 15 16

-        if (u == s) {} // -2 15 16 # wrong warning (always false)
-        if (u == n) {} // -2 15 16 # wrong warning (always false)
-        if (u == N) {} // -2 15 16 # wrong warning (always true)
-        if (u == u) {} // -2 15 16 # wrong warning (always true)
-        if (u == b) {} // -2 15 16 # wrong warning (always false)
-        if (u == o) {} // -2 15 16 # wrong warning (always false)
+        if (u == s) {}
+        if (u == n) {}
+        if (u == N) {}
+        if (u == u) {}
+        if (u == b) {}
+        if (u == o) {}
        if (u == k) {} // 126 15 16

        if (b == s) {} // 126 15 16
        if (b == n) {} // 126 15 16

-        if (b == N) {} // -2 15 16 # wrong warning (always false)
-        if (b == u) {} // -2 15 16 # wrong warning (always false)
-        if (b == b) {} // -1 15 16 # false positive
+        if (b == N) {}
+        if (b == u) {}
+        if (b == b) {}
        if (b == o) {} // 126 15 16
        if (b == k) {} // 126 15 16

        if (o == s) {} // 126 15 16
        if (o == n) {} // 126 15 16
-        if (o == N) {} // -2 15 16 # wrong warning (always false)
-        if (o == u) {} // -2 15 16 # wrong warning (always false)
+        if (o == N) {}
+        if (o == u) {}
        if (o == b) {} // 126 15 16
-        if (o == o) {} // -1 15 16 # false positive
+        if (o == o) {}
        if (o == k) {} // 126 15 16

        if (k == s) {} // 126 15 16
--- a/tests/auto/qml/codemodel/check/expression-statement.qml
+++ b/tests/auto/qml/codemodel/check/expression-statement.qml
@@ -1,4 +1,4 @@
-import Qt 4.7
+import QtQuick 2.0

 Rectangle {
    function foo() {
--- a/tests/auto/qml/codemodel/check/new-expression.qml
+++ b/tests/auto/qml/codemodel/check/new-expression.qml
@@ -1,4 +1,4 @@
-import Qt 4.7
+import QtQuick 2.0

 Item {
    function foo() {
--- a/tests/auto/qml/codemodel/check/properties.qml
+++ b/tests/auto/qml/codemodel/check/properties.qml
@@ -1,4 +1,4 @@
-import QtQuick 1.0
+import QtQuick 2.0

 Item {
   property int width: 200
--- a/tests/auto/qml/codemodel/check/suppression.qml
+++ b/tests/auto/qml/codemodel/check/suppression.qml
@@ -1,4 +1,4 @@
-import Qt 4.7
+import QtQuick 2.0

 Rectangle {
    function foo() {
--- a/tests/auto/qml/codemodel/check/tst_check.cpp
+++ b/tests/auto/qml/codemodel/check/tst_check.cpp
@@ -205,9 +205,6 @@ void tst_Check::test()
            Message missingMessage = expectedMessages.at(i);
            qDebug() << "expected message of type" << missingMessage.type << "on line" << missingMessage.location.startLine;
        }
-        if (path.endsWith("avoid-var.qml"))
-            QEXPECT_FAIL(path.toUtf8(), "currently broken", Continue);
-
        QVERIFY2(false, "more messages expected");
    }
    if (expectedMessages.size() < messages.size()) {
--- a/tests/auto/qml/codemodel/check/unreachable.qml
+++ b/tests/auto/qml/codemodel/check/unreachable.qml
@@ -1,4 +1,4 @@
-import Qt 4.7
+import QtQuick 2.0
 // DEFAULTMSG unreachable
 Item {
    function foo() {
--- a/tests/auto/qml/codemodel/check/useless-blocks.qml
+++ b/tests/auto/qml/codemodel/check/useless-blocks.qml
@@ -1,4 +1,4 @@
-import Qt 4.7
+import QtQuick 2.0

 Rectangle {
    function foo() {