Rust: Handle inherent implementations shadowing trait implementations

Author: paldepind

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

@@ -14,7 +14,13 @@ private import codeql.rust.internal.TypeInference
  * be referenced directly.
  */
 module Impl {
-  private predicate isImplFunction(Function f) { f = any(ImplItemNode impl).getAnAssocItem() }
+  private predicate isInherentImplFunction(Function f) {
+    f = any(Impl impl | not impl.hasTrait()).(ImplItemNode).getAnAssocItem()
+  }
+
+  private predicate isTraitImplFunction(Function f) {
+    f = any(Impl impl | impl.hasTrait()).(ImplItemNode).getAnAssocItem()
+  }
 
   // the following QLdoc is generated: if you need to edit it, do it in the schema file
   /**
@@ -28,16 +34,22 @@ module Impl {
     override Function getStaticTarget() {
       result = resolveMethodCallExpr(this) and
       (
-        // prioritize `impl` methods first
-        isImplFunction(result)
+        // prioritize inherent implementation methods first
+        isInherentImplFunction(result)
         or
-        not isImplFunction(resolveMethodCallExpr(this)) and
+        not isInherentImplFunction(resolveMethodCallExpr(this)) and
         (
-          // then trait methods with default implementations
-          result.hasBody()
+          // then trait implementation methods
+          isTraitImplFunction(result)
           or
-          // and finally trait methods without default implementations
-          not resolveMethodCallExpr(this).hasBody()
+          not isTraitImplFunction(resolveMethodCallExpr(this)) and
+          (
+            // then trait methods with default implementations
+            result.hasBody()
+            or
+            // and finally trait methods without default implementations
+            not resolveMethodCallExpr(this).hasBody()
+          )
         )
       )
     }

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

@@ -974,14 +974,27 @@ private module Cached {
 
     string getField() { result = mce.getIdentifier().getText() }
 
+    int getNumberOfArgs() { result = mce.getArgList().getNumberOfArgs() }
+
     Type resolveTypeAt(TypePath path) { result = inferReceiverType(this, path) }
   }
 
+  /** Holds if a method for `type` with the name `name` and the arity `arity` exists in `impl`. */
+  pragma[nomagic]
+  private predicate methodCandidate(Type type, string name, int arity, Impl impl) {
+    type = impl.(ImplTypeAbstraction).getSelfTy().(TypeReprMention).resolveType() and
+    exists(Function f |
+      f = impl.(ImplItemNode).getASuccessor(name) and
+      f.getParamList().hasSelfParam() and
+      arity = f.getParamList().getNumberOfParams()
+    )
+  }
+
   private module IsInstantiationOfInput implements IsInstantiationOfSig<ReceiverExpr> {
     predicate potentialInstantiationOf(ReceiverExpr receiver, TypeAbstraction impl, TypeMention sub) {
-      sub.resolveType() = receiver.resolveTypeAt(TypePath::nil()) and
-      sub = impl.(ImplTypeAbstraction).getSelfTy().(TypeReprMention) and
-      exists(impl.(ImplItemNode).getASuccessor(receiver.getField()))
+      methodCandidate(receiver.resolveTypeAt(TypePath::nil()), receiver.getField(),
+        receiver.getNumberOfArgs(), impl) and
+      sub = impl.(ImplTypeAbstraction).getSelfTy()
     }
   }
 

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

@@ -399,7 +399,7 @@ mod impl_overlap {
     pub fn f() {
         let x = S1;
         println!("{:?}", x.common_method()); // $ method=S1::common_method SPURIOUS: method=<S1_as_OverlappingTrait>::common_method
-        println!("{:?}", x.common_method_2()); // $ method=S1::common_method_2 SPURIOUS: method=<S1_as_OverlappingTrait>::common_method_2
+        println!("{:?}", x.common_method_2()); // $ method=S1::common_method_2
     }
 }