Simplify Config structure

This simplifies some of the `Config` structure, in particular this
means:

Parameters which are meaningfully equivalent longer stored in
an `Option<T>`, an example of this is `channels`. If you don't
want to join any channels you simply leave it as empty instead.
In effect, `None` is behaviorally equivalent to `vec![]`.

We don't allocate when accessing certain configuration options.
For example, when accessing `channels` we used to allocate a
vector to handle the "empty case", we simply return the slice
corresponding to the list of channels instead.

We skip serializing empty or optional configuration fields.
From a deserialization perspective this is already something
that was mostly supported through use of `Option<T>` and
`#[serde(default)]`.

examples/build-bot.rs

@@ -15,7 +15,7 @@ async fn main() -> irc::error::Result<()> {
     let config = Config {
         nickname: Some("irc-crate-ci".to_owned()),
         server: Some("irc.pdgn.co".to_owned()),
-        use_ssl: Some(true),
+        use_ssl: true,
         ..Default::default()
     };
 

examples/multiserver.rs

@@ -10,14 +10,14 @@ async fn main() -> irc::error::Result<()> {
     let cfg1 = Config {
         nickname: Some("pickles".to_owned()),
         server: Some("irc.mozilla.org".to_owned()),
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
         ..Default::default()
     };
 
     let cfg2 = Config {
         nickname: Some("bananas".to_owned()),
         server: Some("irc.mozilla.org".to_owned()),
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
         ..Default::default()
     };
 

examples/repeater.rs

@@ -5,10 +5,10 @@ use irc::client::prelude::*;
 async fn main() -> irc::error::Result<()> {
     let config = Config {
         nickname: Some("repeater".to_owned()),
-        alt_nicks: Some(vec!["blaster".to_owned(), "smg".to_owned()]),
+        alt_nicks: vec!["blaster".to_owned(), "smg".to_owned()],
         server: Some("irc.mozilla.org".to_owned()),
-        use_ssl: Some(true),
+        use_ssl: true,
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
         burst_window_length: Some(4),
         max_messages_in_burst: Some(4),
         ..Default::default()

examples/simple.rs

@@ -5,9 +5,9 @@ use irc::client::prelude::*;
 async fn main() -> irc::error::Result<()> {
     let config = Config {
         nickname: Some("pickles".to_owned()),
-        alt_nicks: Some(vec!["bananas".to_owned(), "apples".to_owned()]),
+        alt_nicks: vec!["bananas".to_owned(), "apples".to_owned()],
         server: Some("irc.mozilla.org".to_owned()),
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
         ..Default::default()
     };
 

examples/simple_ssl.rs

@@ -6,8 +6,8 @@ async fn main() -> irc::error::Result<()> {
     let config = Config {
         nickname: Some("pickles".to_owned()),
         server: Some("irc.mozilla.org".to_owned()),
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
-        use_ssl: Some(true),
+        use_ssl: true,
         ..Default::default()
     };
 

examples/tooter.rs

@@ -8,7 +8,7 @@ async fn main() -> irc::error::Result<()> {
     let config = Config {
         nickname: Some("mastodon".to_owned()),
         server: Some("irc.mozilla.org".to_owned()),
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
         ..Default::default()
     };
 

examples/tweeter.rs

@@ -8,7 +8,7 @@ async fn main() -> irc::error::Result<()> {
     let config = Config {
         nickname: Some("pickles".to_owned()),
         server: Some("irc.mozilla.org".to_owned()),
-        channels: Some(vec!["#rust-spam".to_owned()]),
+        channels: vec!["#rust-spam".to_owned()],
         ..Default::default()
     };
 

src/client/data/config.rs

@@ -68,77 +68,115 @@ use crate::error::{ConfigError, Result};
 #[derive(Clone, Deserialize, Serialize, Default, PartialEq, Debug)]
 pub struct Config {
     /// A list of the owners of the client by nickname (for bots).
-    pub owners: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    #[serde(default)]
+    pub owners: Vec<String>,
     /// The client's nickname.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub nickname: Option<String>,
     /// The client's NICKSERV password.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub nick_password: Option<String>,
     /// Alternative nicknames for the client, if the default is taken.
-    pub alt_nicks: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    #[serde(default)]
+    pub alt_nicks: Vec<String>,
     /// The client's username.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub username: Option<String>,
     /// The client's real name.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub realname: Option<String>,
     /// The server to connect to.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub server: Option<String>,
     /// The port to connect on.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub port: Option<u16>,
     /// The password to connect to the server.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub password: Option<String>,
     /// Whether or not to use SSL.
     /// Clients will automatically panic if this is enabled without SSL support.
-    pub use_ssl: Option<bool>,
+    #[serde(skip_serializing_if = "is_false")]
+    #[serde(default)]
+    pub use_ssl: bool,
     /// The path to the SSL certificate for this server in DER format.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub cert_path: Option<String>,
     /// The path to a SSL certificate to use for CertFP client authentication in DER format.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub client_cert_path: Option<String>,
     /// The password for the certificate to use in CertFP authentication.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub client_cert_pass: Option<String>,
     /// The encoding type used for this connection.
     /// This is typically UTF-8, but could be something else.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub encoding: Option<String>,
     /// A list of channels to join on connection.
-    pub channels: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    #[serde(default)]
+    pub channels: Vec<String>,
     /// User modes to set on connect. Example: "+RB -x"
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub umodes: Option<String>,
     /// The text that'll be sent in response to CTCP USERINFO requests.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub user_info: Option<String>,
     /// The text that'll be sent in response to CTCP VERSION requests.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub version: Option<String>,
     /// The text that'll be sent in response to CTCP SOURCE requests.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub source: Option<String>,
     /// The amount of inactivity in seconds before the client will ping the server.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub ping_time: Option<u32>,
     /// The amount of time in seconds for a client to reconnect due to no ping response.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub ping_timeout: Option<u32>,
     /// The length in seconds of a rolling window for message throttling. If more than
     /// `max_messages_in_burst` messages are sent within `burst_window_length` seconds, additional
     /// messages will be delayed automatically as appropriate. In particular, in the past
     /// `burst_window_length` seconds, there will never be more than `max_messages_in_burst` messages
     /// sent.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub burst_window_length: Option<u32>,
     /// The maximum number of messages that can be sent in a burst window before they'll be delayed.
     /// Messages are automatically delayed as appropriate.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub max_messages_in_burst: Option<u32>,
     /// Whether the client should use NickServ GHOST to reclaim its primary nickname if it is in
     /// use. This has no effect if `nick_password` is not set.
-    pub should_ghost: Option<bool>,
+    #[serde(skip_serializing_if = "is_false")]
+    #[serde(default)]
+    pub should_ghost: bool,
     /// The command(s) that should be sent to NickServ to recover a nickname. The nickname and
     /// password will be appended in that order after the command.
     /// E.g. `["RECOVER", "RELEASE"]` means `RECOVER nick pass` and `RELEASE nick pass` will be sent
     /// in that order.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    #[serde(default)]
     pub ghost_sequence: Option<Vec<String>>,
     /// Whether or not to use a fake connection for testing purposes. You probably will never want
     /// to enable this, but it is used in unit testing for the `irc` crate.
-    pub use_mock_connection: Option<bool>,
+    #[serde(skip_serializing_if = "is_false")]
+    #[serde(default)]
+    pub use_mock_connection: bool,
     /// The initial value used by the fake connection for testing. You probably will never need to
     /// set this, but it is used in unit testing for the `irc` crate.
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub mock_initial_value: Option<String>,
 
     /// A mapping of channel names to keys for join-on-connect.
-    pub channel_keys: Option<HashMap<String, String>>,
+    #[serde(skip_serializing_if = "HashMap::is_empty")]
+    #[serde(default)]
+    pub channel_keys: HashMap<String, String>,
     /// A map of additional options to be stored in config.
-    pub options: Option<HashMap<String, String>>,
+    #[serde(skip_serializing_if = "HashMap::is_empty")]
+    #[serde(default)]
+    pub options: HashMap<String, String>,
 
     /// The path that this configuration was loaded from.
     ///
@@ -148,6 +186,10 @@ pub struct Config {
     pub path: Option<PathBuf>,
 }
 
+fn is_false(v: &bool) -> bool {
+    !v
+}
+
 impl Config {
     fn with_path<P: AsRef<Path>>(mut self, path: P) -> Config {
         self.path = Some(path.as_ref().to_owned());
@@ -316,17 +358,14 @@ impl Config {
 
     /// Determines whether or not the nickname provided is the owner of the bot.
     pub fn is_owner(&self, nickname: &str) -> bool {
-        self.owners
+        self.owners.iter().find(|n| *n == nickname).is_some()
-            .as_ref()
-            .map(|o| o.contains(&nickname.to_owned()))
-            .unwrap()
     }
 
     /// Gets the nickname specified in the configuration.
     pub fn nickname(&self) -> Result<&str> {
         self.nickname
             .as_ref()
-            .map(|s| &s[..])
+            .map(String::as_str)
             .ok_or_else(|| InvalidConfig {
                 path: self.path(),
                 cause: ConfigError::NicknameNotSpecified,
@@ -336,15 +375,13 @@ impl Config {
     /// Gets the bot's nickserv password specified in the configuration.
     /// This defaults to an empty string when not specified.
     pub fn nick_password(&self) -> &str {
-        self.nick_password.as_ref().map_or("", |s| &s[..])
+        self.nick_password.as_ref().map_or("", String::as_str)
     }
 
     /// Gets the alternate nicknames specified in the configuration.
     /// This defaults to an empty vector when not specified.
-    pub fn alternate_nicknames(&self) -> Vec<&str> {
+    pub fn alternate_nicknames(&self) -> &[String] {
-        self.alt_nicks
+        &self.alt_nicks
-            .as_ref()
-            .map_or(vec![], |v| v.iter().map(|s| &s[..]).collect())
     }
 
     /// Gets the username specified in the configuration.
@@ -367,7 +404,7 @@ impl Config {
     pub fn server(&self) -> Result<&str> {
         self.server
             .as_ref()
-            .map(|s| &s[..])
+            .map(String::as_str)
             .ok_or_else(|| InvalidConfig {
                 path: self.path(),
                 cause: ConfigError::ServerNotSpecified,
@@ -395,28 +432,28 @@ impl Config {
     /// Gets the server password specified in the configuration.
     /// This defaults to a blank string when not specified.
     pub fn password(&self) -> &str {
-        self.password.as_ref().map_or("", |s| &s)
+        self.password.as_ref().map_or("", String::as_str)
     }
 
     /// Gets whether or not to use SSL with this connection.
     /// This defaults to false when not specified.
     pub fn use_ssl(&self) -> bool {
-        self.use_ssl.as_ref().cloned().unwrap_or(false)
+        self.use_ssl
     }
 
     /// Gets the path to the SSL certificate in DER format if specified.
     pub fn cert_path(&self) -> Option<&str> {
-        self.cert_path.as_ref().map(|s| &s[..])
+        self.cert_path.as_ref().map(String::as_str)
     }
 
     /// Gets the path to the client authentication certificate in DER format if specified.
     pub fn client_cert_path(&self) -> Option<&str> {
-        self.client_cert_path.as_ref().map(|s| &s[..])
+        self.client_cert_path.as_ref().map(String::as_str)
     }
 
     /// Gets the password to the client authentication certificate.
     pub fn client_cert_pass(&self) -> &str {
-        self.client_cert_pass.as_ref().map_or("", |s| &s[..])
+        self.client_cert_pass.as_ref().map_or("", String::as_str)
     }
 
     /// Gets the encoding to use for this connection. This requires the encode feature to work.
@@ -427,29 +464,25 @@ impl Config {
 
     /// Gets the channels to join upon connection.
     /// This defaults to an empty vector if it's not specified.
-    pub fn channels(&self) -> Vec<&str> {
+    pub fn channels(&self) -> &[String] {
-        self.channels
+        &self.channels
-            .as_ref()
-            .map_or(vec![], |v| v.iter().map(|s| &s[..]).collect())
     }
 
     /// Gets the key for the specified channel if it exists in the configuration.
     pub fn channel_key(&self, chan: &str) -> Option<&str> {
-        self.channel_keys
+        self.channel_keys.get(chan).map(String::as_str)
-            .as_ref()
-            .and_then(|m| m.get(&chan.to_owned()).map(|s| &s[..]))
     }
 
     /// Gets the user modes to set on connect specified in the configuration.
     /// This defaults to an empty string when not specified.
     pub fn umodes(&self) -> &str {
-        self.umodes.as_ref().map_or("", |s| &s[..])
+        self.umodes.as_ref().map_or("", String::as_str)
     }
 
     /// Gets the string to be sent in response to CTCP USERINFO requests.
     /// This defaults to an empty string when not specified.
     pub fn user_info(&self) -> &str {
-        self.user_info.as_ref().map_or("", |s| &s[..])
+        self.user_info.as_ref().map_or("", String::as_str)
     }
 
     /// Gets the string to be sent in response to CTCP VERSION requests.
@@ -464,7 +497,7 @@ impl Config {
     pub fn source(&self) -> &str {
         self.source
             .as_ref()
-            .map_or("https://github.com/aatxe/irc", |s| &s[..])
+            .map_or("https://github.com/aatxe/irc", String::as_str)
     }
 
     /// Gets the amount of time in seconds for the interval at which the client pings the server.
@@ -500,28 +533,24 @@ impl Config {
     /// Gets whether or not to attempt nickname reclamation using NickServ GHOST.
     /// This defaults to false when not specified.
     pub fn should_ghost(&self) -> bool {
-        self.should_ghost.as_ref().cloned().unwrap_or(false)
+        self.should_ghost
     }
 
     /// Gets the NickServ command sequence to recover a nickname.
     /// This defaults to `["GHOST"]` when not specified.
-    pub fn ghost_sequence(&self) -> Vec<&str> {
+    pub fn ghost_sequence(&self) -> Option<&[String]> {
-        self.ghost_sequence
+        self.ghost_sequence.as_ref().map(Vec::as_slice)
-            .as_ref()
-            .map_or(vec!["GHOST"], |v| v.iter().map(|s| &s[..]).collect())
     }
 
     /// Looks up the specified string in the options map.
     pub fn get_option(&self, option: &str) -> Option<&str> {
-        self.options
+        self.options.get(option).map(String::as_str)
-            .as_ref()
-            .and_then(|o| o.get(&option.to_owned()).map(|s| &s[..]))
     }
 
     /// Gets whether or not to use a mock connection for testing.
     /// This defaults to false when not specified.
     pub fn use_mock_connection(&self) -> bool {
-        self.use_mock_connection.as_ref().cloned().unwrap_or(false)
+        self.use_mock_connection
     }
 
     /// Gets the initial value for the mock connection.
@@ -540,35 +569,16 @@ mod test {
     #[allow(unused)]
     fn test_config() -> Config {
         Config {
-            owners: Some(vec![format!("test")]),
+            owners: vec![format!("test")],
             nickname: Some(format!("test")),
-            nick_password: None,
-            alt_nicks: None,
             username: Some(format!("test")),
             realname: Some(format!("test")),
             password: Some(String::new()),
             umodes: Some(format!("+BR")),
             server: Some(format!("irc.test.net")),
             port: Some(6667),
-            use_ssl: Some(false),
-            cert_path: None,
-            client_cert_path: None,
-            client_cert_pass: None,
             encoding: Some(format!("UTF-8")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
-            channel_keys: None,
-            user_info: None,
-            version: None,
-            source: None,
-            ping_time: None,
-            ping_timeout: None,
-            burst_window_length: None,
-            max_messages_in_burst: None,
-            should_ghost: None,
-            ghost_sequence: None,
-            options: Some(HashMap::new()),
-            use_mock_connection: None,
-            mock_initial_value: None,
 
             ..Default::default()
         }
@@ -577,7 +587,7 @@ mod test {
     #[test]
     fn is_owner() {
         let cfg = Config {
-            owners: Some(vec![format!("test"), format!("test2")]),
+            owners: vec![format!("test"), format!("test2")],
             ..Default::default()
         };
         assert!(cfg.is_owner("test"));
@@ -591,7 +601,7 @@ mod test {
             options: {
                 let mut map = HashMap::new();
                 map.insert(format!("testing"), format!("test"));
-                Some(map)
+                map
             },
             ..Default::default()
         };

src/client/mod.rs

@@ -488,6 +488,8 @@ struct ClientState {
     chanlists: RwLock<HashMap<String, Vec<User>>>,
     /// A thread-safe index to track the current alternative nickname being used.
     alt_nick_index: RwLock<usize>,
+    /// Default ghost sequence to send if one is required but none is configured.
+    default_ghost_sequence: Vec<String>,
 }
 
 impl ClientState {
@@ -497,6 +499,7 @@ impl ClientState {
             config: config,
             chanlists: RwLock::new(HashMap::new()),
             alt_nick_index: RwLock::new(0),
+            default_ghost_sequence: vec![String::from("GHOST")],
         }
     }
 
@@ -520,7 +523,7 @@ impl ClientState {
                 .config()
                 .nickname()
                 .expect("current_nickname should not be callable if nickname is not defined."),
-            i => alt_nicks[i - 1],
+            i => alt_nicks[i - 1].as_str(),
         }
     }
 
@@ -576,7 +579,7 @@ impl ClientState {
                 self.send_umodes()?;
 
                 let config_chans = self.config().channels();
-                for chan in &config_chans {
+                for chan in config_chans {
                     match self.config().channel_key(chan) {
                         Some(key) => self.send_join_with_keys::<&str, &str>(chan, key)?,
                         None => self.send_join(chan)?,
@@ -585,7 +588,7 @@ impl ClientState {
                 let joined_chans = self.chanlists.read();
                 for chan in joined_chans
                     .keys()
-                    .filter(|x| !config_chans.contains(&x.as_str()))
+                    .filter(|x| config_chans.iter().find(|c| c == x).is_none())
                 {
                     self.send_join(chan)?
                 }
@@ -614,10 +617,15 @@ impl ClientState {
             let mut index = self.alt_nick_index.write();
 
             if self.config().should_ghost() && *index != 0 {
-                for seq in &self.config().ghost_sequence() {
+                let seq = match self.config().ghost_sequence() {
+                    Some(seq) => seq,
+                    None => &*self.default_ghost_sequence,
+                };
+
+                for s in seq {
                     self.send(NICKSERV(format!(
                         "{} {} {}",
-                        seq,
+                        s,
                         self.config().nickname()?,
                         self.config().nick_password()
                     )))?;
@@ -1090,13 +1098,13 @@ mod test {
 
     pub fn test_config() -> Config {
         Config {
-            owners: Some(vec![format!("test")]),
+            owners: vec![format!("test")],
             nickname: Some(format!("test")),
-            alt_nicks: Some(vec![format!("test2")]),
+            alt_nicks: vec![format!("test2")],
             server: Some(format!("irc.test.net")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
             user_info: Some(format!("Testing.")),
-            use_mock_connection: Some(true),
+            use_mock_connection: true,
             ..Default::default()
         }
     }
@@ -1157,7 +1165,7 @@ mod test {
         let mut client = Client::from_config(Config {
             mock_initial_value: Some(value.to_owned()),
             nick_password: Some(format!("password")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
             ..test_config()
         })
         .await?;
@@ -1176,11 +1184,11 @@ mod test {
         let mut client = Client::from_config(Config {
             mock_initial_value: Some(value.to_owned()),
             nickname: Some(format!("test")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
             channel_keys: {
                 let mut map = HashMap::new();
                 map.insert(format!("#test2"), format!("password"));
-                Some(map)
+                map
             },
             ..test_config()
         })
@@ -1200,10 +1208,10 @@ mod test {
         let mut client = Client::from_config(Config {
             mock_initial_value: Some(value.to_owned()),
             nickname: Some(format!("test")),
-            alt_nicks: Some(vec![format!("test2")]),
+            alt_nicks: vec![format!("test2")],
             nick_password: Some(format!("password")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
-            should_ghost: Some(true),
+            should_ghost: true,
             ..test_config()
         })
         .await?;
@@ -1223,10 +1231,10 @@ mod test {
         let mut client = Client::from_config(Config {
             mock_initial_value: Some(value.to_owned()),
             nickname: Some(format!("test")),
-            alt_nicks: Some(vec![format!("test2")]),
+            alt_nicks: vec![format!("test2")],
             nick_password: Some(format!("password")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
-            should_ghost: Some(true),
+            should_ghost: true,
             ghost_sequence: Some(vec![format!("RECOVER"), format!("RELEASE")]),
             ..test_config()
         })
@@ -1248,7 +1256,7 @@ mod test {
             mock_initial_value: Some(value.to_owned()),
             nickname: Some(format!("test")),
             umodes: Some(format!("+B")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
             ..test_config()
         })
         .await?;
@@ -1496,7 +1504,7 @@ mod test {
         let mut client = Client::from_config(Config {
             mock_initial_value: Some(value.to_owned()),
             nickname: Some(format!("test")),
-            channels: Some(vec![format!("#test"), format!("#test2")]),
+            channels: vec![format!("#test"), format!("#test2")],
             ..test_config()
         })
         .await?;