From e906bbdb56918ccc900e60ab8a3670c16c8dd4a5 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Thu, 20 Oct 2011 09:45:29 +0200 Subject: [PATCH] QmlJS checks: Warn about extra message suppressions. Reviewed-by: Fawzi Mohamed Change-Id: I4038cd863ba80c1719417cd03b755b047f7d8b5e Reviewed-by: Christian Kamm --- src/libs/qmljs/qmljscheck.cpp | 83 ++++++++++++++----- src/libs/qmljs/qmljscheck.h | 13 +++ src/libs/qmljs/qmljsstaticanalysismessage.cpp | 2 + src/libs/qmljs/qmljsstaticanalysismessage.h | 1 + .../auto/qml/codemodel/check/suppression.qml | 15 ++++ 5 files changed, 94 insertions(+), 20 deletions(-) create mode 100644 tests/auto/qml/codemodel/check/suppression.qml diff --git a/src/libs/qmljs/qmljscheck.cpp b/src/libs/qmljs/qmljscheck.cpp index ad4596fb95d..64055f881d4 100644 --- a/src/libs/qmljs/qmljscheck.cpp +++ b/src/libs/qmljs/qmljscheck.cpp @@ -535,7 +535,11 @@ Check::~Check() QList Check::operator()() { _messages.clear(); + scanCommentsForAnnotations(); + Node::accept(_doc->ast(), this); + warnAboutUnnecessarySuppressions(); + return _messages; } @@ -1147,27 +1151,14 @@ static bool hasOnlySpaces(const QString &s) void Check::addMessage(const Message &message) { if (message.isValid() && _enabledMessages.contains(message.type)) { - // check for 'ignore this message'-type comments - const QString &suppressMessage = message.suppressionString(); - foreach (const SourceLocation &commentLoc, _doc->engine()->comments()) { - if (commentLoc.startLine > message.location.startLine) - break; - if (commentLoc.startLine < message.location.startLine - 1) - continue; - - // only look at comments on the previous line if there's only spaces before the comment - // note: startColumn is 1-based and *after* the starting // or /* - if (commentLoc.startLine == message.location.startLine - 1 - && commentLoc.startColumn > 3) { - const QString &beforeComment = _doc->source().mid(commentLoc.begin() - commentLoc.startColumn + 1, - commentLoc.startColumn - 3); - if (!hasOnlySpaces(beforeComment)) - continue; + if (m_disabledMessageTypesByLine.contains(message.location.startLine)) { + QList &disabledMessages = m_disabledMessageTypesByLine[message.location.startLine]; + for (int i = 0; i < disabledMessages.size(); ++i) { + if (disabledMessages[i].type == message.type) { + disabledMessages[i].wasSuppressed = true; + return; + } } - - const QString &comment = _doc->source().mid(commentLoc.begin(), commentLoc.length); - if (comment.contains(suppressMessage)) - return; } _messages += message; @@ -1179,6 +1170,58 @@ void Check::addMessage(Type type, const SourceLocation &location, const QString addMessage(Message(type, location, arg1, arg2)); } +void Check::scanCommentsForAnnotations() +{ + m_disabledMessageTypesByLine.clear(); + + // find all disable annotations + const QRegExp disableCommentPattern(QLatin1String("@disable M(\\d+)")); + foreach (const SourceLocation &commentLoc, _doc->engine()->comments()) { + const QString &comment = _doc->source().mid(commentLoc.begin(), commentLoc.length); + int lastOffset = -1; + QList disabledMessageTypes; + forever { + lastOffset = disableCommentPattern.indexIn(comment, lastOffset + 1); + if (lastOffset == -1) + break; + MessageTypeAndSuppression entry; + entry.type = static_cast(disableCommentPattern.cap(1).toInt()); + entry.wasSuppressed = false; + entry.suppressionSource = SourceLocation(commentLoc.offset + lastOffset, + disableCommentPattern.matchedLength(), + commentLoc.startLine, + commentLoc.startColumn + lastOffset); + disabledMessageTypes += entry; + } + if (!disabledMessageTypes.isEmpty()) { + int appliesToLine = commentLoc.startLine; + + // if the comment is preceded by spaces only, it applies to the next line + // note: startColumn is 1-based and *after* the starting // or /* + if (commentLoc.startColumn > 3) { + const QString &beforeComment = _doc->source().mid(commentLoc.begin() - commentLoc.startColumn + 1, + commentLoc.startColumn - 3); + if (hasOnlySpaces(beforeComment)) + ++appliesToLine; + } + + m_disabledMessageTypesByLine[appliesToLine] += disabledMessageTypes; + } + } +} + +void Check::warnAboutUnnecessarySuppressions() +{ + QHashIterator< int, QList > it(m_disabledMessageTypesByLine); + while (it.hasNext()) { + it.next(); + foreach (const MessageTypeAndSuppression &entry, it.value()) { + if (!entry.wasSuppressed) + addMessage(WarnUnnecessaryMessageSuppression, entry.suppressionSource); + } + } +} + bool Check::visit(NewExpression *ast) { checkNewExpression(ast->expression); diff --git a/src/libs/qmljs/qmljscheck.h b/src/libs/qmljs/qmljscheck.h index d824d43514b..f15a5f3b840 100644 --- a/src/libs/qmljs/qmljscheck.h +++ b/src/libs/qmljs/qmljscheck.h @@ -119,6 +119,9 @@ private: void addMessage(StaticAnalysis::Type type, const AST::SourceLocation &location, const QString &arg1 = QString(), const QString &arg2 = QString()); + void scanCommentsForAnnotations(); + void warnAboutUnnecessarySuppressions(); + AST::Node *parent(int distance = 0); Document::Ptr _doc; @@ -135,6 +138,16 @@ private: QStack m_idStack; QStack m_propertyStack; + class MessageTypeAndSuppression + { + public: + AST::SourceLocation suppressionSource; + StaticAnalysis::Type type; + bool wasSuppressed; + }; + + QHash< int, QList > m_disabledMessageTypesByLine; + bool _importsOk; }; diff --git a/src/libs/qmljs/qmljsstaticanalysismessage.cpp b/src/libs/qmljs/qmljsstaticanalysismessage.cpp index 6406a237e8d..f9ab8070d83 100644 --- a/src/libs/qmljs/qmljsstaticanalysismessage.cpp +++ b/src/libs/qmljs/qmljsstaticanalysismessage.cpp @@ -118,6 +118,8 @@ StaticAnalysisMessages::StaticAnalysisMessages() tr("do not use comma expressions")); newMsg(WarnAlreadyFormalParameter, Warning, tr("'%1' is already a formal parameter"), 1); + newMsg(WarnUnnecessaryMessageSuppression, Warning, + tr("unnecessary message suppression")); newMsg(WarnAlreadyFunction, Warning, tr("'%1' is already a function"), 1); newMsg(WarnVarUsedBeforeDeclaration, Warning, diff --git a/src/libs/qmljs/qmljsstaticanalysismessage.h b/src/libs/qmljs/qmljsstaticanalysismessage.h index 3d9e00aa89d..0375beebf91 100644 --- a/src/libs/qmljs/qmljsstaticanalysismessage.h +++ b/src/libs/qmljs/qmljsstaticanalysismessage.h @@ -73,6 +73,7 @@ enum Type WarnUnreachable = 28, WarnWith = 29, WarnComma = 30, + WarnUnnecessaryMessageSuppression = 31, WarnAlreadyFormalParameter = 103, WarnAlreadyFunction = 104, WarnVarUsedBeforeDeclaration = 105, diff --git a/tests/auto/qml/codemodel/check/suppression.qml b/tests/auto/qml/codemodel/check/suppression.qml new file mode 100644 index 00000000000..cfc67bd6a84 --- /dev/null +++ b/tests/auto/qml/codemodel/check/suppression.qml @@ -0,0 +1,15 @@ +import Qt 4.7 + +Rectangle { + function foo() { + a + b // 127 9 13 + // @disable M127 + a + b + a + b // @disable M127 + + // @disable M127 31 12 24 + + // @disable M126 31 12 24 + a + b // 127 9 13 + } +}