mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
JavaScript: Autoformat all QL files.
This commit is contained in:
@@ -17,11 +17,7 @@ import javascript
|
||||
* A token that is relevant for this query, that is, an `if`, `else` or `}` token.
|
||||
*/
|
||||
class RelevantToken extends Token {
|
||||
RelevantToken() {
|
||||
exists (string v | v = getValue() |
|
||||
v = "if" or v = "else" or v = "}"
|
||||
)
|
||||
}
|
||||
RelevantToken() { exists(string v | v = getValue() | v = "if" or v = "else" or v = "}") }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -54,9 +50,10 @@ int semanticIndent(RelevantToken tk) {
|
||||
not prevTokenOnSameLine(tk, _, _, _) and
|
||||
result = tk.getLocation().getStartColumn()
|
||||
or
|
||||
exists (RelevantToken prev |
|
||||
exists(RelevantToken prev |
|
||||
prevTokenOnSameLine(tk, "if", prev, "else") or
|
||||
prevTokenOnSameLine(tk, "else", prev, "}") |
|
||||
prevTokenOnSameLine(tk, "else", prev, "}")
|
||||
|
|
||||
result = semanticIndent(prev)
|
||||
)
|
||||
}
|
||||
@@ -64,24 +61,23 @@ int semanticIndent(RelevantToken tk) {
|
||||
/**
|
||||
* Gets the semantic indentation of the `if` token of statement `i`.
|
||||
*/
|
||||
int ifIndent(IfStmt i) {
|
||||
result = semanticIndent(i.getIfToken())
|
||||
}
|
||||
int ifIndent(IfStmt i) { result = semanticIndent(i.getIfToken()) }
|
||||
|
||||
/**
|
||||
* Gets the semantic indentation of the `else` token of statement `i`,
|
||||
* if any.
|
||||
*/
|
||||
int elseIndent(IfStmt i) {
|
||||
result = semanticIndent(i.getElseToken())
|
||||
}
|
||||
int elseIndent(IfStmt i) { result = semanticIndent(i.getElseToken()) }
|
||||
|
||||
from IfStmt outer, IfStmt inner, Token outerIf, Token innerIf, Token innerElse, int outerIndent
|
||||
where inner = outer.getThen().getAChildStmt*() and
|
||||
outerIf = outer.getIfToken() and outerIndent = ifIndent(outer) and
|
||||
innerIf = inner.getIfToken() and innerElse = inner.getElseToken() and
|
||||
outerIndent < ifIndent(inner) and
|
||||
outerIndent = elseIndent(inner) and
|
||||
not outer.getTopLevel().isMinified()
|
||||
where
|
||||
inner = outer.getThen().getAChildStmt*() and
|
||||
outerIf = outer.getIfToken() and
|
||||
outerIndent = ifIndent(outer) and
|
||||
innerIf = inner.getIfToken() and
|
||||
innerElse = inner.getElseToken() and
|
||||
outerIndent < ifIndent(inner) and
|
||||
outerIndent = elseIndent(inner) and
|
||||
not outer.getTopLevel().isMinified()
|
||||
select innerElse, "This else branch belongs to $@, but its indentation suggests it belongs to $@.",
|
||||
innerIf, "this if statement", outerIf, "this other if statement"
|
||||
innerIf, "this if statement", outerIf, "this other if statement"
|
||||
|
||||
@@ -14,9 +14,10 @@ import semmle.javascript.RestrictedLocations
|
||||
import semmle.javascript.frameworks.Emscripten
|
||||
|
||||
from LoopStmt l, BasicBlock body
|
||||
where body = l.getBody().getBasicBlock() and
|
||||
not body.getASuccessor+() = body and
|
||||
not l instanceof EnhancedForLoop and
|
||||
// Emscripten generates lots of `do { ... } while(0);` loops, so exclude
|
||||
not l.getTopLevel() instanceof EmscriptenGeneratedToplevel
|
||||
select (FirstLineOf)l, "This loop executes at most once."
|
||||
where
|
||||
body = l.getBody().getBasicBlock() and
|
||||
not body.getASuccessor+() = body and
|
||||
not l instanceof EnhancedForLoop and
|
||||
// Emscripten generates lots of `do { ... } while(0);` loops, so exclude
|
||||
not l.getTopLevel() instanceof EmscriptenGeneratedToplevel
|
||||
select l.(FirstLineOf), "This loop executes at most once."
|
||||
|
||||
@@ -26,22 +26,21 @@ predicate isThrowOrReturn(Stmt s) {
|
||||
/**
|
||||
* A `return` statement with an operand (that is, not just `return;`).
|
||||
*/
|
||||
class ValueReturn extends ReturnStmt {
|
||||
ValueReturn() { exists(getExpr()) }
|
||||
}
|
||||
class ValueReturn extends ReturnStmt { ValueReturn() { exists(getExpr()) } }
|
||||
|
||||
/** Gets the lexically first explicit return statement in function `f`. */
|
||||
ValueReturn getFirstExplicitReturn(Function f) {
|
||||
result = min(ValueReturn ret |
|
||||
ret.getContainer() = f |
|
||||
ret order by ret.getLocation().getStartLine(), ret.getLocation().getStartColumn()
|
||||
)
|
||||
ret.getContainer() = f
|
||||
|
|
||||
ret
|
||||
order by
|
||||
ret.getLocation().getStartLine(), ret.getLocation().getStartColumn()
|
||||
)
|
||||
}
|
||||
|
||||
/** Gets the number of return statements in function `f`, assuming there is at least one. */
|
||||
int numRet(Function f) {
|
||||
result = strictcount(ReturnStmt ret | ret.getContainer() = f)
|
||||
}
|
||||
int numRet(Function f) { result = strictcount(ReturnStmt ret | ret.getContainer() = f) }
|
||||
|
||||
/**
|
||||
* Holds if `f` is a dual-use constructor, that is, is a function that is meant to be invoked
|
||||
@@ -55,7 +54,7 @@ int numRet(Function f) {
|
||||
*/
|
||||
predicate isDualUseConstructor(Function f) {
|
||||
numRet(f) = 1 and
|
||||
exists (ReturnStmt ret, DataFlow::NewNode new | ret.getContainer() = f |
|
||||
exists(ReturnStmt ret, DataFlow::NewNode new | ret.getContainer() = f |
|
||||
new.asExpr() = ret.getExpr().getUnderlyingValue() and
|
||||
new.getACallee() = f
|
||||
)
|
||||
@@ -67,7 +66,7 @@ predicate isDualUseConstructor(Function f) {
|
||||
* and isn't contained in a `finally` block.
|
||||
*/
|
||||
Stmt getAFallThroughStmt(Function f) {
|
||||
exists (ReachableBasicBlock bb, ControlFlowNode nd |
|
||||
exists(ReachableBasicBlock bb, ControlFlowNode nd |
|
||||
bb.getANode() = nd and
|
||||
nd.isAFinalNode() and
|
||||
f = bb.getContainer() and
|
||||
@@ -78,10 +77,12 @@ Stmt getAFallThroughStmt(Function f) {
|
||||
}
|
||||
|
||||
from Function f, Stmt fallthrough
|
||||
where fallthrough = getAFallThroughStmt(f) and
|
||||
// no control path ends with an implicit return statement of the form `return;`
|
||||
not exists(ReturnStmt ret | ret.getContainer() = f and not exists(ret.getExpr())) and
|
||||
// f doesn't look like a dual-use constructor (which otherwise would trigger a violation)
|
||||
not isDualUseConstructor(f)
|
||||
select (FirstLineOf)fallthrough, "$@ may implicitly return 'undefined' here, while $@ an explicit value is returned.",
|
||||
f, capitalize(f.describe()), getFirstExplicitReturn(f), "elsewhere"
|
||||
where
|
||||
fallthrough = getAFallThroughStmt(f) and
|
||||
// no control path ends with an implicit return statement of the form `return;`
|
||||
not exists(ReturnStmt ret | ret.getContainer() = f and not exists(ret.getExpr())) and
|
||||
// f doesn't look like a dual-use constructor (which otherwise would trigger a violation)
|
||||
not isDualUseConstructor(f)
|
||||
select fallthrough.(FirstLineOf),
|
||||
"$@ may implicitly return 'undefined' here, while $@ an explicit value is returned.", f,
|
||||
capitalize(f.describe()), getFirstExplicitReturn(f), "elsewhere"
|
||||
|
||||
@@ -23,7 +23,8 @@ import javascript
|
||||
* downward.
|
||||
*/
|
||||
predicate bounds(RelationalComparison test, Variable v, string direction) {
|
||||
test.getLesserOperand() = v.getAnAccess() and direction = "upward" or
|
||||
test.getLesserOperand() = v.getAnAccess() and direction = "upward"
|
||||
or
|
||||
test.getGreaterOperand() = v.getAnAccess() and direction = "downward"
|
||||
}
|
||||
|
||||
@@ -36,12 +37,16 @@ predicate bounds(RelationalComparison test, Variable v, string direction) {
|
||||
*/
|
||||
predicate updates(UpdateExpr upd, Variable v, string direction) {
|
||||
upd.getOperand() = v.getAnAccess() and
|
||||
(upd instanceof IncExpr and direction = "upward" or
|
||||
upd instanceof DecExpr and direction = "downward")
|
||||
(
|
||||
upd instanceof IncExpr and direction = "upward"
|
||||
or
|
||||
upd instanceof DecExpr and direction = "downward"
|
||||
)
|
||||
}
|
||||
|
||||
from ForStmt l, Variable v, string d1, string d2
|
||||
where bounds(l.getTest(), v, d1) and
|
||||
updates(l.getUpdate(), v, d2) and
|
||||
d1 != d2
|
||||
where
|
||||
bounds(l.getTest(), v, d1) and
|
||||
updates(l.getUpdate(), v, d2) and
|
||||
d1 != d2
|
||||
select l, "This loop counts " + d2 + ", but its variable is bounded " + d1 + "."
|
||||
|
||||
@@ -12,9 +12,11 @@
|
||||
import javascript
|
||||
|
||||
from Function f, ReturnStmt explicit, ReturnStmt implicit
|
||||
where explicit.getContainer() = f and
|
||||
implicit.getContainer() = f and
|
||||
exists(explicit.getExpr()) and
|
||||
not exists(implicit.getExpr())
|
||||
select implicit, "This return statement implicitly returns 'undefined', whereas $@ returns an explicit value.",
|
||||
explicit, "another return statement in the same function"
|
||||
where
|
||||
explicit.getContainer() = f and
|
||||
implicit.getContainer() = f and
|
||||
exists(explicit.getExpr()) and
|
||||
not exists(implicit.getExpr())
|
||||
select implicit,
|
||||
"This return statement implicitly returns 'undefined', whereas $@ returns an explicit value.",
|
||||
explicit, "another return statement in the same function"
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
import javascript
|
||||
|
||||
from LabeledStmt l, Case c
|
||||
where l = c.getAChildStmt+() and
|
||||
l.getLocation().getStartColumn() = c.getLocation().getStartColumn()
|
||||
select l.getChildExpr(0), "Non-case labels in switch statements are confusing."
|
||||
where
|
||||
l = c.getAChildStmt+() and
|
||||
l.getLocation().getStartColumn() = c.getLocation().getStartColumn()
|
||||
select l.getChildExpr(0), "Non-case labels in switch statements are confusing."
|
||||
|
||||
@@ -1,13 +1,14 @@
|
||||
/**
|
||||
* @name Loop iteration skipped due to shifting
|
||||
* @description Removing elements from an array while iterating over it can cause the loop to skip over some elements,
|
||||
* unless the loop index is decremented accordingly.
|
||||
* unless the loop index is decremented accordingly.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @id js/loop-iteration-skipped-due-to-shifting
|
||||
* @tags correctness
|
||||
* @precision high
|
||||
*/
|
||||
|
||||
import javascript
|
||||
|
||||
/**
|
||||
@@ -24,25 +25,19 @@ class ArrayShiftingCall extends DataFlow::MethodCallNode {
|
||||
(name = "splice" or name = "shift" or name = "unshift")
|
||||
}
|
||||
|
||||
DataFlow::SourceNode getArray() {
|
||||
result = getReceiver().getALocalSource()
|
||||
}
|
||||
DataFlow::SourceNode getArray() { result = getReceiver().getALocalSource() }
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to `splice` on an array.
|
||||
*/
|
||||
class SpliceCall extends ArrayShiftingCall {
|
||||
SpliceCall() {
|
||||
name = "splice"
|
||||
}
|
||||
SpliceCall() { name = "splice" }
|
||||
|
||||
/**
|
||||
* Gets the index from which elements are removed and possibly new elemenst are inserted.
|
||||
*/
|
||||
DataFlow::Node getIndex() {
|
||||
result = getArgument(0)
|
||||
}
|
||||
DataFlow::Node getIndex() { result = getArgument(0) }
|
||||
|
||||
/**
|
||||
* Gets the number of removed elements.
|
||||
@@ -66,21 +61,21 @@ class SpliceCall extends ArrayShiftingCall {
|
||||
*/
|
||||
class ArrayIterationLoop extends ForStmt {
|
||||
DataFlow::SourceNode array;
|
||||
|
||||
LocalVariable indexVariable;
|
||||
|
||||
|
||||
ArrayIterationLoop() {
|
||||
exists (RelationalComparison compare | compare = getTest() |
|
||||
exists(RelationalComparison compare | compare = getTest() |
|
||||
compare.getLesserOperand() = indexVariable.getAnAccess() and
|
||||
compare.getGreaterOperand() = array.getAPropertyRead("length").asExpr()) and
|
||||
compare.getGreaterOperand() = array.getAPropertyRead("length").asExpr()
|
||||
) and
|
||||
getUpdate().(IncExpr).getOperand() = indexVariable.getAnAccess()
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the variable holding the loop variable and current array index.
|
||||
*/
|
||||
LocalVariable getIndexVariable() {
|
||||
result = indexVariable
|
||||
}
|
||||
LocalVariable getIndexVariable() { result = indexVariable }
|
||||
|
||||
/**
|
||||
* Gets the loop entry point.
|
||||
@@ -92,12 +87,10 @@ class ArrayIterationLoop extends ForStmt {
|
||||
/**
|
||||
* Gets a call that potentially shifts the elements of the given array.
|
||||
*/
|
||||
ArrayShiftingCall getAnArrayShiftingCall() {
|
||||
result.getArray() = array
|
||||
}
|
||||
ArrayShiftingCall getAnArrayShiftingCall() { result.getArray() = array }
|
||||
|
||||
/**
|
||||
* Gets a call to `splice` that removes elements from the looped-over array at the current index
|
||||
* Gets a call to `splice` that removes elements from the looped-over array at the current index
|
||||
*
|
||||
* The `splice` call is not guaranteed to be inside the loop body.
|
||||
*/
|
||||
@@ -126,21 +119,21 @@ class ArrayIterationLoop extends ForStmt {
|
||||
hasPathTo(cfg.getAPredecessor()) and
|
||||
getLoopEntry().dominates(cfg.getBasicBlock()) and
|
||||
not hasIndexingManipulation(cfg) and
|
||||
|
||||
// Ignore splice calls guarded by an index equality check.
|
||||
// This indicates that the index of an element is the basis for removal, not its value,
|
||||
// which means it may be okay to skip over elements.
|
||||
not exists (ConditionGuardNode guard, EqualityTest test | cfg = guard |
|
||||
not exists(ConditionGuardNode guard, EqualityTest test | cfg = guard |
|
||||
test = guard.getTest() and
|
||||
test.getAnOperand() = getIndexVariable().getAnAccess() and
|
||||
guard.getOutcome() = test.getPolarity()) and
|
||||
|
||||
guard.getOutcome() = test.getPolarity()
|
||||
) and
|
||||
// Block flow after inspecting an array element other than that at the current index.
|
||||
// For example, if the splice happens after inspecting `array[i + 1]`, then the next
|
||||
// element has already been "looked at" and so it doesn't matter if we skip it.
|
||||
not exists (IndexExpr index | cfg = index |
|
||||
not exists(IndexExpr index | cfg = index |
|
||||
array.flowsToExpr(index.getBase()) and
|
||||
not index.getIndex() = getIndexVariable().getAnAccess())
|
||||
not index.getIndex() = getIndexVariable().getAnAccess()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -160,4 +153,6 @@ class ArrayIterationLoop extends ForStmt {
|
||||
|
||||
from ArrayIterationLoop loop, SpliceCall splice
|
||||
where loop.hasPathThrough(splice, loop.getUpdate().getFirstControlFlowNode())
|
||||
select splice, "Removing an array item without adjusting the loop index '" + loop.getIndexVariable().getName() + "' causes the subsequent array item to be skipped."
|
||||
select splice,
|
||||
"Removing an array item without adjusting the loop index '" + loop.getIndexVariable().getName() +
|
||||
"' causes the subsequent array item to be skipped."
|
||||
|
||||
@@ -26,19 +26,22 @@ predicate misleadingIndentationCandidate(ControlStmt ctrl, Stmt s1, Stmt s2) {
|
||||
not s2 = ctrl.getAControlledStmt()
|
||||
}
|
||||
|
||||
from ControlStmt ctrl, Stmt s1, Stmt s2, string indent, int ctrlStartColumn, int startColumn,
|
||||
File f, int ctrlStartLine, int startLine1, int startLine2
|
||||
where misleadingIndentationCandidate(ctrl, s1, s2) and
|
||||
ctrl.getLocation().hasLocationInfo(f.getAbsolutePath(), ctrlStartLine, ctrlStartColumn, _, _) and
|
||||
// `s1` and `s2` are indented the same
|
||||
s1.getLocation().hasLocationInfo(f.getAbsolutePath(), startLine1, startColumn, _, _) and
|
||||
s2.getLocation().hasLocationInfo(f.getAbsolutePath(), startLine2, startColumn, _, _) and
|
||||
// `s1` is indented relative to `ctrl`
|
||||
startColumn > ctrlStartColumn and
|
||||
// `ctrl`, `s1` and `s2` all have the same indentation character
|
||||
f.hasIndentation(ctrlStartLine, indent, _) and
|
||||
f.hasIndentation(startLine1, indent, _) and
|
||||
f.hasIndentation(startLine2, indent, _) and
|
||||
not s2 instanceof EmptyStmt
|
||||
select (FirstLineOf)s2, "The indentation of this statement suggests that it is controlled by $@, while in fact it is not.",
|
||||
(FirstLineOf)ctrl, "this statement"
|
||||
from
|
||||
ControlStmt ctrl, Stmt s1, Stmt s2, string indent, int ctrlStartColumn, int startColumn, File f,
|
||||
int ctrlStartLine, int startLine1, int startLine2
|
||||
where
|
||||
misleadingIndentationCandidate(ctrl, s1, s2) and
|
||||
ctrl.getLocation().hasLocationInfo(f.getAbsolutePath(), ctrlStartLine, ctrlStartColumn, _, _) and
|
||||
// `s1` and `s2` are indented the same
|
||||
s1.getLocation().hasLocationInfo(f.getAbsolutePath(), startLine1, startColumn, _, _) and
|
||||
s2.getLocation().hasLocationInfo(f.getAbsolutePath(), startLine2, startColumn, _, _) and
|
||||
// `s1` is indented relative to `ctrl`
|
||||
startColumn > ctrlStartColumn and
|
||||
// `ctrl`, `s1` and `s2` all have the same indentation character
|
||||
f.hasIndentation(ctrlStartLine, indent, _) and
|
||||
f.hasIndentation(startLine1, indent, _) and
|
||||
f.hasIndentation(startLine2, indent, _) and
|
||||
not s2 instanceof EmptyStmt
|
||||
select s2.(FirstLineOf),
|
||||
"The indentation of this statement suggests that it is controlled by $@, while in fact it is not.",
|
||||
ctrl.(FirstLineOf), "this statement"
|
||||
|
||||
@@ -16,12 +16,15 @@ import javascript
|
||||
* Gets an iteration variable that loop `for` tests and updates.
|
||||
*/
|
||||
Variable getAnIterationVariable(ForStmt for) {
|
||||
result.getAnAccess().getParentExpr*() = for.getTest() and
|
||||
exists (UpdateExpr upd | upd.getParentExpr*() = for.getUpdate() | upd.getOperand() = result.getAnAccess())
|
||||
result.getAnAccess().getParentExpr*() = for.getTest() and
|
||||
exists(UpdateExpr upd | upd.getParentExpr*() = for.getUpdate() |
|
||||
upd.getOperand() = result.getAnAccess()
|
||||
)
|
||||
}
|
||||
|
||||
from ForStmt outer, ForStmt inner
|
||||
where inner.nestedIn(outer) and
|
||||
getAnIterationVariable(outer) = getAnIterationVariable(inner)
|
||||
select inner.getTest(), "This for statement uses the same loop variable as an enclosing $@.",
|
||||
outer, "for statement"
|
||||
where
|
||||
inner.nestedIn(outer) and
|
||||
getAnIterationVariable(outer) = getAnIterationVariable(inner)
|
||||
select inner.getTest(), "This for statement uses the same loop variable as an enclosing $@.", outer,
|
||||
"for statement"
|
||||
|
||||
@@ -15,8 +15,11 @@ import javascript
|
||||
import semmle.javascript.RestrictedLocations
|
||||
|
||||
from ReturnStmt r, AssignExpr assgn, Variable v
|
||||
where assgn = r.getExpr().stripParens() and
|
||||
v = ((Function)r.getContainer()).getScope().getAVariable() and
|
||||
not v.isCaptured() and
|
||||
assgn.getLhs() = v.getAnAccess()
|
||||
select (FirstLineOf)r, "The assignment to " + v.getName() + " is useless, since it is a local variable and will go out of scope."
|
||||
where
|
||||
assgn = r.getExpr().stripParens() and
|
||||
v = (r.getContainer().(Function)).getScope().getAVariable() and
|
||||
not v.isCaptured() and
|
||||
assgn.getLhs() = v.getAnAccess()
|
||||
select r.(FirstLineOf),
|
||||
"The assignment to " + v.getName() +
|
||||
" is useless, since it is a local variable and will go out of scope."
|
||||
|
||||
@@ -14,7 +14,8 @@ import javascript
|
||||
import semmle.javascript.RestrictedLocations
|
||||
|
||||
from ReturnStmt ret, TopLevel tl
|
||||
where tl = ret.getContainer() and
|
||||
not tl instanceof EventHandlerCode and
|
||||
not tl instanceof NodeModule
|
||||
select (FirstLineOf)ret, "Return statement outside function."
|
||||
where
|
||||
tl = ret.getContainer() and
|
||||
not tl instanceof EventHandlerCode and
|
||||
not tl instanceof NodeModule
|
||||
select ret.(FirstLineOf), "Return statement outside function."
|
||||
|
||||
@@ -20,11 +20,15 @@ import javascript
|
||||
class IncrementExpr extends Expr {
|
||||
IncrementExpr() {
|
||||
// x += e
|
||||
this instanceof AssignAddExpr or
|
||||
this instanceof AssignAddExpr
|
||||
or
|
||||
// ++x or x++
|
||||
this instanceof PreIncExpr or this instanceof PostIncExpr or
|
||||
this instanceof PreIncExpr
|
||||
or
|
||||
this instanceof PostIncExpr
|
||||
or
|
||||
// x = x + e
|
||||
exists (AssignExpr assgn, Variable v | assgn = this |
|
||||
exists(AssignExpr assgn, Variable v | assgn = this |
|
||||
assgn.getTarget() = v.getAnAccess() and
|
||||
assgn.getRhs().(AddExpr).getAnOperand().getUnderlyingReference() = v.getAnAccess()
|
||||
)
|
||||
@@ -35,18 +39,17 @@ class IncrementExpr extends Expr {
|
||||
* Holds if `efl` is a loop whose body increments a variable and does nothing else.
|
||||
*/
|
||||
predicate countingLoop(EnhancedForLoop efl) {
|
||||
exists (ExprStmt inc | inc.getExpr().stripParens() instanceof IncrementExpr |
|
||||
exists(ExprStmt inc | inc.getExpr().stripParens() instanceof IncrementExpr |
|
||||
inc = efl.getBody() or
|
||||
inc = efl.getBody().(BlockStmt).getAStmt()
|
||||
)
|
||||
}
|
||||
|
||||
from EnhancedForLoop efl, PurelyLocalVariable iter
|
||||
where iter = efl.getAnIterationVariable() and
|
||||
not exists (SsaExplicitDefinition ssa | ssa.defines(efl.getIteratorExpr(), iter)) and
|
||||
exists (ReachableBasicBlock body | body.getANode() = efl.getBody() |
|
||||
body.getASuccessor+() = body
|
||||
) and
|
||||
not countingLoop(efl) and
|
||||
not iter.getName().toLowerCase().regexpMatch("(_|dummy|unused).*")
|
||||
select efl.getIterator(), "For loop variable " + iter.getName() + " is not used in the loop body."
|
||||
where
|
||||
iter = efl.getAnIterationVariable() and
|
||||
not exists(SsaExplicitDefinition ssa | ssa.defines(efl.getIteratorExpr(), iter)) and
|
||||
exists(ReachableBasicBlock body | body.getANode() = efl.getBody() | body.getASuccessor+() = body) and
|
||||
not countingLoop(efl) and
|
||||
not iter.getName().toLowerCase().regexpMatch("(_|dummy|unused).*")
|
||||
select efl.getIterator(), "For loop variable " + iter.getName() + " is not used in the loop body."
|
||||
|
||||
@@ -14,16 +14,17 @@ import javascript
|
||||
import semmle.javascript.RestrictedLocations
|
||||
|
||||
from Stmt s
|
||||
where // `s` is unreachable in the CFG
|
||||
s.getFirstControlFlowNode().isUnreachable() and
|
||||
// the CFG does not model all possible exceptional control flow, so be conservative about catch clauses
|
||||
not s instanceof CatchClause and
|
||||
// function declarations are special and always reachable
|
||||
not s instanceof FunctionDeclStmt and
|
||||
// allow a spurious 'break' statement at the end of a switch-case
|
||||
not exists(Case c, int i | i = c.getNumBodyStmt() | (BreakStmt)s = c.getBodyStmt(i-1)) and
|
||||
// ignore ambient statements
|
||||
not s.isAmbient() and
|
||||
// ignore empty statements
|
||||
not s instanceof EmptyStmt
|
||||
select (FirstLineOf)s, "This statement is unreachable."
|
||||
where
|
||||
// `s` is unreachable in the CFG
|
||||
s.getFirstControlFlowNode().isUnreachable() and
|
||||
// the CFG does not model all possible exceptional control flow, so be conservative about catch clauses
|
||||
not s instanceof CatchClause and
|
||||
// function declarations are special and always reachable
|
||||
not s instanceof FunctionDeclStmt and
|
||||
// allow a spurious 'break' statement at the end of a switch-case
|
||||
not exists(Case c, int i | i = c.getNumBodyStmt() | s.(BreakStmt) = c.getBodyStmt(i - 1)) and
|
||||
// ignore ambient statements
|
||||
not s.isAmbient() and
|
||||
// ignore empty statements
|
||||
not s instanceof EmptyStmt
|
||||
select s.(FirstLineOf), "This statement is unreachable."
|
||||
|
||||
@@ -17,18 +17,20 @@ import javascript
|
||||
* We use this to restrict reachability analysis to a small set of containers.
|
||||
*/
|
||||
predicate hasContradictoryGuardNodes(StmtContainer container) {
|
||||
exists (ConditionGuardNode guard |
|
||||
exists(ConditionGuardNode guard |
|
||||
RangeAnalysis::isContradictoryGuardNode(guard) and
|
||||
container = guard.getContainer())
|
||||
container = guard.getContainer()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `block` is reachable and is in a container with contradictory guard nodes.
|
||||
*/
|
||||
predicate isReachable(BasicBlock block) {
|
||||
exists (StmtContainer container |
|
||||
exists(StmtContainer container |
|
||||
hasContradictoryGuardNodes(container) and
|
||||
block = container.getEntryBB())
|
||||
block = container.getEntryBB()
|
||||
)
|
||||
or
|
||||
isReachable(block.getAPredecessor()) and
|
||||
not RangeAnalysis::isContradictoryGuardNode(block.getANode())
|
||||
@@ -50,11 +52,13 @@ predicate isBlockedByContradictoryGuardNodes(BasicBlock block, ConditionGuardNod
|
||||
* Holds if the given guard node is contradictory and causes an expression or statement to be unreachable.
|
||||
*/
|
||||
predicate isGuardNodeWithDeadCode(ConditionGuardNode guard) {
|
||||
exists (BasicBlock block |
|
||||
exists(BasicBlock block |
|
||||
isBlockedByContradictoryGuardNodes(block, guard) and
|
||||
block.getANode() instanceof ExprOrStmt)
|
||||
block.getANode() instanceof ExprOrStmt
|
||||
)
|
||||
}
|
||||
|
||||
from ConditionGuardNode guard
|
||||
where isGuardNodeWithDeadCode(guard)
|
||||
select guard.getTest(), "The condition '" + guard.getTest() + "' is always " + guard.getOutcome().booleanNot() + "."
|
||||
select guard.getTest(),
|
||||
"The condition '" + guard.getTest() + "' is always " + guard.getOutcome().booleanNot() + "."
|
||||
|
||||
@@ -25,9 +25,9 @@ import semmle.javascript.DefensiveProgramming
|
||||
*/
|
||||
predicate isSymbolicConstant(Variable v) {
|
||||
// defined exactly once
|
||||
count (VarDef vd | vd.getAVariable() = v) = 1 and
|
||||
count(VarDef vd | vd.getAVariable() = v) = 1 and
|
||||
// the definition is either a `const` declaration or it assigns a constant to it
|
||||
exists (VarDef vd | vd.getAVariable() = v and count(vd.getAVariable()) = 1 |
|
||||
exists(VarDef vd | vd.getAVariable() = v and count(vd.getAVariable()) = 1 |
|
||||
vd.(VariableDeclarator).getDeclStmt() instanceof ConstDeclStmt or
|
||||
isConstant(vd.getSource())
|
||||
)
|
||||
@@ -46,7 +46,7 @@ predicate isConstant(Expr e) {
|
||||
*/
|
||||
predicate isInitialParameterUse(Expr e) {
|
||||
// unlike `SimpleParameter.getAnInitialUse` this will not include uses we have refinement information for
|
||||
exists (SimpleParameter p, SsaExplicitDefinition ssa |
|
||||
exists(SimpleParameter p, SsaExplicitDefinition ssa |
|
||||
ssa.getDef() = p and
|
||||
ssa.getVariable().getAUse() = e and
|
||||
not p.isRestParameter()
|
||||
@@ -60,11 +60,11 @@ predicate isInitialParameterUse(Expr e) {
|
||||
*/
|
||||
predicate isConstantBooleanReturnValue(Expr e) {
|
||||
// unlike `SourceNode.flowsTo` this will not include uses we have refinement information for
|
||||
exists (DataFlow::CallNode call |
|
||||
exists (call.analyze().getTheBooleanValue()) |
|
||||
e = call.asExpr() or
|
||||
exists(DataFlow::CallNode call | exists(call.analyze().getTheBooleanValue()) |
|
||||
e = call.asExpr()
|
||||
or
|
||||
// also support return values that are assigned to variables
|
||||
exists (SsaExplicitDefinition ssa |
|
||||
exists(SsaExplicitDefinition ssa |
|
||||
ssa.getDef().getSource() = call.asExpr() and
|
||||
ssa.getVariable().getAUse() = e
|
||||
)
|
||||
@@ -113,10 +113,14 @@ predicate whitelist(Expr e) {
|
||||
* is not used for anything other than this truthiness check.
|
||||
*/
|
||||
predicate isExplicitConditional(ASTNode cond, Expr e) {
|
||||
e = cond.(IfStmt).getCondition() or
|
||||
e = cond.(LoopStmt).getTest() or
|
||||
e = cond.(ConditionalExpr).getCondition() or
|
||||
isExplicitConditional(_, cond) and e = cond.(Expr).getUnderlyingValue().(LogicalBinaryExpr).getAnOperand()
|
||||
e = cond.(IfStmt).getCondition()
|
||||
or
|
||||
e = cond.(LoopStmt).getTest()
|
||||
or
|
||||
e = cond.(ConditionalExpr).getCondition()
|
||||
or
|
||||
isExplicitConditional(_, cond) and
|
||||
e = cond.(Expr).getUnderlyingValue().(LogicalBinaryExpr).getAnOperand()
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -132,19 +136,18 @@ predicate isConditional(ASTNode cond, Expr e) {
|
||||
}
|
||||
|
||||
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 cond.(LogAndExpr).getLeftOperand() = op.asExpr() 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 (
|
||||
sel = op.asExpr().stripParens() and
|
||||
msg = "This " + describeExpression(sel) + " always evaluates to " + cv + "."
|
||||
)
|
||||
|
||||
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 cond.(LogAndExpr).getLeftOperand() = op.asExpr() and cv = true and not op.asExpr().isPure()
|
||||
then (
|
||||
sel = cond and msg = "This logical 'and' expression can be replaced with a comma expression."
|
||||
) else (
|
||||
// otherwise we just report that `op` always evaluates to `cv`
|
||||
sel = op.asExpr().stripParens() and
|
||||
msg = "This " + describeExpression(sel) + " always evaluates to " + cv + "."
|
||||
)
|
||||
select sel, msg
|
||||
|
||||
Reference in New Issue
Block a user