diff options
| -rw-r--r-- | tests/auto/qml/qmllint/data/namedSignalParameters.qml | 15 | ||||
| -rw-r--r-- | tests/auto/qml/qmllint/data/tooManySignalParameters.qml | 8 | ||||
| -rw-r--r-- | tests/auto/qml/qmllint/tst_qmllint.cpp | 10 | ||||
| -rw-r--r-- | tools/qmllint/findwarnings.cpp | 65 |
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 6e2a41263e..fe6996d6d9 100644 --- a/tests/auto/qml/qmllint/tst_qmllint.cpp +++ b/tests/auto/qml/qmllint/tst_qmllint.cpp @@ -288,6 +288,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; } |