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.

Author: bors

src/compiletest/header.rs

@@ -159,10 +159,10 @@ fn parse_exec_env(line: &str) -> Option<(~str, ~str)> {
         let mut strs: ~[~str] = nv.splitn('=', 1).map(|s| s.to_owned()).collect();
 
         match strs.len() {
-          1u => (strs.pop(), ~""),
+          1u => (strs.pop().unwrap(), ~""),
           2u => {
-              let end = strs.pop();
-              (strs.pop(), end)
+              let end = strs.pop().unwrap();
+              (strs.pop().unwrap(), end)
           }
           n => fail!("Expected 1 or 2 strings, not {}", n)
         }

src/compiletest/procsrv.rs

@@ -66,8 +66,8 @@ pub fn run(lib_path: &str,
 
             Some(Result {
                 status: status,
-                out: str::from_utf8_owned(output),
-                err: str::from_utf8_owned(error)
+                out: str::from_utf8_owned(output).unwrap(),
+                err: str::from_utf8_owned(error).unwrap()
             })
         },
         None => None

src/compiletest/runtest.rs

@@ -154,7 +154,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
         match props.pp_exact { Some(_) => 1, None => 2 };
 
     let src = File::open(testfile).read_to_end();
-    let src = str::from_utf8_owned(src);
+    let src = str::from_utf8_owned(src).unwrap();
     let mut srcs = ~[src];
 
     let mut round = 0;
@@ -176,7 +176,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
         Some(ref file) => {
             let filepath = testfile.dir_path().join(file);
             let s = File::open(&filepath).read_to_end();
-            str::from_utf8_owned(s)
+            str::from_utf8_owned(s).unwrap()
           }
           None => { srcs[srcs.len() - 2u].clone() }
         };
@@ -308,7 +308,7 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
 
             let adb_arg = format!("export LD_LIBRARY_PATH={}; gdbserver :5039 {}/{}",
                                   config.adb_test_dir.clone(), config.adb_test_dir.clone(),
-                                  str::from_utf8(exe_file.filename().unwrap()));
+                                  str::from_utf8(exe_file.filename().unwrap()).unwrap());
 
             let mut process = procsrv::run_background("", config.adb_path,
                                                       [~"shell",adb_arg.clone()],
@@ -788,7 +788,7 @@ fn make_run_args(config: &config, _props: &TestProps, testfile: &Path) ->
     let exe_file = make_exe_name(config, testfile);
     // FIXME (#9639): This needs to handle non-utf8 paths
     args.push(exe_file.as_str().unwrap().to_owned());
-    let prog = args.shift();
+    let prog = args.shift().unwrap();
     return ProcArgs {prog: prog, args: args};
 }
 
@@ -917,7 +917,7 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
 
     // get bare program string
     let mut tvec: ~[~str] = args.prog.split('/').map(|ts| ts.to_owned()).collect();
