From d360af2cd6bf7fab09d5c83257e9723db7b83651 Mon Sep 17 00:00:00 2001 From: Connor Skees Date: Wed, 21 Jul 2021 09:12:50 -0400 Subject: [PATCH] improve code coverage --- CHANGELOG.md | 1 + src/args.rs | 7 +- src/builtin/functions/meta.rs | 10 +- src/parse/args.rs | 6 ++ src/parse/mod.rs | 6 +- src/value/mod.rs | 28 +++-- src/value/number/mod.rs | 1 + tests/args.rs | 18 ++++ tests/equality.rs | 189 ++++++++++++++++++++++++++++++++++ tests/functions.rs | 22 ++++ tests/list.rs | 15 +++ tests/meta.rs | 8 ++ tests/ordering.rs | 8 ++ tests/selectors.rs | 11 ++ tests/str-escape.rs | 10 ++ tests/values.rs | 5 - 16 files changed, 322 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdb24ef..abff9cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - fix bug in `hue(...)` function in which the value would be incorrect in when the `red` channel was the highest and the green channel was lower than the blue channel - no longer round output from `saturation(...)` function - improve handling of newlines inside and around `@media` +- arglists can be equal to comma separated lists # 0.10.5 diff --git a/src/args.rs b/src/args.rs index e167538..c9aa636 100644 --- a/src/args.rs +++ b/src/args.rs @@ -229,10 +229,13 @@ impl CallArgs { Ok(v) => v, Err(e) => return Err((format!("No argument named ${}.", e), self.1).into()), }; + args.sort_by(|(a1, _), (a2, _)| a1.cmp(a2)); - for arg in args { - vals.push(arg.1?); + + for (_, arg) in args { + vals.push(arg?); } + Ok(vals) } } diff --git a/src/builtin/functions/meta.rs b/src/builtin/functions/meta.rs index 71587d1..002c9e9 100644 --- a/src/builtin/functions/meta.rs +++ b/src/builtin/functions/meta.rs @@ -304,11 +304,15 @@ pub(crate) fn content_exists(args: CallArgs, parser: &mut Parser<'_>) -> SassRes )) } -#[allow(unused_variables)] +#[allow(unused_variables, clippy::needless_pass_by_value)] pub(crate) fn keywords(args: CallArgs, parser: &mut Parser<'_>) -> SassResult { args.max_args(1)?; - drop(args); - todo!("builtin function `keywords` blocked on better handling of call args") + + Err(( + "Builtin function `keywords` is not yet implemented", + args.span(), + ) + .into()) } pub(crate) fn declare(f: &mut GlobalFunctionMap) { diff --git a/src/parse/args.rs b/src/parse/args.rs index 37ec1e9..09c5c94 100644 --- a/src/parse/args.rs +++ b/src/parse/args.rs @@ -357,6 +357,11 @@ impl<'a> Parser<'a> { args.max_args(0)?; return Ok(scope); } + + if !fn_args.0.iter().any(|arg| arg.is_variadic) { + args.max_args(fn_args.len())?; + } + self.scopes.enter_new_scope(); for (idx, mut arg) in fn_args.0.into_iter().enumerate() { if arg.is_variadic { @@ -364,6 +369,7 @@ impl<'a> Parser<'a> { scope.insert_var(arg.name, arg_list); break; } + let val = match args.get(idx, arg.name) { Some(v) => v, None => match arg.default.as_mut() { diff --git a/src/parse/mod.rs b/src/parse/mod.rs index 55951f2..ba4eb73 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -591,10 +591,8 @@ impl<'a> Parser<'a> { self.toks.next(); #[allow(clippy::while_let_on_iterator)] while let Some(tok) = self.toks.next() { - if tok.kind == '*' { - if self.consume_char_if_exists('/') { - break; - } + if tok.kind == '*' && self.consume_char_if_exists('/') { + break; } } } diff --git a/src/value/mod.rs b/src/value/mod.rs index d30e0aa..15e6ebf 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -103,13 +103,23 @@ impl PartialEq for Value { false } } - Value::ArgList(list1) => { - if let Value::ArgList(list2) = other { - list1 == list2 - } else { - false + Value::ArgList(list1) => match other { + Value::ArgList(list2) => list1 == list2, + Value::List(list2, ListSeparator::Comma, ..) => { + if list1.len() != list2.len() { + return false; + } + + for (el1, el2) in list1.iter().zip(list2) { + if &el1.node != el2 { + return false; + } + } + + true } - } + _ => false, + }, } } } @@ -348,11 +358,11 @@ impl Value { num.cmp(&num2.clone().convert(unit2, unit)) } } - v => { + _ => { return Err(( format!( "Undefined operation \"{} {} {}\".", - v.inspect(span)?, + self.inspect(span)?, op, other.inspect(span)? ), @@ -371,7 +381,7 @@ impl Value { ), span, ) - .into()) + .into()); } }) } diff --git a/src/value/number/mod.rs b/src/value/number/mod.rs index 1662898..3fdf38a 100644 --- a/src/value/number/mod.rs +++ b/src/value/number/mod.rs @@ -149,6 +149,7 @@ impl Number { /// Invariants: `from.comparable(&to)` must be true pub fn convert(self, from: &Unit, to: &Unit) -> Self { + debug_assert!(from.comparable(to)); self * UNIT_CONVERSION_TABLE[to][from].clone() } } diff --git a/tests/args.rs b/tests/args.rs index f39457f..12d554e 100644 --- a/tests/args.rs +++ b/tests/args.rs @@ -5,6 +5,10 @@ error!( variable_after_varargs, "@function foo($a..., $b) {\n @return $a;\n}\n", "Error: expected \")\"." ); +error!( + two_varargs, + "@function foo($a..., $b...) {\n @return $a;\n}\n", "Error: expected \")\"." +); error!( varargs_one_period, "@function foo($a.) {\n @return $a;\n}\n", "Error: expected \".\"." @@ -48,6 +52,20 @@ test!( "a {\n color: if(false, unit(foo), red);\n}\n", "a {\n color: red;\n}\n" ); +test!( + silent_comments_in_arg_with_default_val, + "@function foo($a// + :// + red// + ) { + @return $a; + } + + a { + color: foo(); + }", + "a {\n color: red;\n}\n" +); error!( #[ignore = "expects incorrect char, '{'"] nothing_after_open, diff --git a/tests/equality.rs b/tests/equality.rs index 07abba1..f91625d 100644 --- a/tests/equality.rs +++ b/tests/equality.rs @@ -61,3 +61,192 @@ test!( "a {\n color: (0mm: a)==(0cm: a);\n}\n", "a {\n color: true;\n}\n" ); +test!( + true_true_eq, + "a {\n color: true == true;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + false_false_eq, + "a {\n color: false == false;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + true_false_eq, + "a {\n color: true == false;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + false_true_eq, + "a {\n color: false == true;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + true_true_ne, + "a {\n color: true != true;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + false_false_ne, + "a {\n color: false != false;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + true_false_ne, + "a {\n color: true != false;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + false_true_ne, + "a {\n color: true != false;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + important_important_eq, + "a {\n color: !important == !important;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + important_important_ne, + "a {\n color: !important != !important;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + map_color_eq, + "a {\n color: (a: b) == red;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + map_color_ne, + "a {\n color: (a: b) != red;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + bracketed_list_color_eq, + "a {\n color: [] == red;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + bracketed_list_color_ne, + "a {\n color: [] != red;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + function_ref_color_eq, + "a {\n color: get-function(\"red\") == red;\n}\n", + "a {\n color: false;\n}\n" +); +test!( + function_ref_color_ne, + "a {\n color: get-function(\"red\") != red;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + nan_nan_eq, + "a {\n color: (0/0) == (0/0);\n}\n", + "a {\n color: false;\n}\n" +); +test!( + nan_nan_ne, + "a {\n color: (0/0) != (0/0);\n}\n", + "a {\n color: true;\n}\n" +); +test!( + string_bool_ne, + "a {\n color: hi != false;\n}\n", + "a {\n color: true;\n}\n" +); +test!( + lists_differ_only_in_separator_eq, + "a {\n color: (1 2 3) == (1, 2, 3);\n}\n", + "a {\n color: false;\n}\n" +); +test!( + lists_differ_only_in_separator_ne, + "a {\n color: (1 2 3) != (1, 2, 3);\n}\n", + "a {\n color: true;\n}\n" +); +test!( + maps_differ_in_length_eq, + "a {\n color: (a: b) == (a: b, c: d);\n}\n", + "a {\n color: false;\n}\n" +); +test!( + maps_differ_in_length_ne, + "a {\n color: (a: b) != (a: b, c: d);\n}\n", + "a {\n color: true;\n}\n" +); +test!( + arglist_unquoted_string_eq, + "@function foo($a...) { + @return $a == bar; + } + + a { + color: foo(1, 2, 3); + }", + "a {\n color: false;\n}\n" +); +test!( + arglist_equals_self, + "@function foo($a...) { + @return $a == $a; + } + + a { + color: foo(1, 2, 3); + }", + "a {\n color: true;\n}\n" +); +test!( + arglist_equals_self_when_splat_through_other_function, + "@function bar($a, $b...) { + @return $a == $b; + } + + @function foo($a...) { + @return bar($a, $a...); + } + + a { + color: foo(1, 2, 3); + }", + "a {\n color: true;\n}\n" +); +test!( + arglist_equals_does_not_equal_self_when_not_splat, + "@function bar($a, $b...) { + @return $a == $b; + } + + @function foo($a...) { + @return bar($a, $a); + } + + a { + color: foo(1, 2, 3); + }", + "a {\n color: false;\n}\n" +); +test!( + arglist_equals_comma_separated_list, + "@function foo($a...) { + @return $a == (1, 2, 3); + } + + a { + color: foo(1, 2, 3); + }", + "a {\n color: true;\n}\n" +); +test!( + arglist_does_not_equal_space_separated_list, + "@function foo($a...) { + @return $a == (1 2 3); + } + + a { + color: foo(1, 2, 3); + }", + "a {\n color: false;\n}\n" +); diff --git a/tests/functions.rs b/tests/functions.rs index d4e18b9..f8498f3 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -316,6 +316,28 @@ error!( }", "Error: Functions can only contain variable declarations and control directives." ); +error!( + pass_one_arg_to_fn_that_accepts_zero, + "@function foo() { + @return red; + } + + a { + color: foo(1); + }", + "Error: Only 0 arguments allowed, but 1 was passed." +); +error!( + pass_two_args_to_fn_that_accepts_one, + "@function foo($a) { + @return red; + } + + a { + color: foo(1, 2); + }", + "Error: Only 1 argument allowed, but 2 were passed." +); test!( allows_multiline_comment, "@function foo($a) { diff --git a/tests/list.rs b/tests/list.rs index ca2b286..ed7bb6c 100644 --- a/tests/list.rs +++ b/tests/list.rs @@ -372,6 +372,21 @@ test!( "a {\n color: [ /**/ ];\n}\n", "a {\n color: [];\n}\n" ); +test!( + space_separated_inside_comma_separated, + "$a: 1 2 3 == 1, 2, 3; + a { + color: $a; + color: nth($a, 1); + color: nth($a, 2); + }", + "a {\n color: 1 2 false, 2, 3;\n color: 1 2 false;\n color: 2;\n}\n" +); +test!( + whitespace_space_list_number, + "a {\n color: 1 2 3 ;\n}\n", + "a {\n color: 1 2 3;\n}\n" +); error!( invalid_item_in_space_separated_list, "a {\n color: red color * #abc;\n}\n", "Error: Undefined operation \"color * #abc\"." diff --git a/tests/meta.rs b/tests/meta.rs index 7dea6e9..2756963 100644 --- a/tests/meta.rs +++ b/tests/meta.rs @@ -302,3 +302,11 @@ error!( mixin_exists_non_string, "a {color: mixin-exists(12px)}", "Error: $name: 12px is not a string." ); +test!( + local_mixin_exists, + "a { + @mixin a {} + color: mixin-exists($name: a); + }", + "a {\n color: true;\n}\n" +); diff --git a/tests/ordering.rs b/tests/ordering.rs index 98a6085..821f839 100644 --- a/tests/ordering.rs +++ b/tests/ordering.rs @@ -66,3 +66,11 @@ test!( "a {\n color: 2in > 1cm;\n}\n", "a {\n color: true;\n}\n" ); +error!( + strings_not_comparable, + "a {\n color: a > b;\n}\n", "Error: Undefined operation \"a > b\"." +); +error!( + number_and_string_not_comparable, + "a {\n color: 1 > b;\n}\n", "Error: Undefined operation \"1 > b\"." +); diff --git a/tests/selectors.rs b/tests/selectors.rs index 70e061d..4efdf50 100644 --- a/tests/selectors.rs +++ b/tests/selectors.rs @@ -818,6 +818,17 @@ test!( ".foo bar[val=\"//\"] {\n color: &;\n}\n", ".foo bar[val=\"//\"] {\n color: .foo bar[val=\"//\"];\n}\n" ); +test!( + attribute_value_escape_ends_before_non_hex_char, + "[a=\"a\\\\66\"] {\n color: &;\n}\n", + "[a=a\\66] {\n color: [a=a\\66];\n}\n" +); +test!( + #[ignore = "we have to rewrite quoted attribute value parsing somewhat"] + attribute_value_escape_ends_with_whitespace, + "[a=\"a\\\\66 \"] {\n color: &;\n}\n", + "[a=\"a\\\\66 \"] {\n color: [a=\"a\\\\66 \"];\n}\n" +); error!( a_n_plus_b_n_invalid_odd, ":nth-child(ofdd) {\n color: &;\n}\n", "Error: Expected \"odd\"." diff --git a/tests/str-escape.rs b/tests/str-escape.rs index 1c2c010..966d0d2 100644 --- a/tests/str-escape.rs +++ b/tests/str-escape.rs @@ -185,6 +185,16 @@ test!( "a {\n color: \"foo\\\nbar\";\n}\n", "a {\n color: \"foobar\";\n}\n" ); +test!( + escaped_value_over_0xf_in_quoted_string, + "a {\n color: \"#{\"\\1f\"}\";\n}\n", + "a {\n color: \"\\1f\";\n}\n" +); +test!( + escaped_value_over_0xf_in_quoted_string_with_trailing_space, + "a {\n color: \"#{\"\\1f\"} \";\n}\n", + "a {\n color: \"\\1f \";\n}\n" +); error!( newline_after_escape, "a {\n color: \\\n", "Error: Expected escape sequence." diff --git a/tests/values.rs b/tests/values.rs index 05aeb29..cf989ed 100644 --- a/tests/values.rs +++ b/tests/values.rs @@ -110,11 +110,6 @@ test!( "a {\n color: \"'foo' \\\"bar\\\"\";\n}\n", "a {\n color: \"'foo' \\\"bar\\\"\";\n}\n" ); -test!( - whitespace_space_list_number, - "a {\n color: 1 2 3 ;\n}\n", - "a {\n color: 1 2 3;\n}\n" -); test!( whitespace_comma_list_number, "a {\n color: 1 , 2 , 3 ;\n}\n",