auto merge of #11129 : SimonSapin/rust/foo-vs-foo_opt, r=alexcrichton

[On 2013-12-06, I wrote to the rust-dev mailing list](https://mail.mozilla.org/pipermail/rust-dev/2013-December/007263.html):

> Subject: Let’s avoid having both foo() and foo_opt()
>
> We have some functions and methods such as [std::str::from_utf8](http://static.rust-lang.org/doc/master/std/str/fn.from_utf8.html) that may succeed and give a result, or fail when the input is invalid.
>
> 1. Sometimes we assume the input is valid and don’t want to deal with the error case. Task failure works nicely.
>
> 2. Sometimes we do want to do something different on invalid input, so returning an `Option<T>` works best.
>
> And so we end up with both `from_utf8` and `from_utf8`. This particular case is worse because we also have `from_utf8_owned` and `from_utf8_owned_opt`, to cover everything.
>
> Multiplying names like this is just not good design. I’d like to reduce this pattern.
>
> Getting behavior 1. when you have 2. is easy: just call `.unwrap()` on the Option. I think we should rename every `foo_opt()` function or method to just `foo`, remove the old `foo()` behavior, and tell people (through documentation) to use `foo().unwrap()` if they want it back?
>
> The downsides are that unwrap is more verbose and gives less helpful error messages on task failure. But I think it’s worth it.


The email discussion has gone around long enough. Let’s discuss a concrete proposal. For the following functions or methods, I removed `foo` (that caused task failure) and renamed `foo_opt` (that returns `Option`) to just `foo`.

Vector methods:

* `get_opt` (rename only, `get` did not exist as it would have been just `[]`)
* `head_opt`
* `last_opt`
* `pop_opt`
* `shift_opt`
* `remove_opt`

`std::path::BytesContainer` method:

* `container_as_str_opt`

`std::str` functions:

* `from_utf8_opt`
* `from_utf8_owned_opt` (also remove the now unused `not_utf8` condition)

Is there something else that should recieve the same treatement?

I did not rename `recv_opt` on channels based on @brson’s [feedback](https://mail.mozilla.org/pipermail/rust-dev/2013-December/007270.html).

Feel free to pick only some of these commits.

3 files changed, 25 insertions, 54 deletions

diff --git a/src/libstd/path/mod.rs b/src/libstd/path/mod.rs
index 56df14ba763..6464d6021ee 100644
--- a/src/libstd/path/mod.rs
+++ b/src/libstd/path/mod.rs
@@ -189,7 +189,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
     /// If the path is not representable in utf-8, this returns None.
     #[inline]
     fn as_str<'a>(&'a self) -> Option<&'a str> {
-        str::from_utf8_opt(self.as_vec())
+        str::from_utf8(self.as_vec())
     }
 
     /// Returns the path as a byte vector
@@ -220,7 +220,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
     /// See `dirname` for details.
     #[inline]
     fn dirname_str<'a>(&'a self) -> Option<&'a str> {
-        str::from_utf8_opt(self.dirname())
+        str::from_utf8(self.dirname())
     }
     /// Returns the file component of `self`, as a byte vector.
     /// If `self` represents the root of the file hierarchy, returns None.
@@ -230,7 +230,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
     /// See `filename` for details.
     #[inline]
     fn filename_str<'a>(&'a self) -> Option<&'a str> {
-        self.filename().and_then(str::from_utf8_opt)
+        self.filename().and_then(str::from_utf8)
     }
     /// Returns the stem of the filename of `self`, as a byte vector.
     /// The stem is the portion of the filename just before the last '.'.
@@ -252,7 +252,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
     /// See `filestem` for details.
     #[inline]
     fn filestem_str<'a>(&'a self) -> Option<&'a str> {
-        self.filestem().and_then(str::from_utf8_opt)
+        self.filestem().and_then(str::from_utf8)
     }
     /// Returns the extension of the filename of `self`, as an optional byte vector.
     /// The extension is the portion of the filename just after the last '.'.
@@ -275,7 +275,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
     /// See `extension` for details.
     #[inline]
     fn extension_str<'a>(&'a self) -> Option<&'a str> {
-        self.extension().and_then(str::from_utf8_opt)
+        self.extension().and_then(str::from_utf8)
     }
 
     /// Replaces the filename portion of the path with the given byte vector or string.
