Address review comments

Author: hvitved

rust/ql/integration-tests/hello-workspace/exe/src/main.rs

@@ -1,7 +1,10 @@
 use lib::a_module::hello; // $ item=HELLO
 
+use lib::my_macro2; // $ item=my_macro2
+
 mod a_module;
 
 fn main() {
+    my_macro2!(); // $ item=my_macro2
     hello(); // $ item=HELLO
 }

rust/ql/integration-tests/hello-workspace/functions.expected

@@ -1,2 +1,2 @@
-| exe/src/main.rs:5:1:7:1 | fn main |
+| exe/src/main.rs:7:1:10:1 | fn main |
 | lib/src/a_module/mod.rs:1:1:4:1 | fn hello |

rust/ql/integration-tests/hello-workspace/lib/src/a_module/mod.rs

@@ -1,4 +1,4 @@
 pub fn hello() {
-    my_macro!(); // $ item=my_macro
+    my_macro2!(); // $ item=my_macro2
     println!("Hello, world!"); // $ item=println
 } // HELLO

rust/ql/integration-tests/hello-workspace/lib/src/lib.rs

@@ -1,10 +1,17 @@
 #[macro_use]
 mod macros {
-    macro_rules! my_macro {
+    #[macro_export]
+    macro_rules! my_macro1 {
         () => {
             println!("my_macro!");
         };
     }
+    #[macro_export]
+    macro_rules! my_macro2 {
+        () => {
+            $crate::my_macro1!();
+        };
+    }
 }
 
 pub mod a_module;

rust/ql/integration-tests/hello-workspace/path-resolution.expected

@@ -0,0 +1,4 @@
+testFailures
+resolveDollarCrate
+| exe/src/main.rs:8:5:8:14 | $crate | lib/src/lib.rs:0:0:0:0 | Crate(lib@0.1.0) |
+| lib/src/a_module/mod.rs:2:5:2:14 | $crate | lib/src/lib.rs:0:0:0:0 | Crate(lib@0.1.0) |

rust/ql/integration-tests/hello-workspace/path-resolution.ql

@@ -1 +1,10 @@
+import rust
+import codeql.rust.internal.PathResolution
 import utils.test.PathResolutionInlineExpectationsTest
+
+query predicate resolveDollarCrate(RelevantPath p, Crate c) {
+  c = resolvePath(p) and
+  p.isDollarCrate() and
+  p.fromSource() and
+  c.fromSource()
+}

rust/ql/integration-tests/hello-workspace/summary.cargo.expected

@@ -9,8 +9,8 @@
 | Inconsistencies - Path resolution | 0 |
 | Inconsistencies - SSA | 0 |
 | Inconsistencies - data flow | 0 |
-| Lines of code extracted | 15 |
-| Lines of user code extracted | 15 |
-| Macro calls - resolved | 5 |
-| Macro calls - total | 5 |
+| Lines of code extracted | 21 |
+| Lines of user code extracted | 21 |
+| Macro calls - resolved | 10 |
+| Macro calls - total | 10 |
 | Macro calls - unresolved | 0 |

rust/ql/integration-tests/hello-workspace/summary.rust-project.expected

@@ -9,8 +9,8 @@
 | Inconsistencies - Path resolution | 0 |
 | Inconsistencies - SSA | 0 |
 | Inconsistencies - data flow | 0 |
-| Lines of code extracted | 15 |
-| Lines of user code extracted | 15 |
-| Macro calls - resolved | 5 |
-| Macro calls - total | 5 |
+| Lines of code extracted | 21 |
+| Lines of user code extracted | 21 |
+| Macro calls - resolved | 10 |
+| Macro calls - total | 10 |
 | Macro calls - unresolved | 0 |

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

@@ -1559,7 +1559,6 @@ private predicate unqualifiedPathLookup(ItemNode ancestor, string name, Namespac
   exists(RelevantPath path |
     path.isUnqualified(name, encl) and
     ancestor = encl and
-    exists(ns) and
     not name = ["crate", "$crate", "super", "self"]
   |
     pathUsesNamespace(path, ns)
@@ -1668,7 +1667,7 @@ module TraitIsVisible<relevantTraitVisibleSig/2 relevantTraitVisible> {
 }
 
 pragma[nomagic]
-private predicate isMacroExpansion(AstNode expansion, Path macroDefPath) {
+private predicate isDollarCrateSupportedMacroExpansion(Path macroDefPath, AstNode expansion) {
   exists(MacroCall mc |
     expansion = mc.getMacroCallExpansion() and
     macroDefPath = mc.getPath()
@@ -1681,25 +1680,25 @@ private predicate isMacroExpansion(AstNode expansion, Path macroDefPath) {
 }
 
 pragma[nomagic]
-predicate isInMacroExpansion(Path macroDefPath, AstNode n) {
-  isMacroExpansion(n, macroDefPath)
+predicate isInDollarCrateSupportedMacroExpansion(File macroDefFile, AstNode n) {
+  exists(Path macroDefPath |
+    isDollarCrateSupportedMacroExpansion(macroDefPath, n) and
+    macroDefFile = resolvePathCand(macroDefPath).getFile()
+  )
   or
-  isInMacroExpansion(macroDefPath, n.getParentNode())
+  isInDollarCrateSupportedMacroExpansion(macroDefFile, n.getParentNode())
 }
 
 /**
- * Holds if `n` is inside a macro expansion, and the macro _may_ originate from
- * crate `crate`.
+ * Holds if `n` is a `$crate` path, and it may resolve to `crate`.
  *
  * The reason why we cannot be sure is that we need to consider all ancestor macro
  * calls.
  */
 pragma[nomagic]
-predicate isInMacroFromCrateExpansion(CrateItemNode crate, AstNode n) {
-  exists(Path macroDefPath |
-    isInMacroExpansion(macroDefPath, n) and
-    crate.getASourceFile().getFile() = resolvePathCand(macroDefPath).getFile()
-  )
+predicate resolveDollarCrate(RelevantPath p, CrateItemNode crate) {
+  p.isDollarCrate() and
+  isInDollarCrateSupportedMacroExpansion(crate.getASourceFile().getFile(), p)
 }
 
 pragma[nomagic]
@@ -1714,8 +1713,7 @@ private ItemNode resolvePathCand0(RelevantPath path, Namespace ns) {
     else result = res
   )
   or
-  path.isDollarCrate() and
-  isInMacroFromCrateExpansion(result, path) and
+  resolveDollarCrate(path, result) and
   ns = result.getNamespace()
   or
   result = resolvePathCandQualified(_, _, path, ns)