Have tests use OsStrs not OsStrings

src/main.rs

@@ -30,7 +30,7 @@ fn main() {
     logger::configure(env::var_os(vars::EXA_DEBUG));
 
     let args: Vec<_> = env::args_os().skip(1).collect();
-    match Options::parse(&args, &LiveVars) {
+    match Options::parse(args.iter().map(|e| e.as_ref()), &LiveVars) {
         OptionsResult::Ok(options, mut input_paths) => {
 
             // List the current directory by default.

src/options/help.rs

@@ -130,32 +130,26 @@ impl fmt::Display for HelpString {
 #[cfg(test)]
 mod test {
     use crate::options::{Options, OptionsResult};
-    use std::ffi::OsString;
+    use std::ffi::OsStr;
-
-    fn os(input: &'static str) -> OsString {
-        let mut os = OsString::new();
-        os.push(input);
-        os
-    }
 
     #[test]
     fn help() {
-        let args = [ os("--help") ];
+        let args = vec![ OsStr::new("--help") ];
-        let opts = Options::parse(&args, &None);
+        let opts = Options::parse(args, &None);
         assert!(matches!(opts, OptionsResult::Help(_)));
     }
 
     #[test]
     fn help_with_file() {
-        let args = [ os("--help"), os("me") ];
+        let args = vec![ OsStr::new("--help"), OsStr::new("me") ];
-        let opts = Options::parse(&args, &None);
+        let opts = Options::parse(args, &None);
         assert!(matches!(opts, OptionsResult::Help(_)));
     }
 
     #[test]
     fn unhelpful() {
-        let args = [];
+        let args = vec![];
-        let opts = Options::parse(&args, &None);
+        let opts = Options::parse(args, &None);
         assert!(! matches!(opts, OptionsResult::Help(_)))  // no help when --help isn’t passed
     }
 }

src/options/mod.rs

@@ -69,7 +69,7 @@
 //! it’s clear what the user wants.
 
 
-use std::ffi::{OsStr, OsString};
+use std::ffi::OsStr;
 
 use crate::fs::dir_action::DirAction;
 use crate::fs::filter::{FileFilter, GitIgnore};
@@ -120,7 +120,7 @@ impl Options {
     /// for extra options.
     #[allow(unused_results)]
     pub fn parse<'args, I, V>(args: I, vars: &V) -> OptionsResult<'args>
-    where I: IntoIterator<Item = &'args OsString>,
+    where I: IntoIterator<Item = &'args OsStr>,
           V: Vars,
     {
         use crate::options::parser::{Matches, Strictness};
@@ -198,7 +198,7 @@ pub enum OptionsResult<'args> {
 #[cfg(test)]
 pub mod test {
     use crate::options::parser::{Arg, MatchedFlags};
-    use std::ffi::OsString;
+    use std::ffi::OsStr;
 
     #[derive(PartialEq, Debug)]
     pub enum Strictnesses {
@@ -219,27 +219,19 @@ pub mod test {
         use self::Strictnesses::*;
         use crate::options::parser::{Args, Strictness};
 
-        let bits = inputs.into_iter().map(|&o| os(o)).collect::<Vec<OsString>>();
+        let bits = inputs.into_iter().map(OsStr::new).collect::<Vec<_>>();
         let mut result = Vec::new();
 
         if strictnesses == Last || strictnesses == Both {
-            let results = Args(args).parse(bits.iter(), Strictness::UseLastArguments);
+            let results = Args(args).parse(bits.clone(), Strictness::UseLastArguments);
             result.push(get(&results.unwrap().flags));
         }
 
         if strictnesses == Complain || strictnesses == Both {
-            let results = Args(args).parse(bits.iter(), Strictness::ComplainAboutRedundantArguments);
+            let results = Args(args).parse(bits, Strictness::ComplainAboutRedundantArguments);
             result.push(get(&results.unwrap().flags));
         }
 
         result
     }
-
-    /// Creates an `OSStr` (used in tests)
-    #[cfg(test)]
-    fn os(input: &str) -> OsString {
-        let mut os = OsString::new();
-        os.push(input);
-        os
-    }
 }

src/options/parser.rs

@@ -144,7 +144,7 @@ impl Args {
     /// Iterates over the given list of command-line arguments and parses
     /// them into a list of matched flags and free strings.
     pub fn parse<'args, I>(&self, inputs: I, strictness: Strictness) -> Result<Matches<'args>, ParseError>
-    where I: IntoIterator<Item = &'args OsString>
+    where I: IntoIterator<Item = &'args OsStr>
     {
         use std::os::unix::ffi::OsStrExt;
 
@@ -507,24 +507,16 @@ fn split_on_equals(input: &OsStr) -> Option<(&OsStr, &OsStr)> {
 }
 
 
-/// Creates an `OSString` (used in tests)
-#[cfg(test)]
-fn os(input: &'static str) -> OsString {
-    let mut os = OsString::new();
-    os.push(input);
-    os
-}
-
-
 #[cfg(test)]
 mod split_test {
-    use super::{split_on_equals, os};
+    use super::split_on_equals;
+    use std::ffi::{OsStr, OsString};
 
     macro_rules! test_split {
         ($name:ident: $input:expr => None) => {
             #[test]
             fn $name() {
-                assert_eq!(split_on_equals(&os($input)),
+                assert_eq!(split_on_equals(&OsString::from($input)),
                            None);
             }
         };
@@ -532,8 +524,8 @@ mod split_test {
         ($name:ident: $input:expr => $before:expr, $after:expr) => {
             #[test]
             fn $name() {
-                assert_eq!(split_on_equals(&os($input)),
+                assert_eq!(split_on_equals(&OsString::from($input)),
-                           Some((&*os($before), &*os($after))));
+                           Some((OsStr::new($before), OsStr::new($after))));
             }
         };
     }
@@ -555,29 +547,21 @@ mod split_test {
 mod parse_test {
     use super::*;
 
-    pub fn os(input: &'static str) -> OsString {
-        let mut os = OsString::new();
-        os.push(input);
-        os
-    }
-
-
     macro_rules! test {
         ($name:ident: $inputs:expr => frees: $frees:expr, flags: $flags:expr) => {
             #[test]
             fn $name() {
 
-                // Annoyingly the input &strs need to be converted to OsStrings
+                let inputs: &[&'static str] = $inputs.as_ref();
-                let inputs: Vec<OsString> = $inputs.as_ref().into_iter().map(|&o| os(o)).collect();
+                let inputs = inputs.iter().map(OsStr::new);
 
-                // Same with the frees
+                let frees: &[&'static str] = $frees.as_ref();
-                let frees: Vec<OsString> = $frees.as_ref().into_iter().map(|&o| os(o)).collect();
+                let frees  = frees.iter().map(OsStr::new).collect();
-                let frees: Vec<&OsStr> = frees.iter().map(|os| os.as_os_str()).collect();
 
                 let flags = <[_]>::into_vec(Box::new($flags));
 
                 let strictness = Strictness::UseLastArguments;  // this isn’t even used
-                let got = Args(TEST_ARGS).parse(inputs.iter(), strictness);
+                let got = Args(TEST_ARGS).parse(inputs, strictness);
                 let flags = MatchedFlags { flags, strictness };
 
                 let expected = Ok(Matches { frees, flags });
@@ -590,12 +574,10 @@ mod parse_test {
             fn $name() {
                 use self::ParseError::*;
 
-                let strictness = Strictness::UseLastArguments;  // this isn’t even used
+                let inputs = $inputs.iter().map(OsStr::new);
-                let bits = $inputs.as_ref().into_iter()
-                                  .map(|&o| os(o))
-                                  .collect::<Vec<_>>();
 
-                let got = Args(TEST_ARGS).parse(bits.iter(), strictness);
+                let strictness = Strictness::UseLastArguments;  // this isn’t even used
+                let got = Args(TEST_ARGS).parse(inputs, strictness);
                 assert_eq!(got, Err($error));
             }
         };
@@ -663,8 +645,8 @@ mod parse_test {
 
 
     // Unknown args
-    test!(unknown_long:          ["--quiet"]      => error UnknownArgument      { attempt: os("quiet") });
+    test!(unknown_long:          ["--quiet"]      => error UnknownArgument      { attempt: OsString::from("quiet") });
-    test!(unknown_long_eq:       ["--quiet=shhh"] => error UnknownArgument      { attempt: os("quiet") });
+    test!(unknown_long_eq:       ["--quiet=shhh"] => error UnknownArgument      { attempt: OsString::from("quiet") });
     test!(unknown_short:         ["-q"]           => error UnknownShortArgument { attempt: b'q' });
     test!(unknown_short_2nd:     ["-lq"]          => error UnknownShortArgument { attempt: b'q' });
     test!(unknown_short_eq:      ["-q=shhh"]      => error UnknownShortArgument { attempt: b'q' });
@@ -704,7 +686,7 @@ mod matches_test {
 
     #[test]
     fn only_count() {
-        let everything = os("everything");
+        let everything = OsString::from("everything");
 
         let flags = MatchedFlags {
             flags: vec![ (Flag::Short(b'c'), Some(&*everything)) ],
@@ -716,8 +698,8 @@ mod matches_test {
 
     #[test]
     fn rightmost_count() {
-        let everything = os("everything");
+        let everything = OsString::from("everything");
-        let nothing    = os("nothing");
+        let nothing    = OsString::from("nothing");
 
         let flags = MatchedFlags {
             flags: vec![ (Flag::Short(b'c'), Some(&*everything)),

src/options/version.rs

@@ -39,25 +39,19 @@ impl fmt::Display for VersionString {
 #[cfg(test)]
 mod test {
     use crate::options::{Options, OptionsResult};
-    use std::ffi::OsString;
+    use std::ffi::OsStr;
-
-    fn os(input: &'static str) -> OsString {
-        let mut os = OsString::new();
-        os.push(input);
-        os
-    }
 
     #[test]
     fn version() {
-        let args = [ os("--version") ];
+        let args = vec![ OsStr::new("--version") ];
-        let opts = Options::parse(&args, &None);
+        let opts = Options::parse(args, &None);
         assert!(matches!(opts, OptionsResult::Version(_)));
     }
 
     #[test]
     fn version_with_file() {
-        let args = [ os("--version"), os("me") ];
+        let args = vec![ OsStr::new("--version"), OsStr::new("me") ];
-        let opts = Options::parse(&args, &None);
+        let opts = Options::parse(args, &None);
         assert!(matches!(opts, OptionsResult::Version(_)));
     }
 }