summaryrefslogtreecommitdiff
path: root/src/libs/qmljs/qmljscheck.cpp
diff options
context:
space:
mode:
Diffstat (limited to 'src/libs/qmljs/qmljscheck.cpp')
-rw-r--r--src/libs/qmljs/qmljscheck.cpp746
1 files changed, 448 insertions, 298 deletions
diff --git a/src/libs/qmljs/qmljscheck.cpp b/src/libs/qmljs/qmljscheck.cpp
index 59ddb40333..07a9d24732 100644
--- a/src/libs/qmljs/qmljscheck.cpp
+++ b/src/libs/qmljs/qmljscheck.cpp
@@ -34,8 +34,11 @@
#include "qmljsbind.h"
#include "qmljscontext.h"
#include "qmljsevaluate.h"
+#include "qmljsutils.h"
#include "parser/qmljsast_p.h"
+#include <utils/qtcassert.h>
+
#include <QtCore/QDebug>
#include <QtCore/QDir>
#include <QtGui/QColor>
@@ -43,99 +46,14 @@
using namespace QmlJS;
using namespace QmlJS::AST;
-
-QColor QmlJS::toQColor(const QString &qmlColorString)
-{
- QColor color;
- if (qmlColorString.size() == 9 && qmlColorString.at(0) == QLatin1Char('#')) {
- bool ok;
- const int alpha = qmlColorString.mid(1, 2).toInt(&ok, 16);
- if (ok) {
- QString name(qmlColorString.at(0));
- name.append(qmlColorString.right(6));
- if (QColor::isValidColor(name)) {
- color.setNamedColor(name);
- color.setAlpha(alpha);
- }
- }
- } else {
- if (QColor::isValidColor(qmlColorString))
- color.setNamedColor(qmlColorString);
- }
- return color;
-}
-
-SourceLocation QmlJS::locationFromRange(const SourceLocation &start,
- const SourceLocation &end)
-{
- return SourceLocation(start.offset,
- end.end() - start.begin(),
- start.startLine,
- start.startColumn);
-}
-
-SourceLocation QmlJS::fullLocationForQualifiedId(AST::UiQualifiedId *qualifiedId)
-{
- SourceLocation start = qualifiedId->identifierToken;
- SourceLocation end = qualifiedId->identifierToken;
-
- for (UiQualifiedId *iter = qualifiedId; iter; iter = iter->next) {
- if (iter->identifierToken.isValid())
- end = iter->identifierToken;
- }
-
- return locationFromRange(start, end);
-}
-
-
-DiagnosticMessage QmlJS::errorMessage(const AST::SourceLocation &loc, const QString &message)
-{
- return DiagnosticMessage(DiagnosticMessage::Error, loc, message);
-}
-
-namespace {
-class SharedData
-{
-public:
- SharedData()
- {
- validBuiltinPropertyNames.insert(QLatin1String("action"));
- validBuiltinPropertyNames.insert(QLatin1String("bool"));
- validBuiltinPropertyNames.insert(QLatin1String("color"));
- validBuiltinPropertyNames.insert(QLatin1String("date"));
- validBuiltinPropertyNames.insert(QLatin1String("double"));
- validBuiltinPropertyNames.insert(QLatin1String("enumeration"));
- validBuiltinPropertyNames.insert(QLatin1String("font"));
- validBuiltinPropertyNames.insert(QLatin1String("int"));
- validBuiltinPropertyNames.insert(QLatin1String("list"));
- validBuiltinPropertyNames.insert(QLatin1String("point"));
- validBuiltinPropertyNames.insert(QLatin1String("real"));
- validBuiltinPropertyNames.insert(QLatin1String("rect"));
- validBuiltinPropertyNames.insert(QLatin1String("size"));
- validBuiltinPropertyNames.insert(QLatin1String("string"));
- validBuiltinPropertyNames.insert(QLatin1String("time"));
- validBuiltinPropertyNames.insert(QLatin1String("url"));
- validBuiltinPropertyNames.insert(QLatin1String("variant"));
- validBuiltinPropertyNames.insert(QLatin1String("vector3d"));
- validBuiltinPropertyNames.insert(QLatin1String("alias"));
- }
-
- QSet<QString> validBuiltinPropertyNames;
-};
-} // anonymous namespace
-Q_GLOBAL_STATIC(SharedData, sharedData)
-
-bool QmlJS::isValidBuiltinPropertyType(const QString &name)
-{
- return sharedData()->validBuiltinPropertyNames.contains(name);
-}
+using namespace QmlJS::StaticAnalysis;
namespace {
class AssignmentCheck : public ValueVisitor
{
public:
- DiagnosticMessage operator()(
+ Message operator()(
const Document::Ptr &document,
const SourceLocation &location,
const Value *lhsValue,
@@ -143,8 +61,8 @@ public:
Node *ast)
{
_doc = document;
- _message = DiagnosticMessage(DiagnosticMessage::Error, location, QString());
_rhsValue = rhsValue;
+ _location = location;
if (ExpressionStatement *expStmt = cast<ExpressionStatement *>(ast))
_ast = expStmt->expression;
else
@@ -156,23 +74,28 @@ public:
return _message;
}
+ void setMessage(Type type)
+ {
+ _message = Message(type, _location);
+ }
+
virtual void visit(const NumberValue *value)
{
- if (const QmlEnumValue *enumValue = dynamic_cast<const QmlEnumValue *>(value)) {
+ if (const QmlEnumValue *enumValue = value_cast<QmlEnumValue>(value)) {
if (StringLiteral *stringLiteral = cast<StringLiteral *>(_ast)) {
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");
+ && ! _rhsValue->asUnknownValue()) {
+ setMessage(ErrEnumValueMustBeStringOrNumber);
}
} else {
if (cast<TrueLiteral *>(_ast)
|| cast<FalseLiteral *>(_ast)) {
- _message.message = Check::tr("numerical value expected");
+ setMessage(ErrNumberValueExpected);
}
}
}
@@ -184,7 +107,7 @@ public:
if (cast<StringLiteral *>(_ast)
|| cast<NumericLiteral *>(_ast)
|| (unaryMinus && cast<NumericLiteral *>(unaryMinus->expression))) {
- _message.message = Check::tr("boolean value expected");
+ setMessage(ErrBooleanValueExpected);
}
}
@@ -196,14 +119,14 @@ public:
|| (unaryMinus && cast<NumericLiteral *>(unaryMinus->expression))
|| cast<TrueLiteral *>(_ast)
|| cast<FalseLiteral *>(_ast)) {
- _message.message = Check::tr("string value expected");
+ setMessage(ErrStringValueExpected);
}
if (value && value->asUrlValue()) {
if (StringLiteral *literal = cast<StringLiteral *>(_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()) {
@@ -212,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);
}
}
}
@@ -225,7 +147,7 @@ public:
{
if (StringLiteral *stringLiteral = cast<StringLiteral *>(_ast)) {
if (!toQColor(stringLiteral->value.toString()).isValid())
- _message.message = Check::tr("not a valid color");
+ setMessage(ErrInvalidColor);
} else {
visit((StringValue *)0);
}
@@ -233,12 +155,13 @@ public:
virtual void visit(const AnchorLineValue *)
{
- if (! (_rhsValue->asAnchorLineValue() || _rhsValue->asUndefinedValue()))
- _message.message = Check::tr("expected anchor line");
+ if (! (_rhsValue->asAnchorLineValue() || _rhsValue->asUnknownValue()))
+ setMessage(ErrAnchorLineExpected);
}
Document::Ptr _doc;
- DiagnosticMessage _message;
+ Message _message;
+ SourceLocation _location;
const Value *_rhsValue;
ExpressionNode *_ast;
};
@@ -411,11 +334,11 @@ protected:
class MarkUnreachableCode : protected ReachesEndCheck
{
- QList<DiagnosticMessage> _messages;
+ QList<Message> _messages;
bool _emittedWarning;
public:
- QList<DiagnosticMessage> operator()(Node *ast)
+ QList<Message> operator()(Node *ast)
{
_messages.clear();
check(ast);
@@ -438,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;
}
};
@@ -451,10 +374,9 @@ protected:
class DeclarationsCheck : protected Visitor
{
public:
- QList<DiagnosticMessage> operator()(FunctionExpression *function, Check::Options options)
+ QList<Message> operator()(FunctionExpression *function)
{
clear();
- _options = options;
for (FormalParameterList *plist = function->formals; plist; plist = plist->next) {
if (!plist->name.isEmpty())
_formalParameterNames += plist->name.toString();
@@ -464,10 +386,9 @@ public:
return _messages;
}
- QList<DiagnosticMessage> operator()(Node *node, Check::Options options)
+ QList<Message> operator()(Node *node)
{
clear();
- _options = options;
Node::accept(node, this);
return _messages;
}
@@ -503,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;
}
@@ -515,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);
}
@@ -540,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<FunctionExpression *>(ast));
@@ -553,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<FunctionDeclaration *>(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);
}
@@ -579,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<DiagnosticMessage> _messages;
+ QList<Message> _messages;
QStringList _formalParameterNames;
QHash<QString, VariableDeclaration *> _declaredVariables;
QHash<QString, FunctionDeclaration *> _declaredFunctions;
@@ -595,33 +507,52 @@ 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)
+ , _importsOk(false)
{
+ const Imports *imports = context->imports(doc.data());
+ if (imports && !imports->importFailed())
+ _importsOk = true;
+
+ _enabledMessages = Message::allMessageTypes().toSet();
+ disableMessage(HintAnonymousFunctionSpacing);
+ disableMessage(HintDeclareVarsInOneLine);
+ disableMessage(HintDeclarationsShouldBeAtStartOfFunction);
+ disableMessage(HintBinaryOperatorSpacing);
+ disableMessage(HintOneStatementPerLine);
+ disableMessage(HintExtraParentheses);
}
Check::~Check()
{
}
-QList<DiagnosticMessage> Check::operator()()
+QList<Message> Check::operator()()
{
_messages.clear();
+ scanCommentsForAnnotations();
+
Node::accept(_doc->ast(), this);
+ warnAboutUnnecessarySuppressions();
+
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);
@@ -665,11 +596,10 @@ void Check::endVisit(UiObjectInitializer *)
void Check::checkProperty(UiQualifiedId *qualifiedId)
{
- const QString id = Bind::toString(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);
}
@@ -703,35 +633,30 @@ void Check::visitQmlObject(Node *ast, UiQualifiedId *typeId,
}
bool typeError = false;
- const SourceLocation typeErrorLocation = fullLocationForQualifiedId(typeId);
- const ObjectValue *prototype = _context->lookupType(_doc.data(), typeId);
- if (!prototype) {
- typeError = true;
- if (_options & ErrCheckTypeErrors)
- error(typeErrorLocation,
- Check::tr("unknown type"));
- } else {
- PrototypeIterator iter(prototype, _context);
- QList<const ObjectValue *> prototypes = iter.all();
- if (iter.error() != PrototypeIterator::NoError)
+ if (_importsOk) {
+ const SourceLocation typeErrorLocation = fullLocationForQualifiedId(typeId);
+ const ObjectValue *prototype = _context->lookupType(_doc.data(), typeId);
+ if (!prototype) {
typeError = true;
- if (_options & ErrCheckTypeErrors) {
+ addMessage(ErrUnknownComponent, typeErrorLocation);
+ } else {
+ PrototypeIterator iter(prototype, _context);
+ QList<const ObjectValue *> prototypes = iter.all();
+ if (iter.error() != PrototypeIterator::NoError)
+ typeError = true;
const ObjectValue *lastPrototype = prototypes.last();
if (iter.error() == PrototypeIterator::ReferenceResolutionError) {
if (const QmlPrototypeReference *ref =
- dynamic_cast<const QmlPrototypeReference *>(lastPrototype->prototype())) {
- error(typeErrorLocation,
- Check::tr("could not resolve the prototype %1 of %2").arg(
- Bind::toString(ref->qmlTypeName()), lastPrototype->className()));
+ value_cast<QmlPrototypeReference>(lastPrototype->prototype())) {
+ addMessage(ErrCouldNotResolvePrototypeOf, typeErrorLocation,
+ toString(ref->qmlTypeName()), lastPrototype->className());
} else {
- error(typeErrorLocation,
- Check::tr("could not resolve the prototype of %1").arg(
- lastPrototype->className()));
+ addMessage(ErrCouldNotResolvePrototype, typeErrorLocation,
+ lastPrototype->className());
}
} else if (iter.error() == PrototypeIterator::CycleError) {
- error(typeErrorLocation,
- Check::tr("prototype cycle, the last non-repeated object is %1").arg(
- lastPrototype->className()));
+ addMessage(ErrPrototypeCycle, typeErrorLocation,
+ lastPrototype->className());
}
}
}
@@ -762,7 +687,7 @@ bool Check::visit(UiScriptBinding *ast)
ExpressionStatement *expStmt = cast<ExpressionStatement *>(ast->statement);
if (!expStmt) {
- error(loc, Check::tr("expected id"));
+ addMessage(ErrIdExpected, loc);
return false;
}
@@ -771,19 +696,19 @@ bool Check::visit(UiScriptBinding *ast)
id = idExp->name.toString();
} else if (StringLiteral *strExp = cast<StringLiteral *>(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);
@@ -802,9 +727,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);
@@ -827,66 +752,83 @@ bool Check::visit(UiArrayBinding *ast)
bool Check::visit(UiPublicMember *ast)
{
- // check if the member type is valid
- if (!ast->memberType.isEmpty()) {
- 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));
+ if (ast->type == UiPublicMember::Property) {
+ // check if the member type is valid
+ if (!ast->memberType.isEmpty()) {
+ const QString &name = ast->memberType.toString();
+ if (!name.isEmpty() && name.at(0).isLower()) {
+ if (!isValidBuiltinPropertyType(name))
+ addMessage(ErrInvalidPropertyType, ast->typeToken, name);
+ }
+
+ // warn about dubious use of var/variant
+ if (name == QLatin1String("variant") || name == QLatin1String("var")) {
+ Evaluate evaluator(&_scopeChain);
+ const Value *init = evaluator(ast->statement);
+ QString preferedType;
+ if (init->asNumberValue())
+ preferedType = tr("'int' or 'real'");
+ if (init->asStringValue())
+ preferedType = QLatin1String("'string'");
+ if (init->asBooleanValue())
+ preferedType = QLatin1String("'bool'");
+ if (!preferedType.isEmpty())
+ addMessage(HintPreferNonVarPropertyType, ast->typeToken, preferedType);
+ }
}
- }
- checkBindingRhs(ast->statement);
+ checkBindingRhs(ast->statement);
- _scopeBuilder.push(ast);
- Node::accept(ast->statement, this);
- Node::accept(ast->binding, this);
- _scopeBuilder.pop();
+ _scopeBuilder.push(ast);
+ Node::accept(ast->statement, this);
+ Node::accept(ast->binding, this);
+ _scopeBuilder.pop();
+ }
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<const Reference *>(_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<Reference>(_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)
@@ -896,13 +838,22 @@ 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);
+ if (ast->name.isEmpty()) {
+ SourceLocation locfunc = ast->functionToken;
+ SourceLocation loclparen = ast->lparenToken;
+ if (locfunc.isValid() && loclparen.isValid()
+ && (locfunc.startLine != loclparen.startLine
+ || locfunc.end() + 1 != loclparen.begin())) {
+ addMessage(HintAnonymousFunctionSpacing, locationFromRange(locfunc, loclparen));
+ }
}
+ DeclarationsCheck bodyCheck;
+ addMessages(bodyCheck(ast));
+
+ MarkUnreachableCode unreachableCheck;
+ addMessages(unreachableCheck(ast->body));
+
Node::accept(ast->formals, this);
_scopeBuilder.push(ast);
Node::accept(ast->body, this);
@@ -912,9 +863,8 @@ bool Check::visit(FunctionExpression *ast)
static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs)
{
- // we currently use undefined as a "we don't know" value
- if (lhs->asUndefinedValue() || rhs->asUndefinedValue())
- return true;
+ if (lhs->asUnknownValue() || rhs->asUnknownValue())
+ return true; // may coerce or not
if (lhs->asStringValue() && rhs->asNumberValue())
return true; // coerces string to number
@@ -925,7 +875,8 @@ static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs
if (lhs->asObjectValue() && rhs->asStringValue())
return true; // coerces object to primitive
- if (lhs->asBooleanValue() && !rhs->asBooleanValue())
+ if (lhs->asBooleanValue() && (!rhs->asBooleanValue()
+ && !rhs->asUndefinedValue()))
return true; // coerces bool to number
return false;
@@ -933,27 +884,61 @@ static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs
bool Check::visit(BinaryExpression *ast)
{
+ const QString source = _doc->source();
+
+ // check spacing
+ SourceLocation op = ast->operatorToken;
+ if ((op.begin() > 0 && !source.at(op.begin() - 1).isSpace())
+ || (int(op.end()) < source.size() && !source.at(op.end()).isSpace())) {
+ addMessage(HintBinaryOperatorSpacing, op);
+ }
+
+ // check ==, !=
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);
+ Evaluate eval(&_scopeChain);
+ const Value *lhsValue = eval(ast->left);
+ const Value *rhsValue = eval(ast->right);
+ if (shouldAvoidNonStrictEqualityCheck(lhsValue, rhsValue)
+ || shouldAvoidNonStrictEqualityCheck(rhsValue, lhsValue)) {
+ addMessage(MaybeWarnEqualityTypeCoercion, ast->operatorToken);
}
- if (warn) {
- warning(ast->operatorToken, tr("== and != perform type coercion, use === or !== instead to avoid"));
+ }
+
+ // check odd + ++ combinations
+ const QLatin1Char newline('\n');
+ if (ast->op == QSOperator::Add || ast->op == QSOperator::Sub) {
+ QChar match;
+ Type msg;
+ if (ast->op == QSOperator::Add) {
+ match = QLatin1Char('+');
+ msg = WarnConfusingPluses;
+ } else {
+ QTC_CHECK(ast->op == QSOperator::Sub);
+ match = QLatin1Char('-');
+ msg = WarnConfusingMinuses;
+ }
+
+ if (int(op.end()) + 1 < source.size()) {
+ const QChar next = source.at(op.end());
+ if (next.isSpace() && next != newline
+ && source.at(op.end() + 1) == match)
+ addMessage(msg, SourceLocation(op.begin(), 3, op.startLine, op.startColumn));
+ }
+ if (op.begin() >= 2) {
+ const QChar prev = source.at(op.begin() - 1);
+ if (prev.isSpace() && prev != newline
+ && source.at(op.begin() - 2) == match)
+ addMessage(msg, SourceLocation(op.begin() - 2, 3, op.startLine, op.startColumn - 2));
}
}
+
return true;
}
bool Check::visit(Block *ast)
{
if (Node *p = parent()) {
- if (_options & WarnBlocks
- && !cast<UiScriptBinding *>(p)
+ if (!cast<UiScriptBinding *>(p)
&& !cast<UiPublicMember *>(p)
&& !cast<TryStatement *>(p)
&& !cast<Catch *>(p)
@@ -967,12 +952,12 @@ bool Check::visit(Block *ast)
&& !cast<IfStatement *>(p)
&& !cast<SwitchStatement *>(p)
&& !cast<WithStatement *>(p)) {
- warning(ast->lbraceToken, tr("blocks do not introduce a new scope, avoid"));
+ addMessage(WarnBlock, ast->lbraceToken);
}
if (!ast->statements
- && cast<UiPublicMember *>(p)) {
- warning(locationFromRange(ast->firstSourceLocation(), ast->lastSourceLocation()),
- tr("unintentional empty block, use ({}) for empty object literal"));
+ && cast<UiPublicMember *>(p)
+ && ast->lbraceToken.startLine == ast->rbraceToken.startLine) {
+ addMessage(WarnUnintentinalEmptyBlock, locationFromRange(ast->firstSourceLocation(), ast->lastSourceLocation()));
}
}
return true;
@@ -980,25 +965,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<ForStatement *>(p)
&& !cast<LocalForStatement *>(p)) {
- warning(ast->commaToken, tr("avoid comma expressions"));
+ addMessage(WarnComma, ast->commaToken);
}
}
return true;
@@ -1006,7 +989,7 @@ bool Check::visit(Expression *ast)
bool Check::visit(ExpressionStatement *ast)
{
- if (_options & WarnExpressionStatement && ast->expression) {
+ if (ast->expression) {
bool ok = cast<CallExpression *>(ast->expression)
|| cast<DeleteExpression *>(ast->expression)
|| cast<PreDecrementExpression *>(ast->expression)
@@ -1050,8 +1033,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;
@@ -1122,14 +1105,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);
}
}
@@ -1139,10 +1120,110 @@ 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::checkExtraParentheses(ExpressionNode *expression)
+{
+ if (NestedExpression *nested = cast<NestedExpression *>(expression)) {
+ addMessage(HintExtraParentheses, nested->lparenToken);
+ }
+}
+
+void Check::addMessages(const QList<Message> &messages)
+{
+ foreach (const Message &msg, messages)
+ addMessage(msg);
+}
+
+static bool hasOnlySpaces(const QString &s)
+{
+ for (int i = 0; i < s.size(); ++i)
+ if (!s.at(i).isSpace())
+ return false;
+ return true;
+}
+
+void Check::addMessage(const Message &message)
+{
+ if (message.isValid() && _enabledMessages.contains(message.type)) {
+ if (m_disabledMessageTypesByLine.contains(message.location.startLine)) {
+ QList<MessageTypeAndSuppression> &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;
+ }
+ }
+ }
+
+ _messages += message;
+ }
+}
+
+void Check::addMessage(Type type, const SourceLocation &location, const QString &arg1, const QString &arg2)
+{
+ addMessage(Message(type, location, arg1, arg2));
+}
+
+void Check::scanCommentsForAnnotations()
+{
+ m_disabledMessageTypesByLine.clear();
+ const QRegExp disableCommentPattern(Message::suppressionPattern());
+
+ foreach (const SourceLocation &commentLoc, _doc->engine()->comments()) {
+ const QString &comment = _doc->source().mid(commentLoc.begin(), commentLoc.length);
+
+ // enable all checks annotation
+ if (comment.contains(QLatin1String("@enable-all-checks"))) {
+ _enabledMessages = Message::allMessageTypes().toSet();
+ }
+
+ // find all disable annotations
+ int lastOffset = -1;
+ QList<MessageTypeAndSuppression> disabledMessageTypes;
+ forever {
+ lastOffset = disableCommentPattern.indexIn(comment, lastOffset + 1);
+ if (lastOffset == -1)
+ break;
+ MessageTypeAndSuppression entry;
+ entry.type = static_cast<StaticAnalysis::Type>(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<MessageTypeAndSuppression> > it(m_disabledMessageTypesByLine);
+ while (it.hasNext()) {
+ it.next();
+ foreach (const MessageTypeAndSuppression &entry, it.value()) {
+ if (!entry.wasSuppressed)
+ addMessage(WarnUnnecessaryMessageSuppression, entry.suppressionSource);
+ }
}
}
@@ -1155,19 +1236,101 @@ bool Check::visit(NewExpression *ast)
bool Check::visit(NewMemberExpression *ast)
{
checkNewExpression(ast->base);
+
+ // check for Number, Boolean, etc constructor usage
+ if (IdentifierExpression *idExp = cast<IdentifierExpression *>(ast->base)) {
+ const QStringRef name = idExp->name;
+ if (name == QLatin1String("Number")) {
+ addMessage(WarnNumberConstructor, idExp->identifierToken);
+ } else if (name == QLatin1String("Boolean")) {
+ addMessage(WarnBooleanConstructor, idExp->identifierToken);
+ } else if (name == QLatin1String("String")) {
+ addMessage(WarnStringConstructor, idExp->identifierToken);
+ } else if (name == QLatin1String("Object")) {
+ addMessage(WarnObjectConstructor, idExp->identifierToken);
+ } else if (name == QLatin1String("Array")) {
+ bool ok = false;
+ if (ast->arguments && ast->arguments->expression && !ast->arguments->next) {
+ Evaluate evaluate(&_scopeChain);
+ const Value *arg = evaluate(ast->arguments->expression);
+ if (arg->asNumberValue() || arg->asUnknownValue())
+ ok = true;
+ }
+ if (!ok)
+ addMessage(WarnArrayConstructor, idExp->identifierToken);
+ } else if (name == QLatin1String("Function")) {
+ addMessage(WarnFunctionConstructor, idExp->identifierToken);
+ }
+ }
+
return true;
}
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()
+ && name != QLatin1String("String")
+ && name != QLatin1String("Boolean")
+ && name != QLatin1String("Date")
+ && name != QLatin1String("Number")
+ && name != QLatin1String("Object")) {
+ addMessage(WarnExpectedNewWithUppercaseFunction, location);
+ }
+ if (cast<IdentifierExpression *>(ast->base) && name == QLatin1String("eval"))
+ addMessage(WarnEval, location);
+ return true;
+}
+
+bool Check::visit(StatementList *ast)
+{
+ SourceLocation warnStart;
+ SourceLocation warnEnd;
+ unsigned currentLine = 0;
+ for (StatementList *it = ast; it; it = it->next) {
+ if (!it->statement)
+ continue;
+ const SourceLocation itLoc = it->statement->firstSourceLocation();
+ if (itLoc.startLine != currentLine) { // first statement on a line
+ if (warnStart.isValid())
+ addMessage(HintOneStatementPerLine, locationFromRange(warnStart, warnEnd));
+ warnStart = SourceLocation();
+ currentLine = itLoc.startLine;
+ } else { // other statements on the same line
+ if (!warnStart.isValid())
+ warnStart = itLoc;
+ warnEnd = it->statement->lastSourceLocation();
}
}
+ if (warnStart.isValid())
+ addMessage(HintOneStatementPerLine, locationFromRange(warnStart, warnEnd));
+
+ return true;
+}
+
+bool Check::visit(ReturnStatement *ast)
+{
+ checkExtraParentheses(ast->expression);
+ return true;
+}
+
+bool Check::visit(ThrowStatement *ast)
+{
+ checkExtraParentheses(ast->expression);
+ return true;
+}
+
+bool Check::visit(DeleteExpression *ast)
+{
+ checkExtraParentheses(ast->expression);
+ return true;
+}
+
+bool Check::visit(TypeOfExpression *ast)
+{
+ checkExtraParentheses(ast->expression);
return true;
}
@@ -1176,6 +1339,9 @@ bool Check::visit(CallExpression *ast)
/// ### Maybe put this into the context as a helper method.
const Value *Check::checkScopeObjectMember(const UiQualifiedId *id)
{
+ if (!_importsOk)
+ return 0;
+
QList<const ObjectValue *> scopeObjects = _scopeChain.qmlScopeObjects();
if (scopeObjects.isEmpty())
return 0;
@@ -1210,8 +1376,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;
}
@@ -1226,10 +1391,9 @@ const Value *Check::checkScopeObjectMember(const UiQualifiedId *id)
// member lookup
const UiQualifiedId *idPart = id;
while (idPart->next) {
- const ObjectValue *objectValue = value_cast<const ObjectValue *>(value);
+ const ObjectValue *objectValue = value_cast<ObjectValue>(value);
if (! objectValue) {
- error(idPart->identifierToken,
- Check::tr("'%1' does not have members").arg(propertyName));
+ addMessage(ErrDoesNotHaveMembers, idPart->identifierToken, propertyName);
return 0;
}
@@ -1244,9 +1408,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;
}
}
@@ -1256,35 +1418,23 @@ const Value *Check::checkScopeObjectMember(const UiQualifiedId *id)
void Check::checkAssignInCondition(AST::ExpressionNode *condition)
{
- if (_options & WarnAssignInCondition) {
- if (BinaryExpression *binary = cast<BinaryExpression *>(condition)) {
- if (binary->op == QSOperator::Assign)
- warning(binary->operatorToken, tr("avoid assignments in conditions"));
- }
+ if (BinaryExpression *binary = cast<BinaryExpression *>(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;