feedc0de/qt-creator
1
0
Fork 0
forked from qt-creator/qt-creator
Code Pull Requests Activity

qmljs: add check for comparisons not depending on values

Browse Source 
currently we mainly try to warn about primitive == null/undefined or
primitive === non primitive.
There are other that we could warn about null==null null==undefined,
but I feel that they might be triggered too much by clean code.

Change-Id: Id43d838d60a4e13f361be34e4bb38211777a081e
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Fawzi Mohamed
2021-01-29 23:20:31 +01:00
parent cc00af8334
commit 7015ef04b7
4 changed files with 116 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
src/libs/qmljs/qmljscheck.cpp
View File

@@ -1275,6 +1275,30 @@ static bool shouldAvoidNonStrictEqualityCheck(const Value *lhs, const Value *rhs
return false;
}
static bool equalIsAlwaysFalse(const Value *lhs, const Value *rhs)
{
if ((lhs->asNullValue() || lhs->asUndefinedValue())
&& (rhs->asNumberValue() || rhs->asBooleanValue() || rhs->asStringValue()))
return true;
return false;
}
static bool strictCompareConstant(const Value *lhs, const Value *rhs)
{
if (lhs->asUnknownValue() || rhs->asUnknownValue())
return false;
if (lhs->asBooleanValue() && !rhs->asBooleanValue())
return true;
if (lhs->asNumberValue() && !rhs->asNumberValue())
return true;
if (lhs->asStringValue() && !rhs->asStringValue())
return true;
if (lhs->asObjectValue() && (!rhs->asObjectValue() || !rhs->asNullValue() || !rhs->asUndefinedValue()))
return true;
return false;
}
bool Check::visit(BinaryExpression *ast)
{
const QString source = _doc->source();
@@ -1300,6 +1324,18 @@ bool Check::visit(BinaryExpression *ast)
|| shouldAvoidNonStrictEqualityCheck(rhsValue, lhsValue)) {
addMessage(MaybeWarnEqualityTypeCoercion, ast->operatorToken);
}
if (equalIsAlwaysFalse(lhsValue, rhsValue)
|| equalIsAlwaysFalse(rhsValue, lhsValue))
addMessage(WarnLogicalValueDoesNotDependOnValues, ast->operatorToken);
}
if (ast->op == QSOperator::StrictEqual || ast->op == QSOperator::StrictNotEqual) {
Evaluate eval(&_scopeChain);
const Value *lhsValue = eval(ast->left);
const Value *rhsValue = eval(ast->right);
if (strictCompareConstant(lhsValue, rhsValue)
|| strictCompareConstant(rhsValue, lhsValue)) {
addMessage(WarnLogicalValueDoesNotDependOnValues, ast->operatorToken);
}
}
// check odd + ++ combinations

2
src/libs/qmljs/qmljsstaticanalysismessage.cpp
View File

@@ -247,6 +247,8 @@ StaticAnalysisMessages::StaticAnalysisMessages()
tr("Hit maximum recursion limit when visiting AST."));
newMsg(ErrTypeIsInstantiatedRecursively, Error,
tr("Type cannot be instantiated recursively (%1)."), 1);
newMsg(WarnLogicalValueDoesNotDependOnValues, Warning,
tr("Logical value does not depend on actual values"));
}
} // anonymous namespace

1
src/libs/qmljs/qmljsstaticanalysismessage.h
View File

@@ -131,6 +131,7 @@ enum Type
ErrShorterStringValueExpected = 322,
ErrInvalidArrayValueLength = 323,
ErrHitMaximumRecursion = 324,
WarnLogicalValueDoesNotDependOnValues = 325,
WarnDuplicateImport = 400
};

89
tests/auto/qml/codemodel/check/equality-checks.qml
View File

@@ -12,42 +12,42 @@ Rectangle {
if (s == s) {}
if (s == n) {} // 126 15 16
if (s == N) {}
if (s == u) {}
if (s == N) {} // 325 15 16
if (s == u) {} // 325 15 16
if (s == b) {} // 126 15 16
if (s == o) {} // 126 15 16
if (s == k) {} // 126 15 16
if (n == s) {} // 126 15 16
if (n == n) {}
if (n == N) {}
if (n == u) {}
if (n == N) {} // 325 15 16
if (n == u) {} // 325 15 16
if (n == b) {} // 126 15 16
if (n == o) {} // 126 15 16
if (n == k) {} // 126 15 16
if (N == s) {}
if (N == n) {}
if (N == s) {} // 325 15 16
if (N == n) {} // 325 15 16
if (N == N) {}
if (N == u) {}
if (N == b) {}
if (N == b) {} // 325 15 16
if (N == o) {}
if (N == k) {} // 126 15 16
if (u == s) {}
if (u == n) {}
if (u == s) {} // 325 15 16
if (u == n) {} // 325 15 16
if (u == N) {}
if (u == u) {}
if (u == b) {}
if (u == b) {} // 325 15 16
if (u == o) {}
if (u == k) {} // 126 15 16
if (b == s) {} // 126 15 16
if (b == n) {} // 126 15 16
if (b == N) {}
if (b == u) {}
if (b == N) {} // 325 15 16
if (b == u) {} // 325 15 16
if (b == b) {}
if (b == o) {} // 126 15 16
if (b == k) {} // 126 15 16
@@ -67,5 +67,70 @@ Rectangle {
if (k == b) {} // 126 15 16
if (k == o) {} // 126 15 16
if (k == k) {} // 126 15 16
if (s === s) {}
if (s === n) {} // 325 15 17
if (s === N) {} // 325 15 17
if (s === u) {} // 325 15 17
if (s === b) {} // 325 15 17
if (s === o) {} // 325 15 17
if (s === k) {}
if (s !== s) {}
if (s !== n) {} // 325 15 17
if (s !== N) {} // 325 15 17
if (s !== u) {} // 325 15 17
if (s !== b) {} // 325 15 17
if (s !== o) {} // 325 15 17
if (s !== k) {}
if (n === s) {} // 325 15 17
if (n === n) {}
if (n === N) {} // 325 15 17
if (n === u) {} // 325 15 17
if (n === b) {} // 325 15 17
if (n === o) {} // 325 15 17
if (n === k) {}
if (N === s) {} // 325 15 17
if (N === n) {} // 325 15 17
if (N === N) {}
if (N === u) {}
if (N === b) {} // 325 15 17
if (N === o) {} // 325 15 17
if (N === k) {}
if (u === s) {} // 325 15 17
if (u === n) {} // 325 15 17
if (u === N) {}
if (u === u) {}
if (u === b) {} // 325 15 17
if (u === o) {} // 325 15 17
if (u === k) {}
if (b === s) {} // 325 15 17
if (b === n) {} // 325 15 17
if (b === N) {} // 325 15 17
if (b === u) {} // 325 15 17
if (b === b) {}
if (b === o) {} // 325 15 17
if (b === k) {}
if (o === s) {} // 325 15 17
if (o === n) {} // 325 15 17
if (o === N) {} // 325 15 17
if (o === u) {} // 325 15 17
if (o === b) {} // 325 15 17
if (o === o) {} // 325 15 17
if (o === k) {}
if (k === s) {}
if (k === n) {}
if (k === N) {}
if (k === u) {}
if (k === b) {}
if (k === o) {}
if (k === k) {}
}
}