Merge pull request #748 from esben-semmle/js/fix/js/useless-assignment-to-property

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.20/analysis-javascript.md

@@ -31,6 +31,7 @@
 | Unused parameter                           | Fewer false-positive results | This rule no longer flags parameters with leading underscore. |
 | Unused variable, import, function or class | Fewer false-positive results | This rule now flags fewer variables that are implictly used by JSX elements, and no longer flags variables with leading underscore. |
 | Uncontrolled data used in path expression | Fewer false-positive results | This rule now recognizes the Express `root` option, which prevents path traversal. |
+| Useless assignment to property. | Fewer false-positive results | This rule now treats assignments with complex right-hand sides correctly. |
 
 ## Changes to QL libraries
 

javascript/ql/src/Declarations/DeadStoreOfProperty.ql

@@ -78,10 +78,8 @@ predicate isDeadAssignment(string name, DataFlow::PropWrite assign1, DataFlow::P
  */
 bindingset[name]
 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
-    write = block.getNode(i) and
+  exists(ReachableBasicBlock block, int i, int j, Expr e |
+    assign.getWriteNode() = block.getNode(i) and
     e = block.getNode(j) and
     maybeAccessesProperty(e, name)
   |
@@ -108,8 +106,8 @@ predicate noPropAccessBetween(string name, DataFlow::PropWrite assign1, DataFlow
     then
       // same block: check for access between
       not exists(int i1, Expr mid, int i2 |
-        assign1.getWriteNode() = block1.getNode(i1) and
-        assign2.getWriteNode() = block2.getNode(i2) and
+        write1 = block1.getNode(i1) and
+        write2 = block2.getNode(i2) and
         mid = block1.getNode([i1 + 1 .. i2 - 1]) and
         maybeAccessesProperty(mid, name)
       )

javascript/ql/test/query-tests/Declarations/DeadStoreOfProperty/odasa-7636.js

@@ -0,0 +1,12 @@
+(function(o1, o2){
+    o1.p = x ? x : x;
+    o1.p = o1.p + (x ? x : x);
+
+    o2.p = x ? x : x;
+    let v2 = o2.p + (x ? x : x);
+    o2.p = v;
+
+    let v1 = x? x: x;
+    o3.p = v;
+    o3.p = o3.p + (x ? x : x);
+});