Use Result/Err(()) instead of Option/None to indicate a parse error.

Author: SimonSapin

color.rs

@@ -31,22 +31,22 @@ pub enum Color {
 }
 
 
-// Return None on invalid/unsupported value (not a color)
+/// Return `Err(())` on invalid or unsupported value (not a color).
 impl Color {
-    pub fn parse(component_value: &ComponentValue) -> Option<Color> {
+    pub fn parse(component_value: &ComponentValue) -> Result<Color, ()> {
         match *component_value {
             Hash(ref value) | IDHash(ref value) => parse_color_hash(value.as_slice()),
             Ident(ref value) => parse_color_keyword(value.as_slice()),
             Function(ref name, ref arguments)
                 => parse_color_function(name.as_slice(), arguments.as_slice()),
-            _ => None
+            _ => Err(())
         }
     }
 }
 
 
 #[inline]
-fn parse_color_keyword(value: &str) -> Option<Color> {
+fn parse_color_keyword(value: &str) -> Result<Color, ()> {
     let lower_value = value.to_ascii_lower();
     let (r, g, b) = match lower_value.as_slice() {
         "black" => (0., 0., 0.),
@@ -199,33 +199,33 @@ fn parse_color_keyword(value: &str) -> Option<Color> {
         "whitesmoke" => (245., 245., 245.),
         "yellowgreen" => (154., 205., 50.),
 
-        "transparent" => return Some(RGBA(RGBA { red: 0., green: 0., blue: 0., alpha: 0. })),
-        "currentcolor" => return Some(CurrentColor),
-        _ => return None,
+        "transparent" => return Ok(RGBA(RGBA { red: 0., green: 0., blue: 0., alpha: 0. })),
+        "currentcolor" => return Ok(CurrentColor),
+        _ => return Err(()),
     };
-    Some(RGBA(RGBA { red: r / 255., green: g / 255., blue: b / 255., alpha: 1. }))
+    Ok(RGBA(RGBA { red: r / 255., green: g / 255., blue: b / 255., alpha: 1. }))
 }
 
 
 #[inline]
-fn parse_color_hash(value: &str) -> Option<Color> {
+fn parse_color_hash(value: &str) -> Result<Color, ()> {
     macro_rules! from_hex(
         ($c: expr) => {{
             let c = $c;
             match c {
                 '0' .. '9' => c as u8 - ('0' as u8),
                 'a' .. 'f' => c as u8 - ('a' as u8) + 10,
                 'A' .. 'F' => c as u8 - ('A' as u8) + 10,
-                _ => return None  // Not a valid color
+                _ => return Err(())  // Not a valid color
             }
         }};
     )
     macro_rules! to_rgba(
         ($r: expr, $g: expr, $b: expr,) => {
-            Some(RGBA(RGBA { red: $r as f32 / 255.,
-                             green: $g as f32 / 255.,
-                             blue: $b as f32 / 255.,
-                             alpha: 1. }))
+            Ok(RGBA(RGBA { red: $r as f32 / 255.,
+                           green: $g as f32 / 255.,
+                           blue: $b as f32 / 255.,
+                           alpha: 1. }))
         };
     )
 
@@ -240,14 +240,14 @@ fn parse_color_hash(value: &str) -> Option<Color> {
             from_hex!(value.char_at(1)) * 17,
             from_hex!(value.char_at(2)) * 17,
         ),
-        _ => None
+        _ => Err(())
     }
 }
 
 
 #[inline]
 fn parse_color_function(name: &str, arguments: &[ComponentValue])
-                        -> Option<Color> {
+                        -> Result<Color, ()> {
     let lower_name = name.to_ascii_lower();
     let lower_name = lower_name.as_slice();
 
@@ -256,28 +256,28 @@ fn parse_color_function(name: &str, arguments: &[ComponentValue])
         else if "rgb" == lower_name { (true, false) }
         else if "hsl" == lower_name { (false, false) }
         else if "hsla" == lower_name { (false, true) }
-        else { return None };
+        else { return Err(()) };
 
     let mut iter = arguments.skip_whitespace();
     macro_rules! expect_comma(
-        () => ( match iter.next() { Some(&Comma) => {}, _ => { return None } } );
+        () => ( match iter.next() { Some(&Comma) => {}, _ => { return Err(()) } } );
     )
     macro_rules! expect_percentage(
         () => ( match iter.next() {
             Some(&Percentage(ref v)) => v.value,
-            _ => return None,
+            _ => return Err(()),
         });
     )
     macro_rules! expect_integer(
         () => ( match iter.next() {
             Some(&Number(ref v)) if v.int_value.is_some() => v.value,
-            _ => return None,
+            _ => return Err(()),
         });
     )
     macro_rules! expect_number(
         () => ( match iter.next() {
             Some(&Number(ref v)) => v.value,
-            _ => return None,
+            _ => return Err(()),
         });
     )
 
@@ -301,7 +301,7 @@ fn parse_color_function(name: &str, arguments: &[ComponentValue])
                 expect_comma!();
                 blue = (expect_percentage!() / 100.) as f32;
             }
-            _ => return None
+            _ => return Err(())
         };
     } else {
         let hue = expect_number!() / 360.;
@@ -336,8 +336,8 @@ fn parse_color_function(name: &str, arguments: &[ComponentValue])
         1.
     };
     if iter.next().is_none() {
-        Some(RGBA(RGBA { red: red, green: green, blue: blue, alpha: alpha }))
+        Ok(RGBA(RGBA { red: red, green: green, blue: blue, alpha: alpha }))
     } else {
-        None
+        Err(())
     }
 }

nth.rs

@@ -7,15 +7,15 @@ use std::ascii::StrAsciiExt;
 use ast::*;
 
 
-/// Parse the An+B notation, as found in the ``:nth-child()`` selector.
+/// Parse the *An+B* notation, as found in the `:nth-child()` selector.
 /// The input is typically the arguments of a function component value.
-/// Return Some((A, B)), or None for a syntax error.
-pub fn parse_nth(input: &[ComponentValue]) -> Option<(i32, i32)> {
+/// Return `Ok((A, B))`, or `Err(())` for a syntax error.
+pub fn parse_nth(input: &[ComponentValue]) -> Result<(i32, i32), ()> {
     let iter = &mut input.skip_whitespace();
     match iter.next() {
         Some(&Number(ref value)) => match value.int_value {
             Some(b) => parse_end(iter, 0, b as i32),
-            _ => None,
+            _ => Err(()),
         },
         Some(&Dimension(ref value, ref unit)) => match value.int_value {
             Some(a) => {
@@ -26,11 +26,11 @@ pub fn parse_nth(input: &[ComponentValue]) -> Option<(i32, i32)> {
                     "n-" => parse_signless_b(iter, a as i32, -1),
                     _ => match parse_n_dash_digits(unit) {
                         Some(b) => parse_end(iter, a as i32, b),
-                        _ => None
+                        _ => Err(())
                     },
                 }
             },
-            _ => None,
+            _ => Err(()),
         },
         Some(&Ident(ref value)) => {
             let ident = value.as_slice().to_ascii_lower();
@@ -44,11 +44,11 @@ pub fn parse_nth(input: &[ComponentValue]) -> Option<(i32, i32)> {
                 "-n-" => parse_signless_b(iter, -1, -1),
                 _ if ident.starts_with("-") => match parse_n_dash_digits(ident.slice_from(1)) {
                     Some(b) => parse_end(iter, -1, b),
-                    _ => None
+                    _ => Err(())
                 },
                 _ =>  match parse_n_dash_digits(ident) {
                     Some(b) => parse_end(iter, 1, b),
-                    _ => None
+                    _ => Err(())
                 },
             }
         },
@@ -61,47 +61,47 @@ pub fn parse_nth(input: &[ComponentValue]) -> Option<(i32, i32)> {
                     "n-" => parse_signless_b(iter, 1, -1),
                     _ => match parse_n_dash_digits(ident) {
                         Some(b) => parse_end(iter, 1, b),
-                        _ => None
+                        _ => Err(())
                     },
                 }
             },
-            _ => None
+            _ => Err(())
         },
-        _ => None
+        _ => Err(())
     }
 }
 
 
