[css-nesting] Remove DeclarationListParser.

For nesting, we need to parse declaration and nested rules together.
Rather than having DeclarationListParser, organize the parsing code in
two:

 * StyleSheetParser for the top level stylesheet, which has the relevant
   hacks and so on.
 * RuleBodyParser which does potentially everything else.

For that, introduce a new trait that agglomerates all the nesting needs
(so potentially all of at-rule + qualified-rule + declaration parsing).

While at it, change the API a little bit so that these take mutable
references, so that we can avoid some copying.

Author: emilio

src/lib.rs

@@ -82,9 +82,9 @@ pub use crate::nth::parse_nth;
 pub use crate::parser::{BasicParseError, BasicParseErrorKind, ParseError, ParseErrorKind};
 pub use crate::parser::{Delimiter, Delimiters, Parser, ParserInput, ParserState};
 pub use crate::rules_and_declarations::{parse_important, parse_one_declaration};
-pub use crate::rules_and_declarations::{parse_one_rule, RuleListParser};
+pub use crate::rules_and_declarations::{parse_one_rule, StyleSheetParser};
 pub use crate::rules_and_declarations::{AtRuleParser, QualifiedRuleParser};
-pub use crate::rules_and_declarations::{DeclarationListParser, DeclarationParser};
+pub use crate::rules_and_declarations::{RuleBodyParser, RuleBodyItemParser, DeclarationParser};
 pub use crate::serializer::{serialize_identifier, serialize_name, serialize_string};
 pub use crate::serializer::{CssStringWriter, ToCss, TokenSerializationType};
 pub use crate::tokenizer::{SourceLocation, SourcePosition, Token};

src/rules_and_declarations.rs

@@ -50,14 +50,9 @@ pub trait DeclarationParser<'i> {
         &mut self,
         name: CowRcStr<'i>,
         input: &mut Parser<'i, 't>,
-    ) -> Result<Self::Declaration, ParseError<'i, Self::Error>>;
-
-    /// Whether to try to parse qualified rules along with declarations. See
-    /// <https://github.com/w3c/csswg-drafts/issues/7961> for the current state of the discussion.
-    /// This is a low effort opt-in to be able to experiment with it, but it's likely to be needed
-    /// when nesting is less experimental as well (e.g., you probably don't want to allow nesting
-    /// in a style attribute anyways).
-    fn enable_nesting(&self) -> bool { false }
+    ) -> Result<Self::Declaration, ParseError<'i, Self::Error>> {
+        Err(input.new_error(BasicParseErrorKind::UnexpectedToken(Token::Ident(name))))
+    }
 }
 
 /// A trait to provide various parsing of at-rules.
@@ -99,8 +94,6 @@ pub trait AtRuleParser<'i> {
         name: CowRcStr<'i>,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self::Prelude, ParseError<'i, Self::Error>> {
-        let _ = name;
-        let _ = input;
         Err(input.new_error(BasicParseErrorKind::AtRuleInvalid(name)))
     }
 
@@ -140,7 +133,6 @@ pub trait AtRuleParser<'i> {
     ) -> Result<Self::AtRule, ParseError<'i, Self::Error>> {
         let _ = prelude;
         let _ = start;
-        let _ = input;
         Err(input.new_error(BasicParseErrorKind::AtRuleBodyInvalid))
     }
 }
@@ -178,7 +170,6 @@ pub trait QualifiedRuleParser<'i> {
         &mut self,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self::Prelude, ParseError<'i, Self::Error>> {
-        let _ = input;
         Err(input.new_error(BasicParseErrorKind::QualifiedRuleInvalid))
     }
 
@@ -197,24 +188,35 @@ pub trait QualifiedRuleParser<'i> {
     ) -> Result<Self::QualifiedRule, ParseError<'i, Self::Error>> {
         let _ = prelude;
         let _ = start;
-        let _ = input;
         Err(input.new_error(BasicParseErrorKind::QualifiedRuleInvalid))
     }
 }
 
-/// Provides an iterator for declaration list parsing.
-pub struct DeclarationListParser<'i, 't, 'a, P> {
-    /// The input given to `DeclarationListParser::new`
+/// Provides an iterator for rule bodies and declaration lists.
+pub struct RuleBodyParser<'i, 't, 'a, P, I, E> {
+    /// The input given to the parser.
     pub input: &'a mut Parser<'i, 't>,
-
     /// The parser given to `DeclarationListParser::new`
-    pub parser: P,
+    pub parser: &'a mut P,
+
+    _phantom: std::marker::PhantomData<(I, E)>,
 }
 
