qmllint: Warn about unintentional empty blocksHEADdev

If a property binding is a js block without any statements, it could
mean that the user intended to create an object literal instead of an
empty block that returns undefined.

Task-number: QTBUG-129307
Change-Id: If242ddac140cce6479062194b785096ba1f6c0a6
Reviewed-by: Fabian Kosmale <[email protected]>

7 files changed, 40 insertions, 7 deletions

diff --git a/src/qmlcompiler/qqmljsimportvisitor_p.h b/src/qmlcompiler/qqmljsimportvisitor_p.h
index 32f6915b32..c13f09f112 100644
--- a/src/qmlcompiler/qqmljsimportvisitor_p.h
+++ b/src/qmlcompiler/qqmljsimportvisitor_p.h
@@ -305,7 +305,7 @@ protected:
 
     enum class BindingExpressionParseResult { Invalid, Script, Literal, Translation };
     enum class BindingForPropertyDefintion { Yes, No };
-    BindingExpressionParseResult parseBindingExpression(
+    virtual BindingExpressionParseResult parseBindingExpression(
             const QString &name, const QQmlJS::AST::Statement *statement,
             const QQmlJS::AST::UiPublicMember *associatedPropertyDefinition = nullptr);
     bool isImportPrefix(QString prefix) const;
diff --git a/src/qmlcompiler/qqmljslintervisitor.cpp b/src/qmlcompiler/qqmljslintervisitor.cpp
index b5d254a00c..540ba6c535 100644
--- a/src/qmlcompiler/qqmljslintervisitor.cpp
+++ b/src/qmlcompiler/qqmljslintervisitor.cpp
@@ -416,6 +416,22 @@ bool LinterVisitor::visit(ExpressionStatement *ast)
     return true;
 }
 
+QQmlJSImportVisitor::BindingExpressionParseResult LinterVisitor::parseBindingExpression(
+        const QString &name, const QQmlJS::AST::Statement *statement,
+        const QQmlJS::AST::UiPublicMember *associatedPropertyDefinition)
+{
+    if (statement && statement->kind == (int)AST::Node::Kind::Kind_Block) {
+        const auto *block = static_cast<const AST::Block *>(statement);
+        if (!block->statements && associatedPropertyDefinition) {
+            m_logger->log("Unintentional empty block, use ({}) for empty object literal"_L1,
+                          qmlUnintentionalEmptyBlock,
+                          combine(block->lbraceToken, block->rbraceToken));
+        }
+    }
+
+    return QQmlJSImportVisitor::parseBindingExpression(name, statement, associatedPropertyDefinition);
+}
+
 void LinterVisitor::handleLiteralBinding(const QQmlJSMetaPropertyBinding &binding,
                                          const UiPublicMember *associatedPropertyDefinition)
 {
diff --git a/src/qmlcompiler/qqmljslintervisitor_p.h b/src/qmlcompiler/qqmljslintervisitor_p.h
index 416dc972c1..efec9b3abf 100644
--- a/src/qmlcompiler/qqmljslintervisitor_p.h
+++ b/src/qmlcompiler/qqmljslintervisitor_p.h
@@ -92,6 +92,9 @@ private:
     void warnCaseNoFlowControl(QQmlJS::SourceLocation caseToken) const;
     void checkCaseFallthrough(QQmlJS::AST::StatementList *statements, SourceLocation errorLoc,
                               SourceLocation nextLoc);
+    BindingExpressionParseResult parseBindingExpression(
+            const QString &name, const QQmlJS::AST::Statement *statement,
+            const QQmlJS::AST::UiPublicMember *associatedPropertyDefinition = nullptr) override;
     void handleLiteralBinding(const QQmlJSMetaPropertyBinding &binding,
                               const AST::UiPublicMember *associatedPropertyDefinition) override;
 };
diff --git a/src/qmlcompiler/qqmljslogger.cpp b/src/qmlcompiler/qqmljslogger.cpp
index d3cf05fb1d..40bdd9c48d 100644
--- a/src/qmlcompiler/qqmljslogger.cpp
+++ b/src/qmlcompiler/qqmljslogger.cpp
@@ -131,6 +131,8 @@ using namespace Qt::StringLiterals;
       false, false)                                                                                \
     X(qmlUncreatableType, "uncreatable-type", "UncreatableType",                                   \
       "Warn if uncreatable types are created", QtWarningMsg, false, false)                         \
