JS: make AnalyzedFunction public and move getAReturnValue there

Author: Esben Sparre Andreasen

javascript/ql/src/semmle/javascript/dataflow/TypeInference.qll

@@ -239,3 +239,52 @@ class AnalyzedModule extends TopLevel {
     )
   }
 }
+
+/**
+ * Flow analysis for functions.
+ */
+class AnalyzedFunction extends DataFlow::AnalyzedValueNode {
+  override Function astNode;
+
+  override AbstractValue getALocalValue() { result = TAbstractFunction(astNode) }
+
+  /**
+   * Gets a return value for a call to this function.
+   */
+  AbstractValue getAReturnValue() {
+    if astNode.isGenerator() or astNode.isAsync() then
+      result = TAbstractOtherObject()
+    else (
+      // explicit return value
+      result = astNode.getAReturnedExpr().analyze().getALocalValue()
+      or
+      // implicit return value
+      (
+        // either because execution of the function may terminate normally
+        mayReturnImplicitly()
+        or
+        // or because there is a bare `return;` statement
+        exists (ReturnStmt ret | ret = astNode.getAReturnStmt() | not exists(ret.getExpr()))
+      ) and
+      result = TAbstractUndefined()
+    )
+  }
+
+  /**
+   * Holds if the execution of this function may complete normally without
+   * encountering a `return` or `throw` statement.
+   *
+   * Note that this is an overapproximation, that is, the predicate may hold
+   * of functions that cannot actually complete normally, since it does not
+   * account for `finally` blocks and does not check reachability.
+   */
+  private predicate mayReturnImplicitly() {
+    exists (ConcreteControlFlowNode final |
+      final.getContainer() = astNode and
+      final.isAFinalNode() and
+      not final instanceof ReturnStmt and
+      not final instanceof ThrowStmt
+    )
+  }
+
+}

javascript/ql/src/semmle/javascript/dataflow/internal/BasicExprTypeInference.qll

@@ -89,15 +89,6 @@ private class AnalyzedArrayComprehensionExpr extends DataFlow::AnalyzedValueNode
   override AbstractValue getALocalValue() { result = TAbstractOtherObject() }
 }
 
-/**
- * Flow analysis for functions.
- */
-private class AnalyzedFunction extends DataFlow::AnalyzedValueNode {
-  override Function astNode;
-
-  override AbstractValue getALocalValue() { result = TAbstractFunction(astNode) }
-}
-
 /**
  * Flow analysis for class declarations.
  */

javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll

@@ -127,59 +127,19 @@ private class AnalyzedThisInPropertyFunction extends AnalyzedThisExpr {
   }
 }
 
-/**
- * Holds if the execution of function `f` may complete normally without
- * encountering a `return` or `throw` statement.
- *
- * Note that this is an overapproximation, that is, the predicate may hold
- * of functions that cannot actually complete normally, since it does not
- * account for `finally` blocks and does not check reachability.
- */
-predicate mayReturnImplicitly(Function f) {
-  exists (ConcreteControlFlowNode final |
-    final.getContainer() = f and
-    final.isAFinalNode() and
-    not final instanceof ReturnStmt and
-    not final instanceof ThrowStmt
-  )
-}
-
 /**
  * A call with inter-procedural type inference for the return value.
  */
-abstract class CallWithAnalyzedReturnFlow extends DataFlow::CallNode, DataFlow::AnalyzedValueNode {
+abstract class CallWithAnalyzedReturnFlow extends DataFlow::AnalyzedValueNode {
 
   /**
    * Gets a called function.
    */
-  abstract Function getAFunction();
-  
-  /**
-   * Gets a return value for this call.
-   */
-  AbstractValue getAReturnValue() {
-    exists (Function f | f = getAFunction() |
-      if f.isGenerator() or f.isAsync() then
-        result = TAbstractOtherObject()
-      else (
-        // explicit return value
-        result = f.getAReturnedExpr().analyze().getALocalValue()
-        or
-        // implicit return value
-        (
-          // either because execution of the function may terminate normally
-          mayReturnImplicitly(f)
-          or
-          // or because there is a bare `return;` statement
-          exists (ReturnStmt ret | ret = f.getAReturnStmt() | not exists(ret.getExpr()))
-        ) and
-        result = TAbstractUndefined()
-      )
-    )
-  }
+  abstract AnalyzedFunction getACallee();
 
   override AbstractValue getALocalValue() {
-    result = getAReturnValue()
+    result = getACallee().getAReturnValue() and
+    not this instanceof DataFlow::NewNode
   }
 }
 
@@ -194,8 +154,8 @@ private class IIFEWithAnalyzedReturnFlow extends CallWithAnalyzedReturnFlow {
     astNode = iife.getInvocation()
   }
 
-  override Function getAFunction() {
-    result = iife
+  override AnalyzedFunction getACallee() {
+    result = iife.analyze()
   }
 
 }
@@ -238,8 +198,8 @@ private class LocalFunctionCallWithAnalyzedReturnFlow extends CallWithAnalyzedRe
     this = f.getAnInvocation()
   }
 
-  override Function getAFunction() {
-    result = f
+  override AnalyzedFunction getACallee() {
+    result = f.analyze()
   }
 
 }