Address review comments

Author: hvitved

rust/ql/lib/codeql/rust/elements/internal/InvocationExprImpl.qll

@@ -6,6 +6,9 @@ module Impl {
 
   private newtype TArgumentPosition =
     TPositionalArgumentPosition(int i) {
+      // For the type `FunctionPosition` used by type inference, we work with function-call syntax
+      // adjusted positions, so a call like `x.m(a, b, c)` needs positions `0` through `3`; for this
+      // reason, there is no `- 1` after `max(...)` below.
       i in [0 .. max([any(ParamList l).getNumberOfParams(), any(ArgList l).getNumberOfArgs()])]
     } or
     TSelfArgumentPosition() or

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

@@ -343,8 +343,8 @@ private class FunctionDeclaration extends Function {
   }
 }
 
-private class AssocFunction extends FunctionDeclaration {
-  AssocFunction() { this.isAssoc(_) }
+private class AssocFunctionDeclaration extends FunctionDeclaration {
+  AssocFunctionDeclaration() { this.isAssoc(_) }
 }
 
 pragma[nomagic]
@@ -1114,10 +1114,10 @@ private Trait getCallExprTraitQualifier(CallExpr ce) {
 }
 
 pragma[nomagic]
-private predicate nonAssocFunction(ItemNode i) { not i instanceof AssocFunction }
+private predicate nonAssocFunction(ItemNode i) { not i instanceof AssocFunctionDeclaration }
 
 /**
- * A call expression that can resolve to something that is not an associated
+ * A call expression that can only resolve to something that is not an associated
  * function, and hence does not need type inference for resolution.
  */
 private class NonAssocCallExpr extends CallExpr {
@@ -1201,9 +1201,7 @@ private module ContextTyping {
   abstract class ContextTypedCallCand extends AstNode {
     abstract Type getTypeArgument(TypeArgumentPosition apos, TypePath path);
 
-    private predicate hasTypeArgument(TypeArgumentPosition apos) {
-      exists(this.getTypeArgument(apos, _))
-    }
+    predicate hasTypeArgument(TypeArgumentPosition apos) { exists(this.getTypeArgument(apos, _)) }
 
     /**
      * Holds if this call resolves to `target` inside `i`, and the return type
@@ -2209,7 +2207,7 @@ private module AssocFunctionResolution {
      * in `derefChain` and `borrow`.
      */
     pragma[nomagic]
-    AssocFunction resolveCallTarget(
+    AssocFunctionDeclaration resolveCallTarget(
       ImplOrTraitItemNode i, FunctionPosition selfPos, DerefChain derefChain, BorrowKind borrow
     ) {
       exists(AssocFunctionCallCand afcc |
@@ -2288,7 +2286,9 @@ private module AssocFunctionResolution {
       exists(getCallExprPathQualifier(this)) and
       // even if a target cannot be resolved by path resolution, it may still
       // be possible to resolve a blanket implementation (so not `forex`)
-      forall(ItemNode i | i = CallExprImpl::getResolvedFunction(this) | i instanceof AssocFunction)
+      forall(ItemNode i | i = CallExprImpl::getResolvedFunction(this) |
+        i instanceof AssocFunctionDeclaration
+      )
     }
 
     override predicate hasNameAndArity(string name, int arity) {
@@ -2373,7 +2373,9 @@ private module AssocFunctionResolution {
   }
 
   pragma[nomagic]
-  private AssocFunction getAssocFunctionSuccessor(ImplOrTraitItemNode i, string name, int arity) {
+  private AssocFunctionDeclaration getAssocFunctionSuccessor(
+    ImplOrTraitItemNode i, string name, int arity
+  ) {
     result = i.getASuccessor(name) and
     arity = result.getNumberOfParamsInclSelf()
   }
@@ -2488,7 +2490,7 @@ private module AssocFunctionResolution {
     }
 
     pragma[nomagic]
-    AssocFunction resolveCallTargetCand(ImplOrTraitItemNode i) {
+    AssocFunctionDeclaration resolveCallTargetCand(ImplOrTraitItemNode i) {
       exists(string name, int arity |
         this.selfArgIsInstantiationOf(i, name, arity) and
         result = getAssocFunctionSuccessor(i, name, arity)
@@ -2497,7 +2499,7 @@ private module AssocFunctionResolution {
 
     /** Gets the associated function targeted by this call, if any. */
     pragma[nomagic]
-    AssocFunction resolveCallTarget(ImplOrTraitItemNode i) {
+    AssocFunctionDeclaration resolveCallTarget(ImplOrTraitItemNode i) {
       result = this.resolveCallTargetCand(i) and
       not FunctionOverloading::functionResolutionDependsOnArgument(i, result, _, _)
       or
@@ -2896,18 +2898,15 @@ private module FunctionCallMatchingInput implements MatchingWithEnvironmentInput
       result = this.getInferredNonSelfType(pos, path)
     }
 
-    private AssocFunction getTarget(ImplOrTraitItemNode i, string derefChainBorrow) {
+    private AssocFunctionDeclaration getTarget(ImplOrTraitItemNode i, string derefChainBorrow) {
       exists(DerefChain derefChain, BorrowKind borrow |
         derefChainBorrow = encodeDerefChainBorrow(derefChain, borrow) and
         result = super.resolveCallTarget(i, _, derefChain, borrow) // mutual recursion; resolving method calls requires resolving types and vice versa
       )
     }
 
     override Declaration getTarget(string derefChainBorrow) {
-      exists(ImplOrTraitItemNodeOption i, AssocFunction f |
-        f = this.getTarget(i.asSome(), derefChainBorrow) and
-        result = TFunctionDeclaration(i, f)
-      )
+      exists(ImplOrTraitItemNode i | result.isAssocFunction(i, this.getTarget(i, derefChainBorrow)))
     }
 
     pragma[nomagic]
@@ -2926,7 +2925,9 @@ private module FunctionCallMatchingInput implements MatchingWithEnvironmentInput
     }
   }
 
-  private class NonAssocFunctionCallAccess extends Access instanceof NonAssocCallExpr {
+  private class NonAssocFunctionCallAccess extends Access instanceof NonAssocCallExpr,
+    CallExprImpl::CallExprCall
+  {
     pragma[nomagic]
     override Type getTypeArgument(TypeArgumentPosition apos, TypePath path) {
       result = NonAssocCallExpr.super.getTypeArgument(apos, path)
@@ -2949,11 +2950,9 @@ private module FunctionCallMatchingInput implements MatchingWithEnvironmentInput
 
     pragma[nomagic]
     private Declaration getTarget() {
-      exists(ImplOrTraitItemNodeOption i, FunctionDeclaration f |
-        f = super.resolveCallTargetViaPathResolution() and
-        f.isDirectlyFor(i) and
-        result = TFunctionDeclaration(i, f)
-      )
+      result =
+        TFunctionDeclaration(ImplOrTraitItemNodeOption::none_(),
+          super.resolveCallTargetViaPathResolution())
     }
 
     override Declaration getTarget(string derefChainBorrow) {
@@ -2964,9 +2963,16 @@ private module FunctionCallMatchingInput implements MatchingWithEnvironmentInput
     pragma[nomagic]
     override predicate hasUnknownTypeAt(string derefChainBorrow, FunctionPosition pos, TypePath path) {
       derefChainBorrow = noDerefChainBorrow() and
-      exists(ImplOrTraitItemNodeOption i, FunctionDeclaration f |
-        TFunctionDeclaration(i, f) = this.getTarget() and
-        this.hasUnknownTypeAt(i.asSome(), f, pos, path)
+      exists(FunctionDeclaration f, TypeParameter tp |
+        f = super.resolveCallTargetViaPathResolution() and
+        pos.isReturn() and
+        tp = f.getReturnType(_, path) and
+        not tp = f.getParameterType(_, _, _) and
+        // check that no explicit type arguments have been supplied for `tp`
+        not exists(TypeArgumentPosition tapos |
+          this.hasTypeArgument(tapos) and
+          TTypeParamTypeParameter(tapos.asTypeParam()) = tp
+        )
       )
     }
   }
@@ -3135,6 +3141,11 @@ private module TupleLikeConstructionMatchingInput implements MatchingInputSig {
   class Declaration = TupleLikeConstructor;
 
   class Access extends NonAssocCallExpr, ContextTyping::ContextTypedCallCand {
+    Access() {
+      this instanceof CallExprImpl::TupleStructExpr or
+      this instanceof CallExprImpl::TupleVariantExpr
+    }
+
     override Type getTypeArgument(TypeArgumentPosition apos, TypePath path) {
       result = NonAssocCallExpr.super.getTypeArgument(apos, path)
     }

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

@@ -2698,7 +2698,7 @@ mod context_typed {
         let s = Default::default(); // $ target=default type=s:S
         S::f(s); // $ target=f
 
-        let z = free_function(); // $ target=free_function MISSING: type=z:i32
+        let z = free_function(); // $ target=free_function type=z:i32
         x.push(z); // $ target=push
     }
 }

rust/ql/test/library-tests/type-inference/type-inference.expected

@@ -12195,10 +12195,13 @@ inferType
 | main.rs:2698:17:2698:34 | ...::default(...) |  | main.rs:2639:5:2640:13 | S |
 | main.rs:2699:9:2699:15 | ...::f(...) |  | {EXTERNAL LOCATION} | () |
 | main.rs:2699:14:2699:14 | s |  | main.rs:2639:5:2640:13 | S |
+| main.rs:2701:13:2701:13 | z |  | {EXTERNAL LOCATION} | i32 |
+| main.rs:2701:17:2701:31 | free_function(...) |  | {EXTERNAL LOCATION} | i32 |
 | main.rs:2702:9:2702:9 | x |  | {EXTERNAL LOCATION} | Vec |
 | main.rs:2702:9:2702:9 | x | A | {EXTERNAL LOCATION} | Global |
 | main.rs:2702:9:2702:9 | x | T | {EXTERNAL LOCATION} | i32 |
 | main.rs:2702:9:2702:17 | x.push(...) |  | {EXTERNAL LOCATION} | () |
+| main.rs:2702:16:2702:16 | z |  | {EXTERNAL LOCATION} | i32 |
 | main.rs:2709:14:2709:17 | SelfParam |  | main.rs:2707:5:2715:5 | Self [trait MyTrait] |
 | main.rs:2712:14:2712:18 | SelfParam |  | {EXTERNAL LOCATION} | & |
 | main.rs:2712:14:2712:18 | SelfParam | TRef | main.rs:2707:5:2715:5 | Self [trait MyTrait] |