Rust: Improve type inference for closures

Author: hvitved

rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll

@@ -407,6 +407,14 @@ private predicate isPanicMacroCall(MacroExpr me) {
   me.getMacroCall().resolveMacro().(MacroRules).getName().getText() = "panic"
 }
 
+// Due to "binding modes" the type of the pattern is not necessarily the
+// same as the type of the initializer. The pattern being an identifier
+// pattern is sufficient to ensure that this is not the case.
+private predicate identLetStmt(LetStmt let, IdentPat lhs, Expr rhs) {
+  let.getPat() = lhs and
+  let.getInitializer() = rhs
+}
+
 /** Module for inferring certain type information. */
 module CertainTypeInference {
   pragma[nomagic]
@@ -484,11 +492,7 @@ module CertainTypeInference {
       // is not a certain type equality.
       exists(LetStmt let |
         not let.hasTypeRepr() and
-        // Due to "binding modes" the type of the pattern is not necessarily the
-        // same as the type of the initializer. The pattern being an identifier
-        // pattern is sufficient to ensure that this is not the case.
-        let.getPat().(IdentPat) = n1 and
-        let.getInitializer() = n2
+        identLetStmt(let, n1, n2)
       )
       or
       exists(LetExpr let |
@@ -512,6 +516,25 @@ module CertainTypeInference {
           )
         else prefix2.isEmpty()
       )
+    or
+    exists(CallExprImpl::DynamicCallExpr dce, TupleType tt, int i |
+      n1 = dce.getArgList() and
+      tt.getArity() = dce.getNumberOfSyntacticArguments() and
+      n2 = dce.getSyntacticPositionalArgument(i) and
+      prefix1 = TypePath::singleton(tt.getPositionalTypeParameter(i)) and
+      prefix2.isEmpty()
+    )
+    or
+    exists(ClosureExpr ce, int index |
+      n1 = ce and
+      n2 = ce.getParam(index).getPat() and
+      prefix1 = closureParameterPath(ce.getNumberOfParams(), index) and
+      prefix2.isEmpty()
+    )
+    or
+    n1 = any(ClosureExpr ce | not ce.hasRetType() and ce.getClosureBody() = n2) and
+    prefix1 = closureReturnPath() and
+    prefix2.isEmpty()
   }
 
   pragma[nomagic]
@@ -781,17 +804,6 @@ private predicate typeEquality(AstNode n1, TypePath prefix1, AstNode n2, TypePat
     prefix2.isEmpty() and
     s = getRangeType(n1)
   )
-  or
-  exists(ClosureExpr ce, int index |
-    n1 = ce and
-    n2 = ce.getParam(index).getPat() and
-    prefix1 = closureParameterPath(ce.getNumberOfParams(), index) and
-    prefix2.isEmpty()
-  )
-  or
-  n1.(ClosureExpr).getClosureBody() = n2 and
-  prefix1 = closureReturnPath() and
-  prefix2.isEmpty()
 }
 
 /**
@@ -828,6 +840,19 @@ private predicate lubCoercion(AstNode parent, AstNode child, TypePath prefix) {
   prefix.isEmpty()
 }
 
+private Type inferUnknownTypeFromAnnotation(AstNode n, TypePath path) {
+  inferType(n, path) = TUnknownType() and
+  // Normally, these are coercion sites, but in case a type is unknown we
+  // allow for type information to flow from the type annotation.
+  exists(TypeMention tm | result = tm.resolveTypeAt(path) |
+    tm = any(LetStmt let | identLetStmt(let, _, n)).getTypeRepr()
+    or
+    tm = any(ClosureExpr ce | n = ce.getBody()).getRetType().getTypeRepr()
+    or
+    tm = getReturnTypeMention(any(Function f | n = f.getBody()))
+  )
+}
+
 /**
  * Holds if the type tree of `n1` at `prefix1` should be equal to the type tree
  * of `n2` at `prefix2`, but type information should only propagate from `n1` to
@@ -1509,6 +1534,8 @@ private module MethodResolution {
    *    or
    * 4. `MethodCallOperation`: an operation expression, `x + y`, which is syntactic sugar
    *    for `Add::add(x, y)`.
+   * 5. `ClosureMethodCall`: a call to a closure, `c(x)`, which is syntactic sugar for
+   *    `c.call_once(x)`, `c.call_mut(x)`, or `c.call(x)`.
    *
    * Note that only in case 1 and 2 is auto-dereferencing and borrowing allowed.
    *
@@ -1520,7 +1547,7 @@ private module MethodResolution {
   abstract class MethodCall extends Expr {
     abstract predicate hasNameAndArity(string name, int arity);
 
-    abstract Expr getArg(ArgumentPosition pos);
+    abstract AstNode getArg(ArgumentPosition pos);
 
     abstract predicate supportsAutoDerefAndBorrow();
 
@@ -2050,6 +2077,26 @@ private module MethodResolution {
     override Trait getTrait() { super.isOverloaded(result, _, _) }
   }
 
+  private class ClosureMethodCall extends MethodCall instanceof CallExprImpl::DynamicCallExpr {
+    pragma[nomagic]
+    override predicate hasNameAndArity(string name, int arity) {
+      name = "call_once" and // todo: handle call_mut and call
+      arity = 1 // args are passed in a tuple
+    }
+
+    override AstNode getArg(ArgumentPosition pos) {
+      pos.isSelf() and
+      result = super.getFunction()
+      or
+      pos.asPosition() = 0 and
+      result = super.getArgList()
+    }
+
+    override predicate supportsAutoDerefAndBorrow() { any() }
+
+    override Trait getTrait() { result instanceof AnyFnTrait }
+  }
+
   pragma[nomagic]
   private Method getMethodSuccessor(ImplOrTraitItemNode i, string name, int arity) {
     result = i.getASuccessor(name) and
@@ -3860,12 +3907,6 @@ private module InvokedClosureSatisfiesConstraintInput implements
   }
 }
 
-/** Gets the type of `ce` when viewed as an implementation of `FnOnce`. */
-private Type invokedClosureFnTypeAt(InvokedClosureExpr ce, TypePath path) {
-  SatisfiesConstraint<InvokedClosureExpr, InvokedClosureSatisfiesConstraintInput>::satisfiesConstraintType(ce,
-    _, path, result)
-}
-
 /**
  * Gets the root type of a closure.
  *
@@ -3892,73 +3933,39 @@ private TypePath closureParameterPath(int arity, int index) {
       TypePath::singleton(getTupleTypeParameter(arity, index)))
 }
 
-/** Gets the path to the return type of the `FnOnce` trait. */
-private TypePath fnReturnPath() {
-  result = TypePath::singleton(getAssociatedTypeTypeParameter(any(FnOnceTrait t).getOutputType()))
-}
-
-/**
- * Gets the path to the parameter type of the `FnOnce` trait with arity `arity`
- * and index `index`.
- */
 pragma[nomagic]
