Fix false-positive "macro no longer exported" report. (#1043)

The old implementation checked that there's a macro with a matching name
that isn't exported in the linted code, but didn't check that there is
no *exported* macro by that name in the code. This was the cause of
issue #1042.

Fixes #1042.

Cargo.toml

@@ -15,7 +15,7 @@ exclude = [".github/", "brand/", "scripts/", "test_crates/", "test_outputs/", "t
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-trustfall = "0.8.0"
+trustfall = "0.8.0"  # Ensure this matches the `trustfall_core` dev-dependency version below.
 # `cargo_metadata` is used at the API boundary of `trustfall_rustdoc`,
 # so ensure the version we use for `cargo_metadata` here matches what `trustfall_rustdoc` uses too.
 trustfall_rustdoc = { version = "0.19.0", default-features = false, features = ["v32", "v33", "v35", "v36", "v37", "rayon", "rustc-hash"] }
@@ -62,6 +62,7 @@ insta = { version = "1.41.1", features = ["ron", "filters", "toml"] }
 regex = "1.11.1"
 insta-cmd = "0.6.0"
 rayon = "1.10.0"
+trustfall_core = "0.8.0"  # Ensure this matches the `trustfall` version above.
 
 # In dev and test profiles, compile all dependencies with optimizations enabled,
 # but still checking debug assertions and overflows.

src/lints/declarative_macro_missing.ron

@@ -36,7 +36,7 @@ SemverQuery(
         "zero": 0,
         "true": true,
     },
-    error_message: "A `macro_rules` declarative macro cannot be imported by its prior name. The macro may have been renamed or removed entirely.",
+    error_message: "A `macro_rules` declarative macro cannot be invoked by its prior name. The macro may have been renamed or removed entirely.",
     per_result_error_template:  Some("macro_rules! {{name}}, previously in file {{span_filename}}:{{span_begin_line}}"),
     witness: (
         hint_template: r#"{{name}}!(...);"#

src/lints/macro_no_longer_exported.ron

@@ -13,19 +13,35 @@ SemverQuery(
                     ... on Macro {
                         name @output @tag
                         public_api_eligible @filter(op: "=", value: ["$true"])
+
+                        span_: span @optional {
+                            filename @output
+                            begin_line @output
+                        }
                     }
                 }
             }
             current {
-                item {
+                # There's no exported macro under the name we wanted.
+                item @fold @transform(op: "count") @filter(op: "=", value: ["$zero"]) {
+                    ... on Macro {
+                        name @filter(op: "=", value: ["%name"])
+                        public_api_eligible @filter(op: "=", value: ["$true"])
+                    }
+                }
+
+                # There is at least one macro under the same name that is *not* exported.
+                # Perhaps the macro is not deleted, just no longer exported.
+                item @fold @transform(op: "count") @filter(op: ">", value: ["$zero"]) {
                     ... on Macro {
                         name @filter(op: "=", value: ["%name"])
+
                         # Check the macro still exists but is no longer public API
                         # and isn't doc(hidden) (which would be caught by another lint)
                         public_api_eligible @filter(op: "!=", value: ["$true"])
                         doc_hidden @filter(op: "!=", value: ["$true"])
 
-                        span_: span @optional {
+                        non_exported_span_: span @optional {
                             filename @output
                             begin_line @output
                         }
@@ -36,6 +52,7 @@ SemverQuery(
     }"#,
     arguments: {
         "true": true,
+        "zero": 0,
     },
     error_message: "A macro that was previously exported with #[macro_export] is no longer exported. This breaks downstream code that used the macro.",
     per_result_error_template: Some("macro {{name}} in {{span_filename}}:{{span_begin_line}}"),

src/query.rs

@@ -580,15 +580,84 @@ mod tests {
     ) -> (String, Vec<BTreeMap<String, FieldValue>>) {
         let adapter = VersionedRustdocAdapter::new(indexed_crate_new, Some(indexed_crate_old))
             .expect("could not create adapter");
+
         let results_iter = adapter
             .run_query(&semver_query.query, semver_query.arguments.clone())
             .unwrap();
-        (
-            format!("./test_crates/{crate_pair_name}/"),
-            results_iter
-                .map(|res| res.into_iter().map(|(k, v)| (k.to_string(), v)).collect())
-                .collect::<Vec<BTreeMap<_, _>>>(),
-        )
+
+        // Ensure span data inside `@fold` blocks is deterministically ordered,
+        // since the underlying adapter is non-deterministic due to its iteration over hashtables.
+        // Our heuristic for detecting spans inside `@fold` is to look for:
+        // - list-typed outputs
+        // - with names ending in `_begin_line`
+        // - located inside *one* `@fold` level (i.e. their component is directly under the root).
+        let parsed_query = trustfall_core::frontend::parse(adapter.schema(), &semver_query.query)
+            .expect("not a valid query");
+        let fold_keys_and_targets: BTreeMap<&str, Vec<Arc<str>>> = parsed_query
+            .outputs
+            .iter()
+            .filter_map(|(name, output)| {
+                if name.as_ref().ends_with("_begin_line") && output.value_type.is_list() {
+                    if let Some(fold) = parsed_query
+                        .ir_query
+                        .root_component
+                        .folds
+                        .values()
+                        .find(|fold| fold.component.root == parsed_query.vids[&output.vid].root)
+                    {
+                        let targets = parsed_query
+                            .outputs
+                            .values()
+                            .filter_map(|o| {
+                                fold.component
+                                    .vertices
+                                    .contains_key(&o.vid)
+                                    .then_some(Arc::clone(&o.name))
+                            })
+                            .collect();
+                        Some((name.as_ref(), targets))
+                    } else {
+                        None
+                    }
+                } else {
+                    None
+                }
+            })
+            .collect();
+
+        let results = results_iter
+            .map(move |mut res| {
+                // Reorder `@fold`-ed span data in increasing `begin_line` order.
+                for (fold_key, targets) in &fold_keys_and_targets {
+                    let mut data: Vec<(u64, usize)> = res[*fold_key]
+                        .as_vec_with(FieldValue::as_u64)
+                        .expect("fold key was not a list of u64")
+                        .into_iter()
+                        .enumerate()
+                        .map(|(idx, val)| (val, idx))
+                        .collect();
+                    data.sort_unstable();
+                    for target in targets {
+                        res.entry(Arc::clone(target)).and_modify(|value| {
+                            // The output of a `@fold @transform(op: "count")` might not be a list here,
+                            // so ignore such outputs. They don't need reordering anyway.
+                            if let Some(slice) = value.as_slice() {
+                                let new_order = data
+                                    .iter()
+                                    .map(|(_, idx)| slice[*idx].clone())
+                                    .collect::<Vec<_>>()
+                                    .into();
+                                *value = new_order;
+                            }
+                        });
+                    }
+                }
+
+                // Turn the output keys into regular strings.
+                res.into_iter().map(|(k, v)| (k.to_string(), v)).collect()
+            })
+            .collect::<Vec<BTreeMap<_, _>>>();
+        (format!("./test_crates/{crate_pair_name}/"), results)
     }
 
     fn assert_no_false_positives_in_nonchanged_crate(

test_crates/macro_no_longer_exported/new/src/lib.rs

@@ -20,3 +20,20 @@ macro_rules! internal_macro {
         println!("Internal macro");
     };
 }
+
+pub mod foo {
+    // Private macro, which is not exported despite being in a public module.
+    // Macros require `#[macro_export]` or they aren't visible outside their crate.
+    //
+    // This is a breaking change.
+    macro_rules! some_macro {
+        () => {}
+    }
+}
+
+mod bar {
+    // Private macro by the same name.
+    macro_rules! some_macro {
+        () => {}
+    }
+}

test_crates/macro_no_longer_exported/old/src/lib.rs

@@ -18,3 +18,19 @@ macro_rules! internal_macro {
         println!("Internal macro");
     };
 }
+
+mod foo {
+    // Public macro. Exported even though it's in a private module,
+    // because of the `#[macro_export]`.
+    #[macro_export]
+    macro_rules! some_macro {
+        () => {}
+    }
+}
+
+mod bar {
+    // Private macro by the same name.
+    macro_rules! some_macro {
+        () => {}
+    }
+}

test_outputs/query_execution/macro_no_longer_exported.snap

@@ -7,20 +7,51 @@ snapshot_kind: text
   "./test_crates/declarative_macro_missing/": [
     {
       "name": String("will_no_longer_be_exported"),
-      "span_begin_line": Uint64(1),
+      "non_exported_span_begin_line": List([
+        Uint64(1),
+      ]),
+      "non_exported_span_filename": List([
+        String("src/lib.rs"),
+      ]),
+      "span_begin_line": Uint64(7),
       "span_filename": String("src/lib.rs"),
     },
   ],
   "./test_crates/macro_no_longer_exported/": [
     {
       "name": String("example_macro"),
+      "non_exported_span_begin_line": List([
+        Uint64(2),
+      ]),
+      "non_exported_span_filename": List([
+        String("src/lib.rs"),
+      ]),
       "span_begin_line": Uint64(2),
       "span_filename": String("src/lib.rs"),
     },
+    {
+      "name": String("some_macro"),
+      "non_exported_span_begin_line": List([
+        Uint64(29),
+        Uint64(36),
+      ]),
+      "non_exported_span_filename": List([
+        String("src/lib.rs"),
+        String("src/lib.rs"),
+      ]),
+      "span_begin_line": Uint64(26),
+      "span_filename": String("src/lib.rs"),
+    },
   ],
   "./test_crates/macro_now_doc_hidden/": [
     {
       "name": String("becomes_non_exported"),
+      "non_exported_span_begin_line": List([
+        Uint64(36),
+      ]),
+      "non_exported_span_filename": List([
+        String("src/lib.rs"),
+      ]),
       "span_begin_line": Uint64(36),
       "span_filename": String("src/lib.rs"),
     },

test_outputs/witnesses/macro_no_longer_exported.snap

@@ -6,14 +6,19 @@ snapshot_kind: text
 ---
 [["./test_crates/declarative_macro_missing/"]]
 filename = 'src/lib.rs'
-begin_line = 1
+begin_line = 7
 hint = 'will_no_longer_be_exported!(...);'
 
 [["./test_crates/macro_no_longer_exported/"]]
 filename = 'src/lib.rs'
 begin_line = 2
 hint = 'example_macro!(...);'
 
+[["./test_crates/macro_no_longer_exported/"]]
+filename = 'src/lib.rs'
+begin_line = 26
+hint = 'some_macro!(...);'
+
 [["./test_crates/macro_now_doc_hidden/"]]
 filename = 'src/lib.rs'
 begin_line = 36