Skip to content

Commit 7f8b771

Browse files
keithamuskeithamus
committed
fix broken selector parsing
1 parent 16febfe commit 7f8b771

File tree

3 files changed

+108
-74
lines changed

3 files changed

+108
-74
lines changed

‎crates/hdx_ast/src/css/selector/attribute.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use hdx_atom::{atom, Atom};
22
use hdx_lexer::Token;
3-
use hdx_parser::{expect, unexpected, unexpected_ident, Parse, Parser, Result as ParserResult, Spanned};
3+
use hdx_parser::{discard, expect, unexpected, unexpected_ident, Parse, Parser, Result as ParserResult, Spanned};
44
use hdx_writer::{CssWriter, Result as WriterResult, WriteCss};
55
#[cfg(feature = "serde")]
66
use serde::Serialize;
@@ -24,6 +24,9 @@ impl<'a> Parse<'a> for Attribute {
2424
Token::LeftSquare => {
2525
parser.advance();
2626
let (ns_prefix, name) = parse_wq_name(parser)?;
27+
// parse_wq_name advances including WS/Comments so we should discard
28+
// any as attribute selectors aren't WS sensitive.
29+
discard!(parser, Token::Whitespace | Token::Comment(_));
2730
let matcher = match parser.cur() {
2831
Token::RightSquare => {
2932
parser.advance_including_whitespace_and_comments();
@@ -178,13 +181,28 @@ pub enum AttributeModifier {
178181

179182
#[cfg(test)]
180183
mod tests {
184+
use oxc_allocator::Allocator;
181185

182186
use super::*;
187+
use crate::test_helpers::test_write;
183188

184189
#[test]
185190
fn size_test() {
186191
assert_eq!(::std::mem::size_of::<Attribute>(), 40);
187192
assert_eq!(::std::mem::size_of::<AttributeMatch>(), 1);
188193
assert_eq!(::std::mem::size_of::<AttributeMatch>(), 1);
189194
}
195+
196+
#[test]
197+
fn test_writes() {
198+
let allocator = Allocator::default();
199+
test_write::<Attribute>(&allocator, "[foo]", "[foo]");
200+
test_write::<Attribute>(&allocator, "[foo='bar']", "[foo=\"bar\"]");
201+
test_write::<Attribute>(&allocator, "[foo = 'bar']", "[foo=\"bar\"]");
202+
test_write::<Attribute>(&allocator, "[attr*='foo']", "[attr*=\"foo\"]");
203+
test_write::<Attribute>(&allocator, "[|attr='foo']", "[attr=\"foo\"]");
204+
test_write::<Attribute>(&allocator, "[*|attr='foo']", "[*|attr=\"foo\"]");
205+
test_write::<Attribute>(&allocator, "[x|attr='foo']", "[x|attr=\"foo\"]");
206+
test_write::<Attribute>(&allocator, "[attr|='foo']", "[attr|=\"foo\"]");
207+
}
190208
}

‎crates/hdx_ast/src/css/selector/mod.rs

+87-71
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use hdx_atom::Atom;
22
use hdx_lexer::Token;
3-
use hdx_parser::{diagnostics, Parse, Parser, Result as ParserResult, Span, Spanned, unexpected_ident, unexpected};
3+
use hdx_parser::{
4+
diagnostics, expect, unexpected, unexpected_ident, Parse, Parser, Result as ParserResult, Span, Spanned,
5+
};
46
use hdx_writer::{CssWriter, Result as WriterResult, WriteCss};
57
#[cfg(feature = "serde")]
68
use serde::Serialize;
@@ -37,37 +39,38 @@ impl<'a> Parse<'a> for Selector<'a> {
3739
break;
3840
}
3941
}
40-
_ => {}
41-
}
42-
let component = Component::parse(parser)?;
43-
if let Some(Spanned { node, span: component_span }) = components.last() {
44-
match (node, &component.node) {
45-
// A selector like `a /**/ b` would parse as // <Type>, <Descendant>,
46-
// <Descendant>, <Type>. The CSS selector grammar implicitly swallows adjacent
47-
// descendant combinators as whitespace, but due to simplifying AST nodes in our
48-
// parser, it means we must explicitly check for, and elide adjacent descendant
49-
// combinators. Adjacent Descendant Combinator Elision is the name of my metal
50-
// band, btw.
51-
(Component::Combinator(_), Component::Combinator(Combinator::Descendant))
52-
| (Component::Combinator(Combinator::Descendant), Component::Combinator(_)) => {
53-
continue;
54-
}
55-
// Combinators cannot be next to eachother.
56-
(Component::Combinator(_), Component::Combinator(_)) => {
57-
Err(diagnostics::AdjacentSelectorCombinators(
58-
*component_span,
59-
Span::new(span.start, component_span.start),
60-
))?
42+
_ => {
43+
let component = Component::parse(parser)?;
44+
if let Some(Spanned { node, span: component_span }) = components.last() {
45+
match (node, &component.node) {
46+
// A selector like `a /**/ b` would parse as // <Type>, <Descendant>,
47+
// <Descendant>, <Type>. The CSS selector grammar implicitly swallows adjacent
48+
// descendant combinators as whitespace, but due to simplifying AST nodes in our
49+
// parser, it means we must explicitly check for, and elide adjacent descendant
50+
// combinators. Adjacent Descendant Combinator Elision is the name of my metal
51+
// band, btw.
52+
(Component::Combinator(_), Component::Combinator(Combinator::Descendant))
53+
| (Component::Combinator(Combinator::Descendant), Component::Combinator(_)) => {
54+
continue;
55+
}
56+
// Combinators cannot be next to eachother.
57+
(Component::Combinator(_), Component::Combinator(_)) => {
58+
Err(diagnostics::AdjacentSelectorCombinators(
59+
*component_span,
60+
Span::new(span.start, component_span.start),
61+
))?
62+
}
63+
// Types cannot be next to eachother.
64+
(Component::Type(_), Component::Type(_)) => Err(diagnostics::AdjacentSelectorTypes(
65+
*component_span,
66+
Span::new(span.start, component_span.start),
67+
))?,
68+
_ => {}
69+
}
6170
}
62-
// Types cannot be next to eachother.
63-
(Component::Type(_), Component::Type(_)) => Err(diagnostics::AdjacentSelectorTypes(
64-
*component_span,
65-
Span::new(span.start, component_span.start),
66-
))?,
67-
_ => {}
71+
components.push(component);
6872
}
6973
}
70-
components.push(component);
7174
}
7275
Ok(Self { components: parser.boxup(components) }.spanned(span.end(parser.pos())))
7376
}
@@ -119,36 +122,36 @@ impl<'a> Parse<'a> for Component<'a> {
119122
let span = parser.span();
120123
match parser.cur() {
121124
Token::Whitespace => {
122-
parser.advance_including_whitespace_and_comments();
125+
parser.advance_including_whitespace();
123126
Ok(Self::Combinator(Combinator::Descendant).spanned(span.end(parser.pos())))
124127
}
125128
Token::Ident(name) => {
126-
parser.advance_including_whitespace_and_comments();
129+
parser.advance_including_whitespace();
127130
Ok(Self::Type(name.to_ascii_lowercase()).spanned(span))
128131
}
129132
Token::Colon => {
130-
parser.advance_including_whitespace_and_comments();
131-
match parser.cur(){
133+
parser.advance_including_whitespace();
134+
match parser.cur() {
132135
Token::Colon => {
133-
parser.advance_including_whitespace_and_comments();
136+
parser.advance_including_whitespace();
134137
match parser.cur() {
135138
Token::Ident(name) => {
136139
if let Some(selector) = PseudoElement::from_atom(name.clone()) {
137-
parser.advance_including_whitespace_and_comments();
140+
parser.advance_including_whitespace();
138141
Ok(Self::PseudoElement(selector).spanned(span.end(parser.pos())))
139142
} else {
140143
unexpected_ident!(parser, name)
141144
}
142145
}
143-
token => unexpected!(parser, token)
146+
token => unexpected!(parser, token),
144147
}
145148
}
146149
Token::Ident(ident) => {
147150
if let Some(selector) = PseudoClass::from_atom(ident.clone()) {
148-
parser.advance_including_whitespace_and_comments();
151+
parser.advance_including_whitespace();
149152
Ok(Self::PseudoClass(selector).spanned(span.end(parser.pos())))
150153
} else if let Some(e) = LegacyPseudoElement::from_atom(ident.clone()) {
151-
parser.advance_including_whitespace_and_comments();
154+
parser.advance_including_whitespace();
152155
Ok(Self::LegacyPseudoElement(e).spanned(span.end(parser.pos())))
153156
} else {
154157
Err(diagnostics::UnexpectedIdent(ident, parser.span()))?
@@ -158,32 +161,30 @@ impl<'a> Parse<'a> for Component<'a> {
158161
}
159162
}
160163
Token::Hash(name) => {
161-
parser.advance_including_whitespace_and_comments();
164+
parser.advance_including_whitespace();
162165
Ok(Self::Id(name).spanned(span.end(parser.pos())))
163166
}
164167
Token::Delim(char) => match char {
165168
'.' => {
166-
parser.advance_including_whitespace_and_comments();
169+
parser.advance_including_whitespace();
167170
match parser.cur() {
168171
Token::Ident(ident) => {
169-
parser.advance_including_whitespace_and_comments();
172+
parser.advance_including_whitespace();
170173
Ok(Self::Class(ident).spanned(span.end(parser.pos())))
171174
}
172175
_ => Err(diagnostics::Unimplemented(parser.span()))?,
173176
}
174177
}
175-
'*' => {
176-
match parser.peek() {
177-
Token::Delim('|') => {
178-
let (prefix, atom) = parse_wq_name(parser)?;
179-
Ok(Self::NSPrefixedType(parser.boxup((prefix, atom))).spanned(span.end(parser.pos())))
180-
}
181-
_ => {
182-
parser.advance_including_whitespace_and_comments();
183-
Ok(Self::Wildcard.spanned(span.end(parser.pos())))
184-
}
178+
'*' => match parser.peek() {
179+
Token::Delim('|') => {
180+
let (prefix, atom) = parse_wq_name(parser)?;
181+
Ok(Self::NSPrefixedType(parser.boxup((prefix, atom))).spanned(span.end(parser.pos())))
185182
}
186-
}
183+
_ => {
184+
parser.advance_including_whitespace();
185+
Ok(Self::Wildcard.spanned(span.end(parser.pos())))
186+
}
187+
},
187188
_ => Err(diagnostics::Unimplemented(parser.span()))?,
188189
},
189190
Token::LeftSquare => {
@@ -344,41 +345,44 @@ pub struct ANBEvenOdd {
344345

345346
pub(crate) fn parse_wq_name(parser: &mut Parser) -> ParserResult<(NSPrefix, Atom)> {
346347
let nsprefix = match parser.cur() {
347-
Token::Delim('|') => {
348-
parser.advance_including_whitespace_and_comments();
348+
Token::Delim('|') if matches!(parser.peek(), Token::Ident(_)) => {
349+
parser.advance_including_whitespace();
349350
NSPrefix::None
350-
},
351-
Token::Delim('*') => {
352-
parser.advance_including_whitespace_and_comments();
353-
match parser.cur() {
354-
Token::Delim('|') => {
355-
parser.advance_including_whitespace_and_comments();
356-
NSPrefix::Wildcard
357-
},
358-
token => unexpected!(parser, token),
359-
}
351+
}
352+
Token::Delim('*') if matches!(parser.peek(), Token::Delim('|')) => {
353+
parser.advance_including_whitespace();
354+
expect!(parser, Token::Delim('|'));
355+
parser.advance_including_whitespace();
356+
NSPrefix::Wildcard
360357
}
361358
Token::Ident(name) => {
362-
parser.advance_including_whitespace_and_comments();
359+
parser.advance_including_whitespace();
363360
match parser.cur() {
364-
Token::Delim('|') => {
365-
parser.advance_including_whitespace_and_comments();
361+
Token::Delim('|') if matches!(parser.peek(), Token::Ident(_)) => {
362+
parser.advance_including_whitespace();
366363
NSPrefix::Named(name)
367364
}
368-
_ => return Ok((NSPrefix::None, name))
365+
_ => return Ok((NSPrefix::None, name)),
369366
}
370-
},
367+
}
371368
token => unexpected!(parser, token),
372369
};
373370
match parser.cur() {
374-
Token::Ident(name) => Ok((nsprefix, name)),
375-
token => unexpected!(parser, token)
371+
Token::Ident(name) => {
372+
parser.advance_including_whitespace();
373+
Ok((nsprefix, name))
374+
}
375+
token => unexpected!(parser, token),
376376
}
377377
}
378378

379379
#[cfg(test)]
380380
mod test {
381+
use oxc_allocator::Allocator;
382+
381383
use super::*;
384+
use crate::test_helpers::test_write;
385+
382386
#[test]
383387
fn size_test() {
384388
assert_eq!(::std::mem::size_of::<Selector>(), 8);
@@ -394,4 +398,16 @@ mod test {
394398
assert_eq!(::std::mem::size_of::<ANB>(), 8);
395399
assert_eq!(::std::mem::size_of::<ANBEvenOdd>(), 8);
396400
}
401+
402+
#[test]
403+
fn test_writes() {
404+
let allocator = Allocator::default();
405+
test_write::<Component>(&allocator, ":root", ":root");
406+
test_write::<Component>(&allocator, "*", "*");
407+
test_write::<Component>(&allocator, "[attr|='foo']", "[attr|=\"foo\"]");
408+
// test_write::<Component>(&allocator, "*|x", "*|x");
409+
test_write::<Selector>(&allocator, ":root", ":root");
410+
test_write::<Selector>(&allocator, "body [attr|='foo']", "body [attr|=\"foo\"]");
411+
// test_write::<Selector>(&allocator, "*|x :focus-within", "*|x :focus-within");
412+
}
397413
}

‎crates/hdx_lexer/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,14 @@ impl<'a> Lexer<'a> {
115115

116116
pub fn advance_including_whitespace(&mut self) -> Token {
117117
self.include = Include::Whitespace;
118-
let token = self.read_next_token();
118+
let token = self.advance();
119119
self.include = Include::none();
120120
token
121121
}
122122

123123
pub fn advance_including_whitespace_and_comments(&mut self) -> Token {
124124
self.include = Include::all();
125-
let token = self.read_next_token();
125+
let token = self.advance();
126126
self.include = Include::none();
127127
token
128128
}

0 commit comments

Comments
 (0)