hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 03:05:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Merge pull request #380 from asger-semmle/generalize-useless-conditional"

Browse Source 
This reverts commit 28f3b686a7, reversing
changes made to dc3c5a684c.
This commit is contained in:
Asger F
2018-10-31 10:38:38 +00:00
parent 28f3b686a7
commit 2c11844c5b
4 changed files with 13 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
javascript/ql/src/Statements/UselessConditional.ql
View File

@@ -115,21 +115,25 @@ predicate whitelist(Expr e) {
}
/**
* Gets the `&&` expression that defines this guard node, if any.
* Holds if `e` is part of a conditional node `cond` that evaluates
* `e` and checks its value for truthiness.
*/
LogAndExpr getLogAndExpr(ConditionGuardNode guard) {
result.getLeftOperand().stripParens() = guard.getTest()
predicate isConditional(ASTNode cond, Expr e) {
e = cond.(IfStmt).getCondition() or
e = cond.(ConditionalExpr).getCondition() or
e = cond.(LogAndExpr).getLeftOperand() or
e = cond.(LogOrExpr).getLeftOperand()
}
from ConditionGuardNode guard, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg
where guard.getTest() = op.asExpr() and
from ASTNode cond, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg
where isConditional(cond, op.asExpr()) and
cv = op.getTheBooleanValue()and
not whitelist(op.asExpr()) and
// if `cond` is of the form `<non-trivial truthy expr> && <something>`,
// we suggest replacing it with `<non-trivial truthy expr>, <something>`
if exists(getLogAndExpr(guard)) and cv = true and not op.asExpr().isPure() then
(sel = getLogAndExpr(guard) and msg = "This logical 'and' expression can be replaced with a comma expression.")
if cond instanceof LogAndExpr and cv = true and not op.asExpr().isPure() then
(sel = cond and msg = "This logical 'and' expression can be replaced with a comma expression.")
// otherwise we just report that `op` always evaluates to `cv`
else (

5
javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected
View File

@@ -1,4 +1,4 @@
| UselessConditional.js:5:8:5:12 | lines | Variable 'lines' always evaluates to true here. |
| UselessConditional.js:5:7:5:12 | !lines | This expression always evaluates to false. |
| UselessConditional.js:12:34:12:79 | (v = ne ... k] = v) | This logical 'and' expression can be replaced with a comma expression. |
| UselessConditional.js:17:9:17:9 | a | Variable 'a' always evaluates to false here. |
| UselessConditional.js:18:9:18:9 | b | Variable 'b' always evaluates to false here. |
@@ -15,8 +15,7 @@
| UselessConditional.js:65:5:65:5 | x | Variable 'x' always evaluates to true here. |
| UselessConditional.js:76:13:76:13 | x | Variable 'x' always evaluates to true here. |
| UselessConditional.js:82:13:82:13 | x | Variable 'x' always evaluates to true here. |
| UselessConditional.js:94:16:94:16 | x | Variable 'x' always evaluates to false here. |
| UselessConditional.js:101:18:101:18 | x | Variable 'x' always evaluates to false here. |
| UselessConditional.js:89:10:89:16 | x, true | This expression always evaluates to true. |
| UselessConditionalGood.js:58:12:58:13 | x2 | Variable 'x2' always evaluates to false here. |
| UselessConditionalGood.js:69:12:69:13 | xy | Variable 'xy' always evaluates to false here. |
| UselessConditionalGood.js:85:12:85:13 | xy | Variable 'xy' always evaluates to false here. |

13
javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.js
View File

@@ -89,17 +89,4 @@ async function awaitFlow(){
if ((x, true));
});
(function (x, y) {
if (!x) {
while (x) { // NOT OK
f();
}
while (true) { // OK
break;
}
if (true && true) {} // OK
if (y && x) {} // NOT OK
}
});
// semmle-extractor-options: --experimental