Skip to content

Commit 1efef2c

Browse files
committed
JS: Change rule for getPostUpdateForStore
This causes less wobbles in test outputs
1 parent ad52b71 commit 1efef2c

File tree

1 file changed

+8
-7
lines changed

1 file changed

+8
-7
lines changed

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1314,17 +1314,18 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
13141314

13151315
/** Gets the post-update node for which `node` is the corresponding pre-update node. */
13161316
private Node getPostUpdateForStore(Node base) {
1317-
// Some nodes have post-update nodes but should not be targeted by a PropWrite store.
1318-
// Notably, an object literal can have a post-update node it if is an argument to a call,
1319-
// but in this case, we should not target the post-update node, as this would prevent data from
1320-
// flowing into the call.
13211317
exists(Expr expr |
13221318
base = TValueNode(expr) and
13231319
result = TExprPostUpdateNode(expr)
13241320
|
1325-
expr instanceof PropAccess or
1326-
expr instanceof VarAccess or
1327-
expr instanceof ThisExpr
1321+
// When object/array literal appears as an argument to a call, we would generally need two post-update nodes:
1322+
// - one for the stores coming from the properties or array elements (which happen before the call and must flow into the call)
1323+
// - one for the argument position, to propagate the updates that happened during the call
1324+
//
1325+
// However, the first post-update is not actually needed since we are storing into a brand new object, so in the first case
1326+
// we just target the expression directly. In the second case we use the ExprPostUpdateNode.
1327+
not expr instanceof ObjectExpr and
1328+
not expr instanceof ArrayExpr
13281329
)
13291330
or
13301331
exists(ImplicitThisUse use |

0 commit comments

Comments
 (0)