@@ -426,7 +426,7 @@ pub trait GenericPath: Clone + GenericPathUnsafe {
         let t: Option<T> = None;
         if BytesContainer::is_str(t) {
             for p in paths.iter() {
-                self.push(p.container_as_str())
+                self.push(p.container_as_str().unwrap())
             }
         } else {
             for p in paths.iter() {
@@ -499,19 +499,10 @@ pub trait BytesContainer {
     fn container_into_owned_bytes(self) -> ~[u8] {
         self.container_as_bytes().to_owned()
     }
-    /// Returns the receiver interpreted as a utf-8 string
-    ///
-    /// # Failure
-    ///
-    /// Raises `str::null_byte` if not utf-8
-    #[inline]
-    fn container_as_str<'a>(&'a self) -> &'a str {
-        str::from_utf8(self.container_as_bytes())
-    }
     /// Returns the receiver interpreted as a utf-8 string, if possible
     #[inline]
-    fn container_as_str_opt<'a>(&'a self) -> Option<&'a str> {
-        str::from_utf8_opt(self.container_as_bytes())
+    fn container_as_str<'a>(&'a self) -> Option<&'a str> {
+        str::from_utf8(self.container_as_bytes())
     }
     /// Returns whether .container_as_str() is guaranteed to not fail
     // FIXME (#8888): Remove unused arg once ::<for T> works
@@ -589,11 +580,7 @@ impl<'a> BytesContainer for &'a str {
         self.as_bytes()
     }
     #[inline]
-    fn container_as_str<'a>(&'a self) -> &'a str {
-        *self
-    }
-    #[inline]
-    fn container_as_str_opt<'a>(&'a self) -> Option<&'a str> {
+    fn container_as_str<'a>(&'a self) -> Option<&'a str> {
         Some(*self)
     }
     #[inline]
@@ -610,11 +597,7 @@ impl BytesContainer for ~str {
         self.into_bytes()
     }
     #[inline]
-    fn container_as_str<'a>(&'a self) -> &'a str {
-        self.as_slice()
-    }
-    #[inline]
-    fn container_as_str_opt<'a>(&'a self) -> Option<&'a str> {
+    fn container_as_str<'a>(&'a self) -> Option<&'a str> {
         Some(self.as_slice())
     }
     #[inline]
@@ -627,11 +610,7 @@ impl BytesContainer for @str {
         self.as_bytes()
     }
     #[inline]
-    fn container_as_str<'a>(&'a self) -> &'a str {
-        self.as_slice()
-    }
-    #[inline]
-    fn container_as_str_opt<'a>(&'a self) -> Option<&'a str> {
+    fn container_as_str<'a>(&'a self) -> Option<&'a str> {
         Some(self.as_slice())
     }
     #[inline]
diff --git a/src/libstd/path/posix.rs b/src/libstd/path/posix.rs
index b228dc7f1ff..e95bd2d8ca2 100644
--- a/src/libstd/path/posix.rs
+++ b/src/libstd/path/posix.rs
@@ -402,13 +402,13 @@ impl Path {
     /// Returns an iterator that yields each component of the path as Option<&str>.
     /// See components() for details.
     pub fn str_components<'a>(&'a self) -> StrComponents<'a> {
-        self.components().map(str::from_utf8_opt)
+        self.components().map(str::from_utf8)
     }
 
     /// Returns an iterator that yields each component of the path in reverse as Option<&str>.
     /// See components() for details.
     pub fn rev_str_components<'a>(&'a self) -> RevStrComponents<'a> {
-        self.rev_components().map(str::from_utf8_opt)
+        self.rev_components().map(str::from_utf8)
     }
 }
 
