JS: Resolve calls downward in class hierarchy

[asgerf]

Resolves calls targeting this downwards in the class hierarchy, for example:

class Base {
  foo() {
    this.bar(); // calls Sub.bar
  }
}
class Sub extends Base {
  bar() { ... }
}

We already resolve calls upwards in the class hierarchy. One of the risks from resolving both upwards and downwards is that flow can start in one subclass, go upwards to a base class, and then move into a different subclass. This PR mitigates using some features of the shared data flow library involving DataFlowType and viableImplInCallContext.

Both DCA and QA run were very quiet (see backlinks). The DCA run showed 531 new call edges across 95 projects, resulting in 6 new sinks and 694 nodes now being tainted.

[Copilot]

PR Overview

This PR improves the call resolution logic to handle downward calls in the class hierarchy, ensuring that calls to methods on "this" are resolved to subclass implementations when appropriate. Key changes include:

• Addition of test cases in TripleDot to validate downward call resolution.
• Annotated test updates in CallGraphs to reflect calls resolving into subclass methods.
• A new change note documenting the improvements in call resolution.
• A minor update in accessors tests to refactor object creation.

Changes

File	Description
javascript/ql/test/library-tests/TripleDot/subclass.js	Added test cases for downward call resolution in subclasses.
javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/subclasses.js	Updated annotated tests with downward call resolution and added tests for method overriding.
javascript/ql/src/change-notes/2025-02-17-downward-calls.md	Documented the improvements to downward call resolution and data flow handling.
javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/accessors.js	Adjusted the test case to use a stored object reference instead of an inline instantiation.

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

[erik-krogh]

This docstring is outdated.
It's either:

• A (transitive) subclass, where the subclass doesn't have a write to that property.
• A superclass.

[asgerf]

Updated the doc.

where the subclass doesn't have a write to that property.

It may have a write, just not a member declaration or a function stored on this in the constructor.

[erik-krogh]

This makes sense to me, assuming that the shared dataflow library handles the transitive closure.

Similarly the typeStrongerThan1 predicate feels like it needs a transitive closure.
So maybe add a comment there that the shared dataflow handles the transitive closure. (If that's the case)

[asgerf]

After a closer look the shared library actually doesn't do the transitive closure. Good thing you mentioned this.

Updating this to do the transitive closure. Will need another evaluation now.

[erik-krogh]

Transitive closure is not automatically handled by the shared dataflow library for the compatibleTypes predicate?

[asgerf]

Type-compatibility is not transitive. If Base is compatible with Subclass1 and Subclass2 that doesn't mean Subclass1 and Subclass2 are compatible.

[erik-krogh]

Oh, right.

LGTM 👍

[erik-krogh]

Suggested change

	private predicate downwardCall(DataFlowCall call) {
	private predicate mayCallDownwards(DataFlowCall call) {

?

[erik-krogh]

LGTM, assuming the evaluation is good.

[asgerf]

DCA evaluation looks very different now; a regression in vscode and some lost results. Will need to investigate.

[asgerf]

Actually vscode failed in the initial DCA run, it just wasn't obvious enough for me to notice apparently. It failed during the TaintedNodes meta-query which wasn't run in the second run as meta queries were turned off.