Skip to content

Commit 7a1266c

Browse files
author
bors-servo
authored
Auto merge of #180 - upsuper:at-rule-parser, r=SimonSapin
Have at-rule without block handled in two stages as well This change makes `AtRuleParser::parse_prelude` always return a prelude if parsed successfully, so that at-rules which do not accept block can be handled in two phases just like those with block. This is important because at-rules without block usually have side effects. If `parse_prelude` executes the side effect, but `parse_at_rule` later determined that the rule is actually invalid (e.g. because it is followed by a block), there is no way for impl of `AtRuleParser` to undo the side effect. It provides the necessary parser side change for fixing [Gecko bug 1388911](https://bugzilla.mozilla.org/show_bug.cgi?id=1388911). <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-cssparser/180) <!-- Reviewable:end -->
2 parents 7560c3a + 08d510d commit 7a1266c

File tree

5 files changed

+61
-63
lines changed

5 files changed

+61
-63
lines changed

src/css-parsing-tests/one_rule.json

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@
77
"@foo", ["at-rule", "foo", [], null],
88

99
"@foo bar; \t/* comment */", ["at-rule", "foo", [" ", ["ident", "bar"]], null],
10-
" /**/ @foo bar{[(4", ["at-rule", "foo",
10+
" /**/ @foo-with-block bar{[(4", ["at-rule", "foo-with-block",
1111
[" ", ["ident", "bar"]],
1212
[["[]", ["()", ["number", "4", 4, "integer"]]]]
1313
],
1414

15-
"@foo { bar", ["at-rule", "foo", [" "], [" ", ["ident", "bar"]]],
16-
"@foo [ bar", ["at-rule", "foo", [" ", ["[]", " ", ["ident", "bar"]]], null],
15+
"@foo-with-block { bar", ["at-rule", "foo-with-block", [" "], [
16+
" ", ["ident", "bar"]]
17+
],
18+
"@foo [ bar", ["at-rule", "foo",
19+
[" ", ["[]", " ", ["ident", "bar"]]], null
20+
],
1721

1822
" /**/ div > p { color: #aaa; } /**/ ", ["qualified rule",
1923
[["ident", "div"], " ", ">", " ", ["ident", "p"], " "],

src/css-parsing-tests/rule_list.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,20 @@
77
"@foo", [["at-rule", "foo", [], null]],
88

99
"@charset; @foo", [
10-
["at-rule", "charset", [], null],
10+
["error", "invalid"],
1111
["at-rule", "foo", [], null]
1212
],
1313

1414
"@foo bar; \t/* comment */", [["at-rule", "foo", [" ", ["ident", "bar"]], null]],
1515

16-
" /**/ @foo bar{[(4", [["at-rule", "foo",
16+
" /**/ @foo-with-block bar{[(4", [["at-rule", "foo-with-block",
1717
[" ", ["ident", "bar"]],
1818
[["[]", ["()", ["number", "4", 4, "integer"]]]]
1919
]],
2020

21-
"@foo { bar", [["at-rule", "foo", [" "], [" ", ["ident", "bar"]]]],
21+
"@foo-with-block { bar", [["at-rule", "foo-with-block", [" "], [
22+
" ", ["ident", "bar"]]]
23+
],
2224
"@foo [ bar", [["at-rule", "foo", [" ", ["[]", " ", ["ident", "bar"]]], null]],
2325

2426
" /**/ div > p { color: #aaa; } /**/ ", [["qualified rule",

src/css-parsing-tests/stylesheet.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,19 @@
1010

1111
"@foo; @charset 4 {}", [
1212
["at-rule", "foo", [], null],
13-
["at-rule", "charset", [" ", ["number", "4", 4, "integer"], " "], []]
13+
["error", "invalid"]
1414
],
1515

1616
"@foo bar; \t/* comment */", [["at-rule", "foo", [" ", ["ident", "bar"]], null]],
1717

18-
" /**/ @foo bar{[(4", [["at-rule", "foo",
18+
" /**/ @foo-with-block bar{[(4", [["at-rule", "foo-with-block",
1919
[" ", ["ident", "bar"]],
2020
[["[]", ["()", ["number", "4", 4, "integer"]]]]
2121
]],
2222

23-
"@foo { bar", [["at-rule", "foo", [" "], [" ", ["ident", "bar"]]]],
23+
"@foo-with-block { bar", [["at-rule", "foo-with-block", [" "], [
24+
" ", ["ident", "bar"]]]
25+
],
2426
"@foo [ bar", [["at-rule", "foo", [" ", ["[]", " ", ["ident", "bar"]]], null]],
2527

2628
" /**/ div > p { color: #aaa; } /**/ ", [["qualified rule",

src/rules_and_declarations.rs

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,17 @@ pub fn parse_important<'i, 't>(input: &mut Parser<'i, 't>) -> Result<(), BasicPa
2323
/// The return value for `AtRuleParser::parse_prelude`.
2424
/// Indicates whether the at-rule is expected to have a `{ /* ... */ }` block
2525
/// or end with a `;` semicolon.
26-
pub enum AtRuleType<P, R> {
26+
pub enum AtRuleType<P, PB> {
2727
/// The at-rule is expected to end with a `;` semicolon. Example: `@import`.
2828
///
29-
/// The value is the finished representation of an at-rule
30-
/// as returned by `RuleListParser::next` or `DeclarationListParser::next`.
31-
WithoutBlock(R),
29+
/// The value is the representation of all data of the rule which would be
30+
/// handled in rule_without_block.
31+
WithoutBlock(P),
3232

3333
/// The at-rule is expected to have a a `{ /* ... */ }` block. Example: `@media`
3434
///
3535
/// The value is the representation of the "prelude" part of the rule.
36-
WithBlock(P),
37-
38-
/// The at-rule may either have a block or end with a semicolon.
39-
///
40-
/// This is mostly for testing. As of this writing no real CSS at-rule behaves like this.
41-
///
42-
/// The value is the representation of the "prelude" part of the rule.
43-
OptionalBlock(P),
36+
WithBlock(PB),
4437
}
4538

4639
/// A trait to provide various parsing of declaration values.
@@ -85,8 +78,11 @@ pub trait DeclarationParser<'i> {
8578
/// so that `impl AtRuleParser<(), ()> for ... {}` can be used
8679
/// for using `DeclarationListParser` to parse a declartions list with only qualified rules.
8780
pub trait AtRuleParser<'i> {
88-
/// The intermediate representation of an at-rule prelude.
89-
type Prelude;
81+
/// The intermediate representation of prelude of an at-rule without block;
82+
type PreludeNoBlock;
83+
84+
/// The intermediate representation of prelude of an at-rule with block;
85+
type PreludeBlock;
9086

9187
/// The finished representation of an at-rule.
9288
type AtRule;
@@ -112,36 +108,39 @@ pub trait AtRuleParser<'i> {
112108
/// that ends wherever the prelude should end.
113109
/// (Before the next semicolon, the next `{`, or the end of the current block.)
114110
fn parse_prelude<'t>(&mut self, name: CowRcStr<'i>, input: &mut Parser<'i, 't>)
115-
-> Result<AtRuleType<Self::Prelude, Self::AtRule>, ParseError<'i, Self::Error>> {
111+
-> Result<AtRuleType<Self::PreludeNoBlock, Self::PreludeBlock>,
112+
ParseError<'i, Self::Error>> {
116113
let _ = name;
117114
let _ = input;
118115
Err(ParseError::Basic(BasicParseError::AtRuleInvalid(name)))
119116
}
120117

118+
/// End an at-rule which doesn't have block. Return the finished
119+
/// representation of the at-rule.
120+
///
121+
/// This is only called when `parse_prelude` returned `WithoutBlock`, and
122+
/// either the `;` semicolon indeed follows the prelude, or parser is at
123+
/// the end of the input.
124+
fn rule_without_block(&mut self, prelude: Self::PreludeNoBlock) -> Self::AtRule {
125+
let _ = prelude;
126+
panic!("The `AtRuleParser::rule_without_block` method must be overriden \
127+
if `AtRuleParser::parse_prelude` ever returns `AtRuleType::WithoutBlock`.")
128+
}
129+
121130
/// Parse the content of a `{ /* ... */ }` block for the body of the at-rule.
122131
///
123132
/// Return the finished representation of the at-rule
124133
/// as returned by `RuleListParser::next` or `DeclarationListParser::next`,
125134
/// or `Err(())` to ignore the entire at-rule as invalid.
126135
///
127-
/// This is only called when `parse_prelude` returned `WithBlock` or `OptionalBlock`,
128-
/// and a block was indeed found following the prelude.
129-
fn parse_block<'t>(&mut self, prelude: Self::Prelude, input: &mut Parser<'i, 't>)
136+
/// This is only called when `parse_prelude` returned `WithBlock`, and a block
137+
/// was indeed found following the prelude.
138+
fn parse_block<'t>(&mut self, prelude: Self::PreludeBlock, input: &mut Parser<'i, 't>)
130139
-> Result<Self::AtRule, ParseError<'i, Self::Error>> {
131140
let _ = prelude;
132141
let _ = input;
133142
Err(ParseError::Basic(BasicParseError::AtRuleBodyInvalid))
134143
}
135-
136-
/// An `OptionalBlock` prelude was followed by `;`.
137-
///
138-
/// Convert the prelude into the finished representation of the at-rule
139-
/// as returned by `RuleListParser::next` or `DeclarationListParser::next`.
140-
fn rule_without_block(&mut self, prelude: Self::Prelude) -> Self::AtRule {
141-
let _ = prelude;
142-
panic!("The `AtRuleParser::rule_without_block` method must be overriden \
143-
if `AtRuleParser::parse_prelude` ever returns `AtRuleType::OptionalBlock`.")
144-
}
145144
}
146145

147146
/// A trait to provide various parsing of qualified rules.
@@ -460,9 +459,9 @@ fn parse_at_rule<'i: 't, 't, P, E>(start: &ParserState, name: CowRcStr<'i>,
460459
parser.parse_prelude(name, input)
461460
});
462461
match result {
463-
Ok(AtRuleType::WithoutBlock(rule)) => {
462+
Ok(AtRuleType::WithoutBlock(prelude)) => {
464463
match input.next() {
465-
Ok(&Token::Semicolon) | Err(_) => Ok(rule),
464+
Ok(&Token::Semicolon) | Err(_) => Ok(parser.rule_without_block(prelude)),
466465
Ok(&Token::CurlyBracketBlock) => Err(PreciseParseError {
467466
error: ParseError::Basic(BasicParseError::UnexpectedToken(Token::CurlyBracketBlock)),
468467
slice: input.slice_from(start.position()),
@@ -495,21 +494,6 @@ fn parse_at_rule<'i: 't, 't, P, E>(start: &ParserState, name: CowRcStr<'i>,
495494
Ok(_) => unreachable!()
496495
}
497496
}
498-
Ok(AtRuleType::OptionalBlock(prelude)) => {
499-
match input.next() {
500-
Ok(&Token::Semicolon) | Err(_) => Ok(parser.rule_without_block(prelude)),
501-
Ok(&Token::CurlyBracketBlock) => {
502-
// FIXME: https://github.com/rust-lang/rust/issues/42508
503-
parse_nested_block::<'i, 't, _, _, _>(input, move |input| parser.parse_block(prelude, input))
504-
.map_err(|e| PreciseParseError {
505-
error: e,
506-
slice: input.slice_from(start.position()),
507-
location: start.source_location(),
508-
})
509-
}
510-
_ => unreachable!()
511-
}
512-
}
513497
Err(error) => {
514498
let end_position = input.position();
515499
match input.next() {

src/tests.rs

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -745,29 +745,35 @@ impl<'i> DeclarationParser<'i> for JsonParser {
745745
}
746746

747747
impl<'i> AtRuleParser<'i> for JsonParser {
748-
type Prelude = Vec<Json>;
748+
type PreludeNoBlock = Vec<Json>;
749+
type PreludeBlock = Vec<Json>;
749750
type AtRule = Json;
750751
type Error = ();
751752

752753
fn parse_prelude<'t>(&mut self, name: CowRcStr<'i>, input: &mut Parser<'i, 't>)
753-
-> Result<AtRuleType<Vec<Json>, Json>, ParseError<'i, ()>> {
754-
Ok(AtRuleType::OptionalBlock(vec![
754+
-> Result<AtRuleType<Vec<Json>, Vec<Json>>, ParseError<'i, ()>> {
755+
let prelude = vec![
755756
"at-rule".to_json(),
756757
name.to_json(),
757758
Json::Array(component_values_to_json(input)),
758-
]))
759+
];
760+
match_ignore_ascii_case! { &*name,
761+
"media" | "foo-with-block" => Ok(AtRuleType::WithBlock(prelude)),
762+
"charset" => Err(BasicParseError::AtRuleInvalid(name.clone()).into()),
763+
_ => Ok(AtRuleType::WithoutBlock(prelude)),
764+
}
765+
}
766+
767+
fn rule_without_block(&mut self, mut prelude: Vec<Json>) -> Json {
768+
prelude.push(Json::Null);
769+
Json::Array(prelude)
759770
}
760771

761772
fn parse_block<'t>(&mut self, mut prelude: Vec<Json>, input: &mut Parser<'i, 't>)
762773
-> Result<Json, ParseError<'i, ()>> {
763774
prelude.push(Json::Array(component_values_to_json(input)));
764775
Ok(Json::Array(prelude))
765776
}
766-
767-
fn rule_without_block(&mut self, mut prelude: Vec<Json>) -> Json {
768-
prelude.push(Json::Null);
769-
Json::Array(prelude)
770-
}
771777
}
772778

773779
impl<'i> QualifiedRuleParser<'i> for JsonParser {

0 commit comments

Comments
 (0)