Merge pull request #19519 from hvitved/rust-extract-libs-follow-up

Rust: Follow-up work to make path resolution and type inference tests pass again

Author: hvitved

rust/ql/lib/codeql/files/FileSystem.qll

@@ -40,7 +40,9 @@ module Folder = Impl::Folder;
 class File extends Container, Impl::File {
   /** Holds if this file was extracted from ordinary source code. */
   predicate fromSource() {
-    exists(ExtractorStep s | s.getAction() = "Extract" and s.getFile() = this)
+    exists(ExtractorStep s | s.getAction() = "Extract" and s.getFile() = this) and
+    // instead use special extractor step for dependencies?
+    exists(this.getRelativePath())
   }
 
   /**

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

@@ -15,6 +15,7 @@ module Impl {
   private import rust
   private import codeql.rust.elements.internal.generated.ParentChild
   private import codeql.rust.controlflow.ControlFlowGraph
+  private import codeql.rust.elements.internal.MacroCallImpl::Impl as MacroCallImpl
 
   /**
    * Gets the immediate parent of a non-`AstNode` element `e`.
@@ -59,10 +60,20 @@ module Impl {
     }
 
     /** Holds if this node is inside a macro expansion. */
-    predicate isInMacroExpansion() {
-      this = any(MacroCall mc).getMacroCallExpansion()
-      or
-      this.getParentNode().isInMacroExpansion()
+    predicate isInMacroExpansion() { MacroCallImpl::isInMacroExpansion(_, this) }
+
+    /**
+     * Holds if this node exists only as the result of a macro expansion.
+     *
+     * This is the same as `isInMacroExpansion()`, but excludes AST nodes corresponding
+     * to macro arguments.
+     */
+    pragma[nomagic]
+    predicate isFromMacroExpansion() {
+      exists(MacroCall mc |
+        MacroCallImpl::isInMacroExpansion(mc, this) and
+        not this = mc.getATokenTreeNode()
+      )
     }
 
     /**

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

@@ -43,7 +43,7 @@ module Impl {
     File getFile() { result = this.getLocation().getFile() }
 
     /** Holds if this element is from source code. */
-    predicate fromSource() { exists(this.getFile().getRelativePath()) }
+    predicate fromSource() { this.getFile().fromSource() }
   }
 
   private @location_default getDbLocation(Locatable l) {

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

@@ -77,13 +77,76 @@ module LocationImpl {
       )
     }
 
-    /** Holds if this location starts strictly before the specified location. */
+    /** Holds if this location starts before location `that`. */
     pragma[inline]
-    predicate strictlyBefore(Location other) {
-      this.getStartLine() < other.getStartLine()
-      or
-      this.getStartLine() = other.getStartLine() and this.getStartColumn() < other.getStartColumn()
+    predicate startsBefore(Location that) {
+      exists(string f, int sl1, int sc1, int sl2, int sc2 |
+        this.hasLocationInfo(f, sl1, sc1, _, _) and
+        that.hasLocationInfo(f, sl2, sc2, _, _)
+      |
+        sl1 < sl2
+        or
+        sl1 = sl2 and sc1 <= sc2
+      )
+    }
+
+    /** Holds if this location starts strictly before location `that`. */
+    pragma[inline]
+    predicate startsStrictlyBefore(Location that) {
+      exists(string f, int sl1, int sc1, int sl2, int sc2 |
+        this.hasLocationInfo(f, sl1, sc1, _, _) and
+        that.hasLocationInfo(f, sl2, sc2, _, _)
+      |
+        sl1 < sl2
+        or
+        sl1 = sl2 and sc1 < sc2
+      )
+    }
+
+    /** Holds if this location ends after location `that`. */
+    pragma[inline]
+    predicate endsAfter(Location that) {
+      exists(string f, int el1, int ec1, int el2, int ec2 |
+        this.hasLocationInfo(f, _, _, el1, ec1) and
+        that.hasLocationInfo(f, _, _, el2, ec2)
+      |
+        el1 > el2
+        or
+        el1 = el2 and ec1 >= ec2
+      )
     }
+
+    /** Holds if this location ends strictly after location `that`. */
+    pragma[inline]
+    predicate endsStrictlyAfter(Location that) {
+      exists(string f, int el1, int ec1, int el2, int ec2 |
+        this.hasLocationInfo(f, _, _, el1, ec1) and
+        that.hasLocationInfo(f, _, _, el2, ec2)
+      |
+        el1 > el2
+        or
+        el1 = el2 and ec1 > ec2
+      )
+    }
+
+    /**
+     * Holds if this location contains location `that`, meaning that it starts
+     * before and ends after it.
+     */
+    pragma[inline]
+    predicate contains(Location that) { this.startsBefore(that) and this.endsAfter(that) }
+
+    /**
+     * Holds if this location strictlycontains location `that`, meaning that it starts
+     * strictly before and ends strictly after it.
+     */
+    pragma[inline]
+    predicate strictlyContains(Location that) {
+      this.startsStrictlyBefore(that) and this.endsStrictlyAfter(that)
+    }
+
+    /** Holds if this location is from source code. */
+    predicate fromSource() { this.getFile().fromSource() }
   }
 
   class LocationDefault extends Location, TLocationDefault {

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

@@ -11,6 +11,15 @@ private import codeql.rust.elements.internal.generated.MacroCall
  * be referenced directly.
  */
 module Impl {
+  private import rust
+
+  pragma[nomagic]
+  predicate isInMacroExpansion(MacroCall mc, AstNode n) {
+    n = mc.getMacroCallExpansion()
+    or
+    isInMacroExpansion(mc, n.getParentNode())
+  }
+
   // the following QLdoc is generated: if you need to edit it, do it in the schema file
   /**
    * A MacroCall. For example:
@@ -20,5 +29,12 @@ module Impl {
    */
   class MacroCall extends Generated::MacroCall {
     override string toStringImpl() { result = this.getPath().toAbbreviatedString() + "!..." }
+
+    /** Gets an AST node whose location is inside the token tree belonging to this macro call. */
+    pragma[nomagic]
+    AstNode getATokenTreeNode() {
+      isInMacroExpansion(this, result) and
+      this.getTokenTree().getLocation().contains(result.getLocation())
+    }
   }
 }

rust/ql/lib/codeql/rust/frameworks/stdlib/Stdlib.qll

@@ -7,6 +7,7 @@ private import codeql.rust.Concepts
 private import codeql.rust.controlflow.ControlFlowGraph as Cfg
 private import codeql.rust.controlflow.CfgNodes as CfgNodes
 private import codeql.rust.dataflow.DataFlow
+private import codeql.rust.internal.PathResolution
 
 /**
  * A call to the `starts_with` method on a `Path`.
@@ -32,10 +33,8 @@ class OptionEnum extends Enum {
     // todo: replace with canonical path, once calculated in QL
     exists(Crate core, Module m |
       core.getName() = "core" and
-      m = core.getSourceFile().getAnItem() and
-      m.getName().getText() = "option" and
-      this = m.getItemList().getAnItem() and
-      this.getName().getText() = "Option"
+      m = core.getSourceFile().(ItemNode).getASuccessor("option") and
+      this = m.(ItemNode).getASuccessor("Option")
     )
   }
 
@@ -53,10 +52,8 @@ class ResultEnum extends Enum {
     // todo: replace with canonical path, once calculated in QL
     exists(Crate core, Module m |
       core.getName() = "core" and
-      m = core.getSourceFile().getAnItem() and
-      m.getName().getText() = "result" and
-      this = m.getItemList().getAnItem() and
-      this.getName().getText() = "Result"
+      m = core.getSourceFile().(ItemNode).getASuccessor("result") and
+      this = m.(ItemNode).getASuccessor("Result")
     )
   }
 

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

@@ -194,11 +194,11 @@ abstract class ItemNode extends Locatable {
     this = result.(ImplOrTraitItemNode).getAnItemInSelfScope()
     or
     name = "crate" and
-    this = result.(CrateItemNode).getARootModuleNode()
+    this = result.(CrateItemNode).getASourceFile()
     or
     // todo: implement properly
     name = "$crate" and
-    result = any(CrateItemNode crate | this = crate.getARootModuleNode()).(Crate).getADependency*() and
+    result = any(CrateItemNode crate | this = crate.getASourceFile()).(Crate).getADependency*() and
     result.(CrateItemNode).isPotentialDollarCrateTarget()
   }
 
@@ -240,12 +240,6 @@ abstract private class ModuleLikeNode extends ItemNode {
       not mid instanceof ModuleLikeNode
     )
   }
-
-  /**
-   * Holds if this is a root module, meaning either a source file or
-   * the entry module of a crate.
-   */
-  predicate isRoot() { this instanceof SourceFileItemNode }
 }
 
 private class SourceFileItemNode extends ModuleLikeNode, SourceFile {
@@ -267,16 +261,13 @@ private class SourceFileItemNode extends ModuleLikeNode, SourceFile {
 
 class CrateItemNode extends ItemNode instanceof Crate {
   /**
-   * Gets the module node that defines this crate.
-   *
-   * This is either a source file, when the crate is defined in source code,
-   * or a module, when the crate is defined in a dependency.
+   * Gets the source file that defines this crate.
    */
   pragma[nomagic]
-  ModuleLikeNode getModuleNode() { result = super.getSourceFile() }
+  SourceFileItemNode getSourceFile() { result = super.getSourceFile() }
 
   /**
-   * Gets a source file that belongs to this crate, if any.
+   * Gets a source file that belongs to this crate.
    *
    * This is calculated as those source files that can be reached from the entry
    * file of this crate using zero or more `mod` imports, without going through
@@ -294,11 +285,6 @@ class CrateItemNode extends ItemNode instanceof Crate {
     )
   }
 
-  /**
-   * Gets a root module node belonging to this crate.
-   */
-  ModuleLikeNode getARootModuleNode() { result = this.getASourceFile() }
-
   pragma[nomagic]
   predicate isPotentialDollarCrateTarget() {
     exists(string name, RelevantPath p |
@@ -710,7 +696,7 @@ private predicate modImport0(Module m, string name, Folder lookup) {
     // sibling import
     lookup = parent and
     (
-      m.getFile() = any(CrateItemNode c).getModuleNode().(SourceFileItemNode).getFile()
+      m.getFile() = any(CrateItemNode c).getSourceFile().getFile()
       or
       m.getFile().getBaseName() = "mod.rs"
     )
@@ -798,25 +784,18 @@ private predicate fileImportEdge(Module mod, string name, ItemNode item) {
  */
 pragma[nomagic]
 private predicate crateDefEdge(CrateItemNode c, string name, ItemNode i) {
-  i = c.getModuleNode().getASuccessorRec(name) and
+  i = c.getSourceFile().getASuccessorRec(name) and
   not i instanceof Crate
 }
 
 /**
  * Holds if `m` depends on crate `dep` named `name`.
  */
 private predicate crateDependencyEdge(ModuleLikeNode m, string name, CrateItemNode dep) {
-  exists(CrateItemNode c | dep = c.(Crate).getDependency(name) |
-    // entry module/entry source file
-    m = c.getModuleNode()
-    or
-    // entry/transitive source file
+  exists(CrateItemNode c |
+    dep = c.(Crate).getDependency(name) and
     m = c.getASourceFile()
   )
-  or
-  // paths inside the crate graph use the name of the crate itself as prefix,
-  // although that is not valid in Rust
-  dep = any(Crate c | name = c.getName() and m = c.getSourceFile())
 }
 
 private predicate useTreeDeclares(UseTree tree, string name) {
@@ -884,9 +863,9 @@ class RelevantPath extends Path {
 
 private predicate isModule(ItemNode m) { m instanceof Module }
 
-/** Holds if root module `root` contains the module `m`. */
-private predicate rootHasModule(ItemNode root, ItemNode m) =
-  doublyBoundedFastTC(hasChild/2, isRoot/1, isModule/1)(root, m)
+/** Holds if source file `source` contains the module `m`. */
+private predicate rootHasModule(SourceFileItemNode source, ItemNode m) =
+  doublyBoundedFastTC(hasChild/2, isSourceFile/1, isModule/1)(source, m)
 
 pragma[nomagic]
 private ItemNode getOuterScope(ItemNode i) {
@@ -939,14 +918,14 @@ private ItemNode getASuccessorFull(ItemNode pred, string name, Namespace ns) {
   ns = result.getNamespace()
 }
 
-private predicate isRoot(ItemNode root) { root.(ModuleLikeNode).isRoot() }
+private predicate isSourceFile(ItemNode source) { source instanceof SourceFileItemNode }
 
 private predicate hasCratePath(ItemNode i) { any(RelevantPath path).isCratePath(_, i) }
 
 private predicate hasChild(ItemNode parent, ItemNode child) { child.getImmediateParent() = parent }
 
-private predicate rootHasCratePathTc(ItemNode i1, ItemNode i2) =
-  doublyBoundedFastTC(hasChild/2, isRoot/1, hasCratePath/1)(i1, i2)
+private predicate sourceFileHasCratePathTc(ItemNode i1, ItemNode i2) =
+  doublyBoundedFastTC(hasChild/2, isSourceFile/1, hasCratePath/1)(i1, i2)
 
 /**
  * Holds if the unqualified path `p` references a keyword item named `name`, and
@@ -956,10 +935,10 @@ pragma[nomagic]
 private predicate keywordLookup(ItemNode encl, string name, Namespace ns, RelevantPath p) {
   // For `($)crate`, jump directly to the root module
   exists(ItemNode i | p.isCratePath(name, i) |
-    encl.(ModuleLikeNode).isRoot() and
+    encl instanceof SourceFile and
     encl = i
     or
-    rootHasCratePathTc(encl, i)
+    sourceFileHasCratePathTc(encl, i)
   )
   or
   name = ["super", "self"] and
@@ -1176,8 +1155,8 @@ private module Debug {
   private Locatable getRelevantLocatable() {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
-      filepath.matches("%/test_logging.rs") and
-      startline = 163
+      filepath.matches("%/main.rs") and
+      startline = 284
     )
   }
 

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

@@ -1042,7 +1042,7 @@ private module Debug {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
       result.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
       filepath.matches("%/main.rs") and
-      startline = 28
+      startline = 948
     )
   }
 

rust/ql/lib/utils/test/PathResolutionInlineExpectationsTest.qll

@@ -18,7 +18,8 @@ private module ResolveTest implements TestSig {
   private predicate commmentAt(string text, string filepath, int line) {
     exists(Comment c |
       c.getLocation().hasLocationInfo(filepath, line, _, _, _) and
-      c.getCommentText().trim() = text
+      c.getCommentText().trim() = text and
+      c.fromSource()
     )
   }
 
@@ -35,6 +36,8 @@ private module ResolveTest implements TestSig {
     exists(AstNode n |
       not n = any(Path parent).getQualifier() and
       location = n.getLocation() and
+      n.fromSource() and
+      not n.isFromMacroExpansion() and
       element = n.toString() and
       tag = "item"
     |