refactor: Use regular functions rather than FromStr impls (#8178)

Implementing `FromStr` and then calling `parse()` creates an
indirection, which is hard to follow for people who are not familiar
with Rust. @r10s recently voiced this problem when we were
pair-programming, and I agree.

We can decide what exactly to call the new function.

I didn't remove all `FromStr` implementations yet; `FromStr for
Fingerprint`, `FromStr for Params`, `FromStr for MozConfigTag` and
`FromStr for EphemeralTimer` are still left.

src/aheader.rs

@@ -4,9 +4,8 @@
 
 use std::collections::BTreeMap;
 use std::fmt;
-use std::str::FromStr;
 
-use anyhow::{Context as _, Error, Result, bail};
+use anyhow::{Context as _, Result, bail};
 
 use crate::key::{DcKey, SignedPublicKey};
 
@@ -28,10 +27,8 @@ impl fmt::Display for EncryptPreference {
     }
 }
 
-impl FromStr for EncryptPreference {
+impl EncryptPreference {
-    type Err = Error;
+    fn new(s: &str) -> Result<Self> {
-
-    fn from_str(s: &str) -> Result<Self> {
         match s {
             "mutual" => Ok(EncryptPreference::Mutual),
             "nopreference" => Ok(EncryptPreference::NoPreference),
@@ -85,10 +82,8 @@ impl fmt::Display for Aheader {
     }
 }
 
-impl FromStr for Aheader {
+impl Aheader {
-    type Err = Error;
+    pub(crate) fn from_str(s: &str) -> Result<Self> {
-
-    fn from_str(s: &str) -> Result<Self> {
         let mut attributes: BTreeMap<String, String> = s
             .split(';')
             .filter_map(|a| {
@@ -116,7 +111,7 @@ impl FromStr for Aheader {
 
         let prefer_encrypt = attributes
             .remove("prefer-encrypt")
-            .and_then(|raw| raw.parse().ok())
+            .and_then(|raw| EncryptPreference::new(&raw).ok())
             .unwrap_or_default();
 
         let verified = attributes.remove("_verified").is_some();
@@ -144,8 +139,9 @@ mod tests {
 
     #[test]
     fn test_from_str() -> Result<()> {
-        let h: Aheader =
+        let h = Aheader::from_str(&format!(
-            format!("addr=me@mail.com; prefer-encrypt=mutual; keydata={RAWKEY}").parse()?;
+            "addr=me@mail.com; prefer-encrypt=mutual; keydata={RAWKEY}"
+        ))?;
 
         assert_eq!(h.addr, "me@mail.com");
         assert_eq!(h.prefer_encrypt, EncryptPreference::Mutual);
@@ -157,7 +153,7 @@ mod tests {
     #[test]
     fn test_from_str_reset() -> Result<()> {
         let raw = format!("addr=reset@example.com; prefer-encrypt=reset; keydata={RAWKEY}");
-        let h: Aheader = raw.parse()?;
+        let h = Aheader::from_str(&raw)?;
 
         assert_eq!(h.addr, "reset@example.com");
         assert_eq!(h.prefer_encrypt, EncryptPreference::NoPreference);
@@ -167,7 +163,7 @@ mod tests {
     #[test]
     fn test_from_str_non_critical() -> Result<()> {
         let raw = format!("addr=me@mail.com; _foo=one; _bar=two; keydata={RAWKEY}");
-        let h: Aheader = raw.parse()?;
+        let h = Aheader::from_str(&raw)?;
 
         assert_eq!(h.addr, "me@mail.com");
         assert_eq!(h.prefer_encrypt, EncryptPreference::NoPreference);
@@ -177,7 +173,7 @@ mod tests {
     #[test]
     fn test_from_str_superflous_critical() {
         let raw = format!("addr=me@mail.com; _foo=one; _bar=two; other=me; keydata={RAWKEY}");
-        assert!(raw.parse::<Aheader>().is_err());
+        assert!(Aheader::from_str(&raw).is_err());
     }
 
     #[test]

src/mimeparser.rs

@@ -2127,7 +2127,7 @@ async fn parse_gossip_headers(
     let mut gossiped_keys: BTreeMap<String, GossipedKey> = Default::default();
 
     for value in &gossip_headers {
-        let header = match value.parse::<Aheader>() {
+        let header = match Aheader::from_str(value) {
             Ok(header) => header,
             Err(err) => {
                 warn!(context, "Failed parsing Autocrypt-Gossip header: {}", err);

src/reaction.rs

@@ -36,10 +36,7 @@ pub struct Reaction {
     reaction: String,
 }
 
-// We implement From<&str> instead of std::str::FromStr, because
+impl Reaction {
-// FromStr requires error type and reaction parsing never returns an
-// error.
-impl From<&str> for Reaction {
     /// Convert a `&str` into a `Reaction`.
     /// Everything after the first whitespace is ignored.
     ///
@@ -51,7 +48,7 @@ impl From<&str> for Reaction {
     /// reactions is not different from other kinds of spam attacks
     /// such as sending large numbers of large messages, and should be
     /// dealt with the same way, e.g. by blocking the user.
-    fn from(reaction: &str) -> Self {
+    pub fn new(reaction: &str) -> Self {
         let reaction: &str = reaction
             .split_ascii_whitespace()
             .next()
@@ -61,9 +58,7 @@ impl From<&str> for Reaction {
             reaction: reaction.to_string(),
         }
     }
-}
 
-impl Reaction {
     /// Returns true if reaction contains no emoji.
     pub fn is_empty(&self) -> bool {
         self.reaction.is_empty()
@@ -212,7 +207,7 @@ pub async fn send_reaction(context: &Context, msg_id: MsgId, reaction: &str) ->
     let msg = Message::load_from_db(context, msg_id).await?;
     let chat_id = msg.chat_id;
 
-    let reaction: Reaction = reaction.into();
+    let reaction = Reaction::new(reaction);
     let mut reaction_msg = Message::new_text(reaction.as_str().to_string());
     reaction_msg.set_reaction();
     reaction_msg.in_reply_to = Some(msg.rfc724_mid);
@@ -282,7 +277,7 @@ pub async fn get_msg_reactions(context: &Context, msg_id: MsgId) -> Result<React
             |row| {
                 let contact_id: ContactId = row.get(0)?;
                 let reaction: String = row.get(1)?;
-                Ok((contact_id, Reaction::from(reaction.as_str())))
+                Ok((contact_id, Reaction::new(reaction.as_str())))
             },
         )
         .await?;
@@ -361,32 +356,32 @@ mod tests {
     #[test]
     fn test_parse_reaction() {
         // Check that basic set of emojis from RFC 9078 is supported.
-        assert_eq!(Reaction::from("👍").as_str(), "👍");
+        assert_eq!(Reaction::new("👍").as_str(), "👍");
-        assert_eq!(Reaction::from("👎").as_str(), "👎");
+        assert_eq!(Reaction::new("👎").as_str(), "👎");
-        assert_eq!(Reaction::from("😀").as_str(), "😀");
+        assert_eq!(Reaction::new("😀").as_str(), "😀");
-        assert_eq!(Reaction::from("☹").as_str(), "☹");
+        assert_eq!(Reaction::new("☹").as_str(), "☹");
-        assert_eq!(Reaction::from("😢").as_str(), "😢");
+        assert_eq!(Reaction::new("😢").as_str(), "😢");
 
         // Empty string can be used to remove all reactions.
-        assert!(Reaction::from("").is_empty());
+        assert!(Reaction::new("").is_empty());
 
         // Short strings can be used as emojis, could be used to add
         // support for custom emojis via emoji shortcodes.
-        assert_eq!(Reaction::from(":deltacat:").as_str(), ":deltacat:");
+        assert_eq!(Reaction::new(":deltacat:").as_str(), ":deltacat:");
 
         // Check that long strings are not valid emojis.
         assert!(
-            Reaction::from(":foobarbazquuxaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:").is_empty()
+            Reaction::new(":foobarbazquuxaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:").is_empty()
         );
 
         // Multiple reactions separated by spaces or tabs are not supported.
-        assert_eq!(Reaction::from("👍 ❤").as_str(), "👍");
+        assert_eq!(Reaction::new("👍 ❤").as_str(), "👍");
-        assert_eq!(Reaction::from("👍\t❤").as_str(), "👍");
+        assert_eq!(Reaction::new("👍\t❤").as_str(), "👍");
 
-        assert_eq!(Reaction::from("👍\t:foo: ❤").as_str(), "👍");
+        assert_eq!(Reaction::new("👍\t:foo: ❤").as_str(), "👍");
-        assert_eq!(Reaction::from("👍\t:foo: ❤").as_str(), "👍");
+        assert_eq!(Reaction::new("👍\t:foo: ❤").as_str(), "👍");
 
-        assert_eq!(Reaction::from("👍 👍").as_str(), "👍");
+        assert_eq!(Reaction::new("👍 👍").as_str(), "👍");
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -582,7 +577,7 @@ Content-Disposition: reaction\n\
                 assert_eq!(chat_id, expected_chat_id);
                 assert_eq!(msg_id, expected_msg_id);
                 assert_eq!(contact_id, expected_contact_id);
-                assert_eq!(reaction, Reaction::from(expected_reaction));
+                assert_eq!(reaction, Reaction::new(expected_reaction));
             }
             _ => panic!("Unexpected event {event:?}."),
         }

src/receive_imf.rs

@@ -3,6 +3,7 @@
 use std::cmp;
 use std::collections::{BTreeMap, BTreeSet};
 use std::iter;
+use std::str::FromStr as _;
 use std::sync::LazyLock;
 
 use anyhow::{Context as _, Result, ensure};
@@ -1828,7 +1829,7 @@ async fn add_parts(
     // Extract ephemeral timer from the message
     let mut ephemeral_timer = if let Some(value) = mime_parser.get_header(HeaderDef::EphemeralTimer)
     {
-        match value.parse::<EphemeralTimer>() {
+        match EphemeralTimer::from_str(value) {
             Ok(timer) => timer,
             Err(err) => {
                 warn!(context, "Can't parse ephemeral timer \"{value}\": {err:#}.");
@@ -2106,7 +2107,7 @@ async fn add_parts(
                 chat_id,
                 from_id,
                 sort_timestamp,
-                Reaction::from(reaction_str.as_str()),
+                Reaction::new(reaction_str.as_str()),
                 is_incoming_fresh,
             )
             .await?;