-private TypePath fnParameterPath(int arity, int index) {
-  result =
-    TypePath::cons(TTypeParamTypeParameter(any(FnOnceTrait t).getTypeParam()),
-      TypePath::singleton(getTupleTypeParameter(arity, index)))
-}
-
-pragma[nomagic]
-private Type inferDynamicCallExprType(Expr n, TypePath path) {
-  exists(InvokedClosureExpr ce |
-    // Propagate the function's return type to the call expression
-    exists(TypePath path0 | result = invokedClosureFnTypeAt(ce, path0) |
-      n = ce.getCall() and
-      path = path0.stripPrefix(fnReturnPath())
+private Type inferClosureExprType(AstNode n, TypePath path) {
+  exists(ClosureExpr ce |
+    n = ce and
+    (
+      path.isEmpty() and
+      result = closureRootType()
       or
-      // Propagate the function's parameter type to the arguments
-      exists(int index |
-        n = ce.getCall().getSyntacticPositionalArgument(index) and
-        path =
-          path0.stripPrefix(fnParameterPath(ce.getCall().getArgList().getNumberOfArgs(), index))
+      path = TypePath::singleton(TDynTraitTypeParameter(_, any(FnTrait t).getTypeParam())) and
+      result.(TupleType).getArity() = ce.getNumberOfParams()
+      or
+      exists(TypePath path0 |
+        result = ce.getRetType().getTypeRepr().(TypeMention).resolveTypeAt(path0) and
+        path = closureReturnPath().append(path0)
       )
     )
     or
-    // _If_ the invoked expression has the type of a closure, then we propagate
-    // the surrounding types into the closure.
-    exists(int arity, TypePath path0 | ce.getTypeAt(TypePath::nil()) = closureRootType() |
-      // Propagate the type of arguments to the parameter types of closure
-      exists(int index, ArgList args |
-        n = ce and
-        args = ce.getCall().getArgList() and
-        arity = args.getNumberOfArgs() and
-        result = inferType(args.getArg(index), path0) and
-        path = closureParameterPath(arity, index).append(path0)
-      )
-      or
-      // Propagate the type of the call expression to the return type of the closure
-      n = ce and
-      arity = ce.getCall().getArgList().getNumberOfArgs() and
-      result = inferType(ce.getCall(), path0) and
-      path = closureReturnPath().append(path0)
+    exists(Param p |
+      p = ce.getAParam() and
+      not p.hasTypeRepr() and
+      n = p.getPat() and
+      result = TUnknownType() and
+      path.isEmpty()
     )
   )
 }
 
 pragma[nomagic]
-private Type inferClosureExprType(AstNode n, TypePath path) {
-  exists(ClosureExpr ce |
-    n = ce and
-    path.isEmpty() and
-    result = closureRootType()
-    or
-    n = ce and
-    path = TypePath::singleton(TDynTraitTypeParameter(_, any(FnTrait t).getTypeParam())) and
-    result.(TupleType).getArity() = ce.getNumberOfParams()
-    or
-    // Propagate return type annotation to body
-    n = ce.getClosureBody() and
-    result = ce.getRetType().getTypeRepr().(TypeMention).resolveTypeAt(path)
+private TupleType inferArgList(ArgList args, TypePath path) {
+  exists(CallExprImpl::DynamicCallExpr dce |
+    args = dce.getArgList() and
+    result.getArity() = dce.getNumberOfSyntacticArguments() and
+    path.isEmpty()
   )
 }
 
@@ -4005,7 +4012,9 @@ private module Cached {
       or
       i instanceof ImplItemNode and dispatch = false
     |
-      result = call.(MethodResolution::MethodCall).resolveCallTarget(i, _, _) or
+      result = call.(MethodResolution::MethodCall).resolveCallTarget(i, _, _) and
+      not call instanceof CallExprImpl::DynamicCallExpr
+      or
       result = call.(NonMethodResolution::NonMethodCall).resolveCallTargetViaTypeInference(i)
     )
   }
@@ -4115,13 +4124,15 @@ private module Cached {
       or
       result = inferForLoopExprType(n, path)
       or
-      result = inferDynamicCallExprType(n, path)
-      or
       result = inferClosureExprType(n, path)
       or
+      result = inferArgList(n, path)
+      or
       result = inferStructPatType(n, path)
       or
       result = inferTupleStructPatType(n, path)
+      or
+      result = inferUnknownTypeFromAnnotation(n, path)
     )
   }
 }
@@ -4138,8 +4149,8 @@ private module Debug {
   Locatable getRelevantLocatable() {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
-      filepath.matches("%/sqlx.rs") and
-      startline = [56 .. 60]
+      filepath.matches("%/closure.rs") and
+      startline = [10]
     )
   }
 

rust/ql/test/library-tests/type-inference/closure.rs

@@ -63,7 +63,7 @@ mod fn_once_trait {
         };
         let _r = apply(f, true); // $ target=apply type=_r:i64
 
-        let f = |x| x + 1; // $ MISSING: type=x:i64 target=add
+        let f = |x| x + 1; // $ type=x:i64 $ MISSING: target=add
         let _r2 = apply_two(f); // $ target=apply_two certainType=_r2:i64
     }
 }
@@ -100,7 +100,7 @@ mod fn_mut_trait {
         };
         let _r = apply(f, true); // $ target=apply type=_r:i64
 
-        let f = |x| x + 1; // $ MISSING: type=x:i64 target=add
+        let f = |x| x + 1; // $ type=x:i64 $ MISSING: target=add
         let _r2 = apply_two(f); // $ target=apply_two certainType=_r2:i64
     }
 }
