diff --git a/javascript/ql/src/Statements/UselessConditional.ql b/javascript/ql/src/Statements/UselessConditional.ql index a740e8628d8..3f6e55cb20b 100644 --- a/javascript/ql/src/Statements/UselessConditional.ql +++ b/javascript/ql/src/Statements/UselessConditional.ql @@ -115,25 +115,21 @@ predicate whitelist(Expr e) { } /** - * Holds if `e` is part of a conditional node `cond` that evaluates - * `e` and checks its value for truthiness. + * Gets the `&&` expression that defines this guard node, if any. */ -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() +LogAndExpr getLogAndExpr(ConditionGuardNode guard) { + result.getLeftOperand().stripParens() = guard.getTest() } -from ASTNode cond, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg -where isConditional(cond, op.asExpr()) and +from ConditionGuardNode guard, DataFlow::AnalyzedNode op, boolean cv, ASTNode sel, string msg +where guard.getTest() = op.asExpr() and cv = op.getTheBooleanValue()and not whitelist(op.asExpr()) and // if `cond` is of the form ` && `, // we suggest replacing it with `, ` - 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.") + 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.") // otherwise we just report that `op` always evaluates to `cv` else ( diff --git a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected index de351ec51b9..6a89e68f200 100644 --- a/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected +++ b/javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected @@ -1,4 +1,4 @@ -| UselessConditional.js:5:7:5:12 | !lines | This expression always evaluates to false. | +| UselessConditional.js:5:8:5:12 | lines | Variable 'lines' always evaluates to true here. | | 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. |