From 4841cdb31bb8df9f8095245b66fa9a710cfd6ba9 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 11 Aug 2014 17:59:57 +0100 Subject: [PATCH 1/3] Use Result/Err(()) instead of Option/None to indicate a parse error. --- color.rs | 46 +++++++++++++++++++++++----------------------- nth.rs | 34 +++++++++++++++++----------------- tests.rs | 10 +++++----- tokenizer.rs | 14 +++++++------- 4 files changed, 52 insertions(+), 52 deletions(-) diff --git a/color.rs b/color.rs index 27924ce6..54be0b42 100644 --- a/color.rs +++ b/color.rs @@ -33,20 +33,20 @@ pub enum Color { // Return None on invalid/unsupported value (not a color) impl Color { - pub fn parse(component_value: &ComponentValue) -> Option { + pub fn parse(component_value: &ComponentValue) -> Result { 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 { +fn parse_color_keyword(value: &str) -> Result { let lower_value = value.to_ascii_lower(); let (r, g, b) = match lower_value.as_slice() { "black" => (0., 0., 0.), @@ -199,16 +199,16 @@ fn parse_color_keyword(value: &str) -> Option { "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 { +fn parse_color_hash(value: &str) -> Result { macro_rules! from_hex( ($c: expr) => {{ let c = $c; @@ -216,16 +216,16 @@ fn parse_color_hash(value: &str) -> Option { '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 { 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 { + -> Result { 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(()) } } diff --git a/nth.rs b/nth.rs index aeb63aed..b800feba 100644 --- a/nth.rs +++ b/nth.rs @@ -10,12 +10,12 @@ use ast::*; /// 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)> { +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,30 +61,30 @@ 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(()) } } @@ -92,16 +92,16 @@ 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(()), } } diff --git a/tests.rs b/tests.rs index 9ba7922a..e62c57d0 100644 --- a/tests.rs +++ b/tests.rs @@ -199,7 +199,7 @@ fn stylesheet_from_bytes() { fn run_color_tests(json_data: &str, to_json: |result: Option| -> 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::>().as_slice()) + parse_nth(tokenize(input).map(|(c, _)| c).collect::>().as_slice()).ok() }); } diff --git a/tokenizer.rs b/tokenizer.rs index b1335bbb..ed874b7d 100644 --- a/tokenizer.rs +++ b/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 { +fn consume_quoted_string(tokenizer: &mut Tokenizer, single_quote: bool) -> Result { 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), } } From 2a2df62689ce0fc9ece38d49e73372309716203a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 11 Aug 2014 21:00:17 +0100 Subject: [PATCH 2/3] fixup! Use Result/Err(()) instead of Option/None to indicate a parse error. --- color.rs | 2 +- nth.rs | 4 ++-- tokenizer.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/color.rs b/color.rs index 54be0b42..ce9de301 100644 --- a/color.rs +++ b/color.rs @@ -31,7 +31,7 @@ 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) -> Result { match *component_value { diff --git a/nth.rs b/nth.rs index b800feba..6faf3fb5 100644 --- a/nth.rs +++ b/nth.rs @@ -7,9 +7,9 @@ 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. +/// Return `Ok((A, B))`, or `None` for a syntax error. pub fn parse_nth(input: &[ComponentValue]) -> Result<(i32, i32), ()> { let iter = &mut input.skip_whitespace(); match iter.next() { diff --git a/tokenizer.rs b/tokenizer.rs index ed874b7d..edbc1238 100644 --- a/tokenizer.rs +++ b/tokenizer.rs @@ -313,7 +313,7 @@ fn consume_string(tokenizer: &mut Tokenizer, single_quote: bool) -> ComponentVal } -// Return None on syntax error (ie. unescaped newline) +/// Return `Err(())` on syntax error (ie. unescaped newline) fn consume_quoted_string(tokenizer: &mut Tokenizer, single_quote: bool) -> Result { tokenizer.position += 1; // Skip the initial quote let mut string = String::new(); From 9d485fe838c22e51392dd5430d90f26f6eadf357 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 11 Aug 2014 21:03:10 +0100 Subject: [PATCH 3/3] fixup! fixup! Use Result/Err(()) instead of Option/None to indicate a parse error. --- nth.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nth.rs b/nth.rs index 6faf3fb5..b3d6e4c2 100644 --- a/nth.rs +++ b/nth.rs @@ -9,7 +9,7 @@ use ast::*; /// Parse the *An+B* notation, as found in the `:nth-child()` selector. /// The input is typically the arguments of a function component value. -/// Return `Ok((A, B))`, or `None` for a syntax error. +/// 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() {