From 55713a1514adab455ba99d27bd0d2656d221947a Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 28 Sep 2011 15:16:00 +0200 Subject: [PATCH] QmlJS checks: Add severity and unique id to messages. Change-Id: I2cded26524c3f64152107e65d740658e3003ffac Reviewed-on: http://codereview.qt-project.org/5790 Sanity-Review: Qt Sanity Bot Reviewed-by: Fawzi Mohamed --- src/libs/qmljs/qmljs-lib.pri | 6 +- src/libs/qmljs/qmljscheck.cpp | 367 ++++++++++----------- src/libs/qmljs/qmljscheck.h | 43 +-- src/libs/qmljs/qmljsstaticanalysismessage.cpp | 247 ++++++++++++++ src/libs/qmljs/qmljsstaticanalysismessage.h | 135 ++++++++ .../designercore/model/texttomodelmerger.cpp | 11 +- .../qmljseditor/qmljssemanticinfoupdater.cpp | 4 +- src/plugins/qmljseditor/qmltaskmanager.cpp | 8 + tests/auto/qml/codemodel/check/equality-checks.qml | 54 +-- .../qml/codemodel/check/expression-statement.qml | 8 +- tests/auto/qml/codemodel/check/new-expression.qml | 8 +- tests/auto/qml/codemodel/check/tst_check.cpp | 77 ++--- tests/auto/qml/codemodel/check/unreachable.qml | 16 +- tests/auto/qml/codemodel/check/useless-blocks.qml | 8 +- 14 files changed, 665 insertions(+), 327 deletions(-) create mode 100644 src/libs/qmljs/qmljsstaticanalysismessage.cpp create mode 100644 src/libs/qmljs/qmljsstaticanalysismessage.h diff --git a/src/libs/qmljs/qmljs-lib.pri b/src/libs/qmljs/qmljs-lib.pri index 883ebd3da0..e83410e6f0 100644 --- a/src/libs/qmljs/qmljs-lib.pri +++ b/src/libs/qmljs/qmljs-lib.pri @@ -31,7 +31,8 @@ HEADERS += \ $$PWD/qmljsvalueowner.h \ $$PWD/qmljscontext.h \ $$PWD/qmljsscopechain.h \ - $$PWD/qmljsutils.h + $$PWD/qmljsutils.h \ + $$PWD/qmljsstaticanalysismessage.h SOURCES += \ $$PWD/qmljsbind.cpp \ @@ -54,7 +55,8 @@ SOURCES += \ $$PWD/qmljsvalueowner.cpp \ $$PWD/qmljscontext.cpp \ $$PWD/qmljsscopechain.cpp \ - $$PWD/qmljsutils.cpp + $$PWD/qmljsutils.cpp \ + $$PWD/qmljsstaticanalysismessage.cpp RESOURCES += \ $$PWD/qmljs.qrc diff --git a/src/libs/qmljs/qmljscheck.cpp b/src/libs/qmljs/qmljscheck.cpp index 528c49a056..20d9e40efc 100644 --- a/src/libs/qmljs/qmljscheck.cpp +++ b/src/libs/qmljs/qmljscheck.cpp @@ -37,6 +37,8 @@ #include "qmljsutils.h" #include "parser/qmljsast_p.h" +#include + #include #include #include @@ -44,13 +46,14 @@ using namespace QmlJS; using namespace QmlJS::AST; +using namespace QmlJS::StaticAnalysis; namespace { class AssignmentCheck : public ValueVisitor { public: - DiagnosticMessage operator()( + Message operator()( const Document::Ptr &document, const SourceLocation &location, const Value *lhsValue, @@ -58,8 +61,8 @@ public: Node *ast) { _doc = document; - _message = DiagnosticMessage(DiagnosticMessage::Error, location, QString()); _rhsValue = rhsValue; + _location = location; if (ExpressionStatement *expStmt = cast(ast)) _ast = expStmt->expression; else @@ -71,6 +74,11 @@ public: return _message; } + void setMessage(Type type) + { + _message = Message(type, _location); + } + virtual void visit(const NumberValue *value) { if (const QmlEnumValue *enumValue = dynamic_cast(value)) { @@ -78,16 +86,16 @@ public: const QString valueName = stringLiteral->value.toString(); if (!enumValue->keys().contains(valueName)) { - _message.message = Check::tr("unknown value for enum"); + setMessage(ErrInvalidEnumValue); } } else if (! _rhsValue->asStringValue() && ! _rhsValue->asNumberValue() && ! _rhsValue->asUndefinedValue()) { - _message.message = Check::tr("enum value is not a string or number"); + setMessage(ErrEnumValueMustBeStringOrNumber); } } else { if (cast(_ast) || cast(_ast)) { - _message.message = Check::tr("numerical value expected"); + setMessage(ErrNumberValueExpected); } } } @@ -99,7 +107,7 @@ public: if (cast(_ast) || cast(_ast) || (unaryMinus && cast(unaryMinus->expression))) { - _message.message = Check::tr("boolean value expected"); + setMessage(ErrBooleanValueExpected); } } @@ -111,14 +119,14 @@ public: || (unaryMinus && cast(unaryMinus->expression)) || cast(_ast) || cast(_ast)) { - _message.message = Check::tr("string value expected"); + setMessage(ErrStringValueExpected); } if (value && value->asUrlValue()) { if (StringLiteral *literal = cast(_ast)) { QUrl url(literal->value.toString()); if (!url.isValid() && !url.isEmpty()) { - _message.message = Check::tr("not a valid url"); + setMessage(ErrInvalidUrl); } else { QString fileName = url.toLocalFile(); if (!fileName.isEmpty()) { @@ -127,8 +135,7 @@ public: fileName.prepend(_doc->path()); } if (!QFileInfo(fileName).exists()) { - _message.message = Check::tr("file or directory does not exist"); - _message.kind = DiagnosticMessage::Warning; + setMessage(WarnFileOrDirectoryDoesNotExist); } } } @@ -140,7 +147,7 @@ public: { if (StringLiteral *stringLiteral = cast(_ast)) { if (!toQColor(stringLiteral->value.toString()).isValid()) - _message.message = Check::tr("not a valid color"); + setMessage(ErrInvalidColor); } else { visit((StringValue *)0); } @@ -149,11 +156,12 @@ public: virtual void visit(const AnchorLineValue *) { if (! (_rhsValue->asAnchorLineValue() || _rhsValue->asUndefinedValue())) - _message.message = Check::tr("expected anchor line"); + setMessage(ErrAnchorLineExpected); } Document::Ptr _doc; - DiagnosticMessage _message; + Message _message; + SourceLocation _location; const Value *_rhsValue; ExpressionNode *_ast; }; @@ -326,11 +334,11 @@ protected: class MarkUnreachableCode : protected ReachesEndCheck { - QList _messages; + QList _messages; bool _emittedWarning; public: - QList operator()(Node *ast) + QList operator()(Node *ast) { _messages.clear(); check(ast); @@ -353,12 +361,12 @@ protected: return; _emittedWarning = true; - DiagnosticMessage message(DiagnosticMessage::Warning, SourceLocation(), Check::tr("unreachable")); + Message message(WarnUnreachable, SourceLocation()); if (Statement *statement = node->statementCast()) - message.loc = locationFromRange(statement->firstSourceLocation(), statement->lastSourceLocation()); + message.location = locationFromRange(statement->firstSourceLocation(), statement->lastSourceLocation()); else if (ExpressionNode *expr = node->expressionCast()) - message.loc = locationFromRange(expr->firstSourceLocation(), expr->lastSourceLocation()); - if (message.loc.isValid()) + message.location = locationFromRange(expr->firstSourceLocation(), expr->lastSourceLocation()); + if (message.isValid()) _messages += message; } }; @@ -366,10 +374,9 @@ protected: class DeclarationsCheck : protected Visitor { public: - QList operator()(FunctionExpression *function, Check::Options options) + QList operator()(FunctionExpression *function) { clear(); - _options = options; for (FormalParameterList *plist = function->formals; plist; plist = plist->next) { if (!plist->name.isEmpty()) _formalParameterNames += plist->name.toString(); @@ -379,10 +386,9 @@ public: return _messages; } - QList operator()(Node *node, Check::Options options) + QList operator()(Node *node) { clear(); - _options = options; Node::accept(node, this); return _messages; } @@ -418,8 +424,8 @@ protected: bool visit(VariableStatement *ast) { - if (_options & Check::WarnDeclarationsNotStartOfFunction && _seenNonDeclarationStatement) { - warning(ast->declarationKindToken, Check::tr("declarations should be at the start of a function")); + if (_seenNonDeclarationStatement) { + addMessage(HintDeclarationsShouldBeAtStartOfFunction, ast->declarationKindToken); } return true; } @@ -430,21 +436,17 @@ protected: return true; const QString &name = ast->name.toString(); - if (_options & Check::WarnDuplicateDeclaration) { - if (_formalParameterNames.contains(name)) { - warning(ast->identifierToken, Check::tr("already a formal parameter")); - } else if (_declaredFunctions.contains(name)) { - warning(ast->identifierToken, Check::tr("already declared as function")); - } else if (_declaredVariables.contains(name)) { - warning(ast->identifierToken, Check::tr("duplicate declaration")); - } + if (_formalParameterNames.contains(name)) { + addMessage(WarnAlreadyFormalParameter, ast->identifierToken, name); + } else if (_declaredFunctions.contains(name)) { + addMessage(WarnAlreadyFunction, ast->identifierToken, name); + } else if (_declaredVariables.contains(name)) { + addMessage(WarnDuplicateDeclaration, ast->identifierToken, name); } if (_possiblyUndeclaredUses.contains(name)) { - if (_options & Check::WarnUseBeforeDeclaration) { - foreach (const SourceLocation &loc, _possiblyUndeclaredUses.value(name)) { - warning(loc, Check::tr("variable is used before being declared")); - } + foreach (const SourceLocation &loc, _possiblyUndeclaredUses.value(name)) { + addMessage(WarnVarUsedBeforeDeclaration, loc, name); } _possiblyUndeclaredUses.remove(name); } @@ -455,8 +457,8 @@ protected: bool visit(FunctionDeclaration *ast) { - if (_options & Check::WarnDeclarationsNotStartOfFunction &&_seenNonDeclarationStatement) { - warning(ast->functionToken, Check::tr("declarations should be at the start of a function")); + if (_seenNonDeclarationStatement) { + addMessage(HintDeclarationsShouldBeAtStartOfFunction, ast->functionToken); } return visit(static_cast(ast)); @@ -468,22 +470,18 @@ protected: return false; const QString &name = ast->name.toString(); - if (_options & Check::WarnDuplicateDeclaration) { - if (_formalParameterNames.contains(name)) { - warning(ast->identifierToken, Check::tr("already a formal parameter")); - } else if (_declaredVariables.contains(name)) { - warning(ast->identifierToken, Check::tr("already declared as var")); - } else if (_declaredFunctions.contains(name)) { - warning(ast->identifierToken, Check::tr("duplicate declaration")); - } + if (_formalParameterNames.contains(name)) { + addMessage(WarnAlreadyFormalParameter, ast->identifierToken, name); + } else if (_declaredVariables.contains(name)) { + addMessage(WarnAlreadyVar, ast->identifierToken, name); + } else if (_declaredFunctions.contains(name)) { + addMessage(WarnDuplicateDeclaration, ast->identifierToken, name); } if (FunctionDeclaration *decl = cast(ast)) { if (_possiblyUndeclaredUses.contains(name)) { - if (_options & Check::WarnUseBeforeDeclaration) { - foreach (const SourceLocation &loc, _possiblyUndeclaredUses.value(name)) { - warning(loc, Check::tr("function is used before being declared")); - } + foreach (const SourceLocation &loc, _possiblyUndeclaredUses.value(name)) { + addMessage(WarnFunctionUsedBeforeDeclaration, loc, name); } _possiblyUndeclaredUses.remove(name); } @@ -494,13 +492,12 @@ protected: } private: - void warning(const SourceLocation &loc, const QString &message) + void addMessage(Type type, const SourceLocation &loc, const QString &arg1 = QString()) { - _messages.append(DiagnosticMessage(DiagnosticMessage::Warning, loc, message)); + _messages.append(Message(type, loc, arg1)); } - Check::Options _options; - QList _messages; + QList _messages; QStringList _formalParameterNames; QHash _declaredVariables; QHash _declaredFunctions; @@ -510,33 +507,40 @@ private: } // end of anonymous namespace - Check::Check(Document::Ptr doc, const ContextPtr &context) : _doc(doc) , _context(context) , _scopeChain(doc, _context) , _scopeBuilder(&_scopeChain) - , _options(WarnDangerousNonStrictEqualityChecks | WarnBlocks | WarnWith - | WarnVoid | WarnCommaExpression | WarnExpressionStatement - | WarnAssignInCondition | WarnUseBeforeDeclaration | WarnDuplicateDeclaration - | WarnCaseWithoutFlowControlEnd | WarnNonCapitalizedNew - | WarnCallsOfCapitalizedFunctions | WarnUnreachablecode - | ErrCheckTypeErrors) , _lastValue(0) { + _enabledMessages = Message::allMessageTypes().toSet(); + disableMessage(HintAnonymousFunctionSpacing); + disableMessage(HintDeclareVarsInOneLine); + disableMessage(HintDeclarationsShouldBeAtStartOfFunction); } Check::~Check() { } -QList Check::operator()() +QList Check::operator()() { _messages.clear(); Node::accept(_doc->ast(), this); return _messages; } +void Check::enableMessage(Type type) +{ + _enabledMessages.insert(type); +} + +void Check::disableMessage(Type type) +{ + _enabledMessages.remove(type); +} + bool Check::preVisit(Node *ast) { _chain.append(ast); @@ -583,8 +587,7 @@ void Check::checkProperty(UiQualifiedId *qualifiedId) const QString id = toString(qualifiedId); if (id.at(0).isLower()) { if (m_propertyStack.top().contains(id)) { - error(fullLocationForQualifiedId(qualifiedId), - Check::tr("properties can only be assigned once")); + addMessage(ErrPropertiesCanOnlyHaveOneBinding, fullLocationForQualifiedId(qualifiedId)); } m_propertyStack.top().insert(id); } @@ -622,32 +625,25 @@ void Check::visitQmlObject(Node *ast, UiQualifiedId *typeId, const ObjectValue *prototype = _context->lookupType(_doc.data(), typeId); if (!prototype) { typeError = true; - if (_options & ErrCheckTypeErrors) - error(typeErrorLocation, - Check::tr("unknown type")); + addMessage(ErrUnknownComponent, typeErrorLocation); } else { PrototypeIterator iter(prototype, _context); QList prototypes = iter.all(); if (iter.error() != PrototypeIterator::NoError) typeError = true; - if (_options & ErrCheckTypeErrors) { - const ObjectValue *lastPrototype = prototypes.last(); - if (iter.error() == PrototypeIterator::ReferenceResolutionError) { - if (const QmlPrototypeReference *ref = - dynamic_cast(lastPrototype->prototype())) { - error(typeErrorLocation, - Check::tr("could not resolve the prototype %1 of %2").arg( - toString(ref->qmlTypeName()), lastPrototype->className())); - } else { - error(typeErrorLocation, - Check::tr("could not resolve the prototype of %1").arg( - lastPrototype->className())); - } - } else if (iter.error() == PrototypeIterator::CycleError) { - error(typeErrorLocation, - Check::tr("prototype cycle, the last non-repeated object is %1").arg( - lastPrototype->className())); + const ObjectValue *lastPrototype = prototypes.last(); + if (iter.error() == PrototypeIterator::ReferenceResolutionError) { + if (const QmlPrototypeReference *ref = + dynamic_cast(lastPrototype->prototype())) { + addMessage(ErrCouldNotResolvePrototypeOf, typeErrorLocation, + toString(ref->qmlTypeName()), lastPrototype->className()); + } else { + addMessage(ErrCouldNotResolvePrototype, typeErrorLocation, + lastPrototype->className()); } + } else if (iter.error() == PrototypeIterator::CycleError) { + addMessage(ErrPrototypeCycle, typeErrorLocation, + lastPrototype->className()); } } @@ -677,7 +673,7 @@ bool Check::visit(UiScriptBinding *ast) ExpressionStatement *expStmt = cast(ast->statement); if (!expStmt) { - error(loc, Check::tr("expected id")); + addMessage(ErrIdExpected, loc); return false; } @@ -686,19 +682,19 @@ bool Check::visit(UiScriptBinding *ast) id = idExp->name.toString(); } else if (StringLiteral *strExp = cast(expStmt->expression)) { id = strExp->value.toString(); - warning(loc, Check::tr("using string literals for ids is discouraged")); + addMessage(ErrInvalidId, loc); } else { - error(loc, Check::tr("expected id")); + addMessage(ErrIdExpected, loc); return false; } if (id.isEmpty() || (!id.at(0).isLower() && id.at(0) != '_')) { - error(loc, Check::tr("ids must be lower case or start with underscore")); + addMessage(ErrInvalidId, loc); return false; } if (m_idStack.top().contains(id)) { - error(loc, Check::tr("ids must be unique")); + addMessage(ErrDuplicateId, loc); return false; } m_idStack.top().insert(id); @@ -717,9 +713,9 @@ bool Check::visit(UiScriptBinding *ast) const SourceLocation loc = locationFromRange(ast->statement->firstSourceLocation(), ast->statement->lastSourceLocation()); AssignmentCheck assignmentCheck; - DiagnosticMessage message = assignmentCheck(_doc, loc, lhsValue, rhsValue, ast->statement); - if (! message.message.isEmpty()) - _messages += message; + Message message = assignmentCheck(_doc, loc, lhsValue, rhsValue, ast->statement); + if (message.isValid()) + addMessage(message); } checkBindingRhs(ast->statement); @@ -747,7 +743,7 @@ bool Check::visit(UiPublicMember *ast) const QString &name = ast->memberType.toString(); if (!name.isEmpty() && name.at(0).isLower()) { if (!isValidBuiltinPropertyType(name)) - error(ast->typeToken, tr("'%1' is not a valid property type").arg(name)); + addMessage(ErrInvalidPropertyType, ast->typeToken, name); } } @@ -761,47 +757,47 @@ bool Check::visit(UiPublicMember *ast) return false; } -bool Check::visit(IdentifierExpression *ast) +bool Check::visit(IdentifierExpression *) { // currently disabled: too many false negatives return true; - _lastValue = 0; - if (!ast->name.isEmpty()) { - Evaluate evaluator(&_scopeChain); - _lastValue = evaluator.reference(ast); - if (!_lastValue) - error(ast->identifierToken, tr("unknown identifier")); - if (const Reference *ref = value_cast(_lastValue)) { - _lastValue = _context->lookupReference(ref); - if (!_lastValue) - error(ast->identifierToken, tr("could not resolve")); - } - } - return false; +// _lastValue = 0; +// if (!ast->name.isEmpty()) { +// Evaluate evaluator(&_scopeChain); +// _lastValue = evaluator.reference(ast); +// if (!_lastValue) +// addMessage(ErrUnknownIdentifier, ast->identifierToken); +// if (const Reference *ref = value_cast(_lastValue)) { +// _lastValue = _context->lookupReference(ref); +// if (!_lastValue) +// error(ast->identifierToken, tr("could not resolve")); +// } +// } +// return false; } -bool Check::visit(FieldMemberExpression *ast) +bool Check::visit(FieldMemberExpression *) { // currently disabled: too many false negatives return true; - Node::accept(ast->base, this); - if (!_lastValue) - return false; - const ObjectValue *obj = _lastValue->asObjectValue(); - if (!obj) { - error(locationFromRange(ast->base->firstSourceLocation(), ast->base->lastSourceLocation()), - tr("does not have members")); - } - if (!obj || ast->name.isEmpty()) { - _lastValue = 0; - return false; - } - _lastValue = obj->lookupMember(ast->name.toString(), _context); - if (!_lastValue) - error(ast->identifierToken, tr("unknown member")); - return false; +// Node::accept(ast->base, this); +// if (!_lastValue) +// return false; +// const ObjectValue *obj = _lastValue->asObjectValue(); +// if (!obj) { +// error(locationFromRange(ast->base->firstSourceLocation(), ast->base->lastSourceLocation()), +// tr("does not have members")); +// } +// if (!obj || ast->name.isEmpty()) { +// _lastValue = 0; +// return false; +// } +// _lastValue = obj->lookupMember(ast->name.toString(), _context); +// if (!_lastValue) +// error(ast->identifierToken, tr("unknown member")); +// return false; } bool Check::visit(FunctionDeclaration *ast) @@ -812,11 +808,10 @@ bool Check::visit(FunctionDeclaration *ast) bool Check::visit(FunctionExpression *ast) { DeclarationsCheck bodyCheck; - _messages += bodyCheck(ast, _options); - if (_options & WarnUnreachablecode) { - MarkUnreachableCode unreachableCheck; - _messages += unreachableCheck(ast->body); - } + addMessages(bodyCheck(ast)); + + MarkUnreachableCode unreachableCheck; + addMessages(unreachableCheck(ast->body)); Node::accept(ast->formals, this); _scopeBuilder.push(ast); @@ -849,16 +844,12 @@ static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs bool Check::visit(BinaryExpression *ast) { if (ast->op == QSOperator::Equal || ast->op == QSOperator::NotEqual) { - bool warn = _options & WarnAllNonStrictEqualityChecks; - if (!warn && _options & WarnDangerousNonStrictEqualityChecks) { - Evaluate eval(&_scopeChain); - const Value *lhs = eval(ast->left); - const Value *rhs = eval(ast->right); - warn = shouldAvoidNonStrictEqualityCheck(lhs, rhs) - || shouldAvoidNonStrictEqualityCheck(rhs, lhs); - } - if (warn) { - warning(ast->operatorToken, tr("== and != perform type coercion, use === or !== instead to avoid")); + Evaluate eval(&_scopeChain); + const Value *lhs = eval(ast->left); + const Value *rhs = eval(ast->right); + if (shouldAvoidNonStrictEqualityCheck(lhs, rhs) + || shouldAvoidNonStrictEqualityCheck(rhs, lhs)) { + addMessage(MaybeWarnEqualityTypeCoercion, ast->operatorToken); } } return true; @@ -867,8 +858,7 @@ bool Check::visit(BinaryExpression *ast) bool Check::visit(Block *ast) { if (Node *p = parent()) { - if (_options & WarnBlocks - && !cast(p) + if (!cast(p) && !cast(p) && !cast(p) && !cast(p) @@ -882,13 +872,12 @@ bool Check::visit(Block *ast) && !cast(p) && !cast(p) && !cast(p)) { - warning(ast->lbraceToken, tr("blocks do not introduce a new scope, avoid")); + addMessage(WarnBlock, ast->lbraceToken); } if (!ast->statements && (cast(p) || cast(p))) { - warning(locationFromRange(ast->firstSourceLocation(), ast->lastSourceLocation()), - tr("unintentional empty block, use ({}) for empty object literal")); + addMessage(WarnUnintentinalEmptyBlock, ast->firstSourceLocation()); } } return true; @@ -896,25 +885,23 @@ bool Check::visit(Block *ast) bool Check::visit(WithStatement *ast) { - if (_options & WarnWith) - warning(ast->withToken, tr("use of the with statement is not recommended, use a var instead")); + addMessage(WarnWith, ast->withToken); return true; } bool Check::visit(VoidExpression *ast) { - if (_options & WarnVoid) - warning(ast->voidToken, tr("use of void is usually confusing and not recommended")); + addMessage(WarnVoid, ast->voidToken); return true; } bool Check::visit(Expression *ast) { - if (_options & WarnCommaExpression && ast->left && ast->right) { + if (ast->left && ast->right) { Node *p = parent(); if (!cast(p) && !cast(p)) { - warning(ast->commaToken, tr("avoid comma expressions")); + addMessage(WarnComma, ast->commaToken); } } return true; @@ -922,7 +909,7 @@ bool Check::visit(Expression *ast) bool Check::visit(ExpressionStatement *ast) { - if (_options & WarnExpressionStatement && ast->expression) { + if (ast->expression) { bool ok = cast(ast->expression) || cast(ast->expression) || cast(ast->expression) @@ -966,8 +953,8 @@ bool Check::visit(ExpressionStatement *ast) } if (!ok) { - warning(locationFromRange(ast->firstSourceLocation(), ast->lastSourceLocation()), - tr("expression statements should be assignments, calls or delete expressions only")); + addMessage(WarnConfusingExpressionStatement, + locationFromRange(ast->firstSourceLocation(), ast->lastSourceLocation())); } } return true; @@ -1038,14 +1025,12 @@ static QString functionName(ExpressionNode *ast, SourceLocation *location) void Check::checkNewExpression(ExpressionNode *ast) { - if (!(_options & WarnNonCapitalizedNew)) - return; SourceLocation location; const QString name = functionName(ast, &location); if (name.isEmpty()) return; if (!name.at(0).isUpper()) { - warning(location, tr("'new' should only be used with functions that start with an uppercase letter")); + addMessage(WarnNewWithLowercaseFunction, location); } } @@ -1055,11 +1040,27 @@ void Check::checkBindingRhs(Statement *statement) return; DeclarationsCheck bodyCheck; - _messages += bodyCheck(statement, _options); - if (_options & WarnUnreachablecode) { - MarkUnreachableCode unreachableCheck; - _messages += unreachableCheck(statement); - } + addMessages(bodyCheck(statement)); + + MarkUnreachableCode unreachableCheck; + addMessages(unreachableCheck(statement)); +} + +void Check::addMessages(const QList &messages) +{ + foreach (const Message &msg, messages) + addMessage(msg); +} + +void Check::addMessage(const Message &message) +{ + if (message.isValid() && _enabledMessages.contains(message.type)) + _messages += message; +} + +void Check::addMessage(Type type, const SourceLocation &location, const QString &arg1, const QString &arg2) +{ + addMessage(Message(type, location, arg1, arg2)); } bool Check::visit(NewExpression *ast) @@ -1077,12 +1078,10 @@ bool Check::visit(NewMemberExpression *ast) bool Check::visit(CallExpression *ast) { // check for capitalized function name being called - if (_options & WarnCallsOfCapitalizedFunctions) { - SourceLocation location; - const QString name = functionName(ast->base, &location); - if (!name.isEmpty() && name.at(0).isUpper()) { - warning(location, tr("calls of functions that start with an uppercase letter should use 'new'")); - } + SourceLocation location; + const QString name = functionName(ast->base, &location); + if (!name.isEmpty() && name.at(0).isUpper()) { + addMessage(WarnExpectedNewWithUppercaseFunction, location); } return true; } @@ -1126,8 +1125,7 @@ const Value *Check::checkScopeObjectMember(const UiQualifiedId *id) break; } if (!value) { - error(id->identifierToken, - Check::tr("'%1' is not a valid property name").arg(propertyName)); + addMessage(ErrInvalidPropertyName, id->identifierToken, propertyName); return 0; } @@ -1144,8 +1142,7 @@ const Value *Check::checkScopeObjectMember(const UiQualifiedId *id) while (idPart->next) { const ObjectValue *objectValue = value_cast(value); if (! objectValue) { - error(idPart->identifierToken, - Check::tr("'%1' does not have members").arg(propertyName)); + addMessage(ErrDoesNotHaveMembers, idPart->identifierToken, propertyName); return 0; } @@ -1160,9 +1157,7 @@ const Value *Check::checkScopeObjectMember(const UiQualifiedId *id) value = objectValue->lookupMember(propertyName, _context); if (! value) { - error(idPart->identifierToken, - Check::tr("'%1' is not a member of '%2'").arg( - propertyName, objectValue->className())); + addMessage(ErrInvalidMember, idPart->identifierToken, propertyName, objectValue->className()); return 0; } } @@ -1172,35 +1167,23 @@ const Value *Check::checkScopeObjectMember(const UiQualifiedId *id) void Check::checkAssignInCondition(AST::ExpressionNode *condition) { - if (_options & WarnAssignInCondition) { - if (BinaryExpression *binary = cast(condition)) { - if (binary->op == QSOperator::Assign) - warning(binary->operatorToken, tr("avoid assignments in conditions")); - } + if (BinaryExpression *binary = cast(condition)) { + if (binary->op == QSOperator::Assign) + addMessage(WarnAssignmentInCondition, binary->operatorToken); } } void Check::checkEndsWithControlFlow(StatementList *statements, SourceLocation errorLoc) { - if (!statements || !(_options & WarnCaseWithoutFlowControlEnd)) + if (!statements) return; ReachesEndCheck check; if (check(statements)) { - warning(errorLoc, tr("case is not terminated and not empty")); + addMessage(WarnCaseWithoutFlowControl, errorLoc); } } -void Check::error(const AST::SourceLocation &loc, const QString &message) -{ - _messages.append(DiagnosticMessage(DiagnosticMessage::Error, loc, message)); -} - -void Check::warning(const AST::SourceLocation &loc, const QString &message) -{ - _messages.append(DiagnosticMessage(DiagnosticMessage::Warning, loc, message)); -} - Node *Check::parent(int distance) { const int index = _chain.size() - 2 - distance; diff --git a/src/libs/qmljs/qmljscheck.h b/src/libs/qmljs/qmljscheck.h index 11ac979ca4..3826b089d6 100644 --- a/src/libs/qmljs/qmljscheck.h +++ b/src/libs/qmljs/qmljscheck.h @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -57,33 +58,10 @@ public: Check(Document::Ptr doc, const ContextPtr &context); virtual ~Check(); - QList operator()(); - - enum Option { - WarnDangerousNonStrictEqualityChecks = 1 << 0, - WarnAllNonStrictEqualityChecks = 1 << 1, - WarnBlocks = 1 << 2, - WarnWith = 1 << 3, - WarnVoid = 1 << 4, - WarnCommaExpression = 1 << 5, - WarnExpressionStatement = 1 << 6, - WarnAssignInCondition = 1 << 7, - WarnUseBeforeDeclaration = 1 << 8, - WarnDuplicateDeclaration = 1 << 9, - WarnDeclarationsNotStartOfFunction = 1 << 10, - WarnCaseWithoutFlowControlEnd = 1 << 11, - WarnNonCapitalizedNew = 1 << 12, - WarnCallsOfCapitalizedFunctions = 1 << 13, - WarnUnreachablecode = 1 << 14, - ErrCheckTypeErrors = 1 << 15 - }; - Q_DECLARE_FLAGS(Options, Option) - - const Options options() const - { return _options; } - - void setOptions(Options options) - { _options = options; } + QList operator()(); + + void enableMessage(StaticAnalysis::Type type); + void disableMessage(StaticAnalysis::Type type); protected: virtual bool preVisit(AST::Node *ast); @@ -130,8 +108,10 @@ private: void checkNewExpression(AST::ExpressionNode *node); void checkBindingRhs(AST::Statement *statement); - void warning(const AST::SourceLocation &loc, const QString &message); - void error(const AST::SourceLocation &loc, const QString &message); + void addMessages(const QList &messages); + void addMessage(const StaticAnalysis::Message &message); + void addMessage(StaticAnalysis::Type type, const AST::SourceLocation &location, + const QString &arg1 = QString(), const QString &arg2 = QString()); AST::Node *parent(int distance = 0); @@ -141,9 +121,8 @@ private: ScopeChain _scopeChain; ScopeBuilder _scopeBuilder; - QList _messages; - - Options _options; + QList _messages; + QSet _enabledMessages; const Value *_lastValue; QList _chain; diff --git a/src/libs/qmljs/qmljsstaticanalysismessage.cpp b/src/libs/qmljs/qmljsstaticanalysismessage.cpp new file mode 100644 index 0000000000..7b27c235a9 --- /dev/null +++ b/src/libs/qmljs/qmljsstaticanalysismessage.cpp @@ -0,0 +1,247 @@ +/************************************************************************** +** +** This file is part of Qt Creator +** +** Copyright (c) 2011 Nokia Corporation and/or its subsidiary(-ies). +** +** Contact: Nokia Corporation (info@qt.nokia.com) +** +** +** GNU Lesser General Public License Usage +** +** This file may be used under the terms of the GNU Lesser General Public +** License version 2.1 as published by the Free Software Foundation and +** appearing in the file LICENSE.LGPL included in the packaging of this file. +** Please review the following information to ensure the GNU Lesser General +** Public License version 2.1 requirements will be met: +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** Other Usage +** +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** If you have questions regarding the use of this file, please contact +** Nokia at info@qt.nokia.com. +** +**************************************************************************/ + +#include "qmljsstaticanalysismessage.h" + +#include + +#include + +using namespace QmlJS; +using namespace QmlJS::StaticAnalysis; + +namespace { + +class StaticAnalysisMessages +{ + Q_DECLARE_TR_FUNCTIONS(StaticAnalysisMessages) + +public: + class PrototypeMessageData { + public: + Type type; + Severity severity; + QString message; + int placeholders; + }; + + void newMsg(Type type, Severity severity, const QString &message, int placeholders = 0) + { + PrototypeMessageData prototype; + prototype.type = type; + prototype.severity = severity; + prototype.message = message; + prototype.placeholders = placeholders; + QTC_CHECK(placeholders <= 2); + QTC_ASSERT(!messages.contains(type), return); + messages[type] = prototype; + } + + StaticAnalysisMessages(); + QHash messages; +}; + +StaticAnalysisMessages::StaticAnalysisMessages() +{ + newMsg(ErrInvalidEnumValue, Error, + tr("invalid value for enum")); + newMsg(ErrEnumValueMustBeStringOrNumber, Error, + tr("enum value must be a string or a number")); + newMsg(ErrNumberValueExpected, Error, + tr("number value expected")); + newMsg(ErrBooleanValueExpected, Error, + tr("boolean value expected")); + newMsg(ErrStringValueExpected, Error, + tr("string value expected")); + newMsg(ErrInvalidUrl, Error, + tr("invalid URL")); + newMsg(WarnFileOrDirectoryDoesNotExist, Warning, + tr("file or directory does not exist")); + newMsg(ErrInvalidColor, Error, + tr("invalid color")); + newMsg(ErrAnchorLineExpected, Error, + tr("anchor line expected")); + newMsg(ErrPropertiesCanOnlyHaveOneBinding, Error, + tr("duplicate property binding")); + newMsg(ErrIdExpected, Error, + tr("id expected")); + newMsg(ErrInvalidId, Error, + tr("invalid id")); + newMsg(ErrDuplicateId, Error, + tr("duplicate id")); + newMsg(ErrInvalidPropertyName, Error, + tr("invalid property name '%1'"), 1); + newMsg(ErrDoesNotHaveMembers, Error, + tr("'%1' does not have members"), 1); + newMsg(ErrInvalidMember, Error, + tr("'%1' is not a member of '%2'"), 2); + newMsg(WarnAssignmentInCondition, Warning, + tr("assignment in condition")); + newMsg(WarnCaseWithoutFlowControl, Warning, + tr("unterminated non-empty case block")); + newMsg(WarnEval, Warning, + tr("do not use 'eval'")); + newMsg(WarnUnreachable, Warning, + tr("unreachable")); + newMsg(WarnWith, Warning, + tr("do not use 'with'")); + newMsg(WarnComma, Warning, + tr("do not use comma expressions")); + newMsg(WarnAlreadyFormalParameter, Warning, + tr("'%1' is already a formal parameter"), 1); + newMsg(WarnAlreadyFunction, Warning, + tr("'%1' is already a function"), 1); + newMsg(WarnVarUsedBeforeDeclaration, Warning, + tr("var '%1' is used before its declaration"), 1); + newMsg(WarnAlreadyVar, Warning, + tr("'%1' is already a var"), 1); + newMsg(WarnDuplicateDeclaration, Warning, + tr("'%1' is declared more than once"), 1); + newMsg(WarnFunctionUsedBeforeDeclaration, Warning, + tr("function '%1' is used before its declaration"), 1); + newMsg(WarnBooleanConstructor, Warning, + tr("do not use 'Boolean' as a constructor")); + newMsg(WarnStringConstructor, Warning, + tr("do not use 'String' as a constructor")); + newMsg(WarnObjectConstructor, Warning, + tr("do not use 'Object' as a constructor")); + newMsg(WarnArrayConstructor, Warning, + tr("do not use 'Array' as a constructor")); + newMsg(WarnFunctionConstructor, Warning, + tr("do not use 'Function' as a constructor")); + newMsg(HintAnonymousFunctionSpacing, Hint, + tr("the 'function' keyword and the opening parenthesis should be separated by a single space")); + newMsg(WarnBlock, Warning, + tr("do not use stand-alone blocks")); + newMsg(WarnVoid, Warning, + tr("do not use void expressions")); + newMsg(WarnConfusingPluses, Warning, + tr("confusing pluses")); + newMsg(WarnConfusingPreincrement, Warning, + tr("confusing preincrement")); + newMsg(WarnConfusingMinuses, Warning, + tr("confusing minuses")); + newMsg(WarnConfusingPredecrement, Warning, + tr("confusing predecrement")); + newMsg(HintDeclareVarsInOneLine, Hint, + tr("declare all function vars on a single line")); + // unused +// newMsg(HintExtraParentheses, Hint, +// tr("")); + newMsg(MaybeWarnEqualityTypeCoercion, MaybeWarning, + tr("== and != may perform type coercion, use === or !== to avoid")); + newMsg(WarnConfusingExpressionStatement, Warning, + tr("expression statements should be assignments, calls or delete expressions only")); + newMsg(HintDeclarationsShouldBeAtStartOfFunction, Error, + tr("var declarations should be at the start of a function")); + newMsg(HintOneStatementPerLine, Error, + tr("only use one statement per line")); + newMsg(ErrUnknownComponent, Error, + tr("unknown component")); + newMsg(ErrCouldNotResolvePrototypeOf, Error, + tr("could not resolve the prototype '%1'' of '%2'"), 2); + newMsg(ErrCouldNotResolvePrototype, Error, + tr("could not resolve the prototype '%1'"), 1); + newMsg(ErrPrototypeCycle, Error, + tr("prototype cycle, the last non-repeated component is '%1'"), 1); + newMsg(ErrInvalidPropertyType, Error, + tr("invalid property type '%1'"), 1); + newMsg(WarnEqualityTypeCoercion, Warning, + tr("== and != perform type coercion, use === or !== to avoid")); + newMsg(WarnExpectedNewWithUppercaseFunction, Warning, + tr("calls of functions that start with an uppercase letter should use 'new'")); + newMsg(WarnNewWithLowercaseFunction, Warning, + tr("'new' should only be used with functions that start with an uppercase letter")); + newMsg(WarnNumberConstructor, Warning, + tr("do not use 'Number' as a constructor")); + newMsg(HintBinaryOperatorSpacing, Hint, + tr("use spaces around binary operators")); + newMsg(WarnUnintentinalEmptyBlock, Warning, + tr("unintentional empty block, use ({}) for empty object literal")); +} + +} // anonymous namespace + +Q_GLOBAL_STATIC(StaticAnalysisMessages, messages) + +QList Message::allMessageTypes() +{ + return messages()->messages.keys(); +} + +Message::Message() + : type(UnknownType), severity(Hint) +{} + +Message::Message(Type type, AST::SourceLocation location, const QString &arg1, const QString &arg2) + : location(location), type(type) +{ + QTC_ASSERT(messages()->messages.contains(type), return); + const StaticAnalysisMessages::PrototypeMessageData &prototype = messages()->messages.value(type); + severity = prototype.severity; + message = prototype.message; + if (prototype.placeholders == 0) { + if (!arg1.isEmpty() || !arg2.isEmpty()) + qWarning() << "StaticAnalysis message" << type << "expects no arguments"; + } else if (prototype.placeholders == 1) { + if (arg1.isEmpty() || !arg2.isEmpty()) + qWarning() << "StaticAnalysis message" << type << "expects exactly one argument"; + message = message.arg(arg1); + } else if (prototype.placeholders == 2) { + if (arg1.isEmpty() || arg2.isEmpty()) + qWarning() << "StaticAnalysis message" << type << "expects exactly two arguments"; + message = message.arg(arg1, arg2); + } +} + +bool Message::isValid() const +{ + return type != UnknownType && location.isValid() && !message.isEmpty(); +} + +DiagnosticMessage Message::toDiagnosticMessage() const +{ + DiagnosticMessage diagnostic; + switch (severity) { + case Hint: + case MaybeWarning: + case Warning: + diagnostic.kind = DiagnosticMessage::Warning; + break; + default: + diagnostic.kind = DiagnosticMessage::Error; + break; + } + diagnostic.loc = location; + diagnostic.message = message; + return diagnostic; +} diff --git a/src/libs/qmljs/qmljsstaticanalysismessage.h b/src/libs/qmljs/qmljsstaticanalysismessage.h new file mode 100644 index 0000000000..6e9b0c39cc --- /dev/null +++ b/src/libs/qmljs/qmljsstaticanalysismessage.h @@ -0,0 +1,135 @@ +/************************************************************************** +** +** This file is part of Qt Creator +** +** Copyright (c) 2011 Nokia Corporation and/or its subsidiary(-ies). +** +** Contact: Nokia Corporation (info@qt.nokia.com) +** +** +** GNU Lesser General Public License Usage +** +** This file may be used under the terms of the GNU Lesser General Public +** License version 2.1 as published by the Free Software Foundation and +** appearing in the file LICENSE.LGPL included in the packaging of this file. +** Please review the following information to ensure the GNU Lesser General +** Public License version 2.1 requirements will be met: +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** In addition, as a special exception, Nokia gives you certain additional +** rights. These rights are described in the Nokia Qt LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** Other Usage +** +** Alternatively, this file may be used in accordance with the terms and +** conditions contained in a signed written agreement between you and Nokia. +** +** If you have questions regarding the use of this file, please contact +** Nokia at info@qt.nokia.com. +** +**************************************************************************/ + +#ifndef QMLJS_STATICANALYSIS_QMLJSSTATICANALYSISMESSAGE_H +#define QMLJS_STATICANALYSIS_QMLJSSTATICANALYSISMESSAGE_H + +#include "qmljs_global.h" +#include "parser/qmljsengine_p.h" + +namespace QmlJS { +namespace StaticAnalysis { + +enum Severity +{ + Hint, // cosmetic or convention + MaybeWarning, // possibly a warning, insufficient information + Warning, // could cause unintended behavior + MaybeError, // possibly an error, insufficient information + Error // definitely an error +}; + +enum Type +{ + UnknownType = 0, + ErrInvalidEnumValue = 1, + ErrEnumValueMustBeStringOrNumber = 2, + ErrNumberValueExpected = 3, + ErrBooleanValueExpected = 4, + ErrStringValueExpected = 5, + ErrInvalidUrl = 6, + WarnFileOrDirectoryDoesNotExist = 7, + ErrInvalidColor = 8, + ErrAnchorLineExpected = 9, + ErrPropertiesCanOnlyHaveOneBinding = 10, + ErrIdExpected = 11, + ErrInvalidId = 14, + ErrDuplicateId = 15, + ErrInvalidPropertyName = 16, + ErrDoesNotHaveMembers = 17, + ErrInvalidMember = 18, + WarnAssignmentInCondition = 19, + WarnCaseWithoutFlowControl = 20, + WarnEval = 23, + WarnUnreachable = 28, + WarnWith = 29, + WarnComma = 30, + WarnAlreadyFormalParameter = 103, + WarnAlreadyFunction = 104, + WarnVarUsedBeforeDeclaration = 105, + WarnAlreadyVar = 106, + WarnDuplicateDeclaration = 107, + WarnFunctionUsedBeforeDeclaration = 108, + WarnBooleanConstructor = 109, + WarnStringConstructor = 110, + WarnObjectConstructor = 111, + WarnArrayConstructor = 112, + WarnFunctionConstructor = 113, + HintAnonymousFunctionSpacing = 114, + WarnBlock = 115, + WarnVoid = 116, + WarnConfusingPluses = 117, + WarnConfusingPreincrement = 118, + WarnConfusingMinuses = 119, + WarnConfusingPredecrement = 120, + HintDeclareVarsInOneLine = 121, + HintExtraParentheses = 123, + MaybeWarnEqualityTypeCoercion = 126, + WarnConfusingExpressionStatement = 127, + HintDeclarationsShouldBeAtStartOfFunction = 201, + HintOneStatementPerLine = 202, + ErrUnknownComponent = 300, + ErrCouldNotResolvePrototypeOf = 301, + ErrCouldNotResolvePrototype = 302, + ErrPrototypeCycle = 303, + ErrInvalidPropertyType = 304, + WarnEqualityTypeCoercion = 305, + WarnExpectedNewWithUppercaseFunction = 306, + WarnNewWithLowercaseFunction = 307, + WarnNumberConstructor = 308, + HintBinaryOperatorSpacing = 309, + WarnUnintentinalEmptyBlock = 310 +}; + +class QMLJS_EXPORT Message +{ +public: + Message(); + Message(Type type, AST::SourceLocation location, + const QString &arg1 = QString(), + const QString &arg2 = QString()); + + static QList allMessageTypes(); + + bool isValid() const; + DiagnosticMessage toDiagnosticMessage() const; + + AST::SourceLocation location; + QString message; + Type type; + Severity severity; +}; + +} // namespace StaticAnalysis +} // namespace QmlJS + +#endif // QMLJS_STATICANALYSIS_QMLJSSTATICANALYSISMESSAGE_H diff --git a/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp b/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp index 30da2a8e80..607dcfce86 100644 --- a/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp +++ b/src/plugins/qmldesigner/designercore/model/texttomodelmerger.cpp @@ -765,10 +765,13 @@ bool TextToModelMerger::load(const QString &data, DifferenceHandler &differenceH if (view()->checkSemanticErrors()) { Check check(doc, m_scopeChain->context()); - check.setOptions(check.options() & ~Check::ErrCheckTypeErrors); - foreach (const QmlJS::DiagnosticMessage &diagnosticMessage, check()) { - if (diagnosticMessage.isError()) - errors.append(RewriterView::Error(diagnosticMessage, QUrl::fromLocalFile(doc->fileName()))); + check.disableMessage(StaticAnalysis::ErrUnknownComponent); + check.disableMessage(StaticAnalysis::ErrPrototypeCycle); + check.disableMessage(StaticAnalysis::ErrCouldNotResolvePrototype); + check.disableMessage(StaticAnalysis::ErrCouldNotResolvePrototypeOf); + foreach (const StaticAnalysis::Message &message, check()) { + if (message.severity == StaticAnalysis::Error) + errors.append(RewriterView::Error(message.toDiagnosticMessage(), QUrl::fromLocalFile(doc->fileName()))); } if (!errors.isEmpty()) { diff --git a/src/plugins/qmljseditor/qmljssemanticinfoupdater.cpp b/src/plugins/qmljseditor/qmljssemanticinfoupdater.cpp index 8bc7c47a58..3e327dc005 100644 --- a/src/plugins/qmljseditor/qmljssemanticinfoupdater.cpp +++ b/src/plugins/qmljseditor/qmljssemanticinfoupdater.cpp @@ -147,7 +147,9 @@ SemanticInfo SemanticInfoUpdater::semanticInfo(const SemanticInfoUpdaterSource & semanticInfo.m_rootScopeChain = QSharedPointer(scopeChain); QmlJS::Check checker(doc, semanticInfo.context); - semanticInfo.semanticMessages.append(checker()); + foreach (const QmlJS::StaticAnalysis::Message &msg, checker()) { + semanticInfo.semanticMessages += msg.toDiagnosticMessage(); + } return semanticInfo; } diff --git a/src/plugins/qmljseditor/qmltaskmanager.cpp b/src/plugins/qmljseditor/qmltaskmanager.cpp index 2a59adc8f6..74604a338e 100644 --- a/src/plugins/qmljseditor/qmltaskmanager.cpp +++ b/src/plugins/qmljseditor/qmltaskmanager.cpp @@ -87,6 +87,14 @@ static QList convertToTasks(const QList convertToTasks(const QList &messages, const QString &fileName, const QString &category) +{ + QList diagnostics; + foreach (const StaticAnalysis::Message &msg, messages) + diagnostics += msg.toDiagnosticMessage(); + return convertToTasks(diagnostics, fileName, category); +} + void QmlTaskManager::collectMessages( QFutureInterface &future, Snapshot snapshot, QList projectInfos, diff --git a/tests/auto/qml/codemodel/check/equality-checks.qml b/tests/auto/qml/codemodel/check/equality-checks.qml index 7213b6b3af..700331780f 100644 --- a/tests/auto/qml/codemodel/check/equality-checks.qml +++ b/tests/auto/qml/codemodel/check/equality-checks.qml @@ -1,8 +1,8 @@ import Qt 4.7 -// DEFAULTMSG == and != perform type coercion, use === or !== instead to avoid + Rectangle { onXChanged: { - if (0 == undefined) {} // W 15 16 + if (0 == undefined) {} // 126 15 16 } function foo() { @@ -14,47 +14,47 @@ Rectangle { var o = {} if (s == s) {} - if (s == n) {} // W 15 16 + if (s == n) {} // 126 15 16 if (s == N) {} // ### should warn: always false - if (s == u) {} // W 15 16 - if (s == b) {} // W 15 16 - if (s == o) {} // W 15 16 + if (s == u) {} // 126 15 16 + if (s == b) {} // 126 15 16 + if (s == o) {} // 126 15 16 - if (n == s) {} // W 15 16 + if (n == s) {} // 126 15 16 if (n == n) {} if (n == N) {} // ### should warn: always false - if (n == u) {} // W 15 16 - if (n == b) {} // W 15 16 - if (n == o) {} // W 15 16 + if (n == u) {} // 126 15 16 + if (n == b) {} // 126 15 16 + if (n == o) {} // 126 15 16 if (N == s) {} // ### should warn: always false if (N == n) {} // ### should warn: always false if (N == N) {} - if (N == u) {} // W 15 16 + if (N == u) {} // 126 15 16 // ### should warn: always false - if (N == b) {} // W 15 16 + if (N == b) {} // 126 15 16 if (N == o) {} // ### should warn: always false - if (u == s) {} // W 15 16 - if (u == n) {} // W 15 16 - if (u == N) {} // W 15 16 - if (u == u) {} // W 15 16 - if (u == b) {} // W 15 16 - if (u == o) {} // W 15 16 + if (u == s) {} // 126 15 16 + if (u == n) {} // 126 15 16 + if (u == N) {} // 126 15 16 + if (u == u) {} // 126 15 16 + if (u == b) {} // 126 15 16 + if (u == o) {} // 126 15 16 - if (b == s) {} // W 15 16 - if (b == n) {} // W 15 16 + if (b == s) {} // 126 15 16 + if (b == n) {} // 126 15 16 // ### should warn: always false - if (b == N) {} // W 15 16 - if (b == u) {} // W 15 16 + if (b == N) {} // 126 15 16 + if (b == u) {} // 126 15 16 if (b == b) {} - if (b == o) {} // W 15 16 + if (b == o) {} // 126 15 16 - if (o == s) {} // W 15 16 - if (o == n) {} // W 15 16 + if (o == s) {} // 126 15 16 + if (o == n) {} // 126 15 16 if (o == N) {} // ### should warn: always false - if (o == u) {} // W 15 16 - if (o == b) {} // W 15 16 + if (o == u) {} // 126 15 16 + if (o == b) {} // 126 15 16 if (o == o) {} } } diff --git a/tests/auto/qml/codemodel/check/expression-statement.qml b/tests/auto/qml/codemodel/check/expression-statement.qml index 94bcae8041..f3ea9d8ced 100644 --- a/tests/auto/qml/codemodel/check/expression-statement.qml +++ b/tests/auto/qml/codemodel/check/expression-statement.qml @@ -1,13 +1,13 @@ import Qt 4.7 -// DEFAULTMSG expression statements should be assignments, calls or delete expressions only + Rectangle { function foo() { - a // W 9 9 - a + b // W 9 13 + a // 127 9 9 + a + b // 127 9 13 a() delete b a = 12 a += 12 - d().foo // W 9 15 + d().foo // 127 9 15 } } diff --git a/tests/auto/qml/codemodel/check/new-expression.qml b/tests/auto/qml/codemodel/check/new-expression.qml index f1db2bc6a5..31cc4458f4 100644 --- a/tests/auto/qml/codemodel/check/new-expression.qml +++ b/tests/auto/qml/codemodel/check/new-expression.qml @@ -1,17 +1,15 @@ import Qt 4.7 Item { - // DEFAULTMSG 'new' should only be used with functions that start with an uppercase letter function foo() { a = new A a = new A() - a = new a // W 17 17 - a = new a() // W 17 17 + a = new a // 307 17 17 + a = new a() // 307 17 17 } - // DEFAULTMSG calls of functions that start with an uppercase letter should use 'new' function foo() { - a = A() // W 13 13 + a = A() // 306 13 13 a = a() } } diff --git a/tests/auto/qml/codemodel/check/tst_check.cpp b/tests/auto/qml/codemodel/check/tst_check.cpp index 16c020ab0a..d782ab19b7 100644 --- a/tests/auto/qml/codemodel/check/tst_check.cpp +++ b/tests/auto/qml/codemodel/check/tst_check.cpp @@ -51,6 +51,7 @@ using namespace QmlJS; using namespace QmlJS::AST; +using namespace QmlJS::StaticAnalysis; class tst_Check : public QObject { @@ -89,9 +90,9 @@ void tst_Check::initTestCase() CppQmlTypesLoader::defaultQtObjects = CppQmlTypesLoader::loadQmlTypes(QFileInfoList() << builtins, &errors, &warnings); } -static bool offsetComparator(DiagnosticMessage lhs, DiagnosticMessage rhs) +static bool offsetComparator(const Message &lhs, const Message &rhs) { - return lhs.loc.offset < rhs.loc.offset; + return lhs.location.offset < rhs.location.offset; } #define QCOMPARE_NOEXIT(actual, expected) \ @@ -127,72 +128,52 @@ void tst_Check::test() ContextPtr context = Link(snapshot, QStringList(), LibraryInfo())(); Check checker(doc, context); - QList messages = checker(); + QList messages = checker(); std::sort(messages.begin(), messages.end(), &offsetComparator); - const QRegExp errorPattern(" E (\\d+) (\\d+)(.*)"); - const QRegExp warningPattern(" W (\\d+) (\\d+)(.*)"); - const QRegExp defaultmsgPattern(" DEFAULTMSG (.*)"); + const QRegExp messagePattern(" (\\d+) (\\d+) (\\d+)"); - QList expectedMessages; - QString defaultMessage; + QList expectedMessages; foreach (const AST::SourceLocation &comment, doc->engine()->comments()) { const QString text = doc->source().mid(comment.begin(), comment.end() - comment.begin()); - if (defaultmsgPattern.indexIn(text) != -1) { - defaultMessage = defaultmsgPattern.cap(1); + if (messagePattern.indexIn(text) == -1) continue; - } - - const QRegExp *match = 0; - DiagnosticMessage::Kind kind = DiagnosticMessage::Error; - if (errorPattern.indexIn(text) != -1) { - match = &errorPattern; - } else if (warningPattern.indexIn(text) != -1) { - kind = DiagnosticMessage::Warning; - match = &warningPattern; - } - if (!match) - continue; - const int columnStart = match->cap(1).toInt(); - const int columnEnd = match->cap(2).toInt() + 1; - QString message = match->cap(3); - if (message.isEmpty()) - message = defaultMessage; - else - message = message.mid(1); - expectedMessages += DiagnosticMessage( - kind, - SourceLocation( - comment.offset - comment.startColumn + columnStart, - columnEnd - columnStart, - comment.startLine, - columnStart), - message); + const int type = messagePattern.cap(1).toInt(); + const int columnStart = messagePattern.cap(2).toInt(); + const int columnEnd = messagePattern.cap(3).toInt() + 1; + + Message message; + message.location = SourceLocation( + comment.offset - comment.startColumn + columnStart, + columnEnd - columnStart, + comment.startLine, + columnStart), + message.type = static_cast(type); + expectedMessages += message; } for (int i = 0; i < messages.size(); ++i) { - DiagnosticMessage expected; + Message expected; if (i < expectedMessages.size()) expected = expectedMessages.at(i); - DiagnosticMessage actual = messages.at(i); + Message actual = messages.at(i); bool fail = false; - fail |= !QCOMPARE_NOEXIT(actual.message, expected.message); - fail |= !QCOMPARE_NOEXIT(actual.loc.startLine, expected.loc.startLine); + fail |= !QCOMPARE_NOEXIT(actual.location.startLine, expected.location.startLine); if (fail) return; - fail |= !QCOMPARE_NOEXIT(actual.kind, expected.kind); - fail |= !QCOMPARE_NOEXIT(actual.loc.startColumn, expected.loc.startColumn); - fail |= !QCOMPARE_NOEXIT(actual.loc.offset, expected.loc.offset); - fail |= !QCOMPARE_NOEXIT(actual.loc.length, expected.loc.length); + fail |= !QCOMPARE_NOEXIT(actual.type, expected.type); + fail |= !QCOMPARE_NOEXIT(actual.location.startColumn, expected.location.startColumn); + fail |= !QCOMPARE_NOEXIT(actual.location.offset, expected.location.offset); + fail |= !QCOMPARE_NOEXIT(actual.location.length, expected.location.length); if (fail) { - qDebug() << "Failed for message on line" << actual.loc.startLine << actual.message; + qDebug() << "Failed for message on line" << actual.location.startLine << actual.message; return; } } if (expectedMessages.size() > messages.size()) { - DiagnosticMessage missingMessage = expectedMessages.at(messages.size()); - qDebug() << "expected more messages: " << missingMessage.loc.startLine << missingMessage.message; + Message missingMessage = expectedMessages.at(messages.size()); + qDebug() << "expected more messages: " << missingMessage.location.startLine << missingMessage.message; QFAIL("more messages expected"); } } diff --git a/tests/auto/qml/codemodel/check/unreachable.qml b/tests/auto/qml/codemodel/check/unreachable.qml index 2843765fed..b3802214cc 100644 --- a/tests/auto/qml/codemodel/check/unreachable.qml +++ b/tests/auto/qml/codemodel/check/unreachable.qml @@ -3,13 +3,13 @@ import Qt 4.7 Item { function foo() { return - x() // W 9 11 + x() // 28 9 11 x() } function foo() { throw new Object() - x() // W 9 11 + x() // 28 9 11 x() } @@ -26,14 +26,14 @@ Item { return else return - x() // W 9 11 + x() // 28 9 11 } function foo() { try { throw 1 } finally {} - x() // W 9 11 + x() // 28 9 11 } function foo() { @@ -41,7 +41,7 @@ Item { } finally { return } - x() // W 9 11 + x() // 28 9 11 } function foo() { @@ -60,7 +60,7 @@ Item { } catch(a) { return } - x() // W 9 11 + x() // 28 9 11 } function foo() { @@ -85,7 +85,7 @@ Item { default: return } - x() // W 9 11 + x() // 28 9 11 } function foo() { @@ -97,7 +97,7 @@ Item { l3: do { break l1 } while (b); - x() // W 13 15 + x() // 28 13 15 } while (a); x() // reachable via break } diff --git a/tests/auto/qml/codemodel/check/useless-blocks.qml b/tests/auto/qml/codemodel/check/useless-blocks.qml index 6d81c57287..0db7570597 100644 --- a/tests/auto/qml/codemodel/check/useless-blocks.qml +++ b/tests/auto/qml/codemodel/check/useless-blocks.qml @@ -1,17 +1,17 @@ import Qt 4.7 -// DEFAULTMSG blocks do not introduce a new scope, avoid + Rectangle { function foo() { - {} // W 9 9 + {} // 115 9 9 if (a) {} } onXChanged: { - {} // W 9 9 + {} // 115 9 9 while (A) {} } property int d: { - {} // W 9 9 + {} // 115 9 9 } } -- 2.11.0