diff options
| author | Sami Shalayel <sami.shalayel@qt.io> | 2025-11-20 14:34:40 +0100 |
|---|---|---|
| committer | Sami Shalayel <sami.shalayel@qt.io> | 2025-11-26 19:53:53 +0100 |
| commit | 11f9dd7fcbad2abc169d64cecf4ab15b329720cb (patch) | |
| tree | 5fd58eae092a970be1a285b469d996b51ba52719 /src | |
| parent | 9ab13be95a531fc9c1b7f472c75ef8fa880a7206 (diff) | |
qmllint: warn on shadowed properties/signals/methods
Warn when shadowing properties/signals/methods in qmllint. Move the
already existing duplicate property check into its own static method
warnForDuplicates, and extend it to check whether base types also
have a property of the same name. Use it when analysing definitions
of methods, signals and properties in qmljsimportvisitor to warn about
shadowed properties.
As a drive-by change, fix the sourcelocation of the duplicated property
warning to point to the name of the property/signal/method instead of
the "property" or "function" keyword.
To avoid adding more testcases to the already big
tst_qmllint::dirtyQmlSnippets_data(), create a new
tst_qmllint::shadow_data() method for the shadow related tests, and
make tst_qmllint::shadow() call dirtyQmlSnippet(). This allows to reuse
the test code from dirtyQmlSnippet without making dirtyQmlSnippets_data
grow even more.
Adapt some existing test-cases by adding qmllint disable directives
where shadowing was wanted, and disable the shadow category on
tst_qqmljsscope (which does not make use of the qmllint code that reads
qmllint disable directives), as it seems that tst_qqmljsscope tests that
shadowing works as expected (shadowing is a feature, after all...).
I guess this patch might need to be updated in the future once we have
override and virtual properties in QML via QTBUG-98320.
Make the qmlShadow warning non-critical for qmltc, as qmltc supports
shadowing, to avoid breaking the qmltc tests.
Task-number: QTBUG-141854
Change-Id: I74c1b413e2c19a0db34215b34a9b65d43fbef6ce
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>
Diffstat (limited to 'src')
| -rw-r--r-- | src/qmlcompiler/qqmljsimportvisitor.cpp | 60 | ||||
| -rw-r--r-- | src/qmlcompiler/qqmljslogger.cpp | 2 | ||||
| -rw-r--r-- | src/qmlcompiler/qqmljsloggingutils.h | 1 |
3 files changed, 49 insertions, 14 deletions
diff --git a/src/qmlcompiler/qqmljsimportvisitor.cpp b/src/qmlcompiler/qqmljsimportvisitor.cpp index 38b1b1e5a9..54dae65260 100644 --- a/src/qmlcompiler/qqmljsimportvisitor.cpp +++ b/src/qmlcompiler/qqmljsimportvisitor.cpp @@ -1915,20 +1915,49 @@ void QQmlJSImportVisitor::endVisit(UiInlineComponent *component) m_nextIsInlineComponent = false; // might have missed an inline component if file contains invalid QML } +static constexpr QLatin1String s_method = "method"_L1; +static constexpr QLatin1String s_signal = "signal"_L1; +static constexpr QLatin1String s_property = "property"_L1; + +static void warnForDuplicates(const QQmlJSScope::ConstPtr &scope, const QString &name, + QLatin1String type, const QQmlJS::SourceLocation &location, + QQmlJSLogger *logger) +{ + if (scope->hasOwnMethod(name) || scope->hasOwnProperty(name)) + logger->log("Duplicated %1 name \"%2\"."_L1.arg(type, name), qmlDuplicatedName, location); + + static constexpr QLatin1String warningMessage = + "%1 \"%2\" already exists in base type \"%3\", use a different name."_L1; + + if (scope->hasMethod(name)) { + const auto owner = QQmlJSScope::ownerOfMethod(scope, name).scope; + const bool isSignal = + owner->methods(name).front().methodType() == QQmlJSMetaMethodType::Signal; + logger->log(warningMessage.arg(isSignal ? "Signal"_L1 : "Method"_L1, name, + getScopeName(owner, QQmlSA::ScopeType::QMLScope)), + qmlShadow, location); + } + if (scope->hasProperty(name)) { + const auto owner = QQmlJSScope::ownerOfProperty(scope, name).scope; + logger->log(warningMessage.arg("Property"_L1, name, + getScopeName(owner, QQmlSA::ScopeType::QMLScope)), + qmlShadow, location); + } +} + bool QQmlJSImportVisitor::visit(UiPublicMember *publicMember) { switch (publicMember->type) { case UiPublicMember::Signal: { - if (m_currentScope->ownMethods().contains(publicMember->name.toString())) { - m_logger->log(QStringLiteral("Duplicated signal name \"%1\".").arg( - publicMember->name.toString()), qmlDuplicatedName, - publicMember->firstSourceLocation()); - } + const QString signalName = publicMember->name.toString(); + warnForDuplicates(m_currentScope, signalName, s_signal, publicMember->identifierToken, + m_logger); + UiParameterList *param = publicMember->parameters; QQmlJSMetaMethod method; method.setMethodType(QQmlJSMetaMethodType::Signal); method.setReturnTypeName(QStringLiteral("void")); - method.setMethodName(publicMember->name.toString()); + method.setMethodName(signalName); method.setSourceLocation(combine(publicMember->firstSourceLocation(), publicMember->lastSourceLocation())); while (param) { @@ -1943,11 +1972,10 @@ bool QQmlJSImportVisitor::visit(UiPublicMember *publicMember) break; } case UiPublicMember::Property: { - if (m_currentScope->ownProperties().contains(publicMember->name.toString())) { - m_logger->log(QStringLiteral("Duplicated property name \"%1\".").arg( - publicMember->name.toString()), qmlDuplicatedName, - publicMember->firstSourceLocation()); - } + const QString propertyName = publicMember->name.toString(); + warnForDuplicates(m_currentScope, propertyName, s_property, publicMember->identifierToken, + m_logger); + QString typeName = buildName(publicMember->memberType); if (typeName.contains(u'.') && typeName.front().isLower()) { logLowerCaseImport(typeName, publicMember->typeToken, m_logger); @@ -1991,7 +2019,7 @@ bool QQmlJSImportVisitor::visit(UiPublicMember *publicMember) } } QQmlJSMetaProperty prop; - prop.setPropertyName(publicMember->name.toString()); + prop.setPropertyName(propertyName); prop.setIsList(publicMember->typeModifier == QLatin1String("list")); prop.setIsWritable(!publicMember->isReadonly()); prop.setIsFinal(publicMember->isFinal()); @@ -2136,9 +2164,11 @@ void QQmlJSImportVisitor::visitFunctionExpressionHelper(QQmlJS::AST::FunctionExp m_pendingMethodTypeAnnotations << pending; method.setJsFunctionIndex(addFunctionOrExpression(m_currentScope, method.methodName())); - m_currentScope->addOwnMethod(method); - if (m_currentScope->scopeType() != QQmlSA::ScopeType::QMLScope) { + if (m_currentScope->scopeType() == QQmlSA::ScopeType::QMLScope) { + warnForDuplicates(m_currentScope, method.methodName(), s_method, fexpr->identifierToken, + m_logger); + } else { // note: lambda methods have no identifier token const QQmlJS::SourceLocation functionLocation = fexpr->identifierToken.isValid() ? fexpr->identifierToken @@ -2148,6 +2178,8 @@ void QQmlJSImportVisitor::visitFunctionExpressionHelper(QQmlJS::AST::FunctionExp functionLocation, method.returnTypeName(), false }); } + m_currentScope->addOwnMethod(method); + enterEnvironment(QQmlSA::ScopeType::JSFunctionScope, name, fexpr->firstSourceLocation()); } else { addFunctionOrExpression(m_currentScope, QStringLiteral("<anon>")); diff --git a/src/qmlcompiler/qqmljslogger.cpp b/src/qmlcompiler/qqmljslogger.cpp index d98ccaf753..74a1670420 100644 --- a/src/qmlcompiler/qqmljslogger.cpp +++ b/src/qmlcompiler/qqmljslogger.cpp @@ -121,6 +121,8 @@ using namespace Qt::StringLiterals; false, false) \ X(qmlRestrictedType, "restricted-type", "RestrictedType", "Warn about restricted types", \ QtWarningMsg, false, false) \ + X(qmlShadow, "shadow", "Shadow", "Warn about shadowing attributes from a base class", \ + QtWarningMsg, false, false) \ X(qmlSignalParameters, "signal-handler-parameters", "BadSignalHandlerParameters", \ "Warn about bad signal handler parameters", QtWarningMsg, false, false) \ X(qmlStalePropertyRead, "stale-property-read", "StalePropertyRead", \ diff --git a/src/qmlcompiler/qqmljsloggingutils.h b/src/qmlcompiler/qqmljsloggingutils.h index 10dca55ae3..78e876fbeb 100644 --- a/src/qmlcompiler/qqmljsloggingutils.h +++ b/src/qmlcompiler/qqmljsloggingutils.h @@ -84,6 +84,7 @@ extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlRecursionDepthError extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlRedundantOptionalChaining; extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlRequired; extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlRestrictedType; +extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlShadow; extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlSignalParameters; extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlStalePropertyRead; extern const Q_QMLCOMPILER_EXPORT QQmlSA::LoggerWarningId qmlSyntax; |