-type Nth = Option<(i32, i32)>;
+type Nth = Result<(i32, i32), ()>;
 type Iter<'a> = SkipWhitespaceIterator<'a>;
 
 fn parse_b(iter: &mut Iter, a: i32) -> Nth {
     match iter.next() {
-        None => Some((a, 0)),
+        None => Ok((a, 0)),
         Some(&Delim('+')) => parse_signless_b(iter, a, 1),
         Some(&Delim('-')) => parse_signless_b(iter, a, -1),
         Some(&Number(ref value)) => match value.int_value {
             Some(b) if has_sign(value) => parse_end(iter, a, b as i32),
-            _ => None,
+            _ => Err(()),
         },
-        _ => None
+        _ => Err(())
     }
 }
 
 fn parse_signless_b(iter: &mut Iter, a: i32, b_sign: i32) -> Nth {
     match iter.next() {
         Some(&Number(ref value)) => match value.int_value {
             Some(b) if !has_sign(value) => parse_end(iter, a, b_sign * (b as i32)),
-            _ => None,
+            _ => Err(()),
         },
-        _ => None
+        _ => Err(())
     }
 }
 
 fn parse_end(iter: &mut Iter, a: i32, b: i32) -> Nth {
     match iter.next() {
-        None => Some((a, b)),
-        Some(_) => None,
+        None => Ok((a, b)),
+        Some(_) => Err(()),
     }
 }
 

