ruma-api-macros: Fix argument ordering

crates/ruma-api-macros/src/request.rs

@@ -1,5 +1,5 @@
 use std::{
-    collections::BTreeSet,
+    collections::{BTreeMap, BTreeSet},
     convert::{TryFrom, TryInto},
     mem,
 };
@@ -164,8 +164,27 @@ impl Request {
         self.fields.iter().filter(|f| matches!(f, RequestField::Header(..)))
     }
 
-    fn path_fields(&self) -> impl Iterator<Item = &Field> {
-        self.fields.iter().filter_map(RequestField::as_path_field)
+    fn path_fields_ordered(&self) -> impl Iterator<Item = &Field> {
+        let map: BTreeMap<String, &Field> = self
+            .fields
+            .iter()
+            .filter_map(RequestField::as_path_field)
+            .map(|f| (f.ident.as_ref().unwrap().to_string(), f))
+            .collect();
+
+        self.stable_path
+            .as_ref()
+            .or(self.r0_path.as_ref())
+            .or(self.unstable_path.as_ref())
+            .expect("one of the paths to be defined")
+            .value()
+            .split('/')
+            .filter_map(|s| {
+                s.strip_prefix(':')
+                    .map(|s| *map.get(s).expect("path args have already been checked"))
+            })
+            .collect::<Vec<_>>()
+            .into_iter()
     }
 
     fn raw_body_field(&self) -> Option<&Field> {
@@ -249,6 +268,13 @@ impl Request {
     pub(super) fn check(&self) -> syn::Result<()> {
         // TODO: highlight problematic fields
 
+        let path_fields: Vec<_> =
+            self.fields.iter().filter_map(RequestField::as_path_field).collect();
+
+        self.check_path(&path_fields, self.unstable_path.as_ref())?;
+        self.check_path(&path_fields, self.r0_path.as_ref())?;
+        self.check_path(&path_fields, self.stable_path.as_ref())?;
+
         let newtype_body_fields = self.fields.iter().filter(|field| {
             matches!(field, RequestField::NewtypeBody(_) | RequestField::RawBody(_))
         });
@@ -311,6 +337,48 @@ impl Request {
 
         Ok(())
     }
+
+    fn check_path(&self, fields: &[&Field], path: Option<&LitStr>) -> syn::Result<()> {
+        let path = if let Some(lit) = path { lit } else { return Ok(()) };
+
+        let path_args: Vec<_> = path
+            .value()
+            .split('/')
+            .filter_map(|s| s.strip_prefix(':').map(&str::to_string))
+            .collect();
+
+        let field_map: BTreeMap<_, _> =
+            fields.iter().map(|&f| (f.ident.as_ref().unwrap().to_string(), f)).collect();
+
+        // test if all macro fields exist in the path
+        for (name, field) in field_map.iter() {
+            if !path_args.contains(name) {
+                return Err({
+                    let mut err = syn::Error::new_spanned(
+                        field,
+                        "this path argument field is not defined in...",
+                    );
+                    err.combine(syn::Error::new_spanned(path, "...this path."));
+                    err
+                });
+            }
+        }
+
+        // test if all path fields exists in macro fields
+        for arg in &path_args {
+            if !field_map.contains_key(arg) {
+                return Err(syn::Error::new_spanned(
+                    path,
+                    format!(
+                        "a corresponding request path argument field for \"{}\" does not exist",
+                        arg
+                    ),
+                ));
+            }
+        }
+
+        Ok(())
+    }
 }
 
 /// The types of fields that a request can have.

crates/ruma-api-macros/src/request/incoming.rs

@@ -25,7 +25,8 @@ impl Request {
         // except this one. If we get errors about missing fields in IncomingRequest for
         // a path field look here.
         let (parse_request_path, path_vars) = if self.has_path_fields() {
-            let path_vars: Vec<_> = self.path_fields().filter_map(|f| f.ident.as_ref()).collect();
+            let path_vars: Vec<_> =
+                self.path_fields_ordered().filter_map(|f| f.ident.as_ref()).collect();
 
             let parse_request_path = quote! {
                 let (#(#path_vars,)*) = #serde::Deserialize::deserialize(

crates/ruma-api/tests/path_arg_ordering.rs

@@ -0,0 +1,42 @@
+use ruma_api::{ruma_api, IncomingRequest};
+
+ruma_api! {
+    metadata: {
+        description: "Does something.",
+        method: GET,
+        name: "some_path_args",
+        path: "/_matrix/:one/a/:two/b/:three/c",
+        rate_limited: false,
+        authentication: None,
+    }
+
+    request: {
+        #[ruma_api(path)]
+        pub three: String,
+
+        #[ruma_api(path)]
+        pub one: String,
+
+        #[ruma_api(path)]
+        pub two: String,
+    }
+
+    response: {}
+}
+
+#[test]
+fn path_ordering_is_correct() {
+    let request = http::Request::builder()
+        .method("GET")
+        // This explicitly puts wrong values in the URI, as now we rely on the side-supplied
+        // path_args slice, so this is just to ensure it *is* using that slice.
+        .uri("https://www.rust-lang.org/_matrix/non/a/non/b/non/c")
+        .body("")
+        .unwrap();
+
+    let resp = Request::try_from_http_request(request, &["1", "2", "3"]).unwrap();
+
+    assert_eq!(resp.one, "1");
+    assert_eq!(resp.two, "2");
+    assert_eq!(resp.three, "3");
+}

crates/ruma-api/tests/ruma_api_lifetime.rs

@@ -42,7 +42,7 @@ mod nested_types {
             description: "Add an alias to a room.",
             method: PUT,
             name: "create_alias",
-            path: "/_matrix/client/r0/directory/room/:room_alias",
+            path: "/_matrix/client/r0/directory/room",
             rate_limited: false,
             authentication: AccessToken,
         }

crates/ruma-api/tests/ui/03-move-value.rs

@@ -100,6 +100,7 @@ mod plain {
         request: {
             pub q2: Foo,
 
+            #[ruma_api(path)]
             pub bar: String,
 
             #[ruma_api(query)]

crates/ruma-api/tests/ui/05-request-only.rs

@@ -10,7 +10,7 @@ ruma_api! {
         description: "Does something.",
         method: POST, // An `http::Method` constant. No imports required.
         name: "some_endpoint",
-        path: "/_matrix/some/endpoint/:baz",
+        path: "/_matrix/some/endpoint/:foo",
         rate_limited: false,
         authentication: None,
     }
@@ -18,6 +18,7 @@ ruma_api! {
     #[derive(PartialEq)] // Make sure attributes work
     request: {
         // With no attribute on the field, it will be put into the body of the request.
+        #[ruma_api(path)]
         pub foo: String,
     }
 }