Better errors + dptree integration

crates/teloxide-macros/src/button_enum.rs

@@ -17,9 +17,15 @@ impl ButtonEnum {
 
         variants_only_attr![rename];
 
-        let separator = fields_separator
+        let separator = match fields_separator {
-            .map(|(s, _)| s)
+            Some((separator, sp)) => {
-            .unwrap_or_else(|| String::from(DEFAULT_CALLBACK_DATA_SEPARATOR));
+                if separator.is_empty() {
+                    compile_error_at("Separator can't be empty!", sp);
+                }
+                separator
+            }
+            None => String::from(DEFAULT_CALLBACK_DATA_SEPARATOR),
+        };
 
         // We can just always use a separator parser, since the user won't ever interact
         // with that

crates/teloxide-macros/src/fields_stringify.rs

@@ -36,7 +36,9 @@ pub(crate) fn impl_stringify_args_unnamed(
             if stringified.contains(fields_separator) {
                 return ::std::result::Result::Err(StringifyError::SeparatorInUnnamedArgument {
                     enum_variant: std::concat!(#self_string_name, "::", #string_variant).to_owned(),
-                    field: #i
+                    stringified_data: stringified,
+                    separator: fields_separator.to_owned(),
+                    field: #i,
                 });
             }
             stringified
@@ -66,7 +68,9 @@ pub(crate) fn impl_stringify_args_named(
             if stringified.contains(fields_separator) {
                 return ::std::result::Result::Err(StringifyError::SeparatorInNamedArgument {
                     enum_variant: ::std::concat!(#self_string_name, "::", #string_variant).to_owned(),
-                    argument: ::std::stringify!(#name).to_string()
+                    stringified_data: stringified,
+                    separator: fields_separator.to_owned(),
+                    argument: ::std::stringify!(#name).to_string(),
                 });
             }
             stringified

crates/teloxide-macros/src/inline_buttons.rs

@@ -84,7 +84,7 @@ fn impl_parse(
                    #(
                         #matching_values => Ok(#variants_initialization),
                    )*
-                   _ => ::std::result::Result::Err(ParseError::UnknownCommand(enum_variant.to_owned())),
+                   _ => ::std::result::Result::Err(ParseError::UnknownCallbackDataVariant(enum_variant.to_owned())),
               }
          }
     }

crates/teloxide/src/dispatching/handler_ext.rs

@@ -4,9 +4,10 @@ use crate::{
         DpHandlerDescription,
     },
     types::{Me, Message},
-    utils::command::BotCommands,
+    utils::{button::InlineButtons, command::BotCommands},
 };
 use dptree::{di::DependencyMap, Handler};
+use teloxide_core::types::CallbackQuery;
 
 use std::fmt::Debug;
 
@@ -35,6 +36,16 @@ pub trait HandlerExt<Output> {
     where
         C: BotCommands + Send + Sync + 'static;
 
+    /// Returns a handler that accepts a parsed callback query data `D`.
+    ///
+    /// ## Dependency requirements
+    ///
+    ///  - [`crate::types::CallbackQuery`]
+    #[must_use]
+    fn filter_callback_data<D>(self) -> Self
+    where
+        D: InlineButtons + Send + Sync + 'static;
+
     /// Passes [`Dialogue<D, S>`] and `D` as handler dependencies.
     ///
     /// It does so by the following steps:
@@ -80,6 +91,13 @@ where
         self.chain(filter_mention_command::<C, Output>())
     }
 
+    fn filter_callback_data<D>(self) -> Self
+    where
+        D: InlineButtons + Send + Sync + 'static,
+    {
+        self.chain(filter_callback_data::<D, Output>())
+    }
+
     fn enter_dialogue<Upd, S, D>(self) -> Self
     where
         S: Storage<D> + ?Sized + Send + Sync + 'static,
@@ -113,6 +131,28 @@ where
     })
 }
 
