Merge pull request #13780 from github/revert-13767-go/missing-flow-through-receiver

Revert "Go: Fix missing flow through receiver for function variable"

Author: smowton

go/ql/lib/change-notes/2023-07-19-method-call-node.md

@@ -744,7 +744,7 @@ module IR {
 
     override Type getResultType() {
       exists(CallExpr c | this.getBase() = evalExprInstruction(c) |
-        result = c.getCalleeType().getResultType(i)
+        result = c.getTarget().getResultType(i)
       )
       or
       exists(Expr e | this.getBase() = evalExprInstruction(e) |

go/ql/lib/semmle/go/controlflow/IR.qll

@@ -8,7 +8,7 @@ private import DataFlowPrivate
 private predicate isInterfaceCallReceiver(
   DataFlow::CallNode call, DataFlow::Node recv, InterfaceType tp, string m
 ) {
-  call.(DataFlow::MethodCallNode).getReceiver() = recv and
+  call.getReceiver() = recv and
   recv.getType().getUnderlyingType() = tp and
   m = call.getACalleeIncludingExternals().asFunction().getName()
 }
@@ -149,9 +149,7 @@ predicate golangSpecificParamArgFilter(
   // Interface methods calls may be passed strictly to that exact method's model receiver:
   arg.getPosition() != -1
   or
-  exists(Function callTarget |
-    callTarget = call.getNode().(DataFlow::CallNode).getACalleeIncludingExternals().asFunction()
-  |
+  exists(Function callTarget | callTarget = call.getNode().(DataFlow::CallNode).getTarget() |
     not isInterfaceMethod(callTarget)
     or
     callTarget = p.getCallable().asSummarizedCallable().asFunction()

go/ql/lib/semmle/go/dataflow/internal/DataFlowDispatch.qll

@@ -439,8 +439,8 @@ module Public {
     CallNode getCallNode() { result = call }
 
     override Type getType() {
-      exists(SignatureType t | t = call.getCall().getCalleeType() |
-        result = t.getParameterType(t.getNumParameter() - 1)
+      exists(Function f | f = call.getTarget() |
+        result = f.getParameterType(f.getNumParameter() - 1)
       )
     }
 
@@ -474,11 +474,7 @@ module Public {
   class CallNode extends ExprNode {
     override CallExpr expr;
 
-    /**
-     * Gets the declared target of this call, if it exists.
-     *
-     * This doesn't exist when a function is called via a variable.
-     */
+    /** Gets the declared target of this call */
     Function getTarget() { result = expr.getTarget() }
 
     private DataFlow::Node getACalleeSource() { result = getACalleeSource(this) }
@@ -626,40 +622,16 @@ module Public {
     /** Gets a result of this call. */
     Node getAResult() { result = this.getResult(_) }
 
-    /**
-     * Gets the data flow node corresponding to the receiver of this call, if any.
-     *
-     * When a method value is assigned to a variable then when it is called it
-     * looks like a function call, as in the following example.
-     * ```go
-     * file, _ := os.Open("test.txt")
-     * f := file.Close
-     * f()
-     * ```
-     * In this case we use local flow to try to find the receiver (`file` in
-     * the  above example).
-     */
+    /** Gets the data flow node corresponding to the receiver of this call, if any. */
     Node getReceiver() { result = this.getACalleeSource().(MethodReadNode).getReceiver() }
 
     /** Holds if this call has an ellipsis after its last argument. */
     predicate hasEllipsis() { expr.hasEllipsis() }
   }
 
-  /**
-   * A data flow node that represents a call to a method.
-   *
-   * When a method value is assigned to a variable then when it is called it
-   * syntactically looks like a function call, as in the following example.
-   * ```go
-   * file, _ := os.Open("test.txt")
-   * f := file.Close
-   * f()
-   * ```
-   * In this case we use local flow to see whether a method is assigned to the
-   * function.
-   */
+  /** A data flow node that represents a call to a method. */
   class MethodCallNode extends CallNode {
-    MethodCallNode() { getACalleeSource(this) instanceof MethodReadNode }
+    MethodCallNode() { expr.getTarget() instanceof Method }
 
     override Method getTarget() { result = expr.getTarget() }
 

go/ql/lib/semmle/go/dataflow/internal/DataFlowNodes.qll

@@ -7,7 +7,7 @@ module Gqlgen {
   /** An autogenerated file containing gqlgen code. */
   private class GqlgenGeneratedFile extends File {
     GqlgenGeneratedFile() {
-      exists(DataFlow::MethodCallNode call |
+      exists(DataFlow::CallNode call |
         call.getReceiver().getType().hasQualifiedName("github.com/99designs/gqlgen/graphql", _) and
         call.getFile() = this
       )

go/ql/lib/semmle/go/frameworks/Gqlgen.qll

@@ -131,7 +131,7 @@ module NetHttp {
     )
     or
     stack = SummaryComponentStack::argument(-1) and
-    result = call.(DataFlow::MethodCallNode).getReceiver()
+    result = call.getReceiver()
   }
 
   private class ResponseBody extends Http::ResponseBody::Range {

go/ql/lib/semmle/go/frameworks/stdlib/NetHttp.qll

@@ -86,7 +86,7 @@ class ExternalApiDataNode extends DataFlow::Node {
       this = call.getArgument(i)
       or
       // Receiver to a call to a method which returns non trivial value
-      this = call.(DataFlow::MethodCallNode).getReceiver() and
+      this = call.getReceiver() and
       i = -1
     ) and
     // Not defined in the code that is being analyzed

go/ql/lib/semmle/go/security/ExternalAPIs.qll

@@ -33,9 +33,7 @@ module SafeUrlFlow {
 
   /** A function model step using `UnsafeUrlMethod`, considered as a sanitizer for safe URL flow. */
   private class UnsafeUrlMethodEdge extends SanitizerEdge {
-    UnsafeUrlMethodEdge() {
-      this = any(UnsafeUrlMethod um).getACall().(DataFlow::MethodCallNode).getReceiver()
-    }
+    UnsafeUrlMethodEdge() { this = any(UnsafeUrlMethod um).getACall().getReceiver() }
   }
 
   /** Any slicing of the URL, considered as a sanitizer for safe URL flow. */

go/ql/lib/semmle/go/security/SafeUrlFlowCustomizations.qll

@@ -90,7 +90,7 @@ predicate isWritableFileHandle(DataFlow::Node source, DataFlow::CallNode call) {
 /**
  * Holds if `os.File.Close` is called on `sink`.
  */
-predicate isCloseSink(DataFlow::Node sink, DataFlow::MethodCallNode closeCall) {
+predicate isCloseSink(DataFlow::Node sink, DataFlow::CallNode closeCall) {
   // find calls to the os.File.Close function
   closeCall = any(CloseFileFun f).getACall() and
   // that are unhandled
@@ -115,7 +115,7 @@ predicate isCloseSink(DataFlow::Node sink, DataFlow::MethodCallNode closeCall) {
  * Holds if `os.File.Sync` is called on `sink` and the result of the call is neither
  * deferred nor discarded.
  */
-predicate isHandledSync(DataFlow::Node sink, DataFlow::MethodCallNode syncCall) {
+predicate isHandledSync(DataFlow::Node sink, DataFlow::CallNode syncCall) {
   // find a call of the `os.File.Sync` function
   syncCall = any(SyncFileFun f).getACall() and
   // match the sink with the object on which the method is called

go/ql/src/InconsistentCode/UnhandledCloseWritableHandle.ql

@@ -113,7 +113,7 @@ class PrivateUrlFlowsToAuthCodeUrlCall extends DataFlow::Configuration {
     )
   }
 
-  predicate isSinkCall(DataFlow::Node sink, DataFlow::MethodCallNode call) {
+  predicate isSinkCall(DataFlow::Node sink, DataFlow::CallNode call) {
     exists(AuthCodeUrl m | call = m.getACall() | sink = call.getReceiver())
   }