diff options
author | Ulf Hermann <ulf.hermann@qt.io> | 2021-02-11 13:54:10 +0100 |
---|---|---|
committer | Ulf Hermann <ulf.hermann@qt.io> | 2021-02-15 19:18:07 +0100 |
commit | e337ba277beb6d08dbb9ca78e74d6cc22f56e3b6 (patch) | |
tree | 5db664bd00693e28cdde09636a0ea38bcce9bc10 /tools | |
parent | 9f864cc12753a1b9b4d58139df7509b411693e28 (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>
Diffstat (limited to 'tools')
-rw-r--r-- | tools/qmllint/findwarnings.cpp | 65 |
1 files changed, 49 insertions, 16 deletions
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; } |