-impl<'i, 't, 'a, I, P, E: 'i> DeclarationListParser<'i, 't, 'a, P>
-where
-    P: DeclarationParser<'i, Declaration = I, Error = E> + AtRuleParser<'i, AtRule = I, Error = E>,
+/// A parser for a rule body item.
+pub trait RuleBodyItemParser<'i, DeclOrRule, Error: 'i>
+    : DeclarationParser<'i, Declaration = DeclOrRule, Error = Error>
+    + QualifiedRuleParser<'i, QualifiedRule = DeclOrRule, Error = Error>
+    + AtRuleParser<'i, AtRule = DeclOrRule, Error = Error>
 {
+    /// Whether we should attempt to parse declarations. If you know you won't, returning false
+    /// here is slightly faster.
+    fn parse_declarations(&self) -> bool;
+    /// Whether we should attempt to parse qualified rules. If you know you won't, returning false
+    /// would be slightly faster.
+    fn parse_qualified(&self) -> bool;
+}
+
+impl<'i, 't, 'a, P, I, E> RuleBodyParser<'i, 't, 'a, P, I, E> {
     /// Create a new `DeclarationListParser` for the given `input` and `parser`.
     ///
     /// Note that all CSS declaration lists can on principle contain at-rules.
@@ -229,29 +231,27 @@ where
     /// The return type for finished declarations and at-rules also needs to be the same,
     /// since `<DeclarationListParser as Iterator>::next` can return either.
     /// It could be a custom enum.
-    pub fn new(input: &'a mut Parser<'i, 't>, parser: P) -> Self {
-        DeclarationListParser { input, parser }
+    pub fn new(input: &'a mut Parser<'i, 't>, parser: &'a mut P) -> Self {
+        Self { input, parser, _phantom: std::marker::PhantomData }
     }
 }
 
 /// `DeclarationListParser` is an iterator that yields `Ok(_)` for a valid declaration or at-rule
 /// or `Err(())` for an invalid one.
-impl<'i, 't, 'a, I, P, E: 'i> Iterator for DeclarationListParser<'i, 't, 'a, P>
+impl<'i, 't, 'a, I, P, E: 'i> Iterator for RuleBodyParser<'i, 't, 'a, P, I, E>
 where
-    P: DeclarationParser<'i, Declaration = I, Error = E>
-        + AtRuleParser<'i, AtRule = I, Error = E>
-        + QualifiedRuleParser<'i, QualifiedRule = I, Error = E>,
+    P: RuleBodyItemParser<'i, I, E>,
 {
     type Item = Result<I, (ParseError<'i, E>, &'i str)>;
 
     fn next(&mut self) -> Option<Self::Item> {
         loop {
             let start = self.input.state();
-            match self.input.next_including_whitespace_and_comments() {
-                Ok(&Token::WhiteSpace(_)) | Ok(&Token::Comment(_)) | Ok(&Token::Semicolon) => {
-                    continue
+            match self.input.next_including_whitespace_and_comments().ok()? {
+                Token::WhiteSpace(_) | Token::Comment(_) | Token::Semicolon => {
+                    continue;
                 }
-                Ok(&Token::Ident(ref name)) => {
+                Token::Ident(ref name) if self.parser.parse_declarations() => {
                     let name = name.clone();
                     let mut result = {
                         let parser = &mut self.parser;
@@ -261,23 +261,23 @@ where
                         })
                     };
 
-                    if result.is_err() && self.parser.enable_nesting() {
+                    if result.is_err() && self.parser.parse_qualified() {
                         self.input.reset(&start);
-                        result = parse_qualified_rule(&start, self.input, &mut self.parser);
+                        result = parse_qualified_rule(&start, self.input, &mut *self.parser);
                     }
 
                     return Some(result.map_err(|e| (e, self.input.slice_from(start.position()))));
                 }
-                Ok(&Token::AtKeyword(ref name)) => {
+                Token::AtKeyword(ref name) => {
                     let name = name.clone();
-                    return Some(parse_at_rule(&start, name, self.input, &mut self.parser));
+                    return Some(parse_at_rule(&start, name, self.input, &mut *self.parser));
                 }
-                Ok(token) => {
-                    let result = if self.parser.enable_nesting() {
+                token => {
+                    let result = if self.parser.parse_qualified() {
                         self.input.reset(&start);
                         // XXX do we need to, if we fail, consume only until the next semicolon,
                         // rather than until the next `{`?
-                        parse_qualified_rule(&start, self.input, &mut self.parser)
+                        parse_qualified_rule(&start, self.input, &mut *self.parser)
                     } else {
                         let token = token.clone();
                         self.input.parse_until_after(Delimiter::Semicolon, |_| {
@@ -286,66 +286,44 @@ where
                     };
                     return Some(result.map_err(|e| (e, self.input.slice_from(start.position()))));
                 }
-                Err(..) => return None,
             }
         }
     }
 }
 
-/// Provides an iterator for rule list parsing.
-pub struct RuleListParser<'i, 't, 'a, P> {
-    /// The input given to `RuleListParser::new`
+/// Provides an iterator for rule list parsing at the top-level of a stylesheet.
+pub struct StyleSheetParser<'i, 't, 'a, P> {
+    /// The input given.
     pub input: &'a mut Parser<'i, 't>,
 
-    /// The parser given to `RuleListParser::new`
-    pub parser: P,
+    /// The parser given.
+    pub parser: &'a mut P,
 
-    is_stylesheet: bool,
     any_rule_so_far: bool,
 }
 
-impl<'i, 't, 'a, R, P, E: 'i> RuleListParser<'i, 't, 'a, P>
+impl<'i, 't, 'a, R, P, E: 'i> StyleSheetParser<'i, 't, 'a, P>
 where
     P: QualifiedRuleParser<'i, QualifiedRule = R, Error = E>
         + AtRuleParser<'i, AtRule = R, Error = E>,
 {
-    /// Create a new `RuleListParser` for the given `input` at the top-level of a stylesheet
-    /// and the given `parser`.
-    ///
     /// The given `parser` needs to implement both `QualifiedRuleParser` and `AtRuleParser` traits.
-    /// However, either of them can be an empty `impl`
-    /// since the traits provide default implementations of their methods.
+    /// However, either of them can be an empty `impl` since the traits provide default
+    /// implementations of their methods.
     ///
     /// The return type for finished qualified rules and at-rules also needs to be the same,
-    /// since `<RuleListParser as Iterator>::next` can return either.
-    /// It could be a custom enum.
-    pub fn new_for_stylesheet(input: &'a mut Parser<'i, 't>, parser: P) -> Self {
-        RuleListParser {
-            input,
-            parser,
-            is_stylesheet: true,
-            any_rule_so_far: false,
-        }
-    }
-
-    /// Same is `new_for_stylesheet`, but should be used for rule lists inside a block
-    /// such as the body of an `@media` rule.
-    ///
-    /// This differs in that `<!--` and `-->` tokens
-    /// should only be ignored at the stylesheet top-level.
-    /// (This is to deal with legacy workarounds for `<style>` HTML element parsing.)
-    pub fn new_for_nested_rule(input: &'a mut Parser<'i, 't>, parser: P) -> Self {
-        RuleListParser {
+    /// since `<RuleListParser as Iterator>::next` can return either. It could be a custom enum.
+    pub fn new(input: &'a mut Parser<'i, 't>, parser: &'a mut P) -> Self {
+        Self {
             input,
             parser,
-            is_stylesheet: false,
             any_rule_so_far: false,
         }
     }
 }
 
 /// `RuleListParser` is an iterator that yields `Ok(_)` for a rule or `Err(())` for an invalid one.
-impl<'i, 't, 'a, R, P, E: 'i> Iterator for RuleListParser<'i, 't, 'a, P>
+impl<'i, 't, 'a, R, P, E: 'i> Iterator for StyleSheetParser<'i, 't, 'a, P>
 where
     P: QualifiedRuleParser<'i, QualifiedRule = R, Error = E>
         + AtRuleParser<'i, AtRule = R, Error = E>,
@@ -354,13 +332,8 @@ where
 
     fn next(&mut self) -> Option<Self::Item> {
         loop {
-            if self.is_stylesheet {
-                self.input.skip_cdc_and_cdo()
-            } else {
-                self.input.skip_whitespace()
-            }
+            self.input.skip_cdc_and_cdo();
             let start = self.input.state();
-
             let at_keyword = match self.input.next_byte()? {
                 b'@' => match self.input.next_including_whitespace_and_comments() {
                     Ok(&Token::AtKeyword(ref name)) => Some(name.clone()),
@@ -373,7 +346,7 @@ where
             };
 
             if let Some(name) = at_keyword {
-                let first_stylesheet_rule = self.is_stylesheet && !self.any_rule_so_far;
+                let first_stylesheet_rule = !self.any_rule_so_far;
                 self.any_rule_so_far = true;
                 if first_stylesheet_rule && name.eq_ignore_ascii_case("charset") {
                     let delimiters = Delimiter::Semicolon | Delimiter::CurlyBracketBlock;
@@ -384,13 +357,13 @@ where
                         &start,
                         name.clone(),
                         self.input,
-                        &mut self.parser,
-                    ));
+                        &mut *self.parser,
+                    ))
                 }
             } else {
                 self.any_rule_so_far = true;
-                let result = parse_qualified_rule(&start, self.input, &mut self.parser);
-                return Some(result.map_err(|e| (e, self.input.slice_from(start.position()))));
+                let result = parse_qualified_rule(&start, self.input, &mut *self.parser);
+                return Some(result.map_err(|e| (e, self.input.slice_from(start.position()))))
             }
         }
     }