@@ -137,7 +137,7 @@ mod fn_trait {
         };
         let _r = apply(f, true); // $ target=apply type=_r:i64
 
-        let f = |x| x + 1; // $ MISSING: type=x:i64 target=add
+        let f = |x| x + 1; // $ type=x:i64 $ MISSING: target=add
         let _r2 = apply_two(f); // $ target=apply_two certainType=_r2:i64
     }
 }
@@ -183,25 +183,25 @@ mod closure_infer_param {
     }
 
     fn test() {
-        let f = |x| x; // $ MISSING: type=x:i64
+        let f = |x| x; // $ type=x:i64
         let _r = apply1(f, 1i64); // $ target=apply1
 
-        let f = |x| x; // $ MISSING: type=x:i64
+        let f = |x| x; // $ type=x:i64
         let _r = apply2(f, 2i64); // $ target=apply2
 
-        let f = |x| x; // $ MISSING: type=x:i64
+        let f = |x| x; // $ type=x:i64
         let _r = apply3(&f, 3i64); // $ target=apply3
 
-        let f = |x| x; // $ MISSING: type=x:i64
+        let f = |x| x; // $ type=x:i64
         let _r = apply4(f, 4i64); // $ target=apply4
 
         let mut f = |x| x; // $ MISSING: type=x:i64
         let _r = apply5(&mut f, 5i64); // $ target=apply5
 
-        let f = |x| x; // $ MISSING: type=x:i64
+        let f = |x| x; // $ type=x:i64
         let _r = apply6(f, 6i64); // $ target=apply6
 
-        let f = |x| x; // $ MISSING: type=x:i64
+        let f = |x| x; // $ type=x:i64
         let _r = apply7(f, 7i64); // $ target=apply7
     }
 }
