Commit 367e27cd authored by Christian Kamm's avatar Christian Kamm

QmlJS checks: Correct the check for dangerous == and add tests.

Change-Id: Ie0f4062069bf241020868af34ce6d36146b4b0c7
Reviewed-on: http://codereview.qt-project.org/4646Reviewed-by: default avatarThomas Hartmann <Thomas.Hartmann@nokia.com>
parent 97ad582c
......@@ -895,22 +895,24 @@ bool Check::visit(FunctionExpression *ast)
return false;
}
static bool shouldAvoidNonStrictEqualityCheck(ExpressionNode *exp, const Value *other)
static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs)
{
if (NumericLiteral *literal = cast<NumericLiteral *>(exp)) {
if (literal->value == 0 && !other->asNumberValue())
return true;
} else if ((cast<TrueLiteral *>(exp) || cast<FalseLiteral *>(exp)) && !other->asBooleanValue()) {
return true;
} else if (cast<NullExpression *>(exp)) {
// we currently use undefined as a "we don't know" value
if (lhs->asUndefinedValue() || rhs->asUndefinedValue())
return true;
} else if (IdentifierExpression *ident = cast<IdentifierExpression *>(exp)) {
if (ident->name && ident->name->asString() == QLatin1String("undefined"))
return true;
} else if (StringLiteral *literal = cast<StringLiteral *>(exp)) {
if ((!literal->value || literal->value->asString().isEmpty()) && !other->asStringValue())
return true;
}
if (lhs->asStringValue() && rhs->asNumberValue())
return true; // coerces string to number
if (lhs->asObjectValue() && rhs->asNumberValue())
return true; // coerces object to primitive
if (lhs->asObjectValue() && rhs->asStringValue())
return true; // coerces object to primitive
if (lhs->asBooleanValue() && !rhs->asBooleanValue())
return true; // coerces bool to number
return false;
}
......@@ -922,8 +924,8 @@ bool Check::visit(BinaryExpression *ast)
Evaluate eval(&_scopeChain);
const Value *lhs = eval(ast->left);
const Value *rhs = eval(ast->right);
warn = shouldAvoidNonStrictEqualityCheck(ast->left, rhs)
|| shouldAvoidNonStrictEqualityCheck(ast->right, lhs);
warn = shouldAvoidNonStrictEqualityCheck(lhs, rhs)
|| shouldAvoidNonStrictEqualityCheck(rhs, lhs);
}
if (warn) {
warning(ast->operatorToken, tr("== and != perform type coercion, use === or !== instead to avoid"));
......
......@@ -267,6 +267,8 @@ bool Evaluate::visit(AST::ArrayLiteral *)
bool Evaluate::visit(AST::ObjectLiteral *)
{
// ### properties
_result = _valueOwner->newObject();
return false;
}
......
......@@ -6,20 +6,55 @@ Rectangle {
}
function foo() {
var st = ""
var nu = 0
var un = undefined
if (st == "") {} // no warning
if (nu == "") {} // W 16 17
if (un == "") {} // W 16 17
if (st == 0) {} // W 16 17
if (nu == 0) {} // no warning
if (un == 0) {} // W 16 17
if (st == null) {} // W 16 17
if (nu == null) {} // W 16 17
if (un == null) {} // W 16 17
if (st == undefined) {} // W 16 17
if (nu == undefined) {} // W 16 17
if (un == undefined) {} // W 16 17
var s = ""
var n = 0
var N = null
var u = undefined
var b = true
var o = {}
if (s == s) {}
if (s == n) {} // W 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 (n == s) {} // W 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 == s) {} // ### should warn: always false
if (N == n) {} // ### should warn: always false
if (N == N) {}
if (N == u) {} // W 15 16
// ### should warn: always false
if (N == b) {} // W 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 (b == s) {} // W 15 16
if (b == n) {} // W 15 16
// ### should warn: always false
if (b == N) {} // W 15 16
if (b == u) {} // W 15 16
if (b == b) {}
if (b == o) {} // W 15 16
if (o == s) {} // W 15 16
if (o == n) {} // W 15 16
if (o == N) {} // ### should warn: always false
if (o == u) {} // W 15 16
if (o == b) {} // W 15 16
if (o == o) {}
}
}
......@@ -171,9 +171,10 @@ void tst_Check::test()
message);
}
QCOMPARE(expectedMessages.size(), messages.size());
for (int i = 0; i < messages.size(); ++i) {
DiagnosticMessage expected = expectedMessages.at(i);
DiagnosticMessage expected;
if (i < expectedMessages.size())
expected = expectedMessages.at(i);
DiagnosticMessage actual = messages.at(i);
bool fail = false;
fail |= !QCOMPARE_NOEXIT(actual.message, expected.message);
......@@ -189,6 +190,11 @@ void tst_Check::test()
return;
}
}
if (expectedMessages.size() > messages.size()) {
DiagnosticMessage missingMessage = expectedMessages.at(messages.size());
qDebug() << "expected more messages: " << missingMessage.loc.startLine << missingMessage.message;
QFAIL("more messages expected");
}
}
QTEST_MAIN(tst_Check);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment