Skip to content

Commit 1509752

Browse files
semmle-qlcisemmle-qlci
authored
Merge pull request #345 from esben-semmle/js/intro-getUnderlying
Approved by xiemaisi
2 parents e609a95 + c9890fe commit 1509752

18 files changed

Lines changed: 161 additions & 33 deletions

File tree

‎javascript/ql/src/Declarations/DeadStoreOfLocal.ql‎

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,6 @@ predicate deadStoreOfLocal(VarDef vd, PurelyLocalVariable v) {
2525
not exists (SsaExplicitDefinition ssa | ssa.defines(vd, v))
2626
}
2727

28-
/**
29-
* Holds if `e` is an expression evaluating to `null` or `undefined`.
30-
*
31-
* This includes not only direct references to `null` and `undefined`, but
32-
* also `void` expressions and assignments of the form `x = rhs`, where `rhs`
33-
* is itself an expression evaluating to `null` or `undefined`.
34-
*/
35-
predicate isNullOrUndef(Expr e) {
36-
exists (Expr inner |
37-
inner = e.stripParens() |
38-
// `null` or `undefined`
39-
inner instanceof NullLiteral or
40-
inner.(VarAccess).getName() = "undefined" or
41-
inner instanceof VoidExpr or
42-
// recursive case to catch multi-assignments of the form `x = y = null`
43-
isNullOrUndef(inner.(AssignExpr).getRhs())
44-
)
45-
}
46-
4728
/**
4829
* Holds if `e` is an expression that may be used as a default initial value,
4930
* such as `0` or `-1`, or an empty object or array literal.
@@ -57,9 +38,7 @@ predicate isDefaultInit(Expr e) {
5738
// initialising to an empty array or object literal, even if unnecessary,
5839
// can convey useful type information to the reader
5940
e.(ArrayExpr).getSize() = 0 or
60-
e.(ObjectExpr).getNumProperty() = 0 or
61-
// recursive case
62-
isDefaultInit(e.(AssignExpr).getRhs())
41+
e.(ObjectExpr).getNumProperty() = 0
6342
}
6443

6544
from VarDef dead, PurelyLocalVariable v // captured variables may be read by closures, so don't flag them
@@ -77,9 +56,9 @@ where deadStoreOfLocal(dead, v) and
7756
not fd = outer.getBody().(BlockStmt).getAStmt()
7857
) and
7958
// don't flag overwrites with `null` or `undefined`
80-
not isNullOrUndef(dead.getSource()) and
59+
not SyntacticConstants::isNullOrUndefined(dead.getSource()) and
8160
// don't flag default inits that are later overwritten
82-
not (isDefaultInit(dead.getSource()) and dead.isOverwritten(v)) and
61+
not (isDefaultInit(dead.getSource().(Expr).getUnderlyingValue()) and dead.isOverwritten(v)) and
8362
// don't flag assignments in externs
8463
not dead.(ASTNode).inExternsFile() and
8564
// don't flag exported variables

‎javascript/ql/src/semmle/javascript/Constants.qll‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,32 @@ module SyntacticConstants {
127127
class WrappedConstant extends SyntacticConstant {
128128

129129
WrappedConstant() {
130-
stripParens() instanceof SyntacticConstant or
131-
this.(SeqExpr).getLastOperand() instanceof SyntacticConstant or
132-
this.(TypeAssertion).getExpression() instanceof SyntacticConstant or
133-
this.(Assignment).getRhs() instanceof SyntacticConstant
130+
getUnderlyingValue() instanceof SyntacticConstant
134131
}
135132

136133
}
137134

135+
/**
136+
* Holds if `c` evaluates to `undefined`.
137+
*/
138+
predicate isUndefined(SyntacticConstant c) {
139+
c.getUnderlyingValue() instanceof UndefinedConstant
140+
}
141+
142+
/**
143+
* Holds if `c` evaluates to `null`.
144+
*/
145+
predicate isNull(SyntacticConstant c) {
146+
c.getUnderlyingValue() instanceof NullConstant
147+
}
148+
149+
/**
150+
* Holds if `c` evaluates to `null` or `undefined`.
151+
*/
152+
predicate isNullOrUndefined(SyntacticConstant c) {
153+
isUndefined(c) or isNull(c)
154+
}
155+
138156
}
139157

140158
/**

‎javascript/ql/src/semmle/javascript/Expr.qll‎

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,45 @@ class ExprOrType extends @exprortype, Documentable {
4545
)
4646
}
4747

48-
/** Gets this expression or type, with any surrounding parentheses removed. */
48+
/**
49+
* Gets this expression or type, with any surrounding parentheses removed.
50+
*
51+
* Also see `getUnderlyingValue` and `getUnderlyingReference`.
52+
*/
4953
ExprOrType stripParens() { result = this }
54+
55+
/**
56+
* Gets the innermost reference that this expression evaluates to, if any.
57+
*
58+
* Examples:
59+
*
60+
* - a variable or property access: the access itself.
61+
* - a parenthesized expression `(e)`: the underlying reference of `e`.
62+
* - a TypeScript type assertion `e as T`: the underlying reference of `e`.
63+
*
64+
* Also see `getUnderlyingValue` and `stripParens`.
65+
*/
66+
Expr getUnderlyingReference() {
67+
none()
68+
}
69+
70+
/**
71+
* Gets the innermost expression that this expression evaluates to.
72+
*
73+
* Examples:
74+
*
75+
* - a parenthesised expression `(e)`: the underlying value of `e`.
76+
* - a sequence expression `e1, e2`: the underlying value of `e2`.
77+
* - an assignment expression `v = e`: the underlying value of `e`.
78+
* - a TypeScript type assertion `e as T`: the underlying value of `e`.
79+
* - any other expression: the expression itself.
80+
*
81+
* Also see `getUnderlyingReference` and `stripParens`.
82+
*/
83+
Expr getUnderlyingValue() {
84+
result = this
85+
}
86+
5087
}
5188