+    X(qmlUnintentionalEmptyBlock, "unintentional-empty-block", "UnintentionalEmptyBlock",          \
+      "Warn about bindings that contain only an empty block", QtWarningMsg, false, false)          \
     X(qmlUnqualified, "unqualified", "UnqualifiedAccess",                                          \
       "Warn about unqualified identifiers and how to fix them", QtWarningMsg, false, false)        \
     X(qmlUnreachableCode, "unreachable-code", "UnreachableCode", "Warn about unreachable code.",   \
diff --git a/src/qmlcompiler/qqmljsloggingutils.h b/src/qmlcompiler/qqmljsloggingutils.h
index c33fe5a407..05067c7d64 100644
--- a/src/qmlcompiler/qqmljsloggingutils.h
+++ b/src/qmlcompiler/qqmljsloggingutils.h
@@ -88,6 +88,7 @@ extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlSyntaxIdQuotation;
 extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlTopLevelComponent;
 extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlTranslationFunctionMismatch;
 extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlUncreatableType;
+extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlUnintentionalEmptyBlock;
 extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlUnqualified;
 extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlUnreachableCode;
 extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlUnresolvedAlias;
diff --git a/tests/auto/qml/qmllint/data/qdsPlugin/SupportedBlock.ui.qml b/tests/auto/qml/qmllint/data/qdsPlugin/SupportedBlock.ui.qml
index 3cb5ed14cc..0294ef6329 100644
--- a/tests/auto/qml/qmllint/data/qdsPlugin/SupportedBlock.ui.qml
+++ b/tests/auto/qml/qmllint/data/qdsPlugin/SupportedBlock.ui.qml
@@ -2,7 +2,7 @@ import QtQuick
 
 Connections {
     property int ok: 123
-    property int evilBlock: {}
+    property int evilBlock: { 1 }
     property var evilLambda: function() {}
     property var evilFunction: function hello() {}
     property var evilArrow: x => x
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index d2d94b757d..16adcf83fd 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -1462,16 +1462,18 @@ void TestQmllint::dirtyQmlSnippet_data()
             << u"property int qwer: \"Hello\""_s
             << Result{ { { "Cannot assign literal of type string to int"_L1 } } }
             << defaultOptions;
-
-    QTest::newRow("uselessExpressionStatement")
-            << u"property int i: { let x = 0; 0 + 1; return i + 3; }"_s
-            << Result{ { { "Expression statement has no obvious effect."_L1, 1, 30 } } }
+    QTest::newRow("unintentionalEmptyBlock-dirty")
+            << u"property var v: {}"_s
+            << Result{ { { "Unintentional empty block, use ({}) for empty object literal"_L1, 1, 17 } } }
             << defaultOptions;
-
     QTest::newRow("upperCaseId")
             << u"id: Root"_s
             << Result{ { { "Id must start with a lower case letter or an '_'"_L1, 1, 5 } } }
             << defaultOptions;
+    QTest::newRow("uselessExpressionStatement")
+            << u"property int i: { let x = 0; 0 + 1; return i + 3; }"_s
+            << Result{ { { "Expression statement has no obvious effect."_L1, 1, 30 } } }
+            << defaultOptions;
 }
 
 void TestQmllint::dirtyQmlSnippet()
@@ -1532,6 +1534,15 @@ void TestQmllint::cleanQmlSnippet_data()
             << u"Item { component Foo: Item { required property var bla; } }"_s << defaultOptions;
     QTest::newRow("testSnippet") << u"property int qwer: 123"_s << defaultOptions;
     QTest::newRow("underScoreId") << u"id: _Root"_s << defaultOptions;
+    QTest::newRow("unintentionalEmptyBlock-clean")
+            << uR"(
+                    property var p1: ({})
+                    property var p2: {
+                        {}
+                        return {}
+                    }
+                )"_s
+            << defaultOptions;
     QTest::newRow("usefulExpressionStatement") << u"x: y + 3;"_s << defaultOptions;
     QTest::newRow("usefulExpressionStatement") << u"x: 3;"_s << defaultOptions;
     QTest::newRow("void") << u"function f(): void {}"_s << defaultOptions;