@@ -221,15 +221,15 @@ mod implicit_deref {
 
     pub fn test() {
         let x = 0i64;
-        let v = Default::default(); // $ MISSING: type=v:i64 target=default
+        let v = Default::default(); // $ type=v:i64 target=default
         let s = S(v);
-        let _ret = s(x); // $ MISSING: type=_ret:bool
+        let _ret = s(x); // $ type=_ret:bool
 
         let x = 0i32;
-        let v = Default::default(); // $ MISSING: type=v:i32 target=default
+        let v = Default::default(); // $ type=v:i32 target=default
         let s = S(v);
         let s_ref = &s;
-        let _ret = s_ref(x); // $ MISSING: type=_ret:bool
+        let _ret = s_ref(x); // $ type=_ret:bool
 
         // The call below is not an implicit deref, instead it will target
         // `impl<A, F> FnOnce<A> for &F` from

rust/ql/test/library-tests/type-inference/main.rs

@@ -2396,7 +2396,7 @@ mod loops {
         // for loops with arrays
 
         for i in [1, 2, 3] {} // $ type=i:i32
-        for i in [1, 2, 3].map(|x| x + 1) {} // $ target=map MISSING: type=i:i32
+        for i in [1, 2, 3].map(|x| x + 1) {} // $ target=map target=add type=i:i32
         for i in [1, 2, 3].into_iter() {} // $ target=into_iter type=i:i32
 
         let vals1 = [1u8, 2, 3]; // $ type=vals1:TArray.u8
@@ -2896,7 +2896,7 @@ mod arg_trait_bounds {
     }
 
     fn test() {
-        let v = Default::default(); // $ MISSING: type=v:i64 target=default
+        let v = Default::default(); // $ type=v:i64 target=default
         let g = Gen(v);
         let _ = my_get(&g); // $ target=my_get
     }