summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorUlf Hermann <ulf.hermann@qt.io>2021-02-11 13:54:10 +0100
committerUlf Hermann <ulf.hermann@qt.io>2021-02-15 19:18:07 +0100
commite337ba277beb6d08dbb9ca78e74d6cc22f56e3b6 (patch)
tree5db664bd00693e28cdde09636a0ea38bcce9bc10
parent9f864cc12753a1b9b4d58139df7509b411693e28 (diff)
qmllint: Warn about too many or mismatched signal parameters
It's easy to mess this up when you transform your signal handlers into functions. Task-number: QTBUG-89943 Change-Id: If35be2f6828a0e19aada19abb41d8135b0c6ab45 Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io> (cherry picked from commit f0ecad1e99461109e69cd2b0f6271012c20005dd) Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
-rw-r--r--tests/auto/qml/qmllint/data/namedSignalParameters.qml15
-rw-r--r--tests/auto/qml/qmllint/data/tooManySignalParameters.qml8
-rw-r--r--tests/auto/qml/qmllint/tst_qmllint.cpp10
-rw-r--r--tools/qmllint/findwarnings.cpp65
4 files changed, 82 insertions, 16 deletions
diff --git a/tests/auto/qml/qmllint/data/namedSignalParameters.qml b/tests/auto/qml/qmllint/data/namedSignalParameters.qml
new file mode 100644
index 0000000000..00746d5d83
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/namedSignalParameters.qml
@@ -0,0 +1,15 @@
+import QtQuick 2.0
+Window {
+ width: 800
+ height: 600
+ visible: true
+ signal sig(string arg, int argarg)
+ signal sig2(int foo, bool bar)
+
+ onSig: function(argarg) {
+ print("SIG", argarg);
+ }
+
+ onSig2: (foo, bar)=> { sig(foo, bar); }
+}
+
diff --git a/tests/auto/qml/qmllint/data/tooManySignalParameters.qml b/tests/auto/qml/qmllint/data/tooManySignalParameters.qml
new file mode 100644
index 0000000000..f5d5977a99
--- /dev/null
+++ b/tests/auto/qml/qmllint/data/tooManySignalParameters.qml
@@ -0,0 +1,8 @@
+import QtQml
+QtObject {
+ signal sig(string arg, int argarg)
+ onSig: function(arg, b, c, d) {
+ print("SIG", arg, b, c, d);
+ }
+}
+
diff --git a/tests/auto/qml/qmllint/tst_qmllint.cpp b/tests/auto/qml/qmllint/tst_qmllint.cpp
index 97840cc2dd..18b5090dcf 100644
--- a/tests/auto/qml/qmllint/tst_qmllint.cpp
+++ b/tests/auto/qml/qmllint/tst_qmllint.cpp
@@ -282,6 +282,16 @@ void TestQmllint::dirtyQmlCode_data()
<< QStringLiteral("Variable \"argq\" is used before its declaration at 5:9. "
"The declaration is at 6:13.")
<< QString();
+ QTest::newRow("SignalParameterMismatch")
+ << QStringLiteral("namedSignalParameters.qml")
+ << QStringLiteral("Parameter 1 to signal handler for \"onSig\" is called \"argarg\". "
+ "The signal has a parameter of the same name in position 2.")
+ << QStringLiteral("onSig2");
+ QTest::newRow("TooManySignalParameters")
+ << QStringLiteral("tooManySignalParameters.qml")
+ << QStringLiteral("Signal handler for \"onSig\" has more formal parameters "
+ "than the signal it handles.")
+ << QString();
}
void TestQmllint::dirtyQmlCode()
diff --git a/tools/qmllint/findwarnings.cpp b/tools/qmllint/findwarnings.cpp
index 653c5bc209..17e73c770c 100644
--- a/tools/qmllint/findwarnings.cpp
+++ b/tools/qmllint/findwarnings.cpp
@@ -223,35 +223,68 @@ bool FindWarningVisitor::visit(QQmlJS::AST::UiScriptBinding *uisb)
QtWarningMsg,
uisb->firstSourceLocation()
});
+ m_visitFailed = true;
return true;
}
- const auto statement = uisb->statement;
- if (statement->kind == Node::Kind::Kind_ExpressionStatement) {
- if (cast<ExpressionStatement *>(statement)->expression->asFunctionDefinition()) {
- // functions are already handled
- // they do not get names inserted according to the signal, but access their formal
- // parameters
- return true;
- }
- }
-
+ QQmlJSMetaMethod scopeSignal;
for (QQmlJSScope::ConstPtr scope = qmlScope; scope; scope = scope->baseType()) {
const auto methods = scope->ownMethods();
const auto methodsRange = methods.equal_range(signal);
for (auto method = methodsRange.first; method != methodsRange.second; ++method) {
if (method->methodType() != QQmlJSMetaMethod::Signal)
continue;
+ scopeSignal = *method;
+ break;
+ }
+ }
+
+ const auto statement = uisb->statement;
+ if (ExpressionStatement *expr = cast<ExpressionStatement *>(statement)) {
+ if (FunctionExpression *func = expr->expression->asFunctionDefinition()) {
+ // functions are already handled
+ // they do not get names inserted according to the signal, but access their formal
+ // parameters. Let's still check if the names match, though.
+ const QStringList signalParameters = scopeSignal.parameterNames();
+ qsizetype i = 0, end = signalParameters.length();
+ for (FormalParameterList *formal = func->formals;
+ formal; ++i, formal = formal->next) {
+ if (i == end) {
+ m_errors.append({
+ QStringLiteral("Signal handler for \"%2\" has more formal"
+ " parameters than the signal it handles.")
+ .arg(name),
+ QtWarningMsg,
+ uisb->firstSourceLocation()
+ });
+ m_visitFailed = true;
+ }
+
+ const QStringView handlerParameter = formal->element->bindingIdentifier;
+ const qsizetype j = signalParameters.indexOf(handlerParameter);
+ if (j == i || j < 0)
+ continue;
- const auto firstSourceLocation = statement->firstSourceLocation();
- bool hasMultilineStatementBody
- = statement->lastSourceLocation().startLine > firstSourceLocation.startLine;
- m_pendingSingalHandler = firstSourceLocation;
- m_signalHandlers.insert(firstSourceLocation, {*method, hasMultilineStatementBody});
- return true; // If there are multiple candidates for the signal, it's a mess anyway.
+ m_errors.append({
+ QStringLiteral("Parameter %1 to signal handler for \"%2\""
+ " is called \"%3\". The signal has a parameter"
+ " of the same name in position %4.\n")
+ .arg(i + 1).arg(name, handlerParameter).arg(j + 1),
+ QtWarningMsg,
+ uisb->firstSourceLocation()
+ });
+ m_visitFailed = true;
+ }
+
+ return true;
}
}
+ const auto firstSourceLocation = statement->firstSourceLocation();
+ bool hasMultilineStatementBody
+ = statement->lastSourceLocation().startLine > firstSourceLocation.startLine;
+ m_pendingSingalHandler = firstSourceLocation;
+ m_signalHandlers.insert(firstSourceLocation, {scopeSignal, hasMultilineStatementBody});
return true;
}