Address review comments

Author: hvitved

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

@@ -120,7 +120,7 @@ module Stages {
       or
       exists(resolvePath(_))
       or
-      exists(any(ItemNode i).getASuccessor(_))
+      exists(any(ItemNode i).getASuccessorFull(_))
       or
       exists(any(ItemNode i).getASuccessorRec(_))
       or

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

@@ -172,9 +172,14 @@ abstract class ItemNode extends Locatable {
     result = this.(TypeParamItemNode).resolveABound().getASuccessorRec(name).(AssocItemNode)
   }
 
-  /** Gets a successor named `name` of this item, if any. */
+  /**
+   * Gets a successor named `name` of this item, if any.
+   *
+   * Whenever a function exists in both source code and in library code,
+   * both are included
+   */
   cached
-  ItemNode getASuccessor(string name) {
+  ItemNode getASuccessorFull(string name) {
     Stages::PathResolutionStage::ref() and
     result = this.getASuccessorRec(name)
     or
@@ -202,6 +207,22 @@ abstract class ItemNode extends Locatable {
     result.(CrateItemNode).isPotentialDollarCrateTarget()
   }
 
+  /** Gets a successor named `name` of this item, if any. */
+  pragma[nomagic]
+  ItemNode getASuccessor(string name) {
+    result = this.getASuccessorFull(name) and
+    (
+      // when a function exists in both source code and in library code, it is because
+      // we also extracted the source code as library code, and hence we only want
+      // the function from source code
+      result.fromSource()
+      or
+      not result instanceof Function
+      or
+      not this.getASuccessorFull(name).(Function).fromSource()
+    )
+  }
+
   /** Gets the location of this item. */
   Location getLocation() { result = super.getLocation() }
 }
@@ -234,7 +255,7 @@ abstract private class ModuleLikeNode extends ItemNode {
 private class SourceFileItemNode extends ModuleLikeNode, SourceFile {
   pragma[nomagic]
   ModuleLikeNode getSuper() {
-    result = any(ModuleItemNode mod | fileImport(mod, this)).getASuccessor("super")
+    result = any(ModuleItemNode mod | fileImport(mod, this)).getASuccessorFull("super")
   }
 
   override string getName() { result = "(source file)" }
@@ -297,7 +318,7 @@ class CrateItemNode extends ItemNode instanceof Crate {
   predicate isPotentialDollarCrateTarget() {
     exists(string name, RelevantPath p |
       p.isDollarCrateQualifiedPath(name) and
-      exists(this.getASuccessor(name))
+      exists(this.getASuccessorFull(name))
     )
   }
 
@@ -328,10 +349,8 @@ private class ConstItemNode extends AssocItemNode instanceof Const {
     or
     // for trait items from library code, we do not currently know if they
     // have default implementations or not, so we assume they do
-    exists(TraitItemNode t |
-      this = t.getAnAssocItem() and
-      not this.fromSource()
-    )
+    not this.fromSource() and
+    this = any(TraitItemNode t).getAnAssocItem()
   }
 
   override Namespace getNamespace() { result.isValue() }
@@ -373,10 +392,8 @@ class FunctionItemNode extends AssocItemNode instanceof Function {
     or
     // for trait items from library code, we do not currently know if they
     // have default implementations or not, so we assume they do
-    exists(TraitItemNode t |
-      this = t.getAnAssocItem() and
-      not this.fromSource()
-    )
+    not this.fromSource() and
+    this = any(TraitItemNode t).getAnAssocItem()
   }
 
   override Namespace getNamespace() { result.isValue() }
@@ -940,8 +957,8 @@ private predicate unqualifiedPathLookup(ItemNode encl, string name, Namespace ns
 }
 
 pragma[nomagic]
-private ItemNode getASuccessor(ItemNode pred, string name, Namespace ns) {
-  result = pred.getASuccessor(name) and
+private ItemNode getASuccessorFull(ItemNode pred, string name, Namespace ns) {
+  result = pred.getASuccessorFull(name) and
   ns = result.getNamespace()
 }
 
@@ -978,7 +995,7 @@ private predicate keywordLookup(ItemNode encl, string name, Namespace ns, Releva
 
 pragma[nomagic]
 private ItemNode unqualifiedPathLookup(RelevantPath p, Namespace ns) {
-  exists(ItemNode encl, string name | result = getASuccessor(encl, name, ns) |
+  exists(ItemNode encl, string name | result = getASuccessorFull(encl, name, ns) |
     unqualifiedPathLookup(encl, name, ns, p)
     or
     keywordLookup(encl, name, ns, p)
@@ -1002,7 +1019,7 @@ private ItemNode resolvePath0(RelevantPath path, Namespace ns) {
   or
   exists(ItemNode q, string name |
     q = resolvePathQualifier(path, name) and
-    result = getASuccessor(q, name, ns)
+    result = getASuccessorFull(q, name, ns)
   )
   or
   result = resolveUseTreeListItem(_, _, path) and
@@ -1127,12 +1144,12 @@ private ItemNode resolveUseTreeListItem(Use use, UseTree tree, RelevantPath path
     mid = resolveUseTreeListItem(use, midTree) and
     tree = midTree.getUseTreeList().getAUseTree() and
     isUseTreeSubPathUnqualified(tree, path, pragma[only_bind_into](name)) and
-    result = mid.getASuccessor(pragma[only_bind_into](name))
+    result = mid.getASuccessorFull(pragma[only_bind_into](name))
   )
   or
   exists(ItemNode q, string name |
     q = resolveUseTreeListItemQualifier(use, tree, path, name) and
-    result = q.getASuccessor(name)
+    result = q.getASuccessorFull(name)
   )
 }
 
@@ -1162,7 +1179,7 @@ private predicate useImportEdge(Use use, string name, ItemNode item) {
     then
       exists(ItemNode encl, Namespace ns |
         encl.getADescendant() = use and
-        item = getASuccessor(used, name, ns) and
+        item = getASuccessorFull(used, name, ns) and
         // glob imports can be shadowed
         not declares(encl, ns, name) and
         not name = ["super", "self", "Self", "$crate", "crate"]

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

@@ -29,26 +29,7 @@ newtype TType =
 abstract class Type extends TType {
   /** Gets the method `name` belonging to this type, if any. */
   pragma[nomagic]
-  final Function getMethod(string name) {
-    result = this.getAMethod(name) and
-    (
-      // when a method exists in both source code and in library code, it is because
-      // we also extracted the source code as library code, and hence we only want
-      // the method from source code
-      result.fromSource()
-      or
-      not this.getAMethod(name).fromSource()
-    )
-  }
-
-  /**
-   * Gets a method `name` belonging to this type, if any.
-   *
-   * Multiple methods may exist with the same name when it exists in both
-   * source code and in library code.
-   */
-  pragma[nomagic]
-  abstract Function getAMethod(string name);
+  abstract Function getMethod(string name);
 
   /** Gets the struct field `name` belonging to this type, if any. */
   pragma[nomagic]
@@ -93,7 +74,7 @@ abstract class Type extends TType {
 abstract private class StructOrEnumType extends Type {
   abstract ItemNode asItemNode();
 
-  final override Function getAMethod(string name) {
+  final override Function getMethod(string name) {
     result = this.asItemNode().getASuccessor(name) and
     exists(ImplOrTraitItemNode impl | result = impl.getAnAssocItem() |
       impl instanceof Trait
@@ -157,7 +138,7 @@ class TraitType extends Type, TTrait {
 
   TraitType() { this = TTrait(trait) }
 
-  override Function getAMethod(string name) { result = trait.(ItemNode).getASuccessor(name) }
+  override Function getMethod(string name) { result = trait.(ItemNode).getASuccessor(name) }
 
   override StructField getStructField(string name) { none() }
 
@@ -239,7 +220,7 @@ class ImplType extends Type, TImpl {
 
   ImplType() { this = TImpl(impl) }
 
-  override Function getAMethod(string name) { result = impl.(ItemNode).getASuccessor(name) }
+  override Function getMethod(string name) { result = impl.(ItemNode).getASuccessor(name) }
 
   override StructField getStructField(string name) { none() }
 
@@ -266,7 +247,7 @@ class ImplType extends Type, TImpl {
 class ArrayType extends Type, TArrayType {
   ArrayType() { this = TArrayType() }
 
-  override Function getAMethod(string name) { none() }
+  override Function getMethod(string name) { none() }
 
   override StructField getStructField(string name) { none() }
 
@@ -292,7 +273,7 @@ class ArrayType extends Type, TArrayType {
 class RefType extends Type, TRefType {
   RefType() { this = TRefType() }
 
-  override Function getAMethod(string name) { none() }
+  override Function getMethod(string name) { none() }
 
   override StructField getStructField(string name) { none() }
 
@@ -337,7 +318,7 @@ class TypeParamTypeParameter extends TypeParameter, TTypeParamTypeParameter {
 
   TypeParam getTypeParam() { result = typeParam }
 
-  override Function getAMethod(string name) {
+  override Function getMethod(string name) {
     // NOTE: If the type parameter has trait bounds, then this finds methods
     // on the bounding traits.
     result = typeParam.(ItemNode).getASuccessor(name)
@@ -396,7 +377,7 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara
 
   int getIndex() { traitAliasIndex(_, result, typeAlias) }
 
-  override Function getAMethod(string name) { none() }
+  override Function getMethod(string name) { none() }
 
   override string toString() { result = typeAlias.getName().getText() }
 
@@ -407,7 +388,7 @@ class AssociatedTypeTypeParameter extends TypeParameter, TAssociatedTypeTypePara
 
 /** An implicit reference type parameter. */
 class RefTypeParameter extends TypeParameter, TRefTypeParameter {
-  override Function getAMethod(string name) { none() }
+  override Function getMethod(string name) { none() }
 
   override string toString() { result = "&T" }
 
@@ -430,7 +411,7 @@ class SelfTypeParameter extends TypeParameter, TSelfTypeParameter {
 
   override TypeMention getABaseTypeMention() { result = trait }
 
-  override Function getAMethod(string name) {
+  override Function getMethod(string name) {
     // The `Self` type parameter is an implementation of the trait, so it has
     // all the trait's methods.
     result = trait.(ItemNode).getASuccessor(name)