Merge pull request #19945 from github/redsun82/fix-expansion-in-lib

Rust: fix macro expansion in library code

Author: redsun82

rust/extractor/src/config.rs

@@ -71,6 +71,7 @@ pub struct Config {
     pub proc_macro_server: Option<PathBuf>,
     pub skip_path_resolution: bool,
     pub extract_dependencies_as_source: bool,
+    pub force_library_mode: bool, // for testing purposes
 }
 
 impl Config {

rust/extractor/src/main.rs

@@ -293,6 +293,11 @@ fn main() -> anyhow::Result<()> {
     } else {
         (SourceKind::Library, ResolvePaths::No)
     };
+    let (source_mode, source_resolve_paths) = if cfg.force_library_mode {
+        (library_mode, library_resolve_paths)
+    } else {
+        (SourceKind::Source, resolve_paths)
+    };
     let mut processed_files: HashSet<PathBuf, RandomState> =
         HashSet::from_iter(files.iter().cloned());
     for (manifest, files) in map.values().filter(|(_, files)| !files.is_empty()) {
@@ -311,12 +316,10 @@ fn main() -> anyhow::Result<()> {
                         file,
                         &semantics,
                         vfs,
-                        resolve_paths,
-                        SourceKind::Source,
+                        source_resolve_paths,
+                        source_mode,
                     ),
-                    Err(reason) => {
-                        extractor.extract_without_semantics(file, SourceKind::Source, &reason)
-                    }
+                    Err(reason) => extractor.extract_without_semantics(file, source_mode, &reason),
                 };
             }
             for (file_id, file) in vfs.iter() {

rust/extractor/src/qltest.rs

@@ -19,7 +19,7 @@ fn dump_lib() -> anyhow::Result<()> {
         .iter()
         .map(|p| p.file_stem().expect("results of glob must have a name"))
         .filter(|&p| !["main", "lib", "proc_macro"].map(OsStr::new).contains(&p))
-        .map(|p| format!("mod {};", p.to_string_lossy()))
+        .map(|p| format!("pub mod {};", p.to_string_lossy()))
         .join("\n");
     fs::write("lib.rs", lib).context("writing lib.rs")
 }

rust/extractor/src/translate/base.rs

@@ -24,33 +24,31 @@ use ra_ap_syntax::{
 
 impl Emission<ast::Item> for Translator<'_> {
     fn pre_emit(&mut self, node: &ast::Item) -> Option<Label<generated::Item>> {
-        self.prepare_item_expansion(node).map(Into::into)
+        self.item_pre_emit(node).map(Into::into)
     }
 
     fn post_emit(&mut self, node: &ast::Item, label: Label<generated::Item>) {
-        self.emit_item_expansion(node, label);
+        self.item_post_emit(node, label);
     }
 }
 
 impl Emission<ast::AssocItem> for Translator<'_> {
     fn pre_emit(&mut self, node: &ast::AssocItem) -> Option<Label<generated::AssocItem>> {
-        self.prepare_item_expansion(&node.clone().into())
-            .map(Into::into)
+        self.item_pre_emit(&node.clone().into()).map(Into::into)
     }
 
     fn post_emit(&mut self, node: &ast::AssocItem, label: Label<generated::AssocItem>) {
-        self.emit_item_expansion(&node.clone().into(), label.into());
+        self.item_post_emit(&node.clone().into(), label.into());
     }
 }
 
 impl Emission<ast::ExternItem> for Translator<'_> {
     fn pre_emit(&mut self, node: &ast::ExternItem) -> Option<Label<generated::ExternItem>> {
-        self.prepare_item_expansion(&node.clone().into())
-            .map(Into::into)
+        self.item_pre_emit(&node.clone().into()).map(Into::into)
     }
 
     fn post_emit(&mut self, node: &ast::ExternItem, label: Label<generated::ExternItem>) {
-        self.emit_item_expansion(&node.clone().into(), label.into());
+        self.item_post_emit(&node.clone().into(), label.into());
     }
 }
 
@@ -849,35 +847,6 @@ impl<'a> Translator<'a> {
             })
     }
 
