std: Tweak the std::env OsString/String interface

This commit tweaks the interface of the `std::env` module to make it more
ergonomic for common usage:

* `env::var` was renamed to `env::var_os`
* `env::var_string` was renamed to `env::var`
* `env::args` was renamed to `env::args_os`
* `env::args` was re-added as a panicking iterator over string values
* `env::vars` was renamed to `env::vars_os`
* `env::vars` was re-added as a panicking iterator over string values.

This should make common usage (e.g. unicode values everywhere) more ergonomic
as well as "the default". This is also a breaking change due to the differences
of what's yielded from each of these functions, but migration should be fairly
easy as the defaults operate over `String` which is a common type to use.

[breaking-change]

5 files changed, 136 insertions, 51 deletions

diff --git a/src/libstd/dynamic_lib.rs b/src/libstd/dynamic_lib.rs
index bee9a0d0033..c5dd66630b4 100644
--- a/src/libstd/dynamic_lib.rs
+++ b/src/libstd/dynamic_lib.rs
@@ -101,7 +101,7 @@ impl DynamicLibrary {
     /// Returns the current search path for dynamic libraries being used by this
     /// process
     pub fn search_path() -> Vec<Path> {
-        match env::var(DynamicLibrary::envvar()) {
+        match env::var_os(DynamicLibrary::envvar()) {
             Some(var) => env::split_paths(&var).collect(),
             None => Vec::new(),
         }
diff --git a/src/libstd/env.rs b/src/libstd/env.rs
index e73797bc66c..ea18838211f 100644
--- a/src/libstd/env.rs
+++ b/src/libstd/env.rs
@@ -71,17 +71,29 @@ static ENV_LOCK: StaticMutex = MUTEX_INIT;
 
 /// An iterator over a snapshot of the environment variables of this process.
 ///
-/// This iterator is created through `std::env::vars()` and yields `(OsString,
-/// OsString)` pairs.
-pub struct Vars { inner: os_imp::Env }
+/// This iterator is created through `std::env::vars()` and yields `(String,
+/// String)` pairs.
+pub struct Vars { inner: VarsOs }
 
-/// Returns an iterator of (variable, value) pairs, for all the environment
-/// variables of the current process.
+/// An iterator over a snapshot of the environment variables of this process.
+///
+/// This iterator is created through `std::env::vars_os()` and yields
+/// `(OsString, OsString)` pairs.
+pub struct VarsOs { inner: os_imp::Env }
+
+/// Returns an iterator of (variable, value) pairs of strings, for all the
+/// environment variables of the current process.
 ///
 /// The returned iterator contains a snapshot of the process's environment
 /// variables at the time of this invocation, modifications to environment
 /// variables afterwards will not be reflected in the returned iterator.
 ///
+/// # Panics
+///
+/// While iterating, the returned iterator will panic if any key or value in the
+/// environment is not valid unicode. If this is not desired, consider using the
+/// `env::vars_os` function.
+///
 /// # Example
 ///
 /// ```rust
@@ -90,37 +102,50 @@ pub struct Vars { inner: os_imp::Env }
 /// // We will iterate through the references to the element returned by
 /// // env::vars();
 /// for (key, value) in env::vars() {
-///     println!("{:?}: {:?}", key, value);
+///     println!("{}: {}", key, value);
 /// }
 /// ```
 pub fn vars() -> Vars {
-    let _g = ENV_LOCK.lock();
-    Vars { inner: os_imp::env() }
-}
-
-impl Iterator for Vars {
-    type Item = (OsString, OsString);
-    fn next(&mut self) -> Option<(OsString, OsString)> { self.inner.next() }
-    fn size_hint(&self) -> (usize, Option<usize>) { self.inner.size_hint() }
+    Vars { inner: vars_os() }
 }
 
-/// Fetches the environment variable `key` from the current process, returning
-/// None if the variable isn't set.
+/// Returns an iterator of (variable, value) pairs of OS strings, for all the
+/// environment variables of the current process.
+///
+/// The returned iterator contains a snapshot of the process's environment
+/// variables at the time of this invocation, modifications to environment
+/// variables afterwards will not be reflected in the returned iterator.
 ///
 /// # Example
 ///
 /// ```rust
 /// use std::env;
 ///
-/// let key = "HOME";
-/// match env::var(key) {
-///     Some(val) => println!("{}: {:?}", key, val),
-///     None => println!("{} is not defined in the environment.", key)
+/// // We will iterate through the references to the element returned by
+/// // env::vars_os();
+/// for (key, value) in env::vars_os() {
+///     println!("{:?}: {:?}", key, value);
 /// }
 /// ```
-pub fn var<K: ?Sized>(key: &K) -> Option<OsString> where K: AsOsStr {
+pub fn vars_os() -> VarsOs {
     let _g = ENV_LOCK.lock();
-    os_imp::getenv(key.as_os_str())
+    VarsOs { inner: os_imp::env() }
+}
+
+impl Iterator for Vars {
+    type Item = (String, String);
+    fn next(&mut self) -> Option<(String, String)> {
+        self.inner.next().map(|(a, b)| {
+            (a.into_string().unwrap(), b.into_string().unwrap())
+        })
+    }
+    fn size_hint(&self) -> (usize, Option<usize>) { self.inner.size_hint() }
+}
+
+impl Iterator for VarsOs {
+    type Item = (OsString, OsString);
+    fn next(&mut self) -> Option<(OsString, OsString)> { self.inner.next() }
+    fn size_hint(&self) -> (usize, Option<usize>) { self.inner.size_hint() }
 }
 
 /// Fetches the environment variable `key` from the current process.
@@ -135,18 +160,37 @@ pub fn var<K: ?Sized>(key: &K) -> Option<OsString> where K: AsOsStr {
 /// use std::env;
 ///
 /// let key = "HOME";
-/// match env::var_string(key) {
+/// match env::var(key) {
 ///     Ok(val) => println!("{}: {:?}", key, val),
 ///     Err(e) => println!("couldn't interpret {}: {}", key, e),
 /// }
 /// ```
-pub fn var_string<K: ?Sized>(key: &K) -> Result<String, VarError> where K: AsOsStr {
-    match var(key) {
+pub fn var<K: ?Sized>(key: &K) -> Result<String, VarError> where K: AsOsStr {
+    match var_os(key) {
         Some(s) => s.into_string().map_err(VarError::NotUnicode),
         None => Err(VarError::NotPresent)
     }
 }
 
+/// Fetches the environment variable `key` from the current process, returning
+/// None if the variable isn't set.
+///
+/// # Example
+///
+/// ```rust
+/// use std::env;
+///
+/// let key = "HOME";
+/// match env::var_os(key) {
+///     Some(val) => println!("{}: {:?}", key, val),
+///     None => println!("{} is not defined in the environment.", key)
+/// }
+/// ```
+pub fn var_os<K: ?Sized>(key: &K) -> Option<OsString> where K: AsOsStr {
+    let _g = ENV_LOCK.lock();
+    os_imp::getenv(key.as_os_str())
+}
+
 /// Possible errors from the `env::var` method.
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub enum VarError {
@@ -190,7 +234,7 @@ impl Error for VarError {
 ///
 /// let key = "KEY";
 /// env::set_var(key, "VALUE");
-/// assert_eq!(env::var_string(key), Ok("VALUE".to_string()));
+/// assert_eq!(env::var(key), Ok("VALUE".to_string()));
 /// ```
 pub fn set_var<K: ?Sized, V: ?Sized>(k: &K, v: &V)
     where K: AsOsStr, V: AsOsStr
@@ -222,7 +266,7 @@ pub struct SplitPaths<'a> { inner: os_imp::SplitPaths<'a> }
 /// use std::env;
 ///
 /// let key = "PATH";
-/// match env::var(key) {
+/// match env::var_os(key) {
 ///     Some(paths) => {
 ///         for path in env::split_paths(&paths) {
 ///             println!("'{}'", path.display());
@@ -262,7 +306,7 @@ pub struct JoinPathsError {
 /// ```rust
 /// use std::env;
 ///
-/// if let Some(path) = env::var("PATH") {
+/// if let Some(path) = env::var_os("PATH") {
 ///     let mut paths = env::split_paths(&path).collect::<Vec<_>>();
 ///     paths.push(Path::new("/home/xyz/bin"));
 ///     let new_path = env::join_paths(paths.iter()).unwrap();
@@ -376,11 +420,17 @@ pub fn get_exit_status() -> i32 {
     EXIT_STATUS.load(Ordering::SeqCst) as i32
 }
 
-/// An iterator over the arguments of a process, yielding an `OsString` value
+/// An iterator over the arguments of a process, yielding an `String` value
 /// for each argument.
 ///
 /// This structure is created through the `std::env::args` method.
-pub struct Args { inner: os_imp::Args }
+pub struct Args { inner: ArgsOs }
+
+/// An iterator over the arguments of a process, yielding an `OsString` value
+/// for each argument.
+///
+/// This structure is created through the `std::env::args_os` method.
+pub struct ArgsOs { inner: os_imp::Args }
 
 /// Returns the arguments which this program was started with (normally passed
 /// via the command line).
@@ -389,6 +439,12 @@ pub struct Args { inner: os_imp::Args }
 /// set to arbitrary text, and it may not even exist, so this property should
 /// not be relied upon for security purposes.
 ///
+/// # Panics
+///
+/// The returned iterator will panic during iteration if any argument to the
+/// process is not valid unicode. If this is not desired it is recommended to
+/// use the `args_os` function instead.
+///
 /// # Example
 ///
 /// ```rust
@@ -396,14 +452,43 @@ pub struct Args { inner: os_imp::Args }
 ///
 /// // Prints each argument on a separate line
 /// for argument in env::args() {
-///     println!("{:?}", argument);
+///     println!("{}", argument);
 /// }
 /// ```
 pub fn args() -> Args {
-    Args { inner: os_imp::args() }
+    Args { inner: args_os() }
+}
+
+/// Returns the arguments which this program was started with (normally passed
+/// via the command line).
+///
+/// The first element is traditionally the path to the executable, but it can be
+/// set to arbitrary text, and it may not even exist, so this property should
+/// not be relied upon for security purposes.
+///
+/// # Example
+///
+/// ```rust
+/// use std::env;
+///
+/// // Prints each argument on a separate line
+/// for argument in env::args_os() {
+///     println!("{:?}", argument);
+/// }
+/// ```
+pub fn args_os() -> ArgsOs {
+    ArgsOs { inner: os_imp::args() }
 }
 
 impl Iterator for Args {
+    type Item = String;
+    fn next(&mut self) -> Option<String> {
+        self.inner.next().map(|s| s.into_string().unwrap())
+    }
+    fn size_hint(&self) -> (usize, Option<usize>) { self.inner.size_hint() }
+}
+
+impl Iterator for ArgsOs {
     type Item = OsString;
     fn next(&mut self) -> Option<OsString> { self.inner.next() }
     fn size_hint(&self) -> (usize, Option<usize>) { self.inner.size_hint() }
@@ -706,7 +791,7 @@ mod tests {
         let n = format!("TEST{}", rng.gen_ascii_chars().take(10)
                                      .collect::<String>());
         let n = OsString::from_string(n);
-        assert!(var(&n).is_none());
+        assert!(var_os(&n).is_none());
         n
     }
 
@@ -718,7 +803,7 @@ mod tests {
     fn test_set_var() {
         let n = make_rand_name();
         set_var(&n, "VALUE");
-        eq(var(&n), Some("VALUE"));
+        eq(var_os(&n), Some("VALUE"));
     }
 
     #[test]
@@ -726,7 +811,7 @@ mod tests {
         let n = make_rand_name();
         set_var(&n, "VALUE");
         remove_var(&n);
-        eq(var(&n), None);
+        eq(var_os(&n), None);
     }
 
     #[test]
@@ -734,9 +819,9 @@ mod tests {
         let n = make_rand_name();
         set_var(&n, "1");
         set_var(&n, "2");
-        eq(var(&n), Some("2"));
+        eq(var_os(&n), Some("2"));
         set_var(&n, "");
-        eq(var(&n), Some(""));
+        eq(var_os(&n), Some(""));
     }
 
     #[test]
@@ -749,7 +834,7 @@ mod tests {
         }
         let n = make_rand_name();
         set_var(&n, s.as_slice());
-        eq(var(&n), Some(s.as_slice()));
+        eq(var_os(&n), Some(s.as_slice()));
     }
 
     #[test]
@@ -767,22 +852,22 @@ mod tests {
         let n = make_rand_name();
         let s = repeat("x").take(10000).collect::<String>();
         set_var(&n, &s);
-        eq(var(&n), Some(s.as_slice()));
+        eq(var_os(&n), Some(s.as_slice()));
         remove_var(&n);
-        eq(var(&n), None);
+        eq(var_os(&n), None);
     }
 
     #[test]
     fn test_env_set_var() {
         let n = make_rand_name();
 
-        let mut e = vars();
+        let mut e = vars_os();
         set_var(&n, "VALUE");
         assert!(!e.any(|(k, v)| {
             &*k == &*n && &*v == "VALUE"
         }));
 
-        assert!(vars().any(|(k, v)| {
+        assert!(vars_os().any(|(k, v)| {
             &*k == &*n && &*v == "VALUE"
         }));
     }
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index 526b5edd4cb..ca7183a97d4 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -125,7 +125,7 @@ pub fn getcwd() -> IoResult<Path> {
 #[deprecated(since = "1.0.0", reason = "use env::vars instead")]
 #[unstable(feature = "os")]
 pub fn env() -> Vec<(String,String)> {
-    env::vars().map(|(k, v)| {
+    env::vars_os().map(|(k, v)| {
         (k.to_string_lossy().into_owned(), v.to_string_lossy().into_owned())
     }).collect()
 }
@@ -135,7 +135,7 @@ pub fn env() -> Vec<(String,String)> {
 #[deprecated(since = "1.0.0", reason = "use env::vars instead")]
 #[unstable(feature = "os")]
 pub fn env_as_bytes() -> Vec<(Vec<u8>, Vec<u8>)> {
-    env::vars().map(|(k, v)| (byteify(k), byteify(v))).collect()
+    env::vars_os().map(|(k, v)| (byteify(k), byteify(v))).collect()
 }
 
 /// Fetches the environment variable `n` from the current process, returning
@@ -159,10 +159,10 @@ pub fn env_as_bytes() -> Vec<(Vec<u8>, Vec<u8>)> {
 ///     None => println!("{} is not defined in the environment.", key)
 /// }
 /// ```
-#[deprecated(since = "1.0.0", reason = "use env::var or env::var_string instead")]
+#[deprecated(since = "1.0.0", reason = "use env::var or env::var_os instead")]
 #[unstable(feature = "os")]
 pub fn getenv(n: &str) -> Option<String> {
-    env::var_string(n).ok()
+    env::var(n).ok()
 }
 
 /// Fetches the environment variable `n` byte vector from the current process,
@@ -174,7 +174,7 @@ pub fn getenv(n: &str) -> Option<String> {
 #[deprecated(since = "1.0.0", reason = "use env::var instead")]
 #[unstable(feature = "os")]
 pub fn getenv_as_bytes(n: &str) -> Option<Vec<u8>> {
-    env::var(n).map(byteify)
+    env::var_os(n).map(byteify)
 }
 
 #[cfg(unix)]
diff --git a/src/libstd/rt/backtrace.rs b/src/libstd/rt/backtrace.rs
index 2af5a486d0d..ced84d7551e 100644
--- a/src/libstd/rt/backtrace.rs
+++ b/src/libstd/rt/backtrace.rs
@@ -29,7 +29,7 @@ pub fn log_enabled() -> bool {
         _ => {}
     }
 
-    let val = match env::var("RUST_BACKTRACE") {
+    let val = match env::var_os("RUST_BACKTRACE") {
         Some(..) => 2,
         None => 1,
     };
diff --git a/src/libstd/rt/util.rs b/src/libstd/rt/util.rs
index 703dca4d29b..bb57d19ed26 100644
--- a/src/libstd/rt/util.rs
+++ b/src/libstd/rt/util.rs
@@ -52,7 +52,7 @@ pub fn min_stack() -> uint {
         0 => {}
         n => return n - 1,
     }
-    let amt = env::var_string("RUST_MIN_STACK").ok().and_then(|s| s.parse().ok());
+    let amt = env::var("RUST_MIN_STACK").ok().and_then(|s| s.parse().ok());
     let amt = amt.unwrap_or(2 * 1024 * 1024);
     // 0 is our sentinel value, so ensure that we'll never see 0 after
     // initialization has run
@@ -63,7 +63,7 @@ pub fn min_stack() -> uint {
 /// Get's the number of scheduler threads requested by the environment
 /// either `RUST_THREADS` or `num_cpus`.
 pub fn default_sched_threads() -> uint {
-    match env::var_string("RUST_THREADS") {
+    match env::var("RUST_THREADS") {
         Ok(nstr) => {
             let opt_n: Option<uint> = nstr.parse().ok();
             match opt_n {