+/// Returns a handler that accepts a parsed callback query data `D`
+///
+/// A call to this function is the same as
+/// `dptree::entry().filter_callback_data()`.
+///
+/// See [`HandlerExt::filter_callback_data`].
+///
+/// ## Dependency requirements
+///
+///  - [`crate::types::CallbackQuery`]
+#[must_use]
+pub fn filter_callback_data<D, Output>(
+) -> Handler<'static, DependencyMap, Output, DpHandlerDescription>
+where
+    D: InlineButtons + Send + Sync + 'static,
+    Output: Send + Sync + 'static,
+{
+    dptree::filter_map(move |callback_query: CallbackQuery| {
+        callback_query.data.and_then(|data| D::parse(data.as_ref()).ok())
+    })
+}
+
 /// Returns a handler that accepts a parsed command `C` if the command
 /// contains a bot mention, for example `/start@my_bot`.
 ///
@@ -151,13 +191,17 @@ where
 #[cfg(test)]
 #[cfg(feature = "macros")]
 mod tests {
-    use crate::{self as teloxide, dispatching::UpdateFilterExt, utils::command::BotCommands};
+    use crate::{
+        self as teloxide,
+        dispatching::UpdateFilterExt,
+        utils::{button::InlineButtons, command::BotCommands},
+    };
     use chrono::DateTime;
     use dptree::deps;
     use teloxide_core::types::{
-        Chat, ChatFullInfo, ChatId, ChatKind, ChatPrivate, LinkPreviewOptions, Me, MediaKind,
+        CallbackQuery, Chat, ChatFullInfo, ChatId, ChatKind, ChatPrivate, LinkPreviewOptions, Me,
-        MediaText, Message, MessageCommon, MessageId, MessageKind, Update, UpdateId, UpdateKind,
+        MediaKind, MediaText, Message, MessageCommon, MessageId, MessageKind, Update, UpdateId,
-        User, UserId,
+        UpdateKind, User, UserId,
     };
 
     use super::HandlerExt;
@@ -168,78 +212,111 @@ mod tests {
         Test,
     }
 
-    fn make_update(text: String) -> Update {
+    #[derive(InlineButtons, Debug, PartialEq)]
+    enum CallbackButtons {
+        Button1,
+        Button2(String),
+        Button3 { field1: u32 },
+    }
+
+    fn make_from() -> User {
+        User {
+            id: UserId(109_998_024),
+            is_bot: false,
+            first_name: String::from("Laster"),
+            last_name: None,
+            username: Some(String::from("laster_alex")),
+            language_code: Some(String::from("en")),
+            is_premium: false,
+            added_to_attachment_menu: false,
+        }
+    }
+
+    fn make_chat() -> Chat {
+        Chat {
+            id: ChatId(109_998_024),
+            kind: ChatKind::Private(ChatPrivate {
+                username: Some(String::from("Laster")),
+                first_name: Some(String::from("laster_alex")),
+                last_name: None,
+                bio: None,
+                has_private_forwards: None,
+                has_restricted_voice_and_video_messages: None,
+                business_intro: None,
+                business_location: None,
+                business_opening_hours: None,
+                birthdate: None,
+                personal_chat: None,
+            }),
+            photo: None,
+            available_reactions: None,
+            pinned_message: None,
+            message_auto_delete_time: None,
+            has_hidden_members: false,
+            has_aggressive_anti_spam_enabled: false,
+            chat_full_info: ChatFullInfo::default(),
+        }
+    }
+
+    fn make_message(text: String) -> Message {
         let timestamp = 1_569_518_829;
         let date = DateTime::from_timestamp(timestamp, 0).unwrap();
+        Message {
+            via_bot: None,
+            id: MessageId(5042),
+            thread_id: None,
+            from: Some(make_from()),
+            sender_chat: None,
+            is_topic_message: false,
+            sender_business_bot: None,
+            date,
+            chat: make_chat(),
+            kind: MessageKind::Common(MessageCommon {
+                reply_to_message: None,
+                forward_origin: None,
+                external_reply: None,
+                quote: None,
+                edit_date: None,
+                media_kind: MediaKind::Text(MediaText {
+                    text,
+                    entities: vec![],
+                    link_preview_options: Some(LinkPreviewOptions {
+                        is_disabled: true,
+                        url: None,
+                        prefer_small_media: false,
+                        prefer_large_media: false,
+                        show_above_text: false,
+                    }),
+                }),
+                reply_markup: None,
+                author_signature: None,
+                is_automatic_forward: false,
+                has_protected_content: false,
+                reply_to_story: None,
+                sender_boost_count: None,
+                is_from_offline: false,
+                business_connection_id: None,
+            }),
+        }
+    }
+
+    fn make_update(text: String) -> Update {
+        Update { id: UpdateId(326_170_274), kind: UpdateKind::Message(make_message(text)) }
+    }
+
+    fn make_callback_query_update(data: String) -> Update {
         Update {
-            id: UpdateId(326_170_274),
+            id: UpdateId(326_170_275),
-            kind: UpdateKind::Message(Message {
+            kind: UpdateKind::CallbackQuery(CallbackQuery {
-                via_bot: None,
+                id: "5024".to_string(),
-                id: MessageId(5042),
+                from: make_from(),
-                thread_id: None,
+                message: Some(teloxide_core::types::MaybeInaccessibleMessage::Regular(
-                from: Some(User {
+                    make_message("text".to_owned()),
-                    id: UserId(109_998_024),
+                )),
-                    is_bot: false,
+                inline_message_id: None,
-                    first_name: String::from("Laster"),
+                chat_instance: "12345678".to_owned(),
-                    last_name: None,
+                data: Some(data),
-                    username: Some(String::from("laster_alex")),
+                game_short_name: None,
-                    language_code: Some(String::from("en")),
-                    is_premium: false,
-                    added_to_attachment_menu: false,
-                }),
-                sender_chat: None,
-                is_topic_message: false,
-                sender_business_bot: None,
-                date,
-                chat: Chat {
-                    id: ChatId(109_998_024),
-                    kind: ChatKind::Private(ChatPrivate {
-                        username: Some(String::from("Laster")),
-                        first_name: Some(String::from("laster_alex")),
-                        last_name: None,
-                        bio: None,
-                        has_private_forwards: None,
-                        has_restricted_voice_and_video_messages: None,
-                        business_intro: None,
-                        business_location: None,
-                        business_opening_hours: None,
-                        birthdate: None,
-                        personal_chat: None,
-                    }),
-                    photo: None,
-                    available_reactions: None,
-                    pinned_message: None,
-                    message_auto_delete_time: None,
-                    has_hidden_members: false,
-                    has_aggressive_anti_spam_enabled: false,
-                    chat_full_info: ChatFullInfo::default(),
-                },
-                kind: MessageKind::Common(MessageCommon {
-                    reply_to_message: None,
-                    forward_origin: None,
-                    external_reply: None,
-                    quote: None,
-                    edit_date: None,
-                    media_kind: MediaKind::Text(MediaText {
-                        text,
-                        entities: vec![],
-                        link_preview_options: Some(LinkPreviewOptions {
-                            is_disabled: true,
-                            url: None,
-                            prefer_small_media: false,
-                            prefer_large_media: false,
-                            show_above_text: false,
-                        }),
-                    }),
-                    reply_markup: None,
-                    author_signature: None,
-                    is_automatic_forward: false,
-                    has_protected_content: false,
-                    reply_to_story: None,
-                    sender_boost_count: None,
-                    is_from_offline: false,
-                    business_connection_id: None,
-                }),
             }),
         }
     }
@@ -300,4 +377,32 @@ mod tests {
         let result = h.dispatch(deps![update, me.clone()]).await;
         assert!(result.is_continue());
     }
+
+    #[tokio::test]
+    async fn test_filter_callback_data() {
+        let h = dptree::entry().branch(
+            Update::filter_callback_query()
+                .filter_callback_data::<CallbackButtons>()
+                .endpoint(|| async {}),
+        );
+
+        let button = CallbackButtons::Button1;
+        let update = make_callback_query_update(button.stringify().unwrap());
+        let result = h.dispatch(deps![update]).await;
+        assert!(result.is_break());
+
+        let button = CallbackButtons::Button2("SomeData".to_owned());
+        let update = make_callback_query_update(button.stringify().unwrap());
+        let result = h.dispatch(deps![update]).await;
+        assert!(result.is_break());
+
+        let button = CallbackButtons::Button3 { field1: 123 };
+        let update = make_callback_query_update(button.stringify().unwrap());
+        let result = h.dispatch(deps![update]).await;
+        assert!(result.is_break());
+
+        let update = make_callback_query_update("wrong_data".to_string());
+        let result = h.dispatch(deps![update]).await;
+        assert!(result.is_continue());
+    }
 }

crates/teloxide/src/utils/button.rs

@@ -1,5 +1,8 @@
 //! Docs later
+use std::fmt::{Display, Formatter};
+
 use super::command::ParseError;
+use teloxide_core::types::InlineKeyboardButton;
 #[cfg(feature = "macros")]
 pub use teloxide_macros::InlineButtons;
 
@@ -10,6 +13,17 @@ pub trait InlineButtons: Sized {
 
     /// Stringifies the callback data.
     fn stringify(self) -> Result<String, StringifyError>;
+
+    /// Builds an [`InlineKeyboardButton`] from the enum variant
+    ///
+    /// [`InlineKeyboardButton`]: crate::types::InlineKeyboardButton
+    fn build_button<T>(self, text: T) -> Result<InlineKeyboardButton, StringifyError>
+    where
+        T: Into<String>,
+    {
+        let callback_data = self.stringify()?;
+        Ok(InlineKeyboardButton::callback(text.into(), callback_data))
+    }
 }
 
 /// Errors returned from [`InlineButtons::stringify`].
@@ -17,6 +31,79 @@ pub trait InlineButtons: Sized {
 /// [`InlineButtons::stringify`]: InlineButtons::stringify
 #[derive(Debug)]
 pub enum StringifyError {
-    SeparatorInNamedArgument { enum_variant: String, argument: String },
+    SeparatorInNamedArgument {
-    SeparatorInUnnamedArgument { enum_variant: String, field: usize },
+        enum_variant: String,
+        stringified_data: String,
+        separator: String,
+        argument: String,
+    },
+    SeparatorInUnnamedArgument {
+        enum_variant: String,
+        stringified_data: String,
+        separator: String,
+        field: usize,
+    },
 }
+
+fn make_pointers_string(prefix: String, text: String, point_to: String) -> String {
+    let all_indexes: Vec<_> = text.match_indices(&point_to).collect();
+    let prefix_len = prefix.chars().count();
+    let mut pointers_vec = vec![" "; prefix_len + text.chars().count()];
+
+    for (start, matched) in &all_indexes {
+        for (i, _) in matched.chars().enumerate() {
+            pointers_vec[prefix_len + start + i] = "^";
+        }
+    }
+    pointers_vec.join("")
+}
+
+impl Display for StringifyError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
+        match self {
+            Self::SeparatorInNamedArgument {
+                enum_variant,
+                stringified_data,
+                separator,
+                argument,
+            } => {
+                let prefix = format!("self.{argument} == \"");
+                // Makes ^^^ pointers to where the separator is
+                let separator_pointers_string = make_pointers_string(
+                    prefix.clone(),
+                    stringified_data.to_string(),
+                    separator.to_string(),
+                );
+                write!(
+                    f,
+                    "There is a separator \"{separator}\" in `{enum_variant}`: \
+                     \n{prefix}{stringified_data}\"\n{separator_pointers_string}\n\nPlease \
+                     consider changing the separator with \
+                     `#[button(fields_separator=\"NEW_SEPARATOR\")]`"
+                )
+            }
+            Self::SeparatorInUnnamedArgument {
+                enum_variant,
+                stringified_data,
+                separator,
+                field,
+            } => {
+                let prefix = format!("self.{field} == \"");
+                let separator_pointers_string = make_pointers_string(
+                    prefix.clone(),
+                    stringified_data.to_string(),
+                    separator.to_string(),
+                );
+                write!(
+                    f,
+                    "There is a separator \"{separator}\" in `{enum_variant}`: \
+                     \n{prefix}{stringified_data}\"\n{separator_pointers_string}\n\nPlease \
+                     consider changing the separator with \
+                     `#[button(fields_separator=\"NEW_SEPARATOR\")]`"
+                )
+            }
+        }
+    }
+}
+
+impl std::error::Error for StringifyError {}

crates/teloxide/src/utils/command.rs

@@ -266,6 +266,7 @@ pub trait BotCommands: Sized {
 
 pub type PrefixedBotCommand = String;
 pub type BotName = String;
+pub type CallbackDataVariant = String;
 
 /// Errors returned from [`BotCommands::parse`].
 ///
@@ -291,6 +292,11 @@ pub enum ParseError {
     UnknownCommand(PrefixedBotCommand),
     WrongBotName(BotName),
 
+    /// Error for [`InlineButtons`].
+    ///
+    /// [`InlineButtons`]: crate::utils::button::InlineButtons
+    UnknownCallbackDataVariant(CallbackDataVariant),
+
     /// A custom error which you can return from your custom parser.
     Custom(Box<dyn Error + Send + Sync + 'static>),
 }
@@ -472,6 +478,9 @@ impl Display for ParseError {
             ),
             ParseError::IncorrectFormat(e) => write!(f, "Incorrect format of command args: {e}"),
             ParseError::UnknownCommand(e) => write!(f, "Unknown command: {e}"),
+            ParseError::UnknownCallbackDataVariant(v) => {
+                write!(f, "Unknown callback data variant: {v}")
+            }
             ParseError::WrongBotName(n) => write!(f, "Wrong bot name: {n}"),
             ParseError::Custom(e) => write!(f, "{e}"),
         }

crates/teloxide/tests/buttons.rs

@@ -0,0 +1,46 @@
+#[cfg(feature = "macros")]
+use teloxide::utils::button::InlineButtons;
+
+// We put tests here because macro expand in unit tests in module
+// teloxide::utils::button was a failure
+
+#[cfg(feature = "macros")]
+#[derive(InlineButtons, Debug, PartialEq)]
+enum CallbackButtons {
+    Button1,
+    Button2(String),
+    Button3 { field1: u32 },
+}
+
+#[test]
+#[cfg(feature = "macros")]
+fn test_make_button() {
+    use teloxide::types::InlineKeyboardButton;
+
+    let text = "Text for button 1";
+    let actual = CallbackButtons::Button1.build_button(text).unwrap();
+    let expected = InlineKeyboardButton::callback(text, "Button1");
+    assert_eq!(actual, expected);
+}
+
+#[test]
+#[cfg(feature = "macros")]
+fn test_make_button_with_unnamed_args() {
+    use teloxide::types::InlineKeyboardButton;
+
+    let text = "Text for button 2";
+    let actual = CallbackButtons::Button2("data".to_owned()).build_button(text).unwrap();
+    let expected = InlineKeyboardButton::callback(text, "Button2;data");
+    assert_eq!(actual, expected);
+}
+
+#[test]
+#[cfg(feature = "macros")]
+fn test_make_button_with_named_args() {
+    use teloxide::types::InlineKeyboardButton;
+
+    let text = "Text for button 3";
+    let actual = CallbackButtons::Button3 { field1: 23 }.build_button(text).unwrap();
+    let expected = InlineKeyboardButton::callback(text, "Button3;23");
+    assert_eq!(actual, expected);
+}

crates/teloxide/tests/callback_data.rs

@@ -2,7 +2,7 @@
 use teloxide::utils::button::InlineButtons;
 
 // We put tests here because macro expand in unit tests in module
-// teloxide::utils::command was a failure
+// teloxide::utils::button was a failure
 
 #[test]
 #[cfg(feature = "macros")]
@@ -49,10 +49,18 @@ fn stringify_button_error() {
     }
 
     let button = DefaultData::Fruit("test;test2".to_string());
+
     match button.stringify() {
-        Err(StringifyError::SeparatorInUnnamedArgument { enum_variant, field }) => {
+        Err(StringifyError::SeparatorInUnnamedArgument {
+            enum_variant,
+            stringified_data,
+            separator,
+            field,
+        }) => {
             assert_eq!(field, 0);
-            assert_eq!(enum_variant, "DefaultData::Fruit")
+            assert_eq!(enum_variant, "DefaultData::Fruit");
+            assert_eq!(stringified_data, "test;test2");
+            assert_eq!(separator, ";");
         }
         _ => panic!("Expected an error!"),
     }
@@ -121,9 +129,16 @@ fn stringify_button_named_fields_error() {
 
     let button = DefaultData::Fruit { num: 9, data: "test;test2".to_owned() };
     match button.stringify() {
-        Err(StringifyError::SeparatorInNamedArgument { enum_variant, argument }) => {
+        Err(StringifyError::SeparatorInNamedArgument {
+            enum_variant,
+            stringified_data,
+            separator,
+            argument,
+        }) => {
             assert_eq!(argument, "data".to_owned());
-            assert_eq!(enum_variant, "DefaultData::Fruit")
+            assert_eq!(enum_variant, "DefaultData::Fruit");
+            assert_eq!(stringified_data, "test;test2");
+            assert_eq!(separator, ";");
         }
         _ => panic!("Expected an error!"),
     }