Rust: Path resolution improvements

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

@@ -72,9 +72,9 @@ private ItemNode getAChildSuccessor(ItemNode item, string name, SuccessorKind ki
       if item instanceof ImplOrTraitItemNode and result instanceof AssocItem
       then kind.isExternal()
       else
-        if result instanceof Use
-        then kind.isInternal()
-        else kind.isBoth()
+        if result.isPublic()
+        then kind.isBoth()
+        else kind.isInternal()
   )
 }
 
@@ -165,6 +165,20 @@ abstract class ItemNode extends Locatable {
   /** Gets the visibility of this item, if any. */
   abstract Visibility getVisibility();
 
+  /**
+   * Holds if this item is public.
+   *
+   * This is the case when this item either has `pub` visibility (but is not
+   * a `use`; a `use` itself is not visible from the outside), or when this
+   * item is a variant.
+   */
+  predicate isPublic() {
+    exists(this.getVisibility()) and
+    not this instanceof Use
+    or
+    this instanceof Variant
+  }
+
   /** Gets the `i`th type parameter of this item, if any. */
   abstract TypeParam getTypeParam(int i);
 
@@ -380,9 +394,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")
-  }
+  ModuleLikeNode getSuper() { fileImport(result.getAnItemInScope(), this) }
 
   override string getName() { result = "(source file)" }
 