tests.rs

@@ -199,7 +199,7 @@ fn stylesheet_from_bytes() {
 fn run_color_tests(json_data: &str, to_json: |result: Option<Color>| -> json::Json) {
     run_json_tests(json_data, |input| {
         match parse_one_component_value(tokenize(input)) {
-            Ok(component_value) => to_json(Color::parse(&component_value)),
+            Ok(component_value) => to_json(Color::parse(&component_value).ok()),
             Err(_reason) => json::Null,
         }
     });
@@ -235,28 +235,28 @@ fn color3_keywords() {
 #[bench]
 fn bench_color_lookup_red(b: &mut test::Bencher) {
     let ident = parse_one_component_value(tokenize("red")).unwrap();
-    b.iter(|| assert!(Color::parse(&ident).is_some()));
+    b.iter(|| assert!(Color::parse(&ident).is_ok()));
 }
 
 
 #[bench]
 fn bench_color_lookup_lightgoldenrodyellow(b: &mut test::Bencher) {
     let ident = parse_one_component_value(tokenize("lightgoldenrodyellow")).unwrap();
-    b.iter(|| assert!(Color::parse(&ident).is_some()));
+    b.iter(|| assert!(Color::parse(&ident).is_ok()));
 }
 
 
 #[bench]
 fn bench_color_lookup_fail(b: &mut test::Bencher) {
     let ident = parse_one_component_value(tokenize("lightgoldenrodyellowbazinga")).unwrap();
-    b.iter(|| assert!(Color::parse(&ident).is_none()));
+    b.iter(|| assert!(Color::parse(&ident).is_err()));
 }
 
 
 #[test]
 fn nth() {
     run_json_tests(include_str!("css-parsing-tests/An+B.json"), |input| {
-        parse_nth(tokenize(input).map(|(c, _)| c).collect::<Vec<ComponentValue>>().as_slice())
+        parse_nth(tokenize(input).map(|(c, _)| c).collect::<Vec<ComponentValue>>().as_slice()).ok()
     });
 }
 

tokenizer.rs

@@ -307,14 +307,14 @@ fn consume_block_with_location(tokenizer: &mut Tokenizer, ending_token: Componen
 
 fn consume_string(tokenizer: &mut Tokenizer, single_quote: bool) -> ComponentValue {
     match consume_quoted_string(tokenizer, single_quote) {
-        Some(value) => String(value),
-        None => BadString
+        Ok(value) => String(value),
+        Err(()) => BadString
     }
 }
 
 
-// Return None on syntax error (ie. unescaped newline)
-fn consume_quoted_string(tokenizer: &mut Tokenizer, single_quote: bool) -> Option<String> {
+/// Return `Err(())` on syntax error (ie. unescaped newline)
+fn consume_quoted_string(tokenizer: &mut Tokenizer, single_quote: bool) -> Result<String, ()> {
     tokenizer.position += 1;  // Skip the initial quote
     let mut string = String::new();
     while !tokenizer.is_eof() {
@@ -323,7 +323,7 @@ fn consume_quoted_string(tokenizer: &mut Tokenizer, single_quote: bool) -> Optio
             '\'' if single_quote => break,
             '\n' => {
                 tokenizer.position -= 1;
-                return None;
+                return Err(());
             },
             '\\' => {
                 if !tokenizer.is_eof() {
@@ -338,7 +338,7 @@ fn consume_quoted_string(tokenizer: &mut Tokenizer, single_quote: bool) -> Optio
             c => string.push_char(c),
         }
     }
-    Some(string)
+    Ok(string)
 }
 
 
@@ -475,8 +475,8 @@ fn consume_url(tokenizer: &mut Tokenizer) -> ComponentValue {
 
     fn consume_quoted_url(tokenizer: &mut Tokenizer, single_quote: bool) -> ComponentValue {
         match consume_quoted_string(tokenizer, single_quote) {
-            Some(value) => consume_url_end(tokenizer, value),
-            None => consume_bad_url(tokenizer),
+            Ok(value) => consume_url_end(tokenizer, value),
+            Err(()) => consume_bad_url(tokenizer),
         }
     }