-    let prog_short = tvec.pop();
+    let prog_short = tvec.pop().unwrap();
 
     // copy to target
     let copy_result = procsrv::run("", config.adb_path,
@@ -1100,7 +1100,7 @@ fn disassemble_extract(config: &config, _props: &TestProps,
 
 fn count_extracted_lines(p: &Path) -> uint {
     let x = File::open(&p.with_extension("ll")).read_to_end();
-    let x = str::from_utf8_owned(x);
+    let x = str::from_utf8_owned(x).unwrap();
     x.lines().len()
 }
 

src/libextra/base64.rs

@@ -198,7 +198,7 @@ impl<'a> FromBase64 for &'a str {
      *     println!("base64 output: {}", hello_str);
      *     let res = hello_str.from_base64();
      *     if res.is_ok() {
-     *       let optBytes = str::from_utf8_owned_opt(res.unwrap());
+     *       let optBytes = str::from_utf8_owned(res.unwrap());
      *       if optBytes.is_some() {
      *         println!("decoded from base64: {}", optBytes.unwrap());
      *       }

src/libextra/dlist.rs

@@ -1049,11 +1049,11 @@ mod tests {
             match r % 6 {
                 0 => {
                     m.pop_back();
-                    if v.len() > 0 { v.pop(); }
+                    v.pop();
                 }
                 1 => {
                     m.pop_front();
-                    if v.len() > 0 { v.shift(); }
+                    v.shift();
                 }
                 2 | 4 =>  {
                     m.push_front(-i);

src/libextra/ebml.rs

@@ -30,7 +30,7 @@ impl<'doc> Doc<'doc> {
     }
 
     pub fn as_str_slice<'a>(&'a self) -> &'a str {
-        str::from_utf8(self.data.slice(self.start, self.end))
+        str::from_utf8(self.data.slice(self.start, self.end)).unwrap()
     }
 
     pub fn as_str(&self) -> ~str {
@@ -651,7 +651,7 @@ pub mod writer {
         }
 
         pub fn end_tag(&mut self) {
-            let last_size_pos = self.size_positions.pop();
+            let last_size_pos = self.size_positions.pop().unwrap();
             let cur_pos = self.writer.tell();
             self.writer.seek(last_size_pos as i64, io::SeekSet);
             let size = (cur_pos as uint - last_size_pos - 4);

src/libextra/glob.rs

@@ -122,7 +122,7 @@ impl Iterator<Path> for Paths {
                 return None;
             }
 
-            let (path,idx) = self.todo.pop();
+            let (path,idx) = self.todo.pop().unwrap();
             let ref pattern = self.dir_patterns[idx];
 
             if pattern.matches_with(match path.filename_str() {

src/libextra/hex.rs

@@ -96,7 +96,7 @@ impl<'a> FromHex for &'a str {
      *     println!("{}", hello_str);
      *     let bytes = hello_str.from_hex().unwrap();
      *     println!("{:?}", bytes);
-     *     let result_str = str::from_utf8_owned(bytes);
+     *     let result_str = str::from_utf8_owned(bytes).unwrap();
      *     println!("{}", result_str);
      * }
      * ```

src/libextra/json.rs

@@ -312,7 +312,7 @@ impl<'a> Encoder<'a> {
     /// Encode the specified struct into a json str
     pub fn str_encode<T:Encodable<Encoder<'a>>>(to_encode_object: &T) -> ~str  {
         let buff:~[u8] = Encoder::buffer_encode(to_encode_object);
-        str::from_utf8_owned(buff)
+        str::from_utf8_owned(buff).unwrap()
     }
 }
 
@@ -684,7 +684,7 @@ impl Json{
     pub fn to_pretty_str(&self) -> ~str {
         let mut s = MemWriter::new();
         self.to_pretty_writer(&mut s as &mut io::Writer);
-        str::from_utf8_owned(s.unwrap())
+        str::from_utf8_owned(s.unwrap()).unwrap()
     }
 }
 
@@ -1028,7 +1028,7 @@ impl<T : Iterator<char>> Parser<T> {
         while !self.eof() {
             self.parse_whitespace();
 
-            if self.ch != '\"' {
+            if self.ch != '"' {
                 return self.error(~"key must be a string");
             }
 
@@ -1067,7 +1067,7 @@ impl<T : Iterator<char>> Parser<T> {
 
 /// Decodes a json value from an `&mut io::Reader`
 pub fn from_reader(rdr: &mut io::Reader) -> Result<Json, Error> {
-    let s = str::from_utf8_owned(rdr.read_to_end());
+    let s = str::from_utf8_owned(rdr.read_to_end()).unwrap();
     let mut parser = Parser::new(s.chars());
     parser.parse()
 }
@@ -1117,7 +1117,7 @@ impl Decoder {
 impl serialize::Decoder for Decoder {
     fn read_nil(&mut self) -> () {
         debug!("read_nil");
-        match self.stack.pop() {
+        match self.stack.pop().unwrap() {
             Null => (),
             value => self.expected("null", &value)
         }
@@ -1137,15 +1137,15 @@ impl serialize::Decoder for Decoder {
 
     fn read_bool(&mut self) -> bool {
         debug!("read_bool");
-        match self.stack.pop() {
+        match self.stack.pop().unwrap() {
             Boolean(b) => b,
             value => self.expected("boolean", &value)
         }
     }
 
     fn read_f64(&mut self) -> f64 {
         debug!("read_f64");
-        match self.stack.pop() {
+        match self.stack.pop().unwrap() {
             Number(f) => f,
             value => self.expected("number", &value)
         }
@@ -1168,7 +1168,7 @@ impl serialize::Decoder for Decoder {
 
     fn read_str(&mut self) -> ~str {
         debug!("read_str");
-        match self.stack.pop() {
+        match self.stack.pop().unwrap() {
             String(s) => s,
             value => self.expected("string", &value)
         }
@@ -1184,7 +1184,7 @@ impl serialize::Decoder for Decoder {
                             f: |&mut Decoder, uint| -> T)
                             -> T {
         debug!("read_enum_variant(names={:?})", names);
-        let name = match self.stack.pop() {
+        let name = match self.stack.pop().unwrap() {
             String(s) => s,
             Object(mut o) => {
                 let n = match o.pop(&~"variant") {
@@ -1249,7 +1249,7 @@ impl serialize::Decoder for Decoder {
                       -> T {
         debug!("read_struct(name={}, len={})", name, len);
         let value = f(self);
-        self.stack.pop();
+        self.stack.pop().unwrap();
         value
     }
 
@@ -1259,7 +1259,7 @@ impl serialize::Decoder for Decoder {
                             f: |&mut Decoder| -> T)
                             -> T {
         debug!("read_struct_field(name={}, idx={})", name, idx);
-        match self.stack.pop() {
+        match self.stack.pop().unwrap() {
             Object(mut obj) => {
                 let value = match obj.pop(&name.to_owned()) {
                     None => self.missing_field(name, obj),
@@ -1302,15 +1302,15 @@ impl serialize::Decoder for Decoder {
     }
 
     fn read_option<T>(&mut self, f: |&mut Decoder, bool| -> T) -> T {
-        match self.stack.pop() {
+        match self.stack.pop().unwrap() {
             Null => f(self, false),
             value => { self.stack.push(value); f(self, true) }
         }
     }
 
     fn read_seq<T>(&mut self, f: |&mut Decoder, uint| -> T) -> T {
         debug!("read_seq()");
-        let len = match self.stack.pop() {
+        let len = match self.stack.pop().unwrap() {
             List(list) => {
                 let len = list.len();
                 for v in list.move_rev_iter() {
@@ -1330,7 +1330,7 @@ impl serialize::Decoder for Decoder {
 
     fn read_map<T>(&mut self, f: |&mut Decoder, uint| -> T) -> T {
         debug!("read_map()");
-        let len = match self.stack.pop() {
+        let len = match self.stack.pop().unwrap() {
             Object(obj) => {
                 let len = obj.len();
                 for (key, value) in obj.move_iter() {
@@ -1541,7 +1541,7 @@ impl to_str::ToStr for Json {
     fn to_str(&self) -> ~str {
         let mut s = MemWriter::new();
         self.to_writer(&mut s as &mut io::Writer);
-        str::from_utf8_owned(s.unwrap())
+        str::from_utf8_owned(s.unwrap()).unwrap()
     }
 }
 
@@ -1732,7 +1732,7 @@ mod tests {
 
         let mut m = MemWriter::new();
         f(&mut m as &mut io::Writer);
-        str::from_utf8_owned(m.unwrap())
+        str::from_utf8_owned(m.unwrap()).unwrap()
     }
 
     #[test]

src/libextra/num/bigint.rs

@@ -385,7 +385,7 @@ impl Integer for BigUint {
         }
 
         let mut shift = 0;
-        let mut n = *other.data.last();
+        let mut n = *other.data.last().unwrap();
         while n < (1 << BigDigit::bits - 2) {
             n <<= 1;
             shift += 1;
@@ -434,7 +434,7 @@ impl Integer for BigUint {
             }
 
             let an = a.data.slice(a.data.len() - n, a.data.len());
-            let bn = *b.data.last();
+            let bn = *b.data.last().unwrap();
             let mut d = ~[];
             let mut carry = 0;
             for elt in an.rev_iter() {
@@ -798,7 +798,7 @@ impl BigUint {
     /// Determines the fewest bits necessary to express the `BigUint`.
     pub fn bits(&self) -> uint {
         if self.is_zero() { return 0; }
-        let zeros = self.data.last().leading_zeros();
+        let zeros = self.data.last().unwrap().leading_zeros();
         return self.data.len()*BigDigit::bits - (zeros as uint);
     }
 }

src/libextra/priority_queue.rs

@@ -59,7 +59,7 @@ impl<T:Ord> PriorityQueue<T> {
 
     /// Pop the greatest item from the queue - fails if empty
     pub fn pop(&mut self) -> T {
-        let mut item = self.data.pop();
+        let mut item = self.data.pop().unwrap();
         if !self.is_empty() {
             swap(&mut item, &mut self.data[0]);
             self.siftdown(0);
@@ -234,8 +234,8 @@ mod tests {
         sorted.sort();
         let mut heap = PriorityQueue::from_vec(data);
         while !heap.is_empty() {
-            assert_eq!(heap.top(), sorted.last());
-            assert_eq!(heap.pop(), sorted.pop());
+            assert_eq!(heap.top(), sorted.last().unwrap());
+            assert_eq!(heap.pop(), sorted.pop().unwrap());
         }
     }
 

src/libextra/stats.rs

@@ -1001,7 +1001,7 @@ mod tests {
             use std::io::MemWriter;
             let mut m = MemWriter::new();
             write_boxplot(&mut m as &mut io::Writer, s, 30);
-            let out = str::from_utf8_owned(m.unwrap());
+            let out = str::from_utf8_owned(m.unwrap()).unwrap();
             assert_eq!(out, expected);
         }