@@ -1300,7 +1312,8 @@ private predicate useTreeDeclares(UseTree tree, string name) {
  */
 pragma[nomagic]
 private predicate declaresDirectly(ItemNode item, Namespace ns, string name) {
-  exists(ItemNode child, SuccessorKind kind | child = getAChildSuccessor(item, name, kind) |
+  exists(ItemNode child, SuccessorKind kind |
+    child = getAChildSuccessor(item, name, kind) and
     child.getNamespace() = ns and
     kind.isInternalOrBoth()
   )
@@ -1491,6 +1504,13 @@ private ItemNode resolvePathCandQualifier(RelevantPath qualifier, RelevantPath p
   name = path.getText()
 }
 
+pragma[nomagic]
+private Crate getCrate0(Locatable l) { result.getASourceFile().getFile() = l.getFile() }
+
+bindingset[l]
+pragma[inline_late]
+private Crate getCrate(Locatable l) { result = getCrate0(l) }
+
 /**
  * Gets the item that `path` resolves to in `ns` when `qualifier` is the
  * qualifier of `path` and `qualifier` resolves to `q`, if any.
@@ -1501,8 +1521,17 @@ private ItemNode resolvePathCandQualified(
 ) {
   exists(string name, SuccessorKind kind |
     q = resolvePathCandQualifier(qualifier, path, name) and
-    result = getASuccessor(q, name, ns, kind) and
+    result = getASuccessor(q, name, ns, kind)
+  |
     kind.isExternalOrBoth()
+    or
+    // Non-public items are visible to paths in descendant modules of the declaring
+    // module; the declaration may happen via a `use` statement, where the item
+    // being used is _not_ itself in an ancestor module, and we currently don't track
+    // that information in `getASuccessor`. So, for simplicity, we allow for non-public
+    // items when the path and the item are in the same crate.
+    getCrate(path) = getCrate(result) and
+    not result instanceof TypeParam
   )
 }
 
@@ -1646,10 +1675,12 @@ private ItemNode resolveUseTreeListItemQualifier(
 
 pragma[nomagic]
 private ItemNode resolveUseTreeListItem(Use use, UseTree tree) {
-  tree = use.getUseTree() and
-  result = resolvePathCand(tree.getPath())
-  or
-  result = resolveUseTreeListItem(use, tree, tree.getPath(), _)
+  exists(Path path | path = tree.getPath() |
+    tree = use.getUseTree() and
+    result = resolvePathCand(path)
+    or
+    result = resolveUseTreeListItem(use, tree, path, _)
+  )
 }
 
 /** Holds if `use` imports `item` as `name`. */
@@ -1673,7 +1704,10 @@ private predicate useImportEdge(Use use, string name, ItemNode item, SuccessorKi
       item = used and
       (
         not tree.hasRename() and
-        name = item.getName()
+        exists(string pathName |
+          pathName = tree.getPath().getText() and
+          if pathName = "self" then name = item.getName() else name = pathName
+        )
         or
         exists(Rename rename | rename = tree.getRename() |
           name = rename.getName().getText()

rust/ql/test/library-tests/dataflow/sources/CONSISTENCY/PathResolutionConsistency.expected

@@ -11,6 +11,8 @@ multipleCallTargets
 | test.rs:179:30:179:68 | ...::_print(...) |
 | test.rs:188:26:188:105 | ...::_print(...) |
 | test.rs:229:22:229:72 | ... .read_to_string(...) |
+| test.rs:664:22:664:43 | file.read(...) |
+| test.rs:673:22:673:41 | f1.read(...) |
 | test.rs:697:18:697:38 | ...::_print(...) |
 | test.rs:702:18:702:45 | ...::_print(...) |
 | test.rs:720:38:720:42 | ...::_print(...) |

rust/ql/test/library-tests/dataflow/sources/test.rs

@@ -662,7 +662,7 @@ async fn test_async_std_file() -> std::io::Result<()> {
     {
         let mut buffer = [0u8; 100];
         let _bytes = file.read(&mut buffer).await?;
-        sink(&buffer); // $ MISSING: hasTaintFlow="file.txt"
+        sink(&buffer); // $ hasTaintFlow="file.txt"
     }
 
     // --- OpenOptions ---
@@ -671,7 +671,7 @@ async fn test_async_std_file() -> std::io::Result<()> {
         let mut f1 = async_std::fs::OpenOptions::new().open("f1.txt").await?; // $ Alert[rust/summary/taint-sources]
         let mut buffer = [0u8; 1024];
         let _bytes = f1.read(&mut buffer).await?;
-        sink(&buffer); // $ MISSING: hasTaintFlow="f1.txt"
+        sink(&buffer); // $ hasTaintFlow="f1.txt"
     }
 
     Ok(())

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

@@ -1,4 +1,3 @@
 multipleCallTargets
 | main.rs:124:9:124:11 | f(...) |
-| main.rs:774:5:774:7 | f(...) |
 | proc_macro.rs:9:5:9:10 | ...::new(...) |

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

@@ -771,7 +771,7 @@ fn main() {
     my::nested::nested1::nested2::f(); // $ item=I4
     my::f(); // $ item=I38
     nested2::nested3::nested4::f(); // $ item=I12
-    f(); // $ item=I12 $ SPURIOUS: item=I119
+    f(); // $ item=I12
     g(); // $ item=I13
     crate::h(); // $ item=I25
     m1::m2::g(); // $ item=I19

rust/ql/test/library-tests/path-resolution/my2/my3/mod.rs

@@ -7,4 +7,4 @@ pub fn f() {
 use super::super::h; // $ item=I25
 use super::g; // $ item=I9
 
-use super::nested6_f; // $ MISSING: item=I116
+use super::nested6_f; // $ item=I116

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

@@ -361,7 +361,6 @@ resolvePath
 | main.rs:773:5:773:29 | ...::nested4 | my2/nested2.rs:2:5:10:5 | mod nested4 |
 | main.rs:773:5:773:32 | ...::f | my2/nested2.rs:3:9:5:9 | fn f |
 | main.rs:774:5:774:5 | f | my2/nested2.rs:3:9:5:9 | fn f |
-| main.rs:774:5:774:5 | f | my2/nested2.rs:23:9:25:9 | fn f |
 | main.rs:775:5:775:5 | g | my2/nested2.rs:7:9:9:9 | fn g |
 | main.rs:776:5:776:9 | crate | main.rs:0:0:0:0 | Crate(main@0.0.1) |
 | main.rs:776:5:776:12 | ...::h | main.rs:56:1:75:1 | fn h |
@@ -440,6 +439,7 @@ resolvePath
 | my2/my3/mod.rs:8:5:8:9 | super | my2/mod.rs:1:1:23:34 | SourceFile |
 | my2/my3/mod.rs:8:5:8:12 | ...::g | my2/mod.rs:3:1:6:1 | fn g |
 | my2/my3/mod.rs:10:5:10:9 | super | my2/mod.rs:1:1:23:34 | SourceFile |
+| my2/my3/mod.rs:10:5:10:20 | ...::nested6_f | my2/nested2.rs:15:9:17:9 | fn f |
 | my.rs:3:5:3:10 | nested | my.rs:1:1:1:15 | mod nested |
 | my.rs:3:5:3:13 | ...::g | my/nested.rs:19:1:22:1 | fn g |
 | my.rs:11:5:11:5 | g | my/nested.rs:19:1:22:1 | fn g |