@@ -426,7 +426,7 @@ fn normalize_helper<'a>(v: &'a [u8], is_abs: bool) -> Option<~[&'a [u8]]> {
         else if comp == bytes!("..") {
             if is_abs && comps.is_empty() { changed = true }
             else if comps.len() == n_up { comps.push(dot_dot_static); n_up += 1 }
-            else { comps.pop(); changed = true }
+            else { comps.pop().unwrap(); changed = true }
         } else { comps.push(comp) }
     }
     if changed {
@@ -691,7 +691,7 @@ mod tests {
             (s: $path:expr, $op:ident, $exp:expr, opt) => (
                 {
                     let path = Path::new($path);
-                    let left = path.$op().map(|x| str::from_utf8(x));
+                    let left = path.$op().map(|x| str::from_utf8(x).unwrap());
                     assert_eq!(left, $exp);
                 }
             );
diff --git a/src/libstd/path/windows.rs b/src/libstd/path/windows.rs
index 0e5b7894e74..666b8977cc9 100644
--- a/src/libstd/path/windows.rs
+++ b/src/libstd/path/windows.rs
@@ -129,11 +129,7 @@ impl BytesContainer for Path {
         self.into_vec()
     }
     #[inline]
-    fn container_as_str<'a>(&'a self) -> &'a str {
-        self.as_str().unwrap()
-    }
-    #[inline]
-    fn container_as_str_opt<'a>(&'a self) -> Option<&'a str> {
+    fn container_as_str<'a>(&'a self) -> Option<&'a str> {
         self.as_str()
     }
     #[inline]
@@ -146,11 +142,7 @@ impl<'a> BytesContainer for &'a Path {
         self.as_vec()
     }
     #[inline]
-    fn container_as_str<'a>(&'a self) -> &'a str {
-        self.as_str().unwrap()
-    }
-    #[inline]
-    fn container_as_str_opt<'a>(&'a self) -> Option<&'a str> {
+    fn container_as_str<'a>(&'a self) -> Option<&'a str> {
         self.as_str()
     }
     #[inline]
@@ -162,10 +154,10 @@ impl GenericPathUnsafe for Path {
     ///
     /// # Failure
     ///
-    /// Raises the `str::not_utf8` condition if not valid UTF-8.
+    /// Fails if not valid UTF-8.
     #[inline]
     unsafe fn new_unchecked<T: BytesContainer>(path: T) -> Path {
-        let (prefix, path) = Path::normalize_(path.container_as_str());
+        let (prefix, path) = Path::normalize_(path.container_as_str().unwrap());
         assert!(!path.is_empty());
         let mut ret = Path{ repr: path, prefix: prefix, sepidx: None };
         ret.update_sepidx();
@@ -176,9 +168,9 @@ impl GenericPathUnsafe for Path {
     ///
     /// # Failure
     ///
-    /// Raises the `str::not_utf8` condition if not valid UTF-8.
+    /// Fails if not valid UTF-8.
     unsafe fn set_filename_unchecked<T: BytesContainer>(&mut self, filename: T) {
-        let filename = filename.container_as_str();
+        let filename = filename.container_as_str().unwrap();
         match self.sepidx_or_prefix_len() {
             None if ".." == self.repr => {
                 let mut s = str::with_capacity(3 + filename.len());
@@ -224,7 +216,7 @@ impl GenericPathUnsafe for Path {
     /// the new path is relative to. Otherwise, the new path will be treated
     /// as if it were absolute and will replace the receiver outright.
     unsafe fn push_unchecked<T: BytesContainer>(&mut self, path: T) {
-        let path = path.container_as_str();
+        let path = path.container_as_str().unwrap();
         fn is_vol_abs(path: &str, prefix: Option<PathPrefix>) -> bool {
             // assume prefix is Some(DiskPrefix)
             let rest = path.slice_from(prefix_len(prefix));
@@ -311,7 +303,7 @@ impl GenericPathUnsafe for Path {
 impl GenericPath for Path {
     #[inline]
     fn new_opt<T: BytesContainer>(path: T) -> Option<Path> {
-        let s = path.container_as_str_opt();
+        let s = path.container_as_str();
         match s {
             None => None,
             Some(s) => {
@@ -599,7 +591,7 @@ impl Path {
     /// # Failure
     ///
     /// Raises the `null_byte` condition if the vector contains a NUL.
-    /// Raises the `str::not_utf8` condition if invalid UTF-8.
+    /// Fails if invalid UTF-8.
     #[inline]
     pub fn new<T: BytesContainer>(path: T) -> Path {
         GenericPath::new(path)
@@ -1025,7 +1017,7 @@ fn normalize_helper<'a>(s: &'a str, prefix: Option<PathPrefix>) -> (bool,Option<
             };
             if (is_abs || has_abs_prefix) && comps.is_empty() { changed = true }
             else if comps.len() == n_up { comps.push(".."); n_up += 1 }
-            else { comps.pop(); changed = true }
+            else { comps.pop().unwrap(); changed = true }
         } else { comps.push(comp) }
     }
     if !changed && !prefix_is_verbatim(prefix) {