more cleanup

amqp_messaging/src/methods/consume.rs

@@ -1,17 +1,10 @@
 use amqp_core::connection::ChannelHandle;
 use amqp_core::error::ProtocolError;
-use amqp_core::methods::{Bit, ConsumerTag, NoAck, NoLocal, NoWait, QueueName, Table};
+use amqp_core::methods::BasicConsume;
 
-#[allow(clippy::too_many_arguments)]
 pub async fn consume(
     _channel_handle: ChannelHandle,
-    _queue: QueueName,
+    _basic_consume: BasicConsume,
-    _consumer_tag: ConsumerTag,
-    _no_local: NoLocal,
-    _no_ack: NoAck,
-    _exclusive: Bit,
-    _no_wait: NoWait,
-    _arguments: Table,
 ) -> Result<(), ProtocolError> {
     Ok(())
 }

amqp_messaging/src/methods/mod.rs

@@ -22,82 +22,23 @@ pub async fn handle_method(
     info!(?method, "Handling method");
 
     match method {
-        Method::ExchangeDeclare { .. } => amqp_todo!(),
+        Method::ExchangeDeclare(_) => amqp_todo!(),
-        Method::ExchangeDeclareOk => amqp_todo!(),
+        Method::ExchangeDeclareOk(_) => amqp_todo!(),
-        Method::ExchangeDelete { .. } => amqp_todo!(),
+        Method::ExchangeDelete(_) => amqp_todo!(),
-        Method::ExchangeDeleteOk => amqp_todo!(),
+        Method::ExchangeDeleteOk(_) => amqp_todo!(),
-        Method::QueueDeclare {
+        Method::QueueDeclare(queue_declare) => queue::declare(channel_handle, queue_declare).await,
-            queue,
-            passive,
-            durable,
-            exclusive,
-            auto_delete,
-            no_wait,
-            arguments,
-            ..
-        } => {
-            queue::declare(
-                channel_handle,
-                queue,
-                passive,
-                durable,
-                exclusive,
-                auto_delete,
-                no_wait,
-                arguments,
-            )
-            .await
-        }
         Method::QueueDeclareOk { .. } => amqp_todo!(),
-        Method::QueueBind {
+        Method::QueueBind(queue_bind) => queue::bind(channel_handle, queue_bind).await,
-            queue,
+        Method::QueueBindOk(_) => amqp_todo!(),
-            exchange,
-            routing_key,
-            no_wait,
-            arguments,
-            ..
-        } => {
-            queue::bind(
-                channel_handle,
-                queue,
-                exchange,
-                routing_key,
-                no_wait,
-                arguments,
-            )
-            .await
-        }
-        Method::QueueBindOk => amqp_todo!(),
         Method::QueueUnbind { .. } => amqp_todo!(),
-        Method::QueueUnbindOk => amqp_todo!(),
+        Method::QueueUnbindOk(_) => amqp_todo!(),
         Method::QueuePurge { .. } => amqp_todo!(),
         Method::QueuePurgeOk { .. } => amqp_todo!(),
         Method::QueueDelete { .. } => amqp_todo!(),
         Method::QueueDeleteOk { .. } => amqp_todo!(),
         Method::BasicQos { .. } => amqp_todo!(),
-        Method::BasicQosOk => amqp_todo!(),
+        Method::BasicQosOk(_) => amqp_todo!(),
-        Method::BasicConsume {
+        Method::BasicConsume(consume) => consume::consume(channel_handle, consume).await,
-            queue,
-            consumer_tag,
-            no_local,
-            no_ack,
-            exclusive,
-            no_wait,
-            arguments,
-            ..
-        } => {
-            consume::consume(
-                channel_handle,
-                queue,
-                consumer_tag,
-                no_local,
-                no_ack,
-                exclusive,
-                no_wait,
-                arguments,
-            )
-            .await
-        }
         Method::BasicConsumeOk { .. } => amqp_todo!(),
         Method::BasicCancel { .. } => amqp_todo!(),
         Method::BasicCancelOk { .. } => amqp_todo!(),
@@ -110,13 +51,13 @@ pub async fn handle_method(
         Method::BasicReject { .. } => amqp_todo!(),
         Method::BasicRecoverAsync { .. } => amqp_todo!(),
         Method::BasicRecover { .. } => amqp_todo!(),
-        Method::BasicRecoverOk => amqp_todo!(),
+        Method::BasicRecoverOk(_) => amqp_todo!(),
-        Method::TxSelect
+        Method::TxSelect(_)
-        | Method::TxSelectOk
+        | Method::TxSelectOk(_)
-        | Method::TxCommit
+        | Method::TxCommit(_)
-        | Method::TxCommitOk
+        | Method::TxCommitOk(_)
-        | Method::TxRollback
+        | Method::TxRollback(_)
-        | Method::TxRollbackOk => amqp_todo!(),
+        | Method::TxRollbackOk(_) => amqp_todo!(),
         Method::BasicPublish { .. } => {
             unreachable!("Basic.Publish is handled somewhere else because it has a body")
         }

amqp_messaging/src/methods/queue.rs

@@ -1,23 +1,27 @@
 use amqp_core::connection::ChannelHandle;
 use amqp_core::error::{ConException, ProtocolError};
-use amqp_core::methods::{Bit, ExchangeName, NoWait, QueueName, Shortstr, Table};
+use amqp_core::methods::{QueueBind, QueueDeclare};
 use amqp_core::queue::{QueueDeletion, QueueId, RawQueue};
 use amqp_core::{amqp_todo, GlobalData};
 use parking_lot::Mutex;
 use std::sync::atomic::AtomicUsize;
 use std::sync::Arc;
 
-#[allow(clippy::too_many_arguments)]
 pub async fn declare(
     channel_handle: ChannelHandle,
-    queue_name: QueueName,
+    queue_declare: QueueDeclare,
-    passive: Bit,
-    durable: Bit,
-    exclusive: Bit,
-    auto_delete: Bit,
-    no_wait: NoWait,
-    arguments: Table,
 ) -> Result<(), ProtocolError> {
+    let QueueDeclare {
+        queue: queue_name,
+        passive,
+        durable,
+        exclusive,
+        auto_delete,
+        no_wait,
+        arguments,
+        ..
+    } = queue_declare;
+
     if !arguments.is_empty() {
         return Err(ConException::Todo.into());
     }
@@ -58,11 +62,7 @@ pub async fn declare(
 
 pub async fn bind(
     _channel_handle: ChannelHandle,
-    _queue: QueueName,
+    _queue_bind: QueueBind,
-    _exchange: ExchangeName,
-    _routing_key: Shortstr,
-    _no_wait: NoWait,
-    _arguments: Table,
 ) -> Result<(), ProtocolError> {
     amqp_todo!();
 }

amqp_transport/src/connection.rs

@@ -3,7 +3,11 @@ use crate::frame::{ContentHeader, Frame, FrameType};
 use crate::{frame, methods, sasl};
 use amqp_core::connection::{ChannelHandle, ChannelNum, ConnectionHandle, ConnectionId};
 use amqp_core::message::{MessageId, RawMessage, RoutingInformation};
-use amqp_core::methods::{FieldValue, Method, Table};
+use amqp_core::methods::{
+    BasicPublish, ChannelClose, ChannelCloseOk, ChannelOpenOk, ConnectionClose, ConnectionCloseOk,
+    ConnectionOpen, ConnectionOpenOk, ConnectionStart, ConnectionStartOk, ConnectionTune,
+    ConnectionTuneOk, FieldValue, Method, Table,
+};
 use amqp_core::GlobalData;
 use anyhow::Context;
 use bytes::Bytes;
@@ -134,7 +138,7 @@ impl Connection {
     }
 
     async fn start(&mut self) -> Result<()> {
-        let start_method = Method::ConnectionStart {
+        let start_method = Method::ConnectionStart(ConnectionStart {
             version_major: 0,
             version_minor: 9,
             server_properties: server_properties(
@@ -144,7 +148,7 @@ impl Connection {
             ),
             mechanisms: "PLAIN".into(),
             locales: "en_US".into(),
-        };
+        });
 
         debug!(?start_method, "Sending Start method");
         self.send_method(ChannelNum::zero(), start_method).await?;
@@ -152,12 +156,12 @@ impl Connection {
         let start_ok = self.recv_method().await?;
         debug!(?start_ok, "Received Start-Ok");
 
-        if let Method::ConnectionStartOk {
+        if let Method::ConnectionStartOk(ConnectionStartOk {
             mechanism,
             locale,
             response,
             ..
-        } = start_ok
+        }) = start_ok
         {
             ensure_conn(mechanism == "PLAIN")?;
             ensure_conn(locale == "en_US")?;
@@ -171,11 +175,11 @@ impl Connection {
     }
 
     async fn tune(&mut self) -> Result<()> {
-        let tune_method = Method::ConnectionTune {
+        let tune_method = Method::ConnectionTune(ConnectionTune {
             channel_max: CHANNEL_MAX,
             frame_max: FRAME_SIZE_MAX,
             heartbeat: HEARTBEAT_DELAY,
-        };
+        });
 
         debug!("Sending Tune method");
         self.send_method(ChannelNum::zero(), tune_method).await?;
@@ -183,11 +187,11 @@ impl Connection {
         let tune_ok = self.recv_method().await?;
         debug!(?tune_ok, "Received Tune-Ok method");
 
-        if let Method::ConnectionTuneOk {
+        if let Method::ConnectionTuneOk(ConnectionTuneOk {
             channel_max,
             frame_max,
             heartbeat,
-        } = tune_ok
+        }) = tune_ok
         {
             self.channel_max = channel_max;
             self.max_frame_size = usize::try_from(frame_max).unwrap();
@@ -202,15 +206,15 @@ impl Connection {
         let open = self.recv_method().await?;
         debug!(?open, "Received Open method");
 
-        if let Method::ConnectionOpen { virtual_host, .. } = open {
+        if let Method::ConnectionOpen(ConnectionOpen { virtual_host, .. }) = open {
             ensure_conn(virtual_host == "/")?;
         }
 
         self.send_method(
             ChannelNum::zero(),
-            Method::ConnectionOpenOk {
+            Method::ConnectionOpenOk(ConnectionOpenOk {
                 reserved_1: "".to_string(),
-            },
+            }),
         )
         .await?;
 
@@ -242,15 +246,18 @@ impl Connection {
             .map(|channel| channel.status.take());
 
         match method {
-            Method::ConnectionClose {
+            Method::ConnectionClose(ConnectionClose {
                 reply_code,
                 reply_text,
                 class_id,
                 method_id,
-            } => {
+            }) => {
                 info!(%reply_code, %reply_text, %class_id, %method_id, "Closing connection");
-                self.send_method(ChannelNum::zero(), Method::ConnectionCloseOk {})
+                self.send_method(
-                    .await?;
+                    ChannelNum::zero(),
+                    Method::ConnectionCloseOk(ConnectionCloseOk),
+                )
+                .await?;
                 return Err(ProtocolError::GracefulClose.into());
             }
             Method::ChannelOpen { .. } => self.channel_open(frame.channel).await?,
@@ -344,13 +351,13 @@ impl Connection {
         // The only method with content that is sent to the server is Basic.Publish.
         ensure_conn(header.class_id == BASIC_CLASS_ID)?;
 
-        if let Method::BasicPublish {
+        if let Method::BasicPublish(BasicPublish {
             exchange,
             routing_key,
             mandatory,
             immediate,
             ..
-        } = method
+        }) = method
         {
             let message = RawMessage {
                 id: MessageId::random(),
@@ -415,9 +422,9 @@ impl Connection {
 
         self.send_method(
             channel_num,
-            Method::ChannelOpenOk {
+            Method::ChannelOpenOk(ChannelOpenOk {
                 reserved_1: Vec::new(),
-            },
+            }),
         )
         .await?;
 
@@ -425,17 +432,18 @@ impl Connection {
     }
 
     async fn channel_close(&mut self, channel_id: ChannelNum, method: Method) -> Result<()> {
-        if let Method::ChannelClose {
+        if let Method::ChannelClose(ChannelClose {
             reply_code: code,
             reply_text: reason,
             ..
-        } = method
+        }) = method
         {
             info!(%code, %reason, "Closing channel");
 
             if let Some(channel) = self.channels.remove(&channel_id) {
                 drop(channel);
-                self.send_method(channel_id, Method::ChannelCloseOk).await?;
+                self.send_method(channel_id, Method::ChannelCloseOk(ChannelCloseOk))
+                    .await?;
             } else {
                 return Err(ConException::Todo.into());
             }

amqp_transport/src/tests.rs

@@ -1,13 +1,13 @@
 use crate::frame::FrameType;
 use crate::{frame, methods};
 use amqp_core::connection::ChannelNum;
-use amqp_core::methods::{FieldValue, Method};
+use amqp_core::methods::{ConnectionStart, ConnectionStartOk, FieldValue, Method};
 use std::collections::HashMap;
 
 #[tokio::test]
 async fn write_start_ok_frame() {
     let mut payload = Vec::new();
-    let method = Method::ConnectionStart {
+    let method = Method::ConnectionStart(ConnectionStart {
         version_major: 0,
         version_minor: 9,
         server_properties: HashMap::from([(
@@ -16,7 +16,7 @@ async fn write_start_ok_frame() {
         )]),
         mechanisms: "PLAIN".into(),
         locales: "en_US".into(),
-    };
+    });
 
     methods::write::write_method(method, &mut payload).unwrap();
 
@@ -141,7 +141,7 @@ fn read_start_ok_payload() {
 
     assert_eq!(
         method,
-        Method::ConnectionStartOk {
+        Method::ConnectionStartOk(ConnectionStartOk {
             client_properties: HashMap::from([
                 (
                     "product".to_string(),
@@ -179,6 +179,6 @@ fn read_start_ok_payload() {
             mechanism: "PLAIN".to_string(),
             response: "\x00admin\x00".into(),
             locale: "en_US".to_string()
-        }
+        })
     );
 }

xtask/src/codegen/mod.rs

@@ -211,11 +211,31 @@ impl Codegen {
 
         for class in &amqp.classes {
             let enum_name = class.name.to_upper_camel_case();
+            for method in &class.methods {
+                let method_name = method.name.to_upper_camel_case();
+                write!(
+                    self.output,
+                    "    {enum_name}{method_name}({enum_name}{method_name}),"
+                )
+                .ok();
+            }
+        }
+
+        writeln!(self.output, "}}\n").ok();
+
+        // now codegen the individual structs
+        for class in &amqp.classes {
+            let class_name = class.name.to_upper_camel_case();
             for method in &class.methods {
                 let method_name = method.name.to_upper_camel_case();
                 self.doc_comment(&class.doc, 4);
                 self.doc_comment(&method.doc, 4);
-                write!(self.output, "    {enum_name}{method_name}").ok();
+                writeln!(
+                    self.output,
+                    "#[derive(Debug, Clone, PartialEq)]
+pub struct {class_name}{method_name}"
+                )
+                .ok();
                 if !method.fields.is_empty() {
                     writeln!(self.output, " {{").ok();
                     for field in &method.fields {
@@ -225,24 +245,22 @@ impl Codegen {
                             field.asserts.as_ref(),
                         );
                         if !field_docs.is_empty() {
-                            writeln!(self.output, "        /// {field_docs}").ok();
+                            writeln!(self.output, "    /// {field_docs}").ok();
                             if !field.doc.is_empty() {
-                                writeln!(self.output, "        ///").ok();
+                                writeln!(self.output, "    ///").ok();
-                                self.doc_comment(&field.doc, 8);
+                                self.doc_comment(&field.doc, 4);
                             }
                         } else {
-                            self.doc_comment(&field.doc, 8);
+                            self.doc_comment(&field.doc, 4);
                         }
-                        writeln!(self.output, "        {field_name}: {field_type},").ok();
+                        writeln!(self.output, "    pub {field_name}: {field_type},").ok();
                     }
-                    writeln!(self.output, "    }},").ok();
+                    writeln!(self.output, "    }}\n").ok();
                 } else {
-                    writeln!(self.output, ",").ok();
+                    writeln!(self.output, ";\n").ok();
                 }
             }
         }
-
-        writeln!(self.output, "}}\n").ok();
     }
 
     fn amqp_type_to_rust_type(&self, amqp_type: &str) -> &'static str {

xtask/src/codegen/parser.rs

@@ -154,14 +154,14 @@ pub type IResult<'a, T> = nom::IResult<&'a [u8], T, TransError>;
         let method_name = method.name.to_upper_camel_case();
         writeln!(
             self.output,
-            "    Ok((input, Method::{class_name}{method_name} {{"
+            "    Ok((input, Method::{class_name}{method_name}({class_name}{method_name} {{"
         )
         .ok();
         for field in &method.fields {
             let field_name = self.snake_case(&field.name);
             writeln!(self.output, "        {field_name},").ok();
         }
-        writeln!(self.output, "    }}))").ok();
+        writeln!(self.output, "    }})))").ok();
 
         writeln!(self.output, "}}").ok();
     }

xtask/src/codegen/random.rs

@@ -43,7 +43,7 @@ use crate::methods::RandomMethod;
                 let method_name = method.name.to_upper_camel_case();
                 writeln!(
                     self.output,
-                    "                    {i} => Method::{class_name}{method_name} {{"
+                    "                    {i} => Method::{class_name}{method_name}( {class_name}{method_name}{{"
                 )
                 .ok();
                 for field in &method.fields {
@@ -54,7 +54,7 @@ use crate::methods::RandomMethod;
                     )
                     .ok();
                 }
-                writeln!(self.output, "                    }},").ok();
+                writeln!(self.output, "                    }}),").ok();
             }
             writeln!(
                 self.output,

xtask/src/codegen/write.rs

@@ -22,12 +22,16 @@ pub fn write_method<W: Write>(method: Method, mut writer: W) -> Result<(), Trans
             for method in &class.methods {
                 let method_name = method.name.to_upper_camel_case();
                 let method_index = method.index;
-                writeln!(self.output, "        Method::{class_name}{method_name} {{").ok();
+                writeln!(
+                    self.output,
+                    "        Method::{class_name}{method_name}({class_name}{method_name} {{"
+                )
+                .ok();
                 for field in &method.fields {
                     let field_name = self.snake_case(&field.name);
                     writeln!(self.output, "            {field_name},").ok();
                 }
-                writeln!(self.output, "        }} => {{").ok();
+                writeln!(self.output, "        }}) => {{").ok();
                 let [ci0, ci1] = class_index.to_be_bytes();
                 let [mi0, mi1] = method_index.to_be_bytes();
                 writeln!(