Merge pull request #20853 from hvitved/rust/path-resolution-impl-self

Rust: Refine `Self` resolution inside `impl` blocks

Author: hvitved

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

@@ -264,6 +264,9 @@ abstract class ItemNode extends Locatable {
   pragma[nomagic]
   ItemNode getImmediateParent() { this = result.getADescendant() }
 
+  /** Gets a child item of this item, if any. */
+  ItemNode getAChild() { this = result.getImmediateParent() }
+
   /** Gets the immediately enclosing module (or source file) of this item. */
   pragma[nomagic]
   ModuleLikeNode getImmediateParentModule() {
@@ -339,10 +342,13 @@ abstract class ItemNode extends Locatable {
     typeImplEdge(this, _, name, kind, result, useOpt)
     or
     // trait items with default implementations made available in an implementation
-    exists(ImplItemNodeImpl impl, ItemNode trait |
+    exists(ImplItemNodeImpl impl, TraitItemNode trait |
       this = impl and
       trait = impl.resolveTraitTyCand() and
       result = trait.getASuccessor(name, kind, useOpt) and
+      // do not inherit default implementations from super traits; those are inherited by
+      // their `impl` blocks
+      result = trait.getAssocItem(name) and
       result.(AssocItemNode).hasImplementation() and
       kind.isExternalOrBoth() and
       not impl.hasAssocItem(name)
@@ -402,8 +408,14 @@ abstract class ItemNode extends Locatable {
       this instanceof SourceFile and
       builtin(name, result)
       or
-      name = "Self" and
-      this = result.(ImplOrTraitItemNode).getAnItemInSelfScope()
+      exists(ImplOrTraitItemNode i |
+        name = "Self" and
+        this = i.getAnItemInSelfScope()
+      |
+        result = i.(Trait)
+        or
+        result = i.(ImplItemNodeImpl).resolveSelfTyCand()
+      )
       or
       name = "crate" and
       this = result.(CrateItemNode).getASourceFile()
@@ -734,7 +746,7 @@ abstract class ImplOrTraitItemNode extends ItemNode {
   Path getASelfPath() {
     Stages::PathResolutionStage::ref() and
     isUnqualifiedSelfPath(result) and
-    this = unqualifiedPathLookup(result, _, _)
+    result = this.getAnItemInSelfScope().getADescendant()
   }
 
   /** Gets an associated item belonging to this trait or `impl` block. */
@@ -960,7 +972,7 @@ private class ImplItemNodeImpl extends ImplItemNode {
     result = this.resolveSelfTyBuiltin()
   }
 
-  TraitItemNode resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
+  TraitItemNodeImpl resolveTraitTyCand() { result = resolvePathCand(this.getTraitPath()) }
 }
 
 private class StructItemNode extends TypeItemNode, ParameterizableItemNode instanceof Struct {
@@ -1813,15 +1825,7 @@ private module DollarCrateResolution {
 
 pragma[nomagic]
 private ItemNode resolvePathCand0(PathExt path, Namespace ns) {
-  exists(ItemNode res |
-    res = unqualifiedPathLookup(path, ns, _) and
-    if
-      not any(PathExt parent).getQualifier() = path and
-      isUnqualifiedSelfPath(path) and
-      res instanceof ImplItemNode
-    then result = res.(ImplItemNodeImpl).resolveSelfTyCand()
-    else result = res
-  )
+  result = unqualifiedPathLookup(path, ns, _)
   or
   DollarCrateResolution::resolveDollarCrate(path, result) and
   ns = result.getNamespace()
@@ -1883,12 +1887,35 @@ private predicate checkQualifiedVisibility(
   not i instanceof TypeParam
 }
 
+pragma[nomagic]
+private predicate isImplSelfQualifiedPath(
+  ImplItemNode impl, PathExt qualifier, PathExt path, string name
+) {
+  qualifier = impl.getASelfPath() and
+  qualifier = path.getQualifier() and
+  name = path.getText()
+}
+
+private ItemNode resolveImplSelfQualified(PathExt qualifier, PathExt path, Namespace ns) {
+  exists(ImplItemNode impl, string name |
+    isImplSelfQualifiedPath(impl, qualifier, path, name) and
+    result = impl.getAssocItem(name) and
+    ns = result.getNamespace()
+  )
+}
+
 /**
  * Gets the item that `path` resolves to in `ns` when `qualifier` is the
  * qualifier of `path` and `qualifier` resolves to `q`, if any.
  */
 pragma[nomagic]
 private ItemNode resolvePathCandQualified(PathExt qualifier, ItemNode q, PathExt path, Namespace ns) {
+  // Special case for `Self::Assoc`; this always refers to the associated
+  // item in the enclosing `impl` block, if available.
+  q = resolvePathCandQualifier(qualifier, path, _) and
+  result = resolveImplSelfQualified(qualifier, path, ns)
+  or
+  not exists(resolveImplSelfQualified(qualifier, path, ns)) and
   exists(string name, SuccessorKind kind, UseOption useOpt |
     q = resolvePathCandQualifier(qualifier, path, name) and
     result = getASuccessor(q, name, ns, kind, useOpt) and

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

@@ -385,6 +385,8 @@ TypeParamTypeParameter getPtrTypeParameter() {
 /** A type parameter. */
 abstract class TypeParameter extends Type {
   override TypeParameter getPositionalTypeParameter(int i) { none() }
+
+  abstract ItemNode getDeclaringItem();
 }
 
 private class RawTypeParameter = @type_param or @trait or @type_alias or @impl_trait_type_repr;
@@ -403,6 +405,8 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter {
 
   TypeParam getTypeParam() { result = typeParam }
 
+  override ItemNode getDeclaringItem() { result.getTypeParam(_) = typeParam }
+
   override string toString() { result = typeParam.toString() }
 
   override Location getLocation() { result = typeParam.getLocation() }
@@ -436,6 +440,8 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara
   /** Gets the trait that contains this associated type declaration. */
   TraitItemNode getTrait() { result.getAnAssocItem() = typeAlias }
 
+  override ItemNode getDeclaringItem() { result = this.getTrait() }
+
   override string toString() { result = typeAlias.getName().getText() }
 
   override Location getLocation() { result = typeAlias.getLocation() }
@@ -468,6 +474,8 @@ class DynTraitTypeParameter extends TypeParameter, TDynTraitTypeParameter {
     result = [this.getTypeParam().toString(), this.getTypeAlias().getName().toString()]
   }
 
+  override ItemNode getDeclaringItem() { none() }
+
   override string toString() { result = "dyn(" + this.toStringInner() + ")" }
 
   override Location getLocation() { result = n.getLocation() }
@@ -483,6 +491,8 @@ class ImplTraitTypeParameter extends TypeParameter, TImplTraitTypeParameter {
 
   ImplTraitTypeRepr getImplTraitTypeRepr() { result = implTrait }
 
+  override ItemNode getDeclaringItem() { none() }
+
   override string toString() { result = "impl(" + typeParam.toString() + ")" }
 
   override Location getLocation() { result = typeParam.getLocation() }
@@ -502,6 +512,8 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter {
 
   Trait getTrait() { result = trait }
 
+  override ItemNode getDeclaringItem() { result = trait }
+
   override string toString() { result = "Self [" + trait.toString() + "]" }
 
   override Location getLocation() { result = trait.getLocation() }
@@ -529,6 +541,8 @@ class ImplTraitTypeTypeParameter extends ImplTraitType, TypeParameter {
 
   ImplTraitTypeTypeParameter() { impl = function.getAParam().getTypeRepr() }
 
+  override ItemNode getDeclaringItem() { none() }
+
   override Function getFunction() { result = function }
 
   override TypeParameter getPositionalTypeParameter(int i) { none() }

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

@@ -11,7 +11,14 @@ import TypeInference::Consistency
 
 query predicate illFormedTypeMention(TypeMention tm) {
   Consistency::illFormedTypeMention(tm) and
-  not tm instanceof PathTypeReprMention and // avoid overlap with `PathTypeMention`
+  // avoid overlap with `PathTypeMention`
+  not tm instanceof PathTypeReprMention and
+  // known limitation for type mentions that would mention an escaping type parameter
+  not tm =
+    any(PathTypeMention ptm |
+      exists(ptm.resolvePathTypeAt(TypePath::nil())) and
+      not exists(ptm.resolveType())
+    ) and
   // Only include inconsistencies in the source, as we otherwise get
   // inconsistencies from library code in every project.
   tm.fromSource()

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

@@ -77,7 +77,19 @@ class SliceTypeReprMention extends TypeMention instanceof SliceTypeRepr {
   }
 }
 
-abstract class PathTypeMention extends TypeMention, Path { }
+abstract class PathTypeMention extends TypeMention, Path {
+  abstract Type resolvePathTypeAt(TypePath typePath);
+
+  final override Type resolveTypeAt(TypePath typePath) {
+    result = this.resolvePathTypeAt(typePath) and
+    (
+      not result instanceof TypeParameter
+      or
+      // Prevent type parameters from escaping their scope
+      this = result.(TypeParameter).getDeclaringItem().getAChild*().getADescendant()
+    )
+  }
+}
 
 class AliasPathTypeMention extends PathTypeMention {
   TypeAlias resolved;
@@ -94,7 +106,7 @@ class AliasPathTypeMention extends PathTypeMention {
    * Holds if this path resolved to a type alias with a rhs. that has the
    * resulting type at `typePath`.
    */
-  override Type resolveTypeAt(TypePath typePath) {
+  override Type resolvePathTypeAt(TypePath typePath) {
     result = rhs.resolveTypeAt(typePath) and
     not result = pathGetTypeParameter(resolved, _)
     or
@@ -275,7 +287,7 @@ class NonAliasPathTypeMention extends PathTypeMention {
     result = TAssociatedTypeTypeParameter(resolved)
   }
 
-  override Type resolveTypeAt(TypePath typePath) {
+  override Type resolvePathTypeAt(TypePath typePath) {
     typePath.isEmpty() and
     result = this.resolveRootType()
     or
@@ -307,7 +319,9 @@ class ImplSelfMention extends PathTypeMention {
 
   ImplSelfMention() { this = impl.getASelfPath() }
 
-  override Type resolveTypeAt(TypePath typePath) { result = resolveImplSelfTypeAt(impl, typePath) }
+  override Type resolvePathTypeAt(TypePath typePath) {
+    result = resolveImplSelfTypeAt(impl, typePath)
+  }
 }
 
 class PathTypeReprMention extends TypeMention, PathTypeRepr {

rust/ql/test/library-tests/path-resolution/CONSISTENCY/PathResolutionConsistency.expected

@@ -2,5 +2,8 @@ multipleCallTargets
 | main.rs:126:9:126:11 | f(...) |
 | main.rs:366:9:368:16 | ...::f(...) |
 | main.rs:369:9:371:16 | ...::f(...) |
-| main.rs:448:9:452:16 | ...::f(...) |
-| main.rs:453:9:457:16 | ...::f(...) |
+| main.rs:450:9:454:16 | ...::f(...) |
+| main.rs:455:9:459:16 | ...::f(...) |
+| main.rs:565:9:566:15 | ...::Assoc(...) |
+| main.rs:568:9:569:12 | ...::f1(...) |
+| main.rs:571:9:572:12 | ...::f1(...) |

rust/ql/test/library-tests/path-resolution/main.rs

@@ -381,8 +381,9 @@ mod m16 {
     > {
         fn f(&self) -> T; // $ item=I84
 
-        fn g(&self) -> T // $ item=I84
-        ; // I85
+        fn g(&self) -> T {// $ item=I84
+            self.f() // $ item=f
+        } // I85
 
         fn h(&self) -> T { // $ item=I84
             Self::g(&self); // $ item=I85
@@ -436,8 +437,9 @@ mod m16 {
     > // $ item=I89
       for S { // $ item=I90
         fn f(&self) -> S { // $ item=I90
+            Self::g(&self); // $ item=I92
             println!("m16::<S as Trait2<S>>::f"); // $ item=println
-            Self::c // $ MISSING: item=I95
+            Self::c // $ item=I95
         } // I93
     }
 
@@ -466,6 +468,109 @@ mod m16 {
           > // $ item=I86
         >::c; // $ MISSING: item=I95
     } // I83
+
+    trait Trait3 {
+        type AssocType;
+
+        fn f(&self);
+    }
+
+    trait Trait4 {
+        type AssocType;
+
+        fn g(&self);
+    }
+
+    struct S2;
+
+    #[rustfmt::skip]
+    impl Trait3 for S2 { // $ item=Trait3 item=S2
+        type AssocType = i32 // $ item=i32
+        ; // S2Trait3AssocType
+        
+        fn f(&self) {
+            let x: Self::AssocType = 42; // $ item=S2Trait3AssocType
+        } // S2asTrait3::f
+    }
+
+    #[rustfmt::skip]
+    impl Trait4 for S2 { // $ item=Trait4 item=S2
+        type AssocType = bool // $ item=bool
+        ; // S2Trait4AssocType
+
+        fn g(&self) {
+            Self::f(&self); // $ item=S2asTrait3::f
+            S2::f(&self); // $ item=S2asTrait3::f
+            let x: Self::AssocType = true; // $ item=S2Trait4AssocType
+        }
+    }
+
+    trait Trait5 {
+        type Assoc; // Trait5Assoc
+
+        fn Assoc() -> Self::Assoc; // $ item=Trait5Assoc
+    }
+
+    #[rustfmt::skip]
+    impl Trait5 for S { // $ item=Trait5 item=I90
+        type Assoc = i32 // $ item=i32
+        ; // AssocType
+
+        fn Assoc()
+            -> Self::Assoc { // $ item=AssocType
+            Self::Assoc() + 1 // $ item=AssocFunc
+        } // AssocFunc
+    }
+
+    struct S3<T3>(T3); // $ item=T3
+
+    #[rustfmt::skip]
+    impl Trait5 for S3<i32> { // $ item=Trait5 item=S3 item=i32
+        type Assoc = i32 // $ item=i32
+        ; // S3i32AssocType
+
+        fn Assoc()
+            -> Self::Assoc { // $ item=S3i32AssocType
+            Self::Assoc() + 1 // $ item=S3i32AssocFunc
+        } // S3i32AssocFunc
+    }
+
+    #[rustfmt::skip]
+    impl Trait5 for S3<bool> { // $ item=Trait5 item=S3 item=bool
+        type Assoc = bool // $ item=bool
+        ; // S3boolAssocType
+
+        fn Assoc()
+            -> Self::Assoc { // $ item=S3boolAssocType
+            !Self::Assoc() // $ item=S3boolAssocFunc
+        } // S3boolAssocFunc
+    }
+
+    #[rustfmt::skip]
+    impl S3<i32> { // $ item=S3 item=i32
+        fn f1() -> i32 { // $ item=i32
+            0
+        } // S3i32f1
+    }
+
+    #[rustfmt::skip]
+    impl S3<bool> { // $ item=S3 item=bool
+        fn f1() -> bool { // $ item=bool
+            true
+        } // S3boolf1
+    }
+
+    #[rustfmt::skip]
+    fn foo() {
+        S3::<i32>:: // $ item=i32
+        Assoc(); // $ item=S3i32AssocFunc $ SPURIOUS: item=S3boolAssocFunc
+
+        S3::<bool>:: // $ item=bool
+        f1(); // $ item=S3boolf1 $ SPURIOUS: item=S3i32f1
+        
+        S3::<i32>:: // $ item=i32
+        f1(); // $ item=S3i32f1 $ SPURIOUS: item=S3boolf1
+    }
 }
 
 mod trait_visibility {
@@ -805,7 +910,7 @@ mod patterns {
             N0ne => // local variable
                 N0ne
         }
-    } // patterns::test 
+    } // patterns::test
 
     #[rustfmt::skip]
     fn test2() -> Option<i32> { // $ item=Option $ item=i32