-    pub(crate) fn prepare_item_expansion(
-        &mut self,
-        node: &ast::Item,
-    ) -> Option<Label<generated::MacroCall>> {
-        if self.source_kind == SourceKind::Library {
-            // if the item expands via an attribute macro, we want to only emit the expansion
-            if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
-                // we wrap it in a dummy MacroCall to get a single Item label that can replace
-                // the original Item
-                let label = self.trap.emit(generated::MacroCall {
-                    id: TrapId::Star,
-                    attrs: vec![],
-                    path: None,
-                    token_tree: None,
-                });
-                generated::MacroCall::emit_macro_call_expansion(
-                    label,
-                    expanded.into(),
-                    &mut self.trap.writer,
-                );
-                return Some(label);
-            }
-        }
-        if self.is_attribute_macro_target(node) {
-            self.macro_context_depth += 1;
-        }
-        None
-    }
-
     fn process_item_macro_expansion(
         &mut self,
         node: &impl ast::AstNode,
@@ -915,10 +884,6 @@ impl<'a> Translator<'a> {
         &mut self,
         node: &ast::Item,
     ) -> Option<Label<generated::MacroItems>> {
-        if !self.is_attribute_macro_target(node) {
-            return None;
-        }
-        self.macro_context_depth -= 1;
         if self.macro_context_depth > 0 {
             // only expand the outermost attribute macro
             return None;
@@ -927,7 +892,49 @@ impl<'a> Translator<'a> {
         self.process_item_macro_expansion(node, expansion.map(|x| x.value))
     }
 
-    pub(crate) fn emit_item_expansion(&mut self, node: &ast::Item, label: Label<generated::Item>) {
+    pub(crate) fn item_pre_emit(
+        &mut self,
+        node: &ast::Item,
+    ) -> Option<Label<generated::MacroCall>> {
+        if !self.is_attribute_macro_target(node) {
+            return None;
+        }
+        if self.source_kind == SourceKind::Library {
+            // if the item expands via an attribute macro, we want to only emit the expansion
+            if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
+                // we wrap it in a dummy MacroCall to get a single Item label that can replace
+                // the original Item
+                let label = self.trap.emit(generated::MacroCall {
+                    id: TrapId::Star,
+                    attrs: vec![],
+                    path: None,
+                    token_tree: None,
+                });
+                generated::Item::emit_attribute_macro_expansion(
+                    label.into(),
+                    expanded,
+                    &mut self.trap.writer,
+                );
+                self.emit_location(label, node);
+                return Some(label);
+            }
+        }
+        self.macro_context_depth += 1;
+        None
+    }
+
+    pub(crate) fn item_post_emit(&mut self, node: &ast::Item, label: Label<generated::Item>) {
+        if !self.is_attribute_macro_target(node) {
+            return;
+        }
+        // see `item_pre_emit`:
+        // if self.is_attribute_macro_target(node), then we either exited early with `Some(label)`
+        // and are not here, or we did self.macro_context_depth += 1
+        assert!(
+            self.macro_context_depth > 0,
+            "macro_context_depth should be > 0 for an attribute macro target"
+        );
+        self.macro_context_depth -= 1;
         if let Some(expanded) = self.emit_attribute_macro_expansion(node) {
             generated::Item::emit_attribute_macro_expansion(label, expanded, &mut self.trap.writer);
         }

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

@@ -32,7 +32,9 @@ module Impl {
    * ```
    */
   class MacroCall extends Generated::MacroCall {
-    override string toStringImpl() { result = this.getPath().toAbbreviatedString() + "!..." }
+    override string toStringImpl() {
+      if this.hasPath() then result = this.getPath().toAbbreviatedString() + "!..." else result = ""
+    }
 
     /** Gets an AST node whose location is inside the token tree belonging to this macro call. */
     pragma[nomagic]

rust/ql/test/TestUtils.qll

@@ -6,7 +6,7 @@ predicate toBeTested(Element e) {
   (
     not e instanceof Locatable
     or
-    e.(Locatable).fromSource()
+    exists(e.(Locatable).getFile().getRelativePath())
   )
 }
 

rust/ql/test/extractor-tests/canonical_path/canonical_paths.expected

@@ -2,8 +2,8 @@ canonicalPath
 | anonymous.rs:3:1:32:1 | fn canonicals | test::anonymous::canonicals |
 | anonymous.rs:34:1:36:1 | fn other | test::anonymous::other |
 | {EXTERNAL LOCATION} | fn trim | <core::str>::trim |
-| lib.rs:1:1:1:14 | mod anonymous | test::anonymous |
-| lib.rs:2:1:2:12 | mod regular | test::regular |
+| lib.rs:1:1:1:18 | mod anonymous | test::anonymous |
+| lib.rs:2:1:2:16 | mod regular | test::regular |
 | regular.rs:1:1:2:18 | struct Struct | test::regular::Struct |
 | regular.rs:2:12:2:17 | fn eq | <test::regular::Struct as core::cmp::PartialEq>::eq |
 | regular.rs:2:12:2:17 | impl ...::Eq for Struct::<...> { ... } | <test::regular::Struct as core::cmp::Eq> |
@@ -42,8 +42,8 @@ canonicalPaths
 | anonymous.rs:26:5:31:5 | fn usage | None | None |
 | anonymous.rs:34:1:36:1 | fn other | repo::test | crate::anonymous::other |
 | anonymous.rs:35:5:35:23 | struct OtherStruct | None | None |
-| lib.rs:1:1:1:14 | mod anonymous | repo::test | crate::anonymous |
-| lib.rs:2:1:2:12 | mod regular | repo::test | crate::regular |
+| lib.rs:1:1:1:18 | mod anonymous | repo::test | crate::anonymous |
+| lib.rs:2:1:2:16 | mod regular | repo::test | crate::regular |
 | regular.rs:1:1:2:18 | struct Struct | repo::test | crate::regular::Struct |
 | regular.rs:2:12:2:17 | fn eq | repo::test | <crate::regular::Struct as crate::cmp::PartialEq>::eq |
 | regular.rs:2:12:2:17 | impl ...::Eq for Struct::<...> { ... } | None | None |

rust/ql/test/extractor-tests/canonical_path_disabled/canonical_paths.expected

@@ -2,8 +2,8 @@ canonicalPath
 | anonymous.rs:6:1:35:1 | fn canonicals | test::anonymous::canonicals |
 | anonymous.rs:37:1:39:1 | fn other | test::anonymous::other |
 | {EXTERNAL LOCATION} | fn trim | <core::str>::trim |
-| lib.rs:1:1:1:14 | mod anonymous | test::anonymous |
-| lib.rs:2:1:2:12 | mod regular | test::regular |
+| lib.rs:1:1:1:18 | mod anonymous | test::anonymous |
+| lib.rs:2:1:2:16 | mod regular | test::regular |
 | regular.rs:4:1:5:18 | struct Struct | test::regular::Struct |
 | regular.rs:5:12:5:17 | fn eq | <test::regular::Struct as core::cmp::PartialEq>::eq |
 | regular.rs:5:12:5:17 | impl ...::Eq for Struct::<...> { ... } | <test::regular::Struct as core::cmp::Eq> |
@@ -42,8 +42,8 @@ canonicalPaths
 | anonymous.rs:29:5:34:5 | fn usage | None | None |
 | anonymous.rs:37:1:39:1 | fn other | None | None |
 | anonymous.rs:38:5:38:23 | struct OtherStruct | None | None |
-| lib.rs:1:1:1:14 | mod anonymous | None | None |
-| lib.rs:2:1:2:12 | mod regular | None | None |
+| lib.rs:1:1:1:18 | mod anonymous | None | None |
+| lib.rs:2:1:2:16 | mod regular | None | None |
 | regular.rs:4:1:5:18 | struct Struct | None | None |
 | regular.rs:5:12:5:17 | fn eq | None | None |
 | regular.rs:5:12:5:17 | impl ...::Eq for Struct::<...> { ... } | None | None |

rust/ql/test/extractor-tests/generated/Module/Module.expected

@@ -1,21 +1,22 @@
 instances
 | gen_module.rs:3:1:4:8 | mod foo |
 | gen_module.rs:5:1:7:1 | mod bar |
-| lib.rs:1:1:1:15 | mod gen_module |
+| lib.rs:1:1:1:19 | mod gen_module |
 getExtendedCanonicalPath
 | gen_module.rs:3:1:4:8 | mod foo | crate::gen_module::foo |
 | gen_module.rs:5:1:7:1 | mod bar | crate::gen_module::bar |
-| lib.rs:1:1:1:15 | mod gen_module | crate::gen_module |
+| lib.rs:1:1:1:19 | mod gen_module | crate::gen_module |
 getCrateOrigin
 | gen_module.rs:3:1:4:8 | mod foo | repo::test |
 | gen_module.rs:5:1:7:1 | mod bar | repo::test |
-| lib.rs:1:1:1:15 | mod gen_module | repo::test |
+| lib.rs:1:1:1:19 | mod gen_module | repo::test |
 getAttributeMacroExpansion
 getAttr
 getItemList
 | gen_module.rs:5:1:7:1 | mod bar | gen_module.rs:5:9:7:1 | ItemList |
 getName
 | gen_module.rs:3:1:4:8 | mod foo | gen_module.rs:4:5:4:7 | foo |
 | gen_module.rs:5:1:7:1 | mod bar | gen_module.rs:5:5:5:7 | bar |
-| lib.rs:1:1:1:15 | mod gen_module | lib.rs:1:5:1:14 | gen_module |
+| lib.rs:1:1:1:19 | mod gen_module | lib.rs:1:9:1:18 | gen_module |
 getVisibility
+| lib.rs:1:1:1:19 | mod gen_module | lib.rs:1:1:1:3 | Visibility |

rust/ql/test/extractor-tests/generated/Name/Name.expected

@@ -1,8 +1,8 @@
 instances
 | gen_name.rs:3:4:3:12 | test_name |
 | gen_name.rs:7:9:7:11 | foo |
-| lib.rs:1:5:1:12 | gen_name |
+| lib.rs:1:9:1:16 | gen_name |
 getText
 | gen_name.rs:3:4:3:12 | test_name | test_name |
 | gen_name.rs:7:9:7:11 | foo | foo |
-| lib.rs:1:5:1:12 | gen_name | gen_name |
+| lib.rs:1:9:1:16 | gen_name | gen_name |