5289
/** An expression. */
@@ -210,6 +247,15 @@ class ParExpr extends @parexpr, Expr {
210247
override predicate isImpure() {
211248
getExpression().isImpure()
212249
}
250+
251+
override Expr getUnderlyingValue() {
252+
result = getExpression().getUnderlyingValue()
253+
}
254+
255+
override Expr getUnderlyingReference() {
256+
result = getExpression().getUnderlyingReference()
257+
}
258+
213259
}
214260

215261
/** A `null` literal. */
@@ -617,6 +663,11 @@ class SeqExpr extends @seqexpr, Expr {
617663
override string getStringValue() {
618664
result = getLastOperand().getStringValue()
619665
}
666+
667+
override Expr getUnderlyingValue() {
668+
result = getLastOperand().getUnderlyingValue()
669+
}
670+
620671
}
621672

622673
/** A conditional expression. */
@@ -862,6 +913,11 @@ class PropAccess extends @propaccess, Expr {
862913
override ControlFlowNode getFirstControlFlowNode() {
863914
result = getBase().getFirstControlFlowNode()
864915
}
916+
917+
override Expr getUnderlyingReference() {
918+
result = this
919+
}
920+
865921
}
866922

867923
/** A dot expression. */
@@ -1284,10 +1340,17 @@ class Assignment extends @assignment, Expr {
12841340
override ControlFlowNode getFirstControlFlowNode() {
12851341
result = getLhs().getFirstControlFlowNode()
12861342
}
1343+
12871344
}
12881345

12891346
/** A simple assignment expression. */
1290-
class AssignExpr extends @assignexpr, Assignment {}
1347+
class AssignExpr extends @assignexpr, Assignment {
1348+
1349+
override Expr getUnderlyingValue() {
1350+
result = getRhs().getUnderlyingValue()
1351+
}
1352+
1353+
}
12911354

12921355
/** A compound assign expression. */
12931356
abstract class CompoundAssignExpr extends Assignment {}

‎javascript/ql/src/semmle/javascript/TypeScript.qll‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,15 @@ class TypeAssertion extends Expr, @typeassertion {
13151315
override ControlFlowNode getFirstControlFlowNode() {
13161316
result = getExpression().getFirstControlFlowNode()
13171317
}
1318+
1319+
override Expr getUnderlyingValue() {
1320+
result = getExpression().getUnderlyingValue()
1321+
}
1322+
1323+
override Expr getUnderlyingReference() {
1324+
result = getExpression().getUnderlyingReference()
1325+
}
1326+
13181327
}
13191328

13201329
/**

‎javascript/ql/src/semmle/javascript/Variables.qll‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,11 @@ class VarRef extends @varref, Identifier, BindingPattern, LexicalRef {
298298
override VarRef getABindingVarRef() { result = this }
299299

300300
override predicate isImpure() { none() }
301+
302+
override Expr getUnderlyingReference() {
303+
result = this
304+
}
305+
301306
}
302307

303308
/** An identifier that refers to a variable in a non-declaring position. */

‎javascript/ql/test/library-tests/Constants/Constants.expected‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@
5858
| tst.js:47:5:47:5 | 1 |
5959
| tst.js:48:1:48:7 | x.p = 1 |
6060
| tst.js:48:7:48:7 | 1 |
61-
| tst.js:49:1:49:6 | x += 1 |
6261
| tst.js:49:6:49:6 | 1 |
6362
| tst.ts:1:13:1:21 | <number>1 |
6463
| tst.ts:1:21:1:21 | 1 |

‎javascript/ql/test/query-tests/Declarations/ArgumentsRedefined/ArgumentsRedefined.expected‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
| tst.js:3:5:3:13 | arguments | Redefinition of arguments. |
22
| tst.js:7:7:7:15 | arguments | Redefinition of arguments. |
3+
| tst.js:11:11:11:19 | arguments | Redefinition of arguments. |
4+
| tst.js:12:11:12:19 | arguments | Redefinition of arguments. |

‎javascript/ql/test/query-tests/Declarations/ArgumentsRedefined/tst.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,8 @@ function f() {
66
function g(x, y) {
77
var arguments = [y, x]; // NOT OK
88
}
9+
10+
(function (){
11+
for ([arguments] of o);
12+
for ([arguments = 4] of o);
13+
});

‎javascript/ql/test/query-tests/Declarations/AssignmentToConst/AssignmentToConst.expected‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,4 @@
44
| tst.js:7:1:7:6 | y = 23 | Assignment to variable y, which is $@ constant. | tst.js:1:1:1:21 | const x ... y = 42; | declared |
55
| tst.js:10:5:10:10 | y = -1 | Assignment to variable y, which is $@ constant. | tst.js:1:1:1:21 | const x ... y = 42; | declared |
66
| tst.js:13:1:13:3 | ++x | Assignment to variable x, which is $@ constant. | tst.js:1:1:1:21 | const x ... y = 42; | declared |
7+
| tst.js:25:10:25:14 | [ c ] | Assignment to variable c, which is $@ constant. | tst.js:24:5:24:19 | const c = null; | declared |

‎javascript/ql/test/query-tests/Declarations/AssignmentToConst/tst.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,9 @@ var z = 56;
1818
z = 72;
1919

2020
// OK
21-
const s = "hi";
21+
const s = "hi";
22+
23+
(function (){
24+
const c = null;
25+
for ([ c ] of o);
26+
});

0 commit comments

Comments
 (0)