From accf8bb3985f1462188afd8764418e083b0090cf Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 02:02:05 -0400 Subject: [PATCH 01/18] newline at end of test file --- tests/at-root.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/at-root.rs b/tests/at-root.rs index a64e0af..f704ecb 100644 --- a/tests/at-root.rs +++ b/tests/at-root.rs @@ -67,4 +67,4 @@ test!( style_before_at_root, "a {}\n\n@at-root {\n @-ms-viewport { width: device-width; }\n}\n", "@-ms-viewport {\n width: device-width;\n}\n" -); \ No newline at end of file +); From fdca0ca4f2eb8546bb500dfc6ce84d39c253d3e0 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 02:28:35 -0400 Subject: [PATCH 02/18] improve span information for at-rules --- src/parse/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/parse/mod.rs b/src/parse/mod.rs index b0f395a..6dc41f9 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -107,10 +107,12 @@ impl<'a> Parser<'a> { if self.in_function && !stmts.is_empty() { return Ok(stmts); } + self.span_before = *pos; match kind { '@' => { self.toks.next(); let kind_string = self.parse_identifier()?; + self.span_before = kind_string.span; match AtRuleKind::try_from(&kind_string)? { AtRuleKind::Import => stmts.append(&mut self.import()?), AtRuleKind::Mixin => self.parse_mixin()?, @@ -228,7 +230,7 @@ impl<'a> Parser<'a> { } } '\u{0}'..='\u{8}' | '\u{b}'..='\u{1f}' => { - return Err(("expected selector.", self.toks.next().unwrap().pos).into()) + return Err(("expected selector.", *pos).into()) } '}' => { self.toks.next(); @@ -287,6 +289,8 @@ impl<'a> Parser<'a> { return Err(("expected \"{\".", self.span_before).into()); }; + self.span_before = span; + let mut found_curly = false; while let Some(tok) = self.toks.next() { From 1f82d65eacf963e42d173a3db4b3c14b29dd75fb Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 02:35:35 -0400 Subject: [PATCH 03/18] `&` is `null` when at root --- src/parse/value.rs | 8 ++++++-- tests/selectors.rs | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/parse/value.rs b/src/parse/value.rs index 5b7ae5d..ed9bf1f 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -372,8 +372,12 @@ impl<'a> Parser<'a> { } '&' => { let span = self.toks.next().unwrap().pos(); - IntermediateValue::Value(self.super_selectors.last().clone().into_value()) - .span(span) + if self.super_selectors.is_empty() { + IntermediateValue::Value(Value::Null).span(span) + } else { + IntermediateValue::Value(self.super_selectors.last().clone().into_value()) + .span(span) + } } '#' => { if let Some(Token { kind: '{', pos }) = self.toks.peek_forward(1) { diff --git a/tests/selectors.rs b/tests/selectors.rs index 2e8731d..a9412c8 100644 --- a/tests/selectors.rs +++ b/tests/selectors.rs @@ -653,6 +653,11 @@ test!( ":nth-child(ODD) {\n color: &;\n}\n", ":nth-child(odd) {\n color: :nth-child(odd);\n}\n" ); +test!( + super_selector_is_null_when_at_root, + "@mixin foo {\n #{if(&, 'true', 'false')} {\n color: red;\n }\n}\n\n@include foo;\n\na {\n @include foo;\n}\n", + "false {\n color: red;\n}\n\na true {\n color: red;\n}\n" +); error!( a_n_plus_b_n_invalid_odd, ":nth-child(ofdd) {\n color: &;\n}\n", "Error: Expected \"odd\"." From 82ffd0dddeac4639802db80b6fa754ff0a447f6f Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 05:24:42 -0400 Subject: [PATCH 04/18] arglists are lists too --- src/builtin/list.rs | 12 +++++------- src/parse/mod.rs | 18 ++---------------- src/value/mod.rs | 1 + tests/arglist.rs | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 tests/arglist.rs diff --git a/src/builtin/list.rs b/src/builtin/list.rs index 91cdf12..b808da2 100644 --- a/src/builtin/list.rs +++ b/src/builtin/list.rs @@ -1,6 +1,6 @@ use super::{Builtin, GlobalFunctionMap}; -use num_traits::{One, Signed, ToPrimitive, Zero}; +use num_traits::{Signed, ToPrimitive, Zero}; use crate::{ args::CallArgs, @@ -13,12 +13,10 @@ use crate::{ fn length(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> { args.max_args(1)?; - let len = match parser.arg(&mut args, 0, "list")? { - Value::List(v, ..) => Number::from(v.len()), - Value::Map(m) => Number::from(m.len()), - _ => Number::one(), - }; - Ok(Value::Dimension(len, Unit::None)) + Ok(Value::Dimension( + Number::from(parser.arg(&mut args, 0, "list")?.as_list().len()), + Unit::None, + )) } fn nth(mut args: CallArgs, parser: &mut Parser<'_>) -> SassResult<Value> { diff --git a/src/parse/mod.rs b/src/parse/mod.rs index 6dc41f9..4119782 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -856,14 +856,7 @@ impl<'a> Parser<'a> { self.whitespace(); let iter_val_toks = read_until_open_curly_brace(self.toks)?; let iter_val = self.parse_value_from_vec(iter_val_toks)?; - let iter = match iter_val.node.eval(iter_val.span)?.node { - Value::List(v, ..) => v, - Value::Map(m) => m - .into_iter() - .map(|(k, v)| Value::List(vec![k, v], ListSeparator::Space, Brackets::None)) - .collect(), - v => vec![v], - }; + let iter = iter_val.node.eval(iter_val.span)?.node.as_list(); self.toks.next(); self.whitespace(); let mut body = read_until_closing_curly_brace(self.toks)?; @@ -873,14 +866,7 @@ impl<'a> Parser<'a> { let mut stmts = Vec::new(); for row in iter { - let this_iterator = match row { - Value::List(v, ..) => v, - Value::Map(m) => m - .into_iter() - .map(|(k, v)| Value::List(vec![k, v], ListSeparator::Space, Brackets::None)) - .collect(), - v => vec![v], - }; + let this_iterator = row.as_list(); if vars.len() == 1 { if this_iterator.len() == 1 { diff --git a/src/value/mod.rs b/src/value/mod.rs index a8245c9..f24ed4f 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -317,6 +317,7 @@ impl Value { match self { Value::List(v, ..) => v, Value::Map(m) => m.entries(), + Value::ArgList(v) => v.into_iter().map(|val| val.node).collect(), v => vec![v], } } diff --git a/tests/arglist.rs b/tests/arglist.rs new file mode 100644 index 0000000..4636417 --- /dev/null +++ b/tests/arglist.rs @@ -0,0 +1,32 @@ +#![cfg(test)] + +#[macro_use] +mod macros; + +test!( + length_of_empty_arglist, + "@mixin foo($a...) {\n color: length($list: $a);\n}\na {\n @include foo;\n}\n", + "a {\n color: 0;\n}\n" +); +test!( + length_of_arglist_in_mixin, + "@mixin foo($a...) {\n color: length($list: $a);\n}\na {\n @include foo(a, 2, c);\n}\n", + "a {\n color: 3;\n}\n" +); +test!( + arglist_in_at_each, + "@function sum($numbers...) { + $sum: 0; + + @each $number in $numbers { + $sum: $sum + $number; + } + + @return $sum; + } + + a { + width: sum(50px, 30px, 100px); + }", + "a {\n width: 180px;\n}\n" +); From c9ba1317e084554fdc2c9ed577545eaf06c1fcc3 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 05:29:21 -0400 Subject: [PATCH 05/18] workaround @ at-root and interpolated super selectors --- src/parse/value.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parse/value.rs b/src/parse/value.rs index ed9bf1f..386e7f8 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -372,7 +372,7 @@ impl<'a> Parser<'a> { } '&' => { let span = self.toks.next().unwrap().pos(); - if self.super_selectors.is_empty() { + if self.super_selectors.is_empty() && !self.at_root_has_selector && !self.at_root { IntermediateValue::Value(Value::Null).span(span) } else { IntermediateValue::Value(self.super_selectors.last().clone().into_value()) From 191f2dfba4ac401c660f54383298f7dd848c877f Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 05:43:43 -0400 Subject: [PATCH 06/18] refactor @ import based on code review by @pickfire here, https://github.com/connorskees/grass/pull/13 --- src/parse/import.rs | 29 ++++++++++------------------- src/value/map.rs | 4 ---- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/parse/import.rs b/src/parse/import.rs index 68e21a9..bd8e46b 100644 --- a/src/parse/import.rs +++ b/src/parse/import.rs @@ -1,8 +1,4 @@ -use std::{ - ffi::{OsStr, OsString}, - fs, - path::Path, -}; +use std::{ffi::OsStr, fs, path::Path}; use peekmore::PeekMore; @@ -42,7 +38,6 @@ impl<'a> Parser<'a> { let path: &Path = file_name.as_ref(); - let mut rules = Vec::new(); let path_buf = if path.is_absolute() { // todo: test for absolute path imports path.into() @@ -58,24 +53,23 @@ impl<'a> Parser<'a> { // || name.starts_with("http://") || name.starts_with("https://") { todo!("css imports") } - let mut p1 = path_buf.clone(); - p1.push(OsString::from("index.scss")); - let mut p2 = path_buf.clone(); - p2.push(OsString::from("_index.scss")); + let paths = [ path_buf.with_file_name(name).with_extension("scss"), path_buf.with_file_name(format!("_{}.scss", name.to_str().unwrap())), - path_buf, - p1, - p2, + path_buf.clone(), + path_buf.clone().join("index.scss"), + path_buf.clone().join("_index.scss"), ]; + for name in &paths { if name.is_file() { let file = self.map.add_file( name.to_string_lossy().into(), String::from_utf8(fs::read(name)?)?, ); - let rules2 = Parser { + + return Parser { toks: &mut Lexer::new(&file) .collect::<Vec<Token>>() .into_iter() @@ -93,13 +87,10 @@ impl<'a> Parser<'a> { at_root: self.at_root, at_root_has_selector: self.at_root_has_selector, } - .parse()?; - - rules.extend(rules2); - break; + .parse(); } } - Ok(rules) + Ok(Vec::new()) } } diff --git a/src/value/map.rs b/src/value/map.rs index 75e4314..3e1fbd5 100644 --- a/src/value/map.rs +++ b/src/value/map.rs @@ -25,10 +25,6 @@ impl SassMap { Ok(None) } - pub fn len(&self) -> usize { - self.0.len() - } - #[allow(dead_code)] pub fn remove(&mut self, key: &Value) { self.0.retain(|(ref k, ..)| k != key); From 9ccf49010cc030e8c03dfa494eb0c3254b0524c4 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 05:48:01 -0400 Subject: [PATCH 07/18] resolve clippy lints --- src/parse/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/parse/mod.rs b/src/parse/mod.rs index 4119782..fbfd008 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -1147,14 +1147,17 @@ impl<'a> Parser<'a> { Ok(stmts) } + #[allow(clippy::unused_self)] fn parse_extend(&mut self) -> SassResult<()> { todo!("@extend not yet implemented") } + #[allow(clippy::unused_self)] fn parse_supports(&mut self) -> SassResult<Stmt> { todo!("@supports not yet implemented") } + #[allow(clippy::unused_self)] fn parse_keyframes(&mut self) -> SassResult<Stmt> { todo!("@keyframes not yet implemented") } From c957c10678032e2992426dc69dada014f7a735c7 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 05:49:23 -0400 Subject: [PATCH 08/18] simplify NeverEmptyVec::last, as suggested by @pickfire --- src/parse/common.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/parse/common.rs b/src/parse/common.rs index dc2a1bf..d8bbaa5 100644 --- a/src/parse/common.rs +++ b/src/parse/common.rs @@ -18,17 +18,11 @@ impl<T> NeverEmptyVec<T> { } pub fn last(&self) -> &T { - match self.rest.last() { - Some(v) => v, - None => &self.first, - } + self.rest.last().unwrap_or(&self.first) } pub fn last_mut(&mut self) -> &mut T { - match self.rest.last_mut() { - Some(v) => v, - None => &mut self.first, - } + self.rest.last_mut().unwrap_or(&mut self.first) } pub fn push(&mut self, value: T) { From 00bd9f384728a30528717c5f0fbbe1649c2f1184 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 06:09:32 -0400 Subject: [PATCH 09/18] simplify parsing of hex numbers as suggested by @pickfire --- src/parse/value.rs | 110 ++++++++++++++------------------------------- 1 file changed, 34 insertions(+), 76 deletions(-) diff --git a/src/parse/value.rs b/src/parse/value.rs index 386e7f8..957360d 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -554,83 +554,41 @@ impl<'a> Parser<'a> { }); } } - match s.len() { - 3 => { - let v = match u16::from_str_radix(&s, 16) { - Ok(a) => a, - Err(_) => { - return Ok(Value::String(format!("#{}", s), QuoteKind::None) - .span(self.span_before)) - } - }; - let red = (((v & 0xf00) >> 8) * 0x11) as u8; - let green = (((v & 0x0f0) >> 4) * 0x11) as u8; - let blue = ((v & 0x00f) * 0x11) as u8; - Ok( - Value::Color(Box::new(Color::new(red, green, blue, 1, format!("#{}", s)))) - .span(self.span_before), - ) + let v = match u32::from_str_radix(&s, 16) { + Ok(a) => a, + Err(_) => { + return Ok(Value::String(format!("#{}", s), QuoteKind::None).span(self.span_before)) } - 4 => { - let v = match u16::from_str_radix(&s, 16) { - Ok(a) => a, - Err(_) => { - return Ok(Value::String(format!("#{}", s), QuoteKind::None) - .span(self.span_before)) - } - }; - let red = (((v & 0xf000) >> 12) * 0x11) as u8; - let green = (((v & 0x0f00) >> 8) * 0x11) as u8; - let blue = (((v & 0x00f0) >> 4) * 0x11) as u8; - let alpha = ((v & 0x000f) * 0x11) as u8; - Ok(Value::Color(Box::new(Color::new( - red, - green, - blue, - alpha, - format!("#{}", s), - ))) - .span(self.span_before)) - } - 6 => { - let v = match u32::from_str_radix(&s, 16) { - Ok(a) => a, - Err(_) => { - return Ok(Value::String(format!("#{}", s), QuoteKind::None) - .span(self.span_before)) - } - }; - let red = ((v & 0x00ff_0000) >> 16) as u8; - let green = ((v & 0x0000_ff00) >> 8) as u8; - let blue = (v & 0x0000_00ff) as u8; - Ok( - Value::Color(Box::new(Color::new(red, green, blue, 1, format!("#{}", s)))) - .span(self.span_before), - ) - } - 8 => { - let v = match u32::from_str_radix(&s, 16) { - Ok(a) => a, - Err(_) => { - return Ok(Value::String(format!("#{}", s), QuoteKind::None) - .span(self.span_before)) - } - }; - let red = ((v & 0xff00_0000) >> 24) as u8; - let green = ((v & 0x00ff_0000) >> 16) as u8; - let blue = ((v & 0x0000_ff00) >> 8) as u8; - let alpha = (v & 0x0000_00ff) as u8; - Ok(Value::Color(Box::new(Color::new( - red, - green, - blue, - alpha, - format!("#{}", s), - ))) - .span(self.span_before)) - } - _ => Err(("Expected hex digit.", self.span_before).into()), - } + }; + let (red, green, blue, alpha) = match s.len() { + 3 => ( + (((v & 0x0f00) >> 8) * 0x11) as u8, + (((v & 0x00f0) >> 4) * 0x11) as u8, + ((v & 0x000f) * 0x11) as u8, + 1, + ), + 4 => ( + (((v & 0xf000) >> 12) * 0x11) as u8, + (((v & 0x0f00) >> 8) * 0x11) as u8, + (((v & 0x00f0) >> 4) * 0x11) as u8, + ((v & 0x000f) * 0x11) as u8, + ), + 6 => ( + ((v & 0x00ff_0000) >> 16) as u8, + ((v & 0x0000_ff00) >> 8) as u8, + (v & 0x0000_00ff) as u8, + 1, + ), + 8 => ( + ((v & 0xff00_0000) >> 24) as u8, + ((v & 0x00ff_0000) >> 16) as u8, + ((v & 0x0000_ff00) >> 8) as u8, + (v & 0x0000_00ff) as u8, + ), + _ => return Err(("Expected hex digit.", self.span_before).into()), + }; + let color = Color::new(red, green, blue, alpha, format!("#{}", s)); + Ok(Value::Color(Box::new(color)).span(self.span_before)) } fn eat_calc_args(&mut self, buf: &mut String) -> SassResult<()> { From 042935f362bebb31c2cb959ddac222c6342a01c5 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Wed, 17 Jun 2020 06:25:38 -0400 Subject: [PATCH 10/18] further refactor parsing of hex colors --- src/parse/value.rs | 35 ++++++++++++++++------------------- tests/color.rs | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/parse/value.rs b/src/parse/value.rs index 957360d..60d8e53 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -526,41 +526,38 @@ impl<'a> Parser<'a> { } fn parse_hex(&mut self) -> SassResult<Spanned<Value>> { - let mut s = String::with_capacity(8); + let mut s = String::with_capacity(7); + s.push('#'); if self .toks .peek() .ok_or(("Expected identifier.", self.span_before))? .kind - .is_ascii_digit() + .is_ascii_hexdigit() { while let Some(c) = self.toks.peek() { - if !c.kind.is_ascii_hexdigit() || s.len() == 8 { + if !c.kind.is_ascii_hexdigit() { break; } let tok = self.toks.next().unwrap(); self.span_before = self.span_before.merge(tok.pos()); s.push(tok.kind); } + // this branch exists so that we can emit `#` combined with + // identifiers. e.g. `#ooobar` should be emitted exactly as written; + // that is, `#ooobar`. } else { - let i = self.parse_identifier()?; - if i.node.chars().all(|c| c.is_ascii_hexdigit()) { - s = i.node; - self.span_before = self.span_before.merge(i.span); - } else { - return Ok(Spanned { - node: Value::String(format!("#{}", i.node), QuoteKind::None), - span: i.span, - }); - } + let ident = self.parse_identifier()?; + return Ok(Spanned { + node: Value::String(format!("#{}", ident.node), QuoteKind::None), + span: ident.span, + }); } - let v = match u32::from_str_radix(&s, 16) { + let v = match u32::from_str_radix(&s[1..], 16) { Ok(a) => a, - Err(_) => { - return Ok(Value::String(format!("#{}", s), QuoteKind::None).span(self.span_before)) - } + Err(_) => return Ok(Value::String(s, QuoteKind::None).span(self.span_before)), }; - let (red, green, blue, alpha) = match s.len() { + let (red, green, blue, alpha) = match s.len().saturating_sub(1) { 3 => ( (((v & 0x0f00) >> 8) * 0x11) as u8, (((v & 0x00f0) >> 4) * 0x11) as u8, @@ -587,7 +584,7 @@ impl<'a> Parser<'a> { ), _ => return Err(("Expected hex digit.", self.span_before).into()), }; - let color = Color::new(red, green, blue, alpha, format!("#{}", s)); + let color = Color::new(red, green, blue, alpha, s); Ok(Value::Color(Box::new(color)).span(self.span_before)) } diff --git a/tests/color.rs b/tests/color.rs index 2dcd2fe..e518003 100644 --- a/tests/color.rs +++ b/tests/color.rs @@ -582,3 +582,18 @@ test!( "a {\n color: hsl(-1 -1 -1);\n}\n", "a {\n color: black;\n}\n" ); +test!( + interpolation_after_hash_containing_only_hex_chars, + "a {\n color: ##{123};\n color: type-of(##{123});\n}\n", + "a {\n color: #123;\n color: string;\n}\n" +); +test!( + non_hex_chars_after_hash_are_still_touching_hash, + "a {\n color: #ooobar;\n}\n", + "a {\n color: #ooobar;\n}\n" +); +test!( + more_than_8_hex_chars_after_hash, + "a {\n color: #ffffffffff;\n}\n", + "a {\n color: #ffffffffff;\n}\n" +); From 3be87d38b9f389ef8567fab060eb61ced73c8237 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 00:42:43 -0400 Subject: [PATCH 11/18] more accurately parse strange hex colors --- src/parse/value.rs | 26 +++++++++++++++++--------- tests/color.rs | 12 +++++++++++- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/parse/value.rs b/src/parse/value.rs index 60d8e53..827fb02 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -528,15 +528,16 @@ impl<'a> Parser<'a> { fn parse_hex(&mut self) -> SassResult<Spanned<Value>> { let mut s = String::with_capacity(7); s.push('#'); - if self + let first_char = self .toks .peek() .ok_or(("Expected identifier.", self.span_before))? - .kind - .is_ascii_hexdigit() - { + .kind; + let first_is_digit = first_char.is_ascii_digit(); + let first_is_hexdigit = first_char.is_ascii_hexdigit(); + if first_is_digit { while let Some(c) = self.toks.peek() { - if !c.kind.is_ascii_hexdigit() { + if !c.kind.is_ascii_hexdigit() || s.len() == 9 { break; } let tok = self.toks.next().unwrap(); @@ -548,10 +549,17 @@ impl<'a> Parser<'a> { // that is, `#ooobar`. } else { let ident = self.parse_identifier()?; - return Ok(Spanned { - node: Value::String(format!("#{}", ident.node), QuoteKind::None), - span: ident.span, - }); + if first_is_hexdigit + && ident.node.chars().all(|c| c.is_ascii_hexdigit()) + && matches!(ident.node.len(), 3 | 4 | 6 | 8) + { + s.push_str(&ident.node); + } else { + return Ok(Spanned { + node: Value::String(format!("#{}", ident.node), QuoteKind::None), + span: ident.span, + }); + } } let v = match u32::from_str_radix(&s[1..], 16) { Ok(a) => a, diff --git a/tests/color.rs b/tests/color.rs index e518003..d5b5ad0 100644 --- a/tests/color.rs +++ b/tests/color.rs @@ -593,7 +593,17 @@ test!( "a {\n color: #ooobar;\n}\n" ); test!( - more_than_8_hex_chars_after_hash, + more_than_8_hex_chars_after_hash_starts_with_letter, "a {\n color: #ffffffffff;\n}\n", "a {\n color: #ffffffffff;\n}\n" ); +test!( + more_than_8_hex_chars_after_hash_starts_with_number, + "a {\n color: #0000000000;\n}\n", + "a {\n color: #00000000 0;\n}\n" +); +test!( + more_than_8_hex_chars_after_hash_starts_with_number_contains_hex_char, + "a {\n color: #00000000f00;\n}\n", + "a {\n color: #00000000 f00;\n}\n" +); From 7aaf4e7b921703af81a87f40b460dea4cda45322 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 01:08:30 -0400 Subject: [PATCH 12/18] remove `unwrap`s in Value::to_css_string --- src/value/mod.rs | 14 +++++++------- tests/list.rs | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/value/mod.rs b/src/value/mod.rs index f24ed4f..716e71a 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -82,11 +82,11 @@ fn visit_quoted_string(buf: &mut String, force_double_quote: bool, string: &str) buffer.push(hex_char_for(c as u32 >> 4)) } buffer.push(hex_char_for(c as u32 & 0xF)); - if iter.peek().is_none() { - break; - } - let next = iter.peek().unwrap(); + let next = match iter.peek() { + Some(v) => v, + None => break, + }; if next.is_ascii_hexdigit() || next == &' ' || next == &'\t' { buffer.push(' '); @@ -148,7 +148,7 @@ impl Value { Self::List(vals, sep, brackets) => match brackets { Brackets::None => Cow::owned( vals.iter() - .filter(|x| !x.is_null(span).unwrap()) + .filter(|x| !x.clone().is_null(span).unwrap_or(false)) .map(|x| x.to_css_string(span)) .collect::<SassResult<Vec<Cow<'static, str>>>>()? .join(sep.as_str()), @@ -156,7 +156,7 @@ impl Value { Brackets::Bracketed => Cow::owned(format!( "[{}]", vals.iter() - .filter(|x| !x.is_null(span).unwrap()) + .filter(|x| !x.is_null(span).unwrap_or(false)) .map(|x| x.to_css_string(span)) .collect::<SassResult<Vec<Cow<'static, str>>>>()? .join(sep.as_str()), @@ -199,7 +199,7 @@ impl Value { Self::Null => Cow::const_str(""), Self::ArgList(args) => Cow::owned( args.iter() - .filter(|x| !x.is_null(span).unwrap()) + .filter(|x| !x.is_null(span).unwrap_or(false)) .map(|a| Ok(a.node.to_css_string(span)?)) .collect::<SassResult<Vec<Cow<'static, str>>>>()? .join(", "), diff --git a/tests/list.rs b/tests/list.rs index e68c5ae..a396edd 100644 --- a/tests/list.rs +++ b/tests/list.rs @@ -314,3 +314,19 @@ test!( "a {\n color: 3;\n}\n" ); test!(empty_bracketed_list, "a {\n empty: [];\n}\n"); +error!( + invalid_item_in_space_separated_list, + "a {\n color: red color * #abc;\n}\n", "Error: Undefined operation \"color * #abc\"." +); +error!( + invalid_item_in_comma_separated_list, + "a {\n color: red, color * #abc;\n}\n", "Error: Undefined operation \"color * #abc\"." +); +error!( + invalid_item_in_space_separated_list_inside_interpolation, + "a {\n color: #{red color * #abc};\n}\n", "Error: Undefined operation \"color * #abc\"." +); +error!( + invalid_item_in_comma_separated_list_inside_interpolation, + "a {\n color: #{red, color * #abc};\n}\n", "Error: Undefined operation \"color * #abc\"." +); From de8e7048d8b09efa4adbc5cf11e8b303880f9818 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 01:13:23 -0400 Subject: [PATCH 13/18] remove unwrap from variable value parsing --- src/parse/variable.rs | 4 +++- tests/variables.rs | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/parse/variable.rs b/src/parse/variable.rs index a66fe1d..2321f91 100644 --- a/src/parse/variable.rs +++ b/src/parse/variable.rs @@ -123,7 +123,9 @@ impl<'a> Parser<'a> { } Some(..) | None => {} } - val_toks.push(self.toks.next().unwrap()); + if let Some(tok) = self.toks.next() { + val_toks.push(tok); + } } '{' => break, '}' => { diff --git a/tests/variables.rs b/tests/variables.rs index bad5ee6..b697a48 100644 --- a/tests/variables.rs +++ b/tests/variables.rs @@ -143,3 +143,7 @@ error!( invalid_escape, "$\\110000: red;", "Error: Invalid escape sequence." ); +error!( + nothing_after_hash_in_variable_decl, + "$color: #", "Error: Expected identifier." +); From 5fc3748472323878bce6b8a5e072dfc98b6274ce Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 03:09:24 -0400 Subject: [PATCH 14/18] remove most remaining unwraps --- src/parse/args.rs | 12 ++++++--- src/parse/function.rs | 5 +++- src/parse/mixin.rs | 5 +++- src/parse/mod.rs | 44 +++++++++++++++++++++--------- src/parse/value.rs | 2 +- src/utils/number.rs | 6 +---- src/utils/peek_until.rs | 59 +++++++++++++++++++++++----------------- src/utils/read_until.rs | 60 +++++++++++++++++++++++------------------ src/value/mod.rs | 2 +- tests/args.rs | 6 ++++- tests/at-root.rs | 4 +++ tests/each.rs | 4 +++ tests/functions.rs | 4 +++ tests/if.rs | 4 +++ tests/mixins.rs | 4 +++ tests/url.rs | 12 +++++++++ tests/while.rs | 5 +++- 17 files changed, 159 insertions(+), 79 deletions(-) diff --git a/src/parse/args.rs b/src/parse/args.rs index e42abe5..e2422c6 100644 --- a/src/parse/args.rs +++ b/src/parse/args.rs @@ -15,7 +15,10 @@ use super::Parser; impl<'a> Parser<'a> { pub(super) fn parse_func_args(&mut self) -> SassResult<FuncArgs> { let mut args: Vec<FuncArg> = Vec::new(); - let mut close_paren_span: Span = self.toks.peek().unwrap().pos(); + let mut close_paren_span: Span = match self.toks.peek() { + Some(Token { pos, .. }) => *pos, + None => return Err(("expected \")\".", self.span_before).into()), + }; self.whitespace(); while let Some(Token { kind, pos }) = self.toks.next() { @@ -130,9 +133,10 @@ impl<'a> Parser<'a> { .ok_or(("expected \")\".", self.span_before))? .pos(); loop { - match self.toks.peek() { - Some(Token { kind: '$', .. }) => { - let Token { pos, .. } = self.toks.next().unwrap(); + match self.toks.peek().cloned() { + Some(Token { kind: '$', pos }) => { + span = span.merge(pos); + self.toks.next(); let v = self.parse_identifier_no_interpolation(false)?; let whitespace = self.whitespace_or_comment(); if let Some(Token { kind: ':', .. }) = self.toks.peek() { diff --git a/src/parse/function.rs b/src/parse/function.rs index 20aed43..f9d54a8 100644 --- a/src/parse/function.rs +++ b/src/parse/function.rs @@ -35,7 +35,10 @@ impl<'a> Parser<'a> { self.whitespace(); let mut body = read_until_closing_curly_brace(self.toks)?; - body.push(self.toks.next().unwrap()); + body.push(match self.toks.next() { + Some(tok) => tok, + None => return Err(("expected \"}\".", self.span_before).into()), + }); self.whitespace(); let function = Function::new(self.scopes.last().clone(), args, body, span); diff --git a/src/parse/mixin.rs b/src/parse/mixin.rs index ff64b0c..e40102d 100644 --- a/src/parse/mixin.rs +++ b/src/parse/mixin.rs @@ -29,7 +29,10 @@ impl<'a> Parser<'a> { self.whitespace(); let mut body = read_until_closing_curly_brace(self.toks)?; - body.push(self.toks.next().unwrap()); + body.push(match self.toks.next() { + Some(tok) => tok, + None => return Err(("expected \"}\".", self.span_before).into()), + }); // todo: `@include` can only give content when `@content` is present within the body // if `@content` is *not* present and `@include` attempts to give a body, we throw an error diff --git a/src/parse/mod.rs b/src/parse/mod.rs index fbfd008..bc5c1ea 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -171,8 +171,9 @@ impl<'a> Parser<'a> { span, } = self.parse_value_from_vec(toks)?; span.merge(kind_string.span); - if let Some(Token { kind: ';', .. }) = self.toks.peek() { - kind_string.span.merge(self.toks.next().unwrap().pos()); + if let Some(Token { kind: ';', pos }) = self.toks.peek() { + kind_string.span.merge(*pos); + self.toks.next(); } self.warn(&Spanned { node: message.to_css_string(span)?, @@ -186,8 +187,9 @@ impl<'a> Parser<'a> { span, } = self.parse_value_from_vec(toks)?; span.merge(kind_string.span); - if let Some(Token { kind: ';', .. }) = self.toks.peek() { - kind_string.span.merge(self.toks.next().unwrap().pos()); + if let Some(Token { kind: ';', pos }) = self.toks.peek() { + kind_string.span.merge(*pos); + self.toks.next(); } self.debug(&Spanned { node: message.inspect(span)?, @@ -529,7 +531,11 @@ impl<'a> Parser<'a> { if let Some(tok) = self.toks.next() { self.whitespace(); match tok.kind.to_ascii_lowercase() { - 'i' if self.toks.next().unwrap().kind.to_ascii_lowercase() == 'f' => { + 'i' if matches!( + self.toks.peek(), + Some(Token { kind: 'f', .. }) | Some(Token { kind: 'F', .. }) + ) => + { self.toks.next(); let cond = read_until_open_curly_brace(self.toks)?; self.toks.next(); @@ -610,10 +616,10 @@ impl<'a> Parser<'a> { _ => return Err(("expected \"$\".", self.span_before).into()), }; self.whitespace(); - if self.toks.peek().is_none() { - return Err(("Expected \"from\".", var.span).into()); - } - self.span_before = self.toks.peek().unwrap().pos; + self.span_before = match self.toks.peek() { + Some(tok) => tok.pos, + None => return Err(("Expected \"from\".", var.span).into()), + }; if self.parse_identifier()?.node.to_ascii_lowercase() != "from" { return Err(("Expected \"from\".", var.span).into()); } @@ -644,7 +650,10 @@ impl<'a> Parser<'a> { '{' => { return Err(("Expected \"to\" or \"through\".", tok.pos()).into()); } - _ => from_toks.push(self.toks.next().unwrap()), + _ => { + from_toks.push(tok); + self.toks.next(); + } } } self.whitespace(); @@ -766,7 +775,10 @@ impl<'a> Parser<'a> { let mut body = read_until_closing_curly_brace(self.toks)?; - body.push(self.toks.next().unwrap()); + body.push(match self.toks.next() { + Some(tok) => tok, + None => return Err(("expected \"}\".", self.span_before).into()), + }); self.whitespace(); @@ -860,7 +872,10 @@ impl<'a> Parser<'a> { self.toks.next(); self.whitespace(); let mut body = read_until_closing_curly_brace(self.toks)?; - body.push(self.toks.next().unwrap()); + body.push(match self.toks.next() { + Some(tok) => tok, + None => return Err(("expected \"}\".", self.span_before).into()), + }); self.whitespace(); let mut stmts = Vec::new(); @@ -1102,7 +1117,10 @@ impl<'a> Parser<'a> { self.whitespace(); let mut body = read_until_closing_curly_brace(self.toks)?; - body.push(self.toks.next().unwrap()); + body.push(match self.toks.next() { + Some(tok) => tok, + None => return Err(("expected \"}\".", self.span_before).into()), + }); self.whitespace(); diff --git a/src/parse/value.rs b/src/parse/value.rs index 827fb02..b861d70 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -710,7 +710,7 @@ impl<'a> Parser<'a> { } fn peek_interpolation(&mut self) -> SassResult<(Spanned<Value>, usize)> { - let vec = peek_until_closing_curly_brace(self.toks); + let vec = peek_until_closing_curly_brace(self.toks)?; let peek_counter = vec.len(); self.toks.move_forward(1); let val = self.parse_value_from_vec(vec)?; diff --git a/src/utils/number.rs b/src/utils/number.rs index 2d4a57d..704982a 100644 --- a/src/utils/number.rs +++ b/src/utils/number.rs @@ -51,11 +51,7 @@ pub(crate) fn eat_number<I: Iterator<Item = Token>>( ) -> SassResult<Spanned<ParsedNumber>> { let mut whole = String::with_capacity(1); // TODO: merge this span with chars - let span = if let Some(tok) = toks.peek() { - tok.pos() - } else { - todo!() - }; + let span = toks.peek().unwrap().pos; eat_whole_number(toks, &mut whole); if toks.peek().is_none() { diff --git a/src/utils/peek_until.rs b/src/utils/peek_until.rs index 36e31fe..faa1039 100644 --- a/src/utils/peek_until.rs +++ b/src/utils/peek_until.rs @@ -8,19 +8,19 @@ use super::{as_hex, hex_char_for, is_name, is_name_start, IsWhitespace}; pub(crate) fn peek_until_closing_curly_brace<I: Iterator<Item = Token>>( toks: &mut PeekMoreIterator<I>, -) -> Vec<Token> { +) -> SassResult<Vec<Token>> { let mut t = Vec::new(); let mut nesting = 0; - while let Some(tok) = toks.peek() { + while let Some(tok) = toks.peek().cloned() { match tok.kind { q @ '"' | q @ '\'' => { - t.push(*toks.peek().unwrap()); + t.push(tok); toks.move_forward(1); - t.extend(peek_until_closing_quote(toks, q)); + t.extend(peek_until_closing_quote(toks, q)?); } '{' => { nesting += 1; - t.push(*toks.peek().unwrap()); + t.push(tok); toks.move_forward(1); } '}' => { @@ -28,62 +28,71 @@ pub(crate) fn peek_until_closing_curly_brace<I: Iterator<Item = Token>>( break; } else { nesting -= 1; - t.push(*toks.peek().unwrap()); + t.push(tok); toks.move_forward(1); } } '/' => { - let next = *toks.peek_forward(1).unwrap(); - match toks.peek().unwrap().kind { - '/' => peek_until_newline(toks), + let next = *toks + .peek_forward(1) + .ok_or(("Expected expression.", tok.pos))?; + match toks.peek() { + Some(Token { kind: '/', .. }) => peek_until_newline(toks), _ => t.push(next), }; continue; } _ => { - t.push(*toks.peek().unwrap()); + t.push(tok); toks.move_forward(1); } } } peek_whitespace(toks); - t + Ok(t) } fn peek_until_closing_quote<I: Iterator<Item = Token>>( toks: &mut PeekMoreIterator<I>, q: char, -) -> Vec<Token> { +) -> SassResult<Vec<Token>> { let mut t = Vec::new(); - while let Some(tok) = toks.peek() { + while let Some(tok) = toks.peek().cloned() { match tok.kind { '"' if q == '"' => { - t.push(*tok); + t.push(tok); toks.move_forward(1); break; } '\'' if q == '\'' => { - t.push(*tok); + t.push(tok); toks.move_forward(1); break; } '\\' => { - t.push(*tok); - t.push(*toks.peek_forward(1).unwrap()); + t.push(tok); + t.push(match toks.peek_forward(1) { + Some(tok) => *tok, + None => return Err((format!("Expected {}.", q), tok.pos).into()), + }); } '#' => { - t.push(*tok); - let next = toks.peek().unwrap(); + t.push(tok); + let next = match toks.peek() { + Some(tok) => tok, + None => return Err((format!("Expected {}.", q), tok.pos).into()), + }; if next.kind == '{' { - t.push(*toks.peek_forward(1).unwrap()); - t.append(&mut peek_until_closing_curly_brace(toks)); + t.push(*next); + toks.peek_forward(1); + t.append(&mut peek_until_closing_curly_brace(toks)?); } } - _ => t.push(*tok), + _ => t.push(tok), } toks.move_forward(1); } - t + Ok(t) } fn peek_until_newline<I: Iterator<Item = Token>>(toks: &mut PeekMoreIterator<I>) { @@ -166,13 +175,13 @@ pub(crate) fn peek_ident_no_interpolation<I: Iterator<Item = Token>>( .ok_or(("Expected identifier.", span_before))? .pos(); let mut text = String::new(); - if toks.peek().unwrap().kind == '-' { + if let Some(Token { kind: '-', .. }) = toks.peek() { toks.peek_forward(1); text.push('-'); if toks.peek().is_none() { return Ok(Spanned { node: text, span }); } - if toks.peek().unwrap().kind == '-' { + if let Some(Token { kind: '-', .. }) = toks.peek() { toks.peek_forward(1); text.push('-'); text.push_str(&peek_ident_body_no_interpolation(toks, unit, span)?.node); diff --git a/src/utils/read_until.rs b/src/utils/read_until.rs index 81e49d2..9a18054 100644 --- a/src/utils/read_until.rs +++ b/src/utils/read_until.rs @@ -28,10 +28,10 @@ pub(crate) fn read_until_open_curly_brace<I: Iterator<Item = Token>>( } '\\' => { t.push(toks.next().unwrap()); - if toks.peek().is_some() { - t.push(toks.next().unwrap()); - } - continue; + t.push(match toks.next() { + Some(tok) => tok, + None => continue, + }); } q @ '"' | q @ '\'' => { t.push(toks.next().unwrap()); @@ -89,9 +89,10 @@ pub(crate) fn read_until_closing_curly_brace<I: Iterator<Item = Token>>( } '\\' => { t.push(toks.next().unwrap()); - if toks.peek().is_some() { - t.push(toks.next().unwrap()); - } + t.push(match toks.next() { + Some(tok) => tok, + None => continue, + }); } _ => t.push(toks.next().unwrap()), } @@ -120,16 +121,21 @@ pub(crate) fn read_until_closing_quote<I: Iterator<Item = Token>>( } '\\' => { t.push(tok); - if toks.peek().is_some() { - t.push(toks.next().unwrap()); - } + t.push(match toks.next() { + Some(tok) => tok, + None => return Err((format!("Expected {}.", q), tok.pos).into()), + }); } '#' => { t.push(tok); - let next = toks.peek().unwrap(); - if next.kind == '{' { - t.push(toks.next().unwrap()); - t.append(&mut read_until_closing_curly_brace(toks)?); + match toks.peek() { + Some(tok @ Token { kind: '{', .. }) => { + t.push(*tok); + toks.next(); + t.append(&mut read_until_closing_curly_brace(toks)?); + } + Some(..) => continue, + None => return Err((format!("Expected {}.", q), tok.pos).into()), } } _ => t.push(tok), @@ -156,9 +162,10 @@ pub(crate) fn read_until_semicolon_or_closing_curly_brace<I: Iterator<Item = Tok } '\\' => { t.push(toks.next().unwrap()); - if toks.peek().is_some() { - t.push(toks.next().unwrap()); - } + t.push(match toks.next() { + Some(tok) => tok, + None => continue, + }); } '"' | '\'' => { let quote = toks.next().unwrap(); @@ -217,11 +224,11 @@ pub(crate) fn read_until_closing_paren<I: Iterator<Item = Token>>( continue; } '\\' => { - t.push(tok); - if toks.peek().is_some() { - t.push(toks.next().unwrap()); - } - continue; + t.push(toks.next().unwrap()); + t.push(match toks.next() { + Some(tok) => tok, + None => continue, + }); } _ => {} } @@ -253,10 +260,11 @@ pub(crate) fn read_until_closing_square_brace<I: Iterator<Item = Token>>( continue; } '\\' => { - t.push(tok); - if toks.peek().is_some() { - t.push(toks.next().unwrap()); - } + t.push(toks.next().unwrap()); + t.push(match toks.next() { + Some(tok) => tok, + None => continue, + }); } _ => {} } diff --git a/src/value/mod.rs b/src/value/mod.rs index 716e71a..800de76 100644 --- a/src/value/mod.rs +++ b/src/value/mod.rs @@ -148,7 +148,7 @@ impl Value { Self::List(vals, sep, brackets) => match brackets { Brackets::None => Cow::owned( vals.iter() - .filter(|x| !x.clone().is_null(span).unwrap_or(false)) + .filter(|x| !x.is_null(span).unwrap_or(false)) .map(|x| x.to_css_string(span)) .collect::<SassResult<Vec<Cow<'static, str>>>>()? .join(sep.as_str()), diff --git a/tests/args.rs b/tests/args.rs index 04a7569..21354e9 100644 --- a/tests/args.rs +++ b/tests/args.rs @@ -46,7 +46,11 @@ test!( "a {\n color: 2;\n}\n" ); error!( - #[ignore = "does not fail"] + #[ignore = "expects incorrect char, '{'"] nothing_after_open, "a { color:rgb(; }", "Error: expected \")\"." ); +error!( + nothing_after_open_paren_in_fn_args, + "@function foo(", "Error: expected \")\"." +); diff --git a/tests/at-root.rs b/tests/at-root.rs index f704ecb..c919255 100644 --- a/tests/at-root.rs +++ b/tests/at-root.rs @@ -68,3 +68,7 @@ test!( "a {}\n\n@at-root {\n @-ms-viewport { width: device-width; }\n}\n", "@-ms-viewport {\n width: device-width;\n}\n" ); +error!( + missing_closing_curly_brace, + "@at-root {", "Error: expected \"}\"." +); diff --git a/tests/each.rs b/tests/each.rs index 59220f7..aa406c7 100644 --- a/tests/each.rs +++ b/tests/each.rs @@ -48,3 +48,7 @@ test!( "a {\n @each $i in 1 2 3 {\n color: type-of($i);\n }\n}\n", "a {\n color: number;\n color: number;\n color: number;\n}\n" ); +error!( + missing_closing_curly_brace, + "@each $i in 1 {", "Error: expected \"}\"." +); diff --git a/tests/functions.rs b/tests/functions.rs index d9de492..aa8d96c 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -95,3 +95,7 @@ error!( double_comma_args, "@function foo($a,$b,,) {}", "Error: expected \")\"." ); +error!( + body_missing_closing_curly_brace, + "@function foo() {", "Error: expected \"}\"." +); diff --git a/tests/if.rs b/tests/if.rs index 13a0f43..f8a5fcd 100644 --- a/tests/if.rs +++ b/tests/if.rs @@ -149,3 +149,7 @@ error!(unclosed_sgl_quote, "@if true ' {}", "Error: Expected '."); error!(unclosed_call_args, "@if a({}", "Error: expected \")\"."); error!(nothing_after_div, "@if a/", "Error: Expected expression."); error!(multiline_error, "@if \"\n\"{}", "Error: Expected \"."); +error!( + nothing_after_i_after_else, + "@if true {} @else i", "Error: expected \"{\"." +); diff --git a/tests/mixins.rs b/tests/mixins.rs index 28a2e21..c92e338 100644 --- a/tests/mixins.rs +++ b/tests/mixins.rs @@ -233,6 +233,10 @@ error!( undefined_mixin, "a {@include foo;}", "Error: Undefined mixin." ); +error!( + body_missing_closing_curly_brace, + "@mixin foo() {", "Error: expected \"}\"." +); test!( include_empty_args_no_semicolon, "@mixin c {}\n\na {\n @include c()\n}\n", diff --git a/tests/url.rs b/tests/url.rs index a83b3c3..0373825 100644 --- a/tests/url.rs +++ b/tests/url.rs @@ -134,3 +134,15 @@ test!( "a {\n color: UrL(http://foo);\n}\n", "a {\n color: url(http://foo);\n}\n" ); +error!( + url_nothing_after_forward_slash_in_interpolation, + "a { color: url(#{/", "Error: Expected expression." +); +error!( + url_nothing_after_backslash_in_interpolation_in_quote, + "a { color: url(#{\"\\", "Error: Expected \"." +); +error!( + url_nothing_after_hash_in_interpolation_in_quote, + "a { color: url(#{\"#", "Error: Expected \"." +); diff --git a/tests/while.rs b/tests/while.rs index 4725a72..3af9ac5 100644 --- a/tests/while.rs +++ b/tests/while.rs @@ -107,7 +107,6 @@ test!( }", "result {\n root_default: initial;\n root_implicit: inner;\n root_explicit: inner;\n}\n" ); - test!( if_inside_while, "$continue_outer: true; @@ -126,3 +125,7 @@ test!( }", "a {\n color: red;\n}\n\na {\n color: blue;\n}\n" ); +error!( + missing_closing_curly_brace, + "@while true {", "Error: expected \"}\"." +); From f5e4c7e2269206d6c3215aea4b3309db74b54929 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 16:54:19 -0400 Subject: [PATCH 15/18] fix regression in mixin scoping --- CHANGELOG.md | 3 ++- src/parse/mixin.rs | 2 ++ tests/mixins.rs | 5 +++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bdde4f..6831f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # 0.9.1 - - fix issue in which `@at-root` would panic when placed after a ruleset + - fix regression in which `@at-root` would panic when placed after a ruleset + - fix regression related to `@mixin` scoping and outer, local variables # 0.9.0 diff --git a/src/parse/mixin.rs b/src/parse/mixin.rs index e40102d..32b589c 100644 --- a/src/parse/mixin.rs +++ b/src/parse/mixin.rs @@ -116,6 +116,8 @@ impl<'a> Parser<'a> { } .parse()?; + self.scopes.pop(); + Ok(body) } diff --git a/tests/mixins.rs b/tests/mixins.rs index c92e338..a5cf865 100644 --- a/tests/mixins.rs +++ b/tests/mixins.rs @@ -242,3 +242,8 @@ test!( "@mixin c {}\n\na {\n @include c()\n}\n", "" ); +test!( + local_variable_declared_before_mixin_is_still_in_scope, + "@mixin foo {}\n\na {\n $a: red;\n @include foo;\n color: $a;\n}\n", + "a {\n color: red;\n}\n" +); From 947b4a3e15e7784dd756430b3997ad8729c47e95 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 17:18:31 -0400 Subject: [PATCH 16/18] resolve regression in function scoping --- src/parse/function.rs | 12 +++++++++--- src/parse/mixin.rs | 7 +++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/parse/function.rs b/src/parse/function.rs index f9d54a8..efafff8 100644 --- a/src/parse/function.rs +++ b/src/parse/function.rs @@ -61,8 +61,9 @@ impl<'a> Parser<'a> { } pub fn eval_function(&mut self, mut function: Function, args: CallArgs) -> SassResult<Value> { - self.scopes.push(self.scopes.last().clone()); self.eval_fn_args(&mut function, args)?; + self.scopes.push(function.scope); + let mut return_value = Parser { toks: &mut function.body.into_iter().peekmore(), map: self.map, @@ -79,7 +80,9 @@ impl<'a> Parser<'a> { at_root_has_selector: self.at_root_has_selector, } .parse()?; + self.scopes.pop(); + debug_assert!(return_value.len() <= 1); match return_value .pop() @@ -91,11 +94,12 @@ impl<'a> Parser<'a> { } fn eval_fn_args(&mut self, function: &mut Function, mut args: CallArgs) -> SassResult<()> { + self.scopes.push(self.scopes.last().clone()); for (idx, arg) in function.args.0.iter_mut().enumerate() { if arg.is_variadic { let span = args.span(); let arg_list = Value::ArgList(self.variadic_args(args)?); - self.scopes.last_mut().insert_var( + function.scope.insert_var( arg.name.clone(), Spanned { node: arg_list, @@ -117,8 +121,10 @@ impl<'a> Parser<'a> { }; self.scopes .last_mut() - .insert_var(mem::take(&mut arg.name), val)?; + .insert_var(arg.name.clone(), val.clone())?; + function.scope.insert_var(mem::take(&mut arg.name), val)?; } + self.scopes.pop(); Ok(()) } } diff --git a/src/parse/mixin.rs b/src/parse/mixin.rs index 32b589c..0a34aec 100644 --- a/src/parse/mixin.rs +++ b/src/parse/mixin.rs @@ -126,7 +126,7 @@ impl<'a> Parser<'a> { } fn eval_mixin_args(&mut self, mixin: &mut Mixin, mut args: CallArgs) -> SassResult<()> { - let mut scope = self.scopes.last().clone(); + self.scopes.push(self.scopes.last().clone()); for (idx, arg) in mixin.args.0.iter_mut().enumerate() { if arg.is_variadic { let span = args.span(); @@ -152,9 +152,12 @@ impl<'a> Parser<'a> { } }, }; - scope.insert_var(arg.name.clone(), val.clone())?; + self.scopes + .last_mut() + .insert_var(arg.name.clone(), val.clone())?; mixin.scope.insert_var(mem::take(&mut arg.name), val)?; } + self.scopes.pop(); Ok(()) } } From 1a5301d0fa1ef27cfd397f258f20f8bb1289ad42 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Thu, 18 Jun 2020 18:14:35 -0400 Subject: [PATCH 17/18] resolve regression in `@mixin` scoping --- CHANGELOG.md | 3 ++- src/parse/common.rs | 8 ++++++++ src/parse/function.rs | 8 +++----- src/parse/mixin.rs | 11 +++-------- src/parse/variable.rs | 5 +++++ tests/functions.rs | 22 ++++++++++++++++++++++ 6 files changed, 43 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6831f9a..3bdc0e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # 0.9.1 + - fix regression in which `@at-root` would panic when placed after a ruleset - - fix regression related to `@mixin` scoping and outer, local variables + - fix regression related to `@mixin` and `@function` scoping when combined with outer, local variables # 0.9.0 diff --git a/src/parse/common.rs b/src/parse/common.rs index d8bbaa5..a34c093 100644 --- a/src/parse/common.rs +++ b/src/parse/common.rs @@ -25,6 +25,14 @@ impl<T> NeverEmptyVec<T> { self.rest.last_mut().unwrap_or(&mut self.first) } + pub fn first(&mut self) -> &T { + &self.first + } + + pub fn first_mut(&mut self) -> &mut T { + &mut self.first + } + pub fn push(&mut self, value: T) { self.rest.push(value) } diff --git a/src/parse/function.rs b/src/parse/function.rs index efafff8..a8f26a1 100644 --- a/src/parse/function.rs +++ b/src/parse/function.rs @@ -3,7 +3,6 @@ use std::mem; use codemap::Spanned; use peekmore::PeekMore; -use super::{Parser, Stmt}; use crate::{ args::CallArgs, atrule::Function, @@ -13,6 +12,8 @@ use crate::{ Token, }; +use super::{NeverEmptyVec, Parser, Stmt}; + impl<'a> Parser<'a> { pub(super) fn parse_function(&mut self) -> SassResult<()> { if self.in_mixin { @@ -62,13 +63,12 @@ impl<'a> Parser<'a> { pub fn eval_function(&mut self, mut function: Function, args: CallArgs) -> SassResult<Value> { self.eval_fn_args(&mut function, args)?; - self.scopes.push(function.scope); let mut return_value = Parser { toks: &mut function.body.into_iter().peekmore(), map: self.map, path: self.path, - scopes: self.scopes, + scopes: &mut NeverEmptyVec::new(function.scope), global_scope: self.global_scope, super_selectors: self.super_selectors, span_before: self.span_before, @@ -81,8 +81,6 @@ impl<'a> Parser<'a> { } .parse()?; - self.scopes.pop(); - debug_assert!(return_value.len() <= 1); match return_value .pop() diff --git a/src/parse/mixin.rs b/src/parse/mixin.rs index 0a34aec..9a14f9e 100644 --- a/src/parse/mixin.rs +++ b/src/parse/mixin.rs @@ -6,13 +6,12 @@ use crate::{ args::{CallArgs, FuncArgs}, atrule::Mixin, error::SassResult, - scope::Scope, utils::read_until_closing_curly_brace, value::Value, Token, }; -use super::{Parser, Stmt}; +use super::{NeverEmptyVec, Parser, Stmt}; impl<'a> Parser<'a> { pub(super) fn parse_mixin(&mut self) -> SassResult<()> { @@ -41,7 +40,7 @@ impl<'a> Parser<'a> { // this is blocked on figuring out just how to check for this. presumably we could have a check // not when parsing initially, but rather when `@include`ing to see if an `@content` was found. - let mixin = Mixin::new(Scope::new(), args, body, false); + let mixin = Mixin::new(self.scopes.last().clone(), args, body, false); if self.at_root { self.global_scope.insert_mixin(name, mixin); @@ -97,13 +96,11 @@ impl<'a> Parser<'a> { let mut mixin = self.scopes.last().get_mixin(name, self.global_scope)?; self.eval_mixin_args(&mut mixin, args)?; - self.scopes.push(mixin.scope); - let body = Parser { toks: &mut mixin.body, map: self.map, path: self.path, - scopes: self.scopes, + scopes: &mut NeverEmptyVec::new(mixin.scope), global_scope: self.global_scope, super_selectors: self.super_selectors, span_before: self.span_before, @@ -116,8 +113,6 @@ impl<'a> Parser<'a> { } .parse()?; - self.scopes.pop(); - Ok(body) } diff --git a/src/parse/variable.rs b/src/parse/variable.rs index 2321f91..e95e5a1 100644 --- a/src/parse/variable.rs +++ b/src/parse/variable.rs @@ -81,6 +81,11 @@ impl<'a> Parser<'a> { scope.insert_var(ident.clone(), value.value.clone())?; } } + if self.scopes.first().var_exists_no_global(&ident) { + self.scopes + .first_mut() + .insert_var(ident.clone(), value.value.clone())?; + } self.scopes.last_mut().insert_var(ident, value.value)?; } Ok(()) diff --git a/tests/functions.rs b/tests/functions.rs index aa8d96c..783a040 100644 --- a/tests/functions.rs +++ b/tests/functions.rs @@ -99,3 +99,25 @@ error!( body_missing_closing_curly_brace, "@function foo() {", "Error: expected \"}\"." ); +test!( + does_not_modify_local_variables, + "@function bar($color-name) { + @if $color-name==bar { + @error bar; + } + + $color: bar; + @return null; + } + + @function foo($a, $b) { + @return \"success!\"; + } + + a { + $color: foo; + + color: foo(bar($color), bar($color)); + }", + "a {\n color: \"success!\";\n}\n" +); From 47c4a421ac6d8b1670b9d5525d11f92f71718321 Mon Sep 17 00:00:00 2001 From: ConnorSkees <39542938+ConnorSkees@users.noreply.github.com> Date: Fri, 19 Jun 2020 22:47:06 -0400 Subject: [PATCH 18/18] upgrade dependencies --- Cargo.toml | 20 ++++++++++---------- src/parse/ident.rs | 2 +- src/parse/mod.rs | 4 ++-- src/parse/style.rs | 8 ++++---- src/parse/value.rs | 12 ++++++------ src/parse/variable.rs | 2 +- src/selector/attribute.rs | 4 ++-- src/selector/parse.rs | 4 ++-- src/utils/number.rs | 2 +- src/utils/peek_until.rs | 18 +++++++++--------- 10 files changed, 38 insertions(+), 38 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ac2165a..405c2ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ required-features = ["commandline"] [lib] name = "grass" path = "src/lib.rs" -crate-type = ["cdylib", "rlib"] +# crate-type = ["cdylib", "rlib"] bench = false [[bench]] @@ -45,15 +45,15 @@ harness = false [dependencies] -clap = { version = "2.33.0", optional = true } -num-rational = "0.2.3" -num-bigint = "0.2.6" -num-traits = "0.2.11" -once_cell = "1.3.1" +clap = { version = "2.33.1", optional = true } +num-rational = "0.3.0" +num-bigint = "0.3.0" +num-traits = "0.2.12" +once_cell = "1.4.0" rand = { version = "0.7.3", optional = true } codemap = "0.1.3" -peekmore = "0.4.0" -wasm-bindgen = { version = "0.2.60", optional = true } +peekmore = "0.5.1" +wasm-bindgen = { version = "0.2.63", optional = true } beef = "0.4.4" # criterion is not a dev-dependency because it makes tests take too # long to compile, and you cannot make dev-dependencies optional @@ -76,8 +76,8 @@ profiling = [] bench = ["criterion"] [dev-dependencies] -tempfile = "3" -paste = "0.1" +tempfile = "3.1.0" +paste = "0.1.17" [profile.release] debug = true diff --git a/src/parse/ident.rs b/src/parse/ident.rs index 793fe5c..b2adaff 100644 --- a/src/parse/ident.rs +++ b/src/parse/ident.rs @@ -66,7 +66,7 @@ impl<'a> Parser<'a> { let interpolation = self.parse_interpolation()?; buf.push_str(&interpolation.node.to_css_string(interpolation.span)?); } else { - self.toks.reset_view(); + self.toks.reset_cursor(); break; } } diff --git a/src/parse/mod.rs b/src/parse/mod.rs index bc5c1ea..bb69261 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -479,7 +479,7 @@ impl<'a> Parser<'a> { self.read_until_newline(); } _ => { - self.toks.reset_view(); + self.toks.reset_cursor(); return found_whitespace; } }, @@ -520,7 +520,7 @@ impl<'a> Parser<'a> { self.toks.peek_forward(1); let ident = peek_ident_no_interpolation(self.toks, false, pos)?; if ident.as_str() != "else" { - self.toks.reset_view(); + self.toks.reset_cursor(); break; } self.toks.take(4).for_each(drop); diff --git a/src/parse/style.rs b/src/parse/style.rs index 18c98ee..18dd5e1 100644 --- a/src/parse/style.rs +++ b/src/parse/style.rs @@ -49,11 +49,11 @@ impl<'a> Parser<'a> { while let Some(tok) = self.toks.peek() { match tok.kind { ';' | '}' => { - self.toks.reset_view(); + self.toks.reset_cursor(); break; } '{' => { - self.toks.reset_view(); + self.toks.reset_cursor(); return None; } '(' => { @@ -98,10 +98,10 @@ impl<'a> Parser<'a> { if let Some(first_char) = self.toks.peek() { if first_char.kind == '#' { if !matches!(self.toks.peek_forward(1), Some(Token { kind: '{', .. })) { - self.toks.reset_view(); + self.toks.reset_cursor(); return Ok(SelectorOrStyle::Selector(String::new())); } - self.toks.reset_view(); + self.toks.reset_cursor(); } else if !is_name_start(first_char.kind) && first_char.kind != '-' { return Ok(SelectorOrStyle::Selector(String::new())); } diff --git a/src/parse/value.rs b/src/parse/value.rs index b861d70..abc6a0d 100644 --- a/src/parse/value.rs +++ b/src/parse/value.rs @@ -382,10 +382,10 @@ impl<'a> Parser<'a> { '#' => { if let Some(Token { kind: '{', pos }) = self.toks.peek_forward(1) { self.span_before = *pos; - self.toks.reset_view(); + self.toks.reset_cursor(); return Some(self.parse_ident_value()); } - self.toks.reset_view(); + self.toks.reset_cursor(); self.toks.next(); let hex = match self.parse_hex() { Ok(v) => v, @@ -660,7 +660,7 @@ impl<'a> Parser<'a> { peek_counter += peek_whitespace(self.toks); while let Some(tok) = self.toks.peek() { let kind = tok.kind; - self.toks.move_forward(1); + self.toks.advance_cursor(); peek_counter += 1; if kind == '!' || kind == '%' @@ -673,7 +673,7 @@ impl<'a> Parser<'a> { buf.push_str(&self.peek_escape()?); } else if kind == '#' { if let Some(Token { kind: '{', .. }) = self.toks.peek() { - self.toks.move_forward(1); + self.toks.advance_cursor(); peek_counter += 1; let (interpolation, count) = self.peek_interpolation()?; peek_counter += count; @@ -705,14 +705,14 @@ impl<'a> Parser<'a> { break; } } - self.toks.reset_view(); + self.toks.reset_cursor(); Ok(None) } fn peek_interpolation(&mut self) -> SassResult<(Spanned<Value>, usize)> { let vec = peek_until_closing_curly_brace(self.toks)?; let peek_counter = vec.len(); - self.toks.move_forward(1); + self.toks.advance_cursor(); let val = self.parse_value_from_vec(vec)?; Ok(( Spanned { diff --git a/src/parse/variable.rs b/src/parse/variable.rs index e95e5a1..18b703d 100644 --- a/src/parse/variable.rs +++ b/src/parse/variable.rs @@ -171,7 +171,7 @@ impl<'a> Parser<'a> { default = true; } "important" => { - self.toks.reset_view(); + self.toks.reset_cursor(); val_toks.push(self.toks.next().unwrap()); continue; } diff --git a/src/selector/attribute.rs b/src/selector/attribute.rs index abac689..2646feb 100644 --- a/src/selector/attribute.rs +++ b/src/selector/attribute.rs @@ -46,14 +46,14 @@ fn attribute_name(parser: &mut Parser<'_>, start: Span) -> SassResult<QualifiedN } match parser.toks.peek_forward(1) { Some(v) if v.kind == '=' => { - parser.toks.reset_view(); + parser.toks.reset_cursor(); return Ok(QualifiedName { ident: name_or_namespace.node, namespace: Namespace::None, }); } Some(..) => { - parser.toks.reset_view(); + parser.toks.reset_cursor(); } None => return Err(("expected more input.", name_or_namespace.span).into()), } diff --git a/src/selector/parse.rs b/src/selector/parse.rs index 17f3043..c592589 100644 --- a/src/selector/parse.rs +++ b/src/selector/parse.rs @@ -222,11 +222,11 @@ impl<'a, 'b> SelectorParser<'a, 'b> { match self.parser.toks.peek_forward(1) { Some(Token { kind, .. }) if is_name_start(*kind) || kind == &'-' || kind == &'\\' => { - self.parser.toks.reset_view(); + self.parser.toks.reset_cursor(); true } Some(..) | None => { - self.parser.toks.reset_view(); + self.parser.toks.reset_cursor(); false } } diff --git a/src/utils/number.rs b/src/utils/number.rs index 704982a..1394c32 100644 --- a/src/utils/number.rs +++ b/src/utils/number.rs @@ -106,7 +106,7 @@ pub(crate) fn eat_number<I: Iterator<Item = Token>>( break; } - toks.reset_view(); + toks.reset_cursor(); whole.push_str(&dec); diff --git a/src/utils/peek_until.rs b/src/utils/peek_until.rs index faa1039..9f5b307 100644 --- a/src/utils/peek_until.rs +++ b/src/utils/peek_until.rs @@ -15,13 +15,13 @@ pub(crate) fn peek_until_closing_curly_brace<I: Iterator<Item = Token>>( match tok.kind { q @ '"' | q @ '\'' => { t.push(tok); - toks.move_forward(1); + toks.advance_cursor(); t.extend(peek_until_closing_quote(toks, q)?); } '{' => { nesting += 1; t.push(tok); - toks.move_forward(1); + toks.advance_cursor(); } '}' => { if nesting == 0 { @@ -29,7 +29,7 @@ pub(crate) fn peek_until_closing_curly_brace<I: Iterator<Item = Token>>( } else { nesting -= 1; t.push(tok); - toks.move_forward(1); + toks.advance_cursor(); } } '/' => { @@ -44,7 +44,7 @@ pub(crate) fn peek_until_closing_curly_brace<I: Iterator<Item = Token>>( } _ => { t.push(tok); - toks.move_forward(1); + toks.advance_cursor(); } } } @@ -61,12 +61,12 @@ fn peek_until_closing_quote<I: Iterator<Item = Token>>( match tok.kind { '"' if q == '"' => { t.push(tok); - toks.move_forward(1); + toks.advance_cursor(); break; } '\'' if q == '\'' => { t.push(tok); - toks.move_forward(1); + toks.advance_cursor(); break; } '\\' => { @@ -90,7 +90,7 @@ fn peek_until_closing_quote<I: Iterator<Item = Token>>( } _ => t.push(tok), } - toks.move_forward(1); + toks.advance_cursor(); } Ok(t) } @@ -100,7 +100,7 @@ fn peek_until_newline<I: Iterator<Item = Token>>(toks: &mut PeekMoreIterator<I>) if tok.kind == '\n' { break; } - toks.move_forward(1); + toks.advance_cursor(); } } @@ -111,7 +111,7 @@ fn peek_whitespace<I: Iterator<Item = W>, W: IsWhitespace>(s: &mut PeekMoreItera break; } found_whitespace = true; - s.move_forward(1); + s.advance_cursor(); } found_whitespace }