Skip to content

Commit 7938bc4

Browse files
committed
improve alert message for js/useless-assignment-to-local
1 parent 0b4bfed commit 7938bc4

File tree

3 files changed

+27
-1
lines changed

3 files changed

+27
-1
lines changed

javascript/ql/src/Declarations/DeadStoreOfLocal.ql

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,23 @@ predicate deadStoreOfLocal(VarDef vd, PurelyLocalVariable v) {
2929
not exists(SsaExplicitDefinition ssa | ssa.defines(vd, v))
3030
}
3131

32+
/**
33+
* Holds if there exists another definition of the variable `v` that dominates `dead`.
34+
*/
35+
predicate hasDominatingDef(VarDef dead, PurelyLocalVariable v) {
36+
exists(VarDef otherDef | not otherDef = dead and otherDef.getAVariable() = v |
37+
dead.getBasicBlock().getASuccessor+() = otherDef.getBasicBlock()
38+
or
39+
exists(ReachableBasicBlock bb, int i, int j |
40+
bb = otherDef.getBasicBlock() and bb = dead.getBasicBlock()
41+
|
42+
bb.defAt(i, v, dead) and
43+
bb.defAt(j, v, otherDef) and
44+
j > i
45+
)
46+
)
47+
}
48+
3249
from VarDef dead, PurelyLocalVariable v, string msg
3350
where
3451
deadStoreOfLocal(dead, v) and
@@ -63,7 +80,7 @@ where
6380
(
6481
// To avoid confusion about the meaning of "definition" and "declaration" we avoid
6582
// the term "definition" when the alert location is a variable declaration.
66-
if dead instanceof VariableDeclarator
83+
if dead instanceof VariableDeclarator and hasDominatingDef(dead, v)
6784
then msg = "The initial value of " + v.getName() + " is unused, since it is always overwritten."
6885
else msg = "This definition of " + v.getName() + " is useless, since its value is never read."
6986
)

javascript/ql/test/query-tests/Declarations/DeadStoreOfLocal/DeadStoreOfLocal.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@
1010
| tst.js:51:6:51:11 | x = 23 | The initial value of x is unused, since it is always overwritten. |
1111
| tst.js:132:7:132:13 | {x} = o | The initial value of x is unused, since it is always overwritten. |
1212
| tst.js:162:6:162:14 | [x] = [0] | The initial value of x is unused, since it is always overwritten. |
13+
| tst.js:172:7:172:17 | nSign = foo | This definition of nSign is useless, since its value is never read. |

javascript/ql/test/query-tests/Declarations/DeadStoreOfLocal/tst.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,3 +166,11 @@ function v() {
166166
x;
167167
y;
168168
});
169+
170+
(function() {
171+
if (something()) {
172+
var nSign = foo;
173+
} else {
174+
console.log(nSign);
175+
}
176+
})()

0 commit comments

Comments
 (0)