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

Author: hvitved

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`.

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

@@ -196,11 +196,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()
   }
 
@@ -281,12 +281,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 {
@@ -312,16 +306,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
@@ -339,11 +330,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 |
@@ -985,7 +971,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"
     )
@@ -1073,25 +1059,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) {
@@ -1159,9 +1138,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) {
@@ -1214,14 +1193,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
@@ -1231,10 +1210,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
@@ -1449,12 +1428,8 @@ private predicate preludeEdge(SourceFile f, string name, ItemNode i) {
 private import codeql.rust.frameworks.stdlib.Bultins as Builtins
 
 pragma[nomagic]
-private predicate builtinEdge(ModuleLikeNode m, string name, ItemNode i) {
-  (
-    m instanceof SourceFile
-    or
-    m = any(CrateItemNode c).getModuleNode()
-  ) and
+private predicate builtinEdge(SourceFile source, string name, ItemNode i) {
+  exists(source) and
   exists(SourceFileItemNode builtins |
     builtins.getFile().getParentContainer() instanceof Builtins::BuiltinsFolder and
     i = builtins.getASuccessorRec(name)

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

@@ -1232,7 +1232,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"
     |

rust/ql/test/TestUtils.qll

@@ -1,12 +1,20 @@
 private import rust
 
-predicate toBeTested(Element e) { not e instanceof CrateElement and not e instanceof Builtin }
+predicate toBeTested(Element e) {
+  not e instanceof CrateElement and
+  not e instanceof Builtin and
+  (
+    not e instanceof Locatable
+    or
+    e.(Locatable).fromSource()
+  ) and
+  not e.(AstNode).isFromMacroExpansion()
+}
 
 class CrateElement extends Element {
   CrateElement() {
     this instanceof Crate or
-    this instanceof NamedCrate or
-    any(Crate c).getSourceFile() = this.(AstNode).getParentNode*()
+    this instanceof NamedCrate
   }
 }
 

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

@@ -16,21 +16,21 @@ mod my4 {
 }
 
 pub use my4::my5::f as nested_f; // $ item=I201
-
+#[rustfmt::skip]
 type Result<
     T, // T
 > = ::std::result::Result<
     T, // $ item=T
-    String,
->; // my::Result
+    String,> // $ item=Result
+; // my::Result
 
 fn int_div(
     x: i32, // $ item=i32
     y: i32, // $ item=i32
 ) -> Result<i32> // $ item=my::Result $ item=i32
 {
     if y == 0 {
-        return Err("Div by zero".to_string());
+        return Err("Div by zero".to_string()); // $ item=Err
     }
-    Ok(x / y)
+    Ok(x / y) // $ item=Ok
 }