From be9eb8e0b0fc9dc786a347490351e79add25065d Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Wed, 14 Jul 2021 01:40:11 -0400 Subject: [PATCH] improve code coverage also identified dead code and a bug in saturation and hue fns --- src/color/mod.rs | 52 ++++++++++++++-------------- src/output.rs | 12 ++----- src/parse/keyframes.rs | 1 + src/parse/value/css_function.rs | 59 ++++++++++++++----------------- src/parse/variable.rs | 2 -- tests/color.rs | 45 ++++++++++++++++++++++++ tests/keyframes.rs | 40 ++++++++++++++++++++- tests/meta.rs | 8 +++++ tests/min-max.rs | 61 +++++++++++++++++++++++++++++++++ tests/str-escape.rs | 4 +++ tests/units.rs | 10 ++++++ tests/variables.rs | 19 ++++++++++ 12 files changed, 241 insertions(+), 72 deletions(-) diff --git a/src/color/mod.rs b/src/color/mod.rs index 59fea6f..218ac4a 100644 --- a/src/color/mod.rs +++ b/src/color/mod.rs @@ -237,25 +237,23 @@ impl Color { let red = self.red() / Number::from(255); let green = self.green() / Number::from(255); let blue = self.blue() / Number::from(255); + let min = min(&red, min(&green, &blue)).clone(); let max = max(&red, max(&green, &blue)).clone(); - if min == max { - return Number::zero(); - } - let mut hue = if blue == max { - Number::from(4) + (red - green) / (max - min) - } else if green == max { - Number::from(2) + (blue - red) / (max - min) + let delta = max.clone() - min.clone(); + + let hue = if min == max { + Number::zero() + } else if max == red { + Number::from(60_u8) * (green - blue) / delta + } else if max == green { + Number::from(120_u8) + Number::from(60_u8) * (blue - red) / delta } else { - (green - blue) / (max - min) + Number::from(240_u8) + Number::from(60_u8) * (red - green) / delta }; - if hue.is_negative() { - hue += Number::from(360); - } - - (hue * Number::from(60)).round() + hue % Number::from(360) } /// Calculate saturation from RGBA values @@ -275,14 +273,18 @@ impl Color { return Number::zero(); } - let d = max.clone() - min.clone(); - let mm = max + min; - let s = d / if mm > Number::one() { - Number::from(2) - mm - } else { - mm - }; - (s * Number::from(100)).round() + let delta = max.clone() - min.clone(); + + let sum = max + min; + + let s = delta + / if sum > Number::one() { + Number::from(2) - sum + } else { + sum + }; + + s * Number::from(100) } /// Calculate luminance from RGBA values @@ -392,11 +394,6 @@ impl Color { ); if saturation.is_zero() { - let luminance = if luminance > Number::from(100) { - Number::from(100) - } else { - luminance - }; let val = luminance * Number::from(255); let repr = repr(&val, &val, &val, &alpha); return Color::new_hsla(val.clone(), val.clone(), val, alpha, hsla, repr); @@ -455,11 +452,14 @@ impl Color { if weight.is_zero() { return self.clone(); } + let red = Number::from(u8::max_value()) - self.red(); let green = Number::from(u8::max_value()) - self.green(); let blue = Number::from(u8::max_value()) - self.blue(); let repr = repr(&red, &green, &blue, &self.alpha()); + let inverse = Color::new_rgba(red, green, blue, self.alpha(), repr); + inverse.mix(self, weight) } diff --git a/src/output.rs b/src/output.rs index a737b85..14e1628 100644 --- a/src/output.rs +++ b/src/output.rs @@ -293,18 +293,10 @@ impl Formatter for CompressedFormatter { let mut selectors = selectors.iter(); if let Some(selector) = selectors.next() { - match selector { - KeyframesSelector::To => write!(buf, "to")?, - KeyframesSelector::From => write!(buf, "from")?, - KeyframesSelector::Percent(p) => write!(buf, "{}%", p)?, - } + write!(buf, "{}", selector)?; } for selector in selectors { - match selector { - KeyframesSelector::To => write!(buf, ",to")?, - KeyframesSelector::From => write!(buf, ",from")?, - KeyframesSelector::Percent(p) => write!(buf, ",{}%", p)?, - } + write!(buf, ",{}", selector)?; } write!(buf, "{{")?; diff --git a/src/parse/keyframes.rs b/src/parse/keyframes.rs index a09c9fa..38b18c7 100644 --- a/src/parse/keyframes.rs +++ b/src/parse/keyframes.rs @@ -181,6 +181,7 @@ impl<'a> Parser<'a> { c => string.push(c), } } + Err(("expected \"{\".", span).into()) } diff --git a/src/parse/value/css_function.rs b/src/parse/value/css_function.rs index 01ec78d..207f9e3 100644 --- a/src/parse/value/css_function.rs +++ b/src/parse/value/css_function.rs @@ -60,6 +60,7 @@ impl<'a> Parser<'a> { return Err(("expected \"(\".", self.span_before).into()); } }; + while let Some(tok) = self.toks.next() { span = span.merge(tok.pos()); match tok.kind { @@ -73,6 +74,7 @@ impl<'a> Parser<'a> { _ => return Err(("expected \"(\".", span).into()), } } + Ok(string) } @@ -84,45 +86,36 @@ impl<'a> Parser<'a> { self.whitespace(); while let Some(tok) = self.toks.next() { - let kind = tok.kind; - - if kind == '!' - || kind == '%' - || kind == '&' - || ('*'..='~').contains(&kind) - || kind as u32 >= 0x0080 - { - buf.push(kind); - } else if kind == '\\' { - buf.push_str(&self.parse_escape(false)?); - } else if kind == '#' { - if let Some(Token { kind: '{', .. }) = self.toks.peek() { - self.toks.next(); - let interpolation = self.parse_interpolation()?; - match interpolation.node { - Value::String(ref s, ..) => buf.push_str(s), - v => buf.push_str(v.to_css_string(interpolation.span)?.borrow()), - }; - } else { - buf.push('#'); + match tok.kind { + '!' | '%' | '&' | '*'..='~' | '\u{80}'..=char::MAX => buf.push(tok.kind), + '#' => { + if self.consume_char_if_exists('{') { + let interpolation = self.parse_interpolation()?; + match interpolation.node { + Value::String(ref s, ..) => buf.push_str(s), + v => buf.push_str(v.to_css_string(interpolation.span)?.borrow()), + }; + } else { + buf.push('#'); + } } - } else if kind == ')' { - buf.push(')'); - - return Ok(Some(buf)); - } else if kind.is_whitespace() { - self.whitespace(); - - if let Some(Token { kind: ')', .. }) = self.toks.peek() { - self.toks.next(); + ')' => { buf.push(')'); return Ok(Some(buf)); } + ' ' | '\t' | '\n' | '\r' => { + self.whitespace(); - break; - } else { - break; + if self.consume_char_if_exists(')') { + buf.push(')'); + + return Ok(Some(buf)); + } + + break; + } + _ => break, } } diff --git a/src/parse/variable.rs b/src/parse/variable.rs index 871eb98..c50679c 100644 --- a/src/parse/variable.rs +++ b/src/parse/variable.rs @@ -52,8 +52,6 @@ impl<'a> Parser<'a> { return Ok(()); } - var_value?.node - } else if self.at_root { var_value?.node } else { if self.scopes.default_var_exists(ident) { diff --git a/tests/color.rs b/tests/color.rs index badfb9d..42a7b90 100644 --- a/tests/color.rs +++ b/tests/color.rs @@ -743,3 +743,48 @@ test!( "a {\n color: type-of(r#{e}d);\n}\n", "a {\n color: string;\n}\n" ); +test!( + color_equality_differ_in_green_channel, + "a {\n color: rgb(1, 1, 1) == rgb(1, 2, 1);\n}\n", + "a {\n color: false;\n}\n" +); +test!( + color_equality_differ_in_blue_channel, + "a {\n color: rgb(1, 1, 1) == rgb(1, 1, 2);\n}\n", + "a {\n color: false;\n}\n" +); +test!( + color_equality_differ_in_alpha_channel, + "a {\n color: rgba(1, 1, 1, 1.0) == rgba(1, 1, 1, 0.5);\n}\n", + "a {\n color: false;\n}\n" +); +test!( + hue_of_rgb_is_negative, + "a {\n color: hue(rgb(255, 0, 1));\n}\n", + "a {\n color: 359.7647058824deg;\n}\n" +); +test!( + saturation_of_rgb_all_channels_equal, + "a {\n color: saturation(rgb(125, 125, 125));\n}\n", + "a {\n color: 0%;\n}\n" +); +test!( + saturation_of_rgb_min_and_max_above_1, + "a {\n color: saturation(rgb(255, 248, 248));\n}\n", + "a {\n color: 100%;\n}\n" +); +test!( + saturation_of_rgb_min_and_max_below_1, + "a {\n color: saturation(rgb(88, 88, 87));\n}\n", + "a {\n color: 0.5714285714%;\n}\n" +); +test!( + invert_weight_zero_is_nop, + "a {\n color: invert(#0f0f0f, 0);\n}\n", + "a {\n color: #0f0f0f;\n}\n" +); +test!( + mix_combined_weight_is_normalized_weight, + "a {\n color: mix(rgba(255, 20, 0, 0), rgba(0, 20, 255, 1), 100);\n}\n", + "a {\n color: rgba(255, 20, 0, 0);\n}\n" +); diff --git a/tests/keyframes.rs b/tests/keyframes.rs index a2cf6b9..3632692 100644 --- a/tests/keyframes.rs +++ b/tests/keyframes.rs @@ -1,7 +1,6 @@ #[macro_use] mod macros; -// @content inside keyframes test!( content_inside_keyframes, "@mixin foo { @@ -137,3 +136,42 @@ test!( }", "@keyframes foo {\n 12.5% {\n color: red;\n }\n}\n" ); +test!( + keyframes_hash_in_name, + "@keyframes #identifier { + to { + color: red; + } + }", + "@keyframes #identifier {\n to {\n color: red;\n }\n}\n" +); +test!( + keyframes_interpolated_selector, + "@keyframes foo { + #{t}o { + color: red; + } + }", + "@keyframes foo {\n to {\n color: red;\n }\n}\n" +); +error!( + keyframes_denies_selector_with_hash, + "@keyframes foo { + #to { + color: red; + } + }", + "Error: Expected \"to\" or \"from\"." +); +error!( + keyframes_nothing_after_forward_slash_in_selector, + "@keyframes foo { a/", "Error: Expected selector." +); +error!( + keyframes_no_ident_after_forward_slash_in_selector, + "@keyframes foo { a/ {} }", "Error: expected selector." +); +error!( + keyframes_nothing_after_selector, + "@keyframes foo { a", "Error: expected \"{\"." +); diff --git a/tests/meta.rs b/tests/meta.rs index 93eafe8..7dea6e9 100644 --- a/tests/meta.rs +++ b/tests/meta.rs @@ -286,6 +286,14 @@ test!( "@function a(){} a {\n color: function-exists($name: a)\n}\n", "a {\n color: true;\n}\n" ); +test!( + function_exists_not_global, + "a { + @function foo () {} + color: function-exists($name: 'foo'); + }", + "a {\n color: true;\n}\n" +); error!( function_exists_non_string, "a {color: function-exists(12px)}", "Error: $name: 12px is not a string." diff --git a/tests/min-max.rs b/tests/min-max.rs index fb14f3d..6071ca6 100644 --- a/tests/min-max.rs +++ b/tests/min-max.rs @@ -186,3 +186,64 @@ test!( "a {\n color: min(calc(1 /* #{5} */ 2));\n}\n", "a {\n color: min(calc(1 /* #{5} */ 2));\n}\n" ); +test!( + min_uppercase, + "a {\n color: MIN(1);\n}\n", + "a {\n color: min(1);\n}\n" +); +test!( + max_uppercase, + "a {\n color: MAX(1);\n}\n", + "a {\n color: max(1);\n}\n" +); + +test!( + min_parenthesis_around_arg, + "a {\n color: min((1));\n}\n", + "a {\n color: min((1));\n}\n" +); +error!( + min_parenthesis_around_arg_with_comma, + "a {\n color: min((1, 1));\n}\n", "Error: 1, 1 is not a number." +); +error!( + min_hash_without_interpolation, + "a {\n color: min(#a);\n}\n", "Error: #a is not a number." +); +error!( + min_calc_no_parens, + "a {\n color: min(calc);\n}\n", "Error: calc is not a number." +); +error!( + min_env_no_parens, + "a {\n color: min(env);\n}\n", "Error: env is not a number." +); +error!( + min_var_no_parens, + "a {\n color: min(var);\n}\n", "Error: var is not a number." +); +error!( + min_min_unfinished, + "a {\n color: min(mi);\n}\n", "Error: mi is not a number." +); +error!( + min_max_unfinished, + "a {\n color: min(ma);\n}\n", "Error: ma is not a number." +); +error!( + min_min_no_parens, + "a {\n color: min(min);\n}\n", "Error: min is not a number." +); +error!( + min_max_no_parens, + "a {\n color: min(max);\n}\n", "Error: max is not a number." +); +error!( + min_min_invalid, + "a {\n color: min(min(#));\n}\n", "Error: Expected identifier." +); +test!( + min_calc_parens_no_args, + "a {\n color: min(calc());\n}\n", + "a {\n color: min(calc());\n}\n" +); diff --git a/tests/str-escape.rs b/tests/str-escape.rs index a77b93f..1c2c010 100644 --- a/tests/str-escape.rs +++ b/tests/str-escape.rs @@ -185,3 +185,7 @@ test!( "a {\n color: \"foo\\\nbar\";\n}\n", "a {\n color: \"foobar\";\n}\n" ); +error!( + newline_after_escape, + "a {\n color: \\\n", "Error: Expected escape sequence." +); diff --git a/tests/units.rs b/tests/units.rs index baa7510..9df8ff7 100644 --- a/tests/units.rs +++ b/tests/units.rs @@ -176,6 +176,16 @@ test!( "a {\n color: unit(1-\\65);\n}\n", "a {\n color: \"-e\";\n}\n" ); +test!( + viewport_relative_comparable_same, + "a {\n color: comparable(1vw, 2vw);\n}\n", + "a {\n color: true;\n}\n" +); +test!( + viewport_relative_comparable_different, + "a {\n color: comparable(1vw, 1vh);\n}\n", + "a {\n color: false;\n}\n" +); error!( display_single_div_with_none_numerator, "a {\n color: (1 / 1em);\n}\n", "Error: 1em^-1 isn't a valid CSS value." diff --git a/tests/variables.rs b/tests/variables.rs index 82b7a3f..47a8e25 100644 --- a/tests/variables.rs +++ b/tests/variables.rs @@ -405,3 +405,22 @@ error!( only_semicolon_after_hash_in_variable_decl, "$color: #;", "Error: Expected identifier." ); + +test!( + variable_name_begins_with_escape, + "$\\69: red; + + a { + color: $\\69; + }", + "a {\n color: red;\n}\n" +); +test!( + variable_name_contains_escape, + "$a\\69: red; + + a { + color: $a\\69; + }", + "a {\n color: red;\n}\n" +);