Move around Boxes to fix clippy::large_enum_variant (serenity-rs#…

…1929)

src/client/bridge/gateway/mod.rs

@@ -63,14 +63,12 @@ pub use self::shard_runner_message::{ChunkGuildFilter, ShardRunnerMessage};
 use crate::gateway::ConnectionStage;
 
 /// A message either for a [`ShardManager`] or a [`ShardRunner`].
-// Once we can use `Box` as part of a pattern, we will reconsider boxing.
-#[allow(clippy::large_enum_variant)]
 #[derive(Clone, Debug)]
 pub enum ShardClientMessage {
     /// A message intended to be worked with by a [`ShardManager`].
     Manager(ShardManagerMessage),
     /// A message intended to be worked with by a [`ShardRunner`].
-    Runner(ShardRunnerMessage),
+    Runner(Box<ShardRunnerMessage>),
 }
 
 /// A message for a [`ShardManager`] relating to an operation with a shard.

src/client/bridge/gateway/shard_messenger.rs

@@ -249,7 +249,7 @@ impl ShardMessenger {
     /// Returns a [`TrySendError`] if the shard's receiver was closed.
     #[inline]
     pub fn send_to_shard(&self, msg: ShardRunnerMessage) -> Result<(), TrySendError<InterMessage>> {
-        self.tx.unbounded_send(InterMessage::Client(Box::new(ShardClientMessage::Runner(msg))))
+        self.tx.unbounded_send(InterMessage::Client(ShardClientMessage::Runner(Box::new(msg))))
     }
 
     /// Sets a new filter for an event collector.

src/client/bridge/gateway/shard_queuer.rs

@@ -244,7 +244,7 @@ impl ShardQueuer {
         if let Some(runner) = self.runners.lock().await.get(&shard_id) {
             let shutdown = ShardManagerMessage::Shutdown(shard_id, code);
             let client_msg = ShardClientMessage::Manager(shutdown);
-            let msg = InterMessage::Client(Box::new(client_msg));
+            let msg = InterMessage::Client(client_msg);
 
             if let Err(why) = runner.runner_tx.tx.unbounded_send(msg) {
                 warn!(

src/client/bridge/gateway/shard_runner.rs

@@ -347,120 +347,113 @@ impl ShardRunner {
     #[instrument(skip(self))]
     async fn handle_rx_value(&mut self, value: InterMessage) -> bool {
         match value {
-            InterMessage::Client(value) => match *value {
-                ShardClientMessage::Manager(ShardManagerMessage::Restart(id)) => {
-                    self.checked_shutdown(id, 4000).await
-                },
-                ShardClientMessage::Manager(ShardManagerMessage::Shutdown(id, code)) => {
-                    self.checked_shutdown(id, code).await
-                },
-                ShardClientMessage::Manager(ShardManagerMessage::ShutdownAll) => {
-                    // This variant should never be received.
-                    warn!("[ShardRunner {:?}] Received a ShutdownAll?", self.shard.shard_info(),);
+            InterMessage::Client(value) => match value {
+                ShardClientMessage::Manager(msg) => match msg {
+                    ShardManagerMessage::Restart(id) => self.checked_shutdown(id, 4000).await,
+                    ShardManagerMessage::Shutdown(id, code) => {
+                        self.checked_shutdown(id, code).await
+                    },
+                    ShardManagerMessage::ShutdownAll => {
+                        // This variant should never be received.
+                        warn!(
+                            "[ShardRunner {:?}] Received a ShutdownAll?",
+                            self.shard.shard_info(),
+                        );
 
-                    true
-                },
-                ShardClientMessage::Manager(
+                        true
+                    },
                     ShardManagerMessage::ShardUpdate {
                         ..
                     }
                     | ShardManagerMessage::ShutdownInitiated
-                    | ShardManagerMessage::ShutdownFinished(_),
-                ) => {
-                    // nb: not sent here
-
-                    true
-                },
-                ShardClientMessage::Manager(
+                    | ShardManagerMessage::ShutdownFinished(_) => {
+                        // nb: not sent here
+                        true
+                    },
                     ShardManagerMessage::ShardDisallowedGatewayIntents
                     | ShardManagerMessage::ShardInvalidAuthentication
-                    | ShardManagerMessage::ShardInvalidGatewayIntents,
-                ) => {
-                    // These variants should never be received.
-                    warn!("[ShardRunner {:?}] Received a ShardError?", self.shard.shard_info(),);
+                    | ShardManagerMessage::ShardInvalidGatewayIntents => {
+                        // These variants should never be received.
+                        warn!("[ShardRunner {:?}] Received a ShardError?", self.shard.shard_info(),);
 
-                    true
-                },
-                ShardClientMessage::Runner(ShardRunnerMessage::ChunkGuild {
-                    guild_id,
-                    limit,
-                    filter,
-                    nonce,
-                }) => {
-                    self.shard.chunk_guild(guild_id, limit, filter, nonce.as_deref()).await.is_ok()
-                },
-                ShardClientMessage::Runner(ShardRunnerMessage::Close(code, reason)) => {
-                    let reason = reason.unwrap_or_default();
-                    let close = CloseFrame {
-                        code: code.into(),
-                        reason: Cow::from(reason),
-                    };
-                    self.shard.client.close(Some(close)).await.is_ok()
-                },
-                ShardClientMessage::Runner(ShardRunnerMessage::Message(msg)) => {
-                    self.shard.client.send(msg).await.is_ok()
-                },
-                ShardClientMessage::Runner(ShardRunnerMessage::SetActivity(activity)) => {
-                    // To avoid a clone of `activity`, we do a little bit of
-                    // trickery here:
-                    //
-                    // First, we obtain a reference to the current presence of
-                    // the shard, and create a new presence tuple of the new
-                    // activity we received over the channel as well as the
-                    // online status that the shard already had.
-                    //
-                    // We then (attempt to) send the websocket message with the
-                    // status update, expressively returning:
-                    //
-                    // - whether the message successfully sent
-                    // - the original activity we received over the channel
-                    self.shard.set_activity(activity);
-
-                    self.shard.update_presence().await.is_ok()
-                },
-                ShardClientMessage::Runner(ShardRunnerMessage::SetPresence(status, activity)) => {
-                    self.shard.set_presence(status, activity);
-
-                    self.shard.update_presence().await.is_ok()
-                },
-                ShardClientMessage::Runner(ShardRunnerMessage::SetStatus(status)) => {
-                    self.shard.set_status(status);
-
-                    self.shard.update_presence().await.is_ok()
+                        true
+                    },
                 },
-                #[cfg(feature = "collector")]
-                ShardClientMessage::Runner(ShardRunnerMessage::SetEventFilter(collector)) => {
-                    self.event_filters.push(collector);
+                ShardClientMessage::Runner(msg) => match *msg {
+                    ShardRunnerMessage::ChunkGuild {
+                        guild_id,
+                        limit,
+                        filter,
+                        nonce,
+                    } => self
+                        .shard
+                        .chunk_guild(guild_id, limit, filter, nonce.as_deref())
+                        .await
+                        .is_ok(),
+                    ShardRunnerMessage::Close(code, reason) => {
+                        let reason = reason.unwrap_or_default();
+                        let close = CloseFrame {
+                            code: code.into(),
+                            reason: Cow::from(reason),
+                        };
+                        self.shard.client.close(Some(close)).await.is_ok()
+                    },
+                    ShardRunnerMessage::Message(msg) => self.shard.client.send(msg).await.is_ok(),
+                    ShardRunnerMessage::SetActivity(activity) => {
+                        // To avoid a clone of `activity`, we do a little bit of
+                        // trickery here:
+                        //
+                        // First, we obtain a reference to the current presence of
+                        // the shard, and create a new presence tuple of the new
+                        // activity we received over the channel as well as the
+                        // online status that the shard already had.
+                        //
+                        // We then (attempt to) send the websocket message with the
+                        // status update, expressively returning:
+                        //
+                        // - whether the message successfully sent
+                        // - the original activity we received over the channel
+                        self.shard.set_activity(activity);
+                        self.shard.update_presence().await.is_ok()
+                    },
+                    ShardRunnerMessage::SetPresence(status, activity) => {
+                        self.shard.set_presence(status, activity);
+                        self.shard.update_presence().await.is_ok()
+                    },
+                    ShardRunnerMessage::SetStatus(status) => {
+                        self.shard.set_status(status);
+                        self.shard.update_presence().await.is_ok()
+                    },
+                    #[cfg(feature = "collector")]
+                    ShardRunnerMessage::SetEventFilter(collector) => {
+                        self.event_filters.push(collector);
 
-                    true
-                },
-                #[cfg(feature = "collector")]
-                ShardClientMessage::Runner(ShardRunnerMessage::SetMessageFilter(collector)) => {
-                    self.message_filters.push(collector);
+                        true
+                    },
+                    #[cfg(feature = "collector")]
+                    ShardRunnerMessage::SetMessageFilter(collector) => {
+                        self.message_filters.push(collector);
 
-                    true
-                },
-                #[cfg(feature = "collector")]
-                ShardClientMessage::Runner(ShardRunnerMessage::SetReactionFilter(collector)) => {
-                    self.reaction_filters.push(collector);
+                        true
+                    },
+                    #[cfg(feature = "collector")]
+                    ShardRunnerMessage::SetReactionFilter(collector) => {
+                        self.reaction_filters.push(collector);
 
-                    true
-                },
-                #[cfg(feature = "collector")]
-                ShardClientMessage::Runner(ShardRunnerMessage::SetComponentInteractionFilter(
-                    collector,
-                )) => {
-                    self.component_interaction_filters.push(collector);
+                        true
+                    },
+                    #[cfg(feature = "collector")]
+                    ShardRunnerMessage::SetComponentInteractionFilter(collector) => {
+                        self.component_interaction_filters.push(collector);
 
-                    true
-                },
-                #[cfg(feature = "collector")]
-                ShardClientMessage::Runner(ShardRunnerMessage::SetModalInteractionFilter(
-                    collector,
-                )) => {
-                    self.modal_interaction_filters.push(collector);
+                        true
+                    },
+                    #[cfg(feature = "collector")]
+                    ShardRunnerMessage::SetModalInteractionFilter(collector) => {
+                        self.modal_interaction_filters.push(collector);
 
-                    true
+                        true
+                    },
                 },
             },
             InterMessage::Json(value) => {

src/client/bridge/gateway/shard_runner_message.rs

@@ -23,8 +23,6 @@ pub enum ChunkGuildFilter {
 }
 
 /// A message to send from a shard over a WebSocket.
-// Once we can use `Box` as part of a pattern, we will reconsider boxing.
-#[allow(clippy::large_enum_variant)]
 #[derive(Clone, Debug)]
 pub enum ShardRunnerMessage {
     /// Indicates that the client is to send a member chunk message.

src/error.rs

@@ -95,7 +95,7 @@ pub enum Error {
     ///
     /// [`http`]: crate::http
     #[cfg(feature = "http")]
-    Http(Box<HttpError>),
+    Http(HttpError),
     /// An error from the `tungstenite` crate.
     #[cfg(feature = "gateway")]
     Tungstenite(TungsteniteError),
@@ -149,7 +149,7 @@ impl From<TungsteniteError> for Error {
 #[cfg(feature = "http")]
 impl From<HttpError> for Error {
     fn from(e: HttpError) -> Error {
-        Error::Http(Box::new(e))
+        Error::Http(e)
     }
 }
 

src/gateway/mod.rs

@@ -150,7 +150,7 @@ impl fmt::Display for ConnectionStage {
 #[non_exhaustive]
 pub enum InterMessage {
     #[cfg(feature = "client")]
-    Client(Box<ShardClientMessage>),
+    Client(ShardClientMessage),
     Json(Value),
 }
 

src/http/client.rs

@@ -4061,7 +4061,7 @@ impl Http {
         if response.status().is_success() {
             Ok(response)
         } else {
-            Err(Error::Http(Box::new(HttpError::from_response(response).await)))
+            Err(Error::Http(HttpError::from_response(response).await))
         }
     }
 
@@ -4080,7 +4080,7 @@ impl Http {
         debug!("Expected {}, got {}", expected, response.status());
         trace!("Unsuccessful response: {:?}", response);
 
-        Err(Error::Http(Box::new(HttpError::from_response(response).await)))
+        Err(Error::Http(HttpError::from_response(response).await))
     }
 }
 

src/http/ratelimiting.rs

@@ -450,18 +450,13 @@ mod tests {
     fn test_parse_header_errors() {
         let headers = headers();
 
-        // This macro wouldn't be needed if `Error::Http` didn't
-        // box its error, or if box patterns were stable.
-        macro_rules! is_err {
-            ($header:expr, $err:pat) => {
-                match parse_header::<i64>(&headers, $header).unwrap_err() {
-                    Error::Http(x) => matches!(*x, $err),
-                    _ => false,
-                }
-            };
-        }
-
-        assert!(is_err!("x-bad-num", HttpError::RateLimitI64F64));
-        assert!(is_err!("x-bad-unicode", HttpError::RateLimitUtf8));
+        assert!(matches!(
+            parse_header::<i64>(&headers, "x-bad-num").unwrap_err(),
+            Error::Http(HttpError::RateLimitI64F64)
+        ));
+        assert!(matches!(
+            parse_header::<i64>(&headers, "x-bad-unicode").unwrap_err(),
+            Error::Http(HttpError::RateLimitUtf8)
+        ));
     }
 }