From 120d1e78da4358621014fe8a20a2024a64696ef9 Mon Sep 17 00:00:00 2001
From: Cormac Relf <web@cormacrelf.net>
Date: Wed, 29 May 2024 14:55:18 -0500
Subject: [PATCH] Generate deterministic names for 'uri' macros.

---
 core/codegen/src/attribute/route/mod.rs |  6 ++-
 core/codegen/src/syn_ext.rs             | 17 +++---
 core/codegen/tests/route-uniqueness.rs  | 72 +++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 9 deletions(-)
 create mode 100644 core/codegen/tests/route-uniqueness.rs

diff --git a/core/codegen/src/attribute/route/mod.rs b/core/codegen/src/attribute/route/mod.rs
index b5fb5968..3dc28c8d 100644
--- a/core/codegen/src/attribute/route/mod.rs
+++ b/core/codegen/src/attribute/route/mod.rs
@@ -209,9 +209,11 @@ fn internal_uri_macro_decl(route: &Route) -> TokenStream {
     // Generate a unique macro name based on the route's metadata.
     let macro_name = route.handler.sig.ident.prepend(crate::URI_MACRO_PREFIX);
     let inner_macro_name = macro_name.uniqueify_with(|mut hasher| {
-        route.handler.sig.ident.hash(&mut hasher);
+        route.attr.method.0.hash(&mut hasher);
         route.attr.uri.path().hash(&mut hasher);
-        route.attr.uri.query().hash(&mut hasher)
+        route.attr.uri.query().hash(&mut hasher);
+        route.attr.data.as_ref().map(|d| d.value.hash(&mut hasher));
+        route.attr.format.as_ref().map(|f| f.0.hash(&mut hasher));
     });
 
     let route_uri = route.attr.uri.to_string();
diff --git a/core/codegen/src/syn_ext.rs b/core/codegen/src/syn_ext.rs
index 78adf5eb..f1105e17 100644
--- a/core/codegen/src/syn_ext.rs
+++ b/core/codegen/src/syn_ext.rs
@@ -80,18 +80,21 @@ impl IdentExt for syn::Ident {
         self.prepend(crate::ROCKET_IDENT_PREFIX)
     }
 
+    /// Create a unqiue version of the ident `self` based on the hash of `self`,
+    /// its span, the current call site, and any additional information provided
+    /// by the closure `f`.
+    ///
+    /// Span::source_file() / line / col are not stable, but the byte span and
+    /// some kind of scope identifier do appear in the `Debug` representation
+    /// for `Span`. And they seem to be consistent across compilations: "#57
+    /// bytes(106..117)" at the time of writing. So we use that.
     fn uniqueify_with<F: FnMut(&mut dyn Hasher)>(&self, mut f: F) -> syn::Ident {
-        use std::sync::atomic::{AtomicUsize, Ordering};
         use std::collections::hash_map::DefaultHasher;
 
-        // Keep a global counter (+ thread ID later) to generate unique ids.
-        static COUNTER: AtomicUsize = AtomicUsize::new(0);
-
         let mut hasher = DefaultHasher::new();
         self.hash(&mut hasher);
-        std::process::id().hash(&mut hasher);
-        std::thread::current().id().hash(&mut hasher);
-        COUNTER.fetch_add(1, Ordering::AcqRel).hash(&mut hasher);
+        format!("{:?}", self.span()).hash(&mut hasher);
+        format!("{:?}", Span::call_site()).hash(&mut hasher);
         f(&mut hasher);
 
         self.append(&format!("_{}", hasher.finish()))
diff --git a/core/codegen/tests/route-uniqueness.rs b/core/codegen/tests/route-uniqueness.rs
new file mode 100644
index 00000000..c8cc9f6b
--- /dev/null
+++ b/core/codegen/tests/route-uniqueness.rs
@@ -0,0 +1,72 @@
+#[macro_use] extern crate rocket;
+
+#[get("/")]
+fn index() { }
+
+mod module {
+    // This one has all the same macro inputs, and we need it to
+    // generate a crate-wide unique identifier for the macro it
+    // defines.
+    #[get("/")]
+    pub fn index() { }
+}
+
+// Makes sure that the hashing of the proc macro's call site span
+// is enough, even if we're inside a declarative macro
+macro_rules! gen_routes {
+    () => {
+        #[get("/")]
+        pub fn index() { }
+
+        pub mod two {
+            #[get("/")]
+            pub fn index() { }
+        }
+    }
+}
+
+mod module2 {
+    gen_routes!();
+
+    pub mod module3 {
+        gen_routes!();
+    }
+}
+
+#[test]
+fn test_uri_reachability() {
+    use rocket::http::Status;
+    use rocket::local::blocking::Client;
+
+    let rocket = rocket::build()
+        .mount("/", routes![index])
+        .mount("/module", routes![module::index])
+        .mount("/module2", routes![module2::index])
+        .mount("/module2/two", routes![module2::two::index])
+        .mount("/module2/module3", routes![module2::module3::index])
+        .mount("/module2/module3/two", routes![module2::module3::two::index]);
+
+    let uris = rocket.routes()
+        .map(|r| r.uri.base().to_string())
+        .collect::<Vec<_>>();
+
+    let client = Client::debug(rocket).unwrap();
+    for uri in uris {
+        let response = client.get(uri).dispatch();
+        assert_eq!(response.status(), Status::Ok);
+    }
+}
+
+#[test]
+fn test_uri_calls() {
+    let uris = [
+        uri!(index()),
+        uri!(module::index()),
+        uri!(module2::index()),
+        uri!(module2::two::index()),
+        uri!(module2::module3::index()),
+        uri!(module2::module3::two::index()),
+    ];
+
+    assert!(uris.iter().all(|uri| uri == "/"));
+}