JS: substitute Assignment for DataFlow::PropWrite

Author: Esben Sparre Andreasen

javascript/ql/src/Declarations/DeadStoreOfProperty.ql

@@ -15,31 +15,30 @@ import DeadStore
 /**
  * Holds if `assign` definitely assigns property `name` of `base`.
  */
-predicate unambiguousPropWrite(DataFlow::SourceNode base, string name, Assignment assign) {
-  exists(DataFlow::PropWrite lhs |
-    assign.getLhs().flow() = lhs and
-    base.getAPropertyWrite(name) = lhs and
-    not exists (DataFlow::SourceNode otherBase |
-      otherBase != base and
-      lhs = otherBase.getAPropertyWrite(name)
-    )
+predicate unambiguousPropWrite(DataFlow::SourceNode base, string name, DataFlow::PropWrite write) {
+  write = base.getAPropertyWrite(name) and
+  not exists (DataFlow::SourceNode otherBase |
+    otherBase != base and
+    write = otherBase.getAPropertyWrite(name)
   )
 }
 
 /**
  * Holds if `assign1` and `assign2` both assign property `name` of the same object, and `assign2` post-dominates `assign1`.
  */
-predicate postDominatedPropWrite(string name, Assignment assign1, Assignment assign2) {
-  exists (DataFlow::SourceNode base, ReachableBasicBlock block1, ReachableBasicBlock block2 |
-    block1 = assign1.getBasicBlock() and
-    block2 = assign2.getBasicBlock() and
+predicate postDominatedPropWrite(string name, DataFlow::PropWrite assign1, DataFlow::PropWrite assign2) {
+  exists (ControlFlowNode write1, ControlFlowNode write2, DataFlow::SourceNode base, ReachableBasicBlock block1, ReachableBasicBlock block2 |
+    write1 = assign1.getWriteNode() and
+    write2 = assign2.getWriteNode() and
+    block1 = write1.getBasicBlock() and
+    block2 = write2.getBasicBlock() and
     unambiguousPropWrite(base, name, assign1) and
     unambiguousPropWrite(base, name, assign2) and
     block2.postDominates(block1) and
     (block1 = block2 implies
       exists (int i1, int i2 |
-        assign1 = block1.getNode(i1) and
-        assign2 = block2.getNode(i2) and
+        write1 = block1.getNode(i1) and
+        write2 = block2.getNode(i2) and
         i1 < i2
       )
     )
@@ -59,7 +58,7 @@ predicate maybeAccessesProperty(Expr e, string name) {
 /**
  * Holds if `assign1` and `assign2` both assign property `name`, but `assign1` is dead because of `assign2`.
  */
-predicate isDeadAssignment(string name, Assignment assign1, Assignment assign2) {
+predicate isDeadAssignment(string name, DataFlow::PropWrite assign1, DataFlow::PropWrite assign2) {
   postDominatedPropWrite(name, assign1, assign2) and
   noPropAccessBetween(name, assign1, assign2) and
   not isDOMProperty(name)
@@ -70,10 +69,11 @@ predicate isDeadAssignment(string name, Assignment assign1, Assignment assign2)
  * `after` indicates if the access happens before or after the node for `assign`.
  */
 bindingset[name]
-predicate maybeAccessesAssignedPropInBlock(string name, Assignment assign, boolean after) {
-  exists (ReachableBasicBlock block, int i, int j, Expr e |
+predicate maybeAccessesAssignedPropInBlock(string name, DataFlow::PropWrite assign, boolean after) {
+  exists (ControlFlowNode write, ReachableBasicBlock block, int i, int j, Expr e |
+    write = assign.getWriteNode() and
     block = assign.getBasicBlock() and
-    assign = block.getNode(i) and
+    write = block.getNode(i) and
     e = block.getNode(j) and
     maybeAccessesProperty(e, name) |
     after = true and i < j
@@ -86,15 +86,17 @@ predicate maybeAccessesAssignedPropInBlock(string name, Assignment assign, boole
  * Holds if `assign1` and `assign2` both assign property `name`, and the assigned property may be accessed between the two assignments.
  */
 bindingset[name]
-predicate noPropAccessBetween(string name, Assignment assign1, Assignment assign2) {
-  exists (ReachableBasicBlock block1, ReachableBasicBlock block2 |
-    assign1.getBasicBlock() = block1 and
-    assign2.getBasicBlock() = block2 and
+predicate noPropAccessBetween(string name, DataFlow::PropWrite assign1, DataFlow::PropWrite assign2) {
+  exists (ControlFlowNode write1, ControlFlowNode write2, ReachableBasicBlock block1, ReachableBasicBlock block2 |
+    write1 = assign1.getWriteNode() and
+    write2 = assign2.getWriteNode() and
+    write1.getBasicBlock() = block1 and
+    write2.getBasicBlock() = block2 and
     if block1 = block2 then
       // same block: check for access between
       not exists (int i1, Expr mid, int i2 |
-        assign1 = block1.getNode(i1) and
-        assign2 = block2.getNode(i2) and
+        assign1.getWriteNode() = block1.getNode(i1) and
+        assign2.getWriteNode() = block2.getNode(i2) and
         mid = block1.getNode([i1+1..i2-1]) and
         maybeAccessesProperty(mid, name)
       )
@@ -115,23 +117,26 @@ predicate noPropAccessBetween(string name, Assignment assign1, Assignment assign
   )
 }
 
-from string name, Assignment assign1, Assignment assign2
+from string name, DataFlow::PropWrite assign1, DataFlow::PropWrite assign2
 where isDeadAssignment(name, assign1, assign2) and
       // whitelist
       not (
         // Google Closure Compiler pattern: `o.p = o['p'] = v`
         exists (PropAccess p1, PropAccess p2 |
-          p1 = assign1.getLhs() and
-          p2 = assign2.getLhs() |
+          p1 = assign1.getAstNode() and
+          p2 = assign2.getAstNode() |
           p1 instanceof DotExpr and p2 instanceof IndexExpr
           or
           p2 instanceof DotExpr and p1 instanceof IndexExpr
         )
         or
         // don't flag overwrites for default values
-        isDefaultInit(assign1.getRhs().getUnderlyingValue())
+        isDefaultInit(assign1.getRhs().asExpr().getUnderlyingValue())
         or
         // don't flag assignments in externs
-        assign1.inExternsFile()
+        assign1.getAstNode().inExternsFile()
+        or
+        // exclude result from js/overwritten-property
+        assign2.getBase() instanceof DataFlow::ObjectLiteralNode
       )
-select assign1, "This write to property '" + name + "' is useless, since $@ always overrides it.", assign2, "another property write"
+select assign1.getWriteNode(), "This write to property '" + name + "' is useless, since $@ always overrides it.", assign2.getWriteNode(), "another property write"