fix(parser): handle comments, whitespace, and scripts between bare JSX siblings in Astro expressions

Author: Princesseuh

crates/oxc_parser/src/astro/jsx.rs

@@ -473,18 +473,33 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
     /// Astro-specific: Peek ahead past `<` to check if the next token starts a JSX element
     /// or fragment. Used in binary expression context to distinguish `<div>` from `< comparison`.
     ///
-    /// Returns `true` if `<` is followed by an identifier, keyword, or `>` (fragment).
-    /// Does not consume any tokens.
+    /// Returns `true` if `<` is followed by an identifier, keyword, `>` (fragment), or a
+    /// complete `<!-- -->` comment. Does not consume any tokens.
     pub(crate) fn is_astro_jsx_after_lt(&mut self) -> bool {
         let checkpoint = self.checkpoint();
         self.bump_any(); // bump `<`
         let next_kind = self.cur_kind();
-        let is_jsx =
-            next_kind == Kind::RAngle || next_kind == Kind::Ident || next_kind.is_any_keyword();
+        let is_jsx = next_kind == Kind::RAngle
+            || next_kind == Kind::Ident
+            || next_kind.is_any_keyword()
+            || self.at_astro_html_comment();
         self.rewind(checkpoint);
         is_jsx
     }
 
+    /// `true` when the bytes right after the just-bumped `<` open a complete
+    /// `<!-- … -->` comment, letting it continue a run of sibling JSX.
+    ///
+    /// Requiring a real `-->` keeps the JS `< !--x` (negated pre-decrement) a
+    /// comparison, and stops the binary parser looping on a `<!--` it can't consume.
+    fn at_astro_html_comment(&self) -> bool {
+        // `prev_token_end` is the byte right after `<` (no whitespace skipped),
+        // so a `< !--` comparison is excluded; only `<!--` matches.
+        self.source_text
+            .get(self.prev_token_end as usize..)
+            .is_some_and(|rest| rest.starts_with("!--") && rest[3..].contains("-->"))
+    }
+
     /// Astro-specific: Parse multiple JSX elements in binary expression context.
     pub(crate) fn parse_astro_multiple_jsx_in_expression(
         &mut self,
@@ -502,17 +517,44 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
         }
 
         while self.at(Kind::LAngle) {
+            // The JS lexer skips whitespace between siblings; record where the
+            // gap starts so it can be preserved (see the helper call below).
+            let ws_start = self.prev_token_end;
             let checkpoint = self.checkpoint();
             let child_span = self.start_span();
             self.bump_any(); // bump `<`
 
             let kind = self.cur_kind();
             if kind == Kind::RAngle {
+                self.push_astro_inter_sibling_whitespace(&mut children, ws_start, child_span);
                 let fragment = self.parse_astro_jsx_fragment(child_span, false);
                 children.push(JSXChild::Fragment(fragment));
             } else if kind == Kind::Ident || kind.is_any_keyword() {
-                let element = self.parse_astro_jsx_element(child_span, false);
-                children.push(JSXChild::Element(element));
+                self.push_astro_inter_sibling_whitespace(&mut children, ws_start, child_span);
+                // `<script>` needs the dedicated raw-text path like every other
+                // JSX-children site; otherwise its body parses as JSX and a close
+                // tag in a template literal (`` `</article>` ``) becomes a stray error.
+                if self.cur_src() == "script" {
+                    children.push(self.parse_astro_script_in_jsx(child_span));
+                } else {
+                    let element = self.parse_astro_jsx_element(child_span, false);
+                    children.push(JSXChild::Element(element));
+                }
+            } else if kind == Kind::Bang
+                && let Some(comment) = self.parse_html_comment_in_jsx(child_span)
+            {
+                // HTML comment in a sibling run. The comment parser leaves the lexer
+                // in JSX-child mode, but this loop is JS-expression context. Resync to
+                // JS tokens so the closing `)` ends the list instead of being read as text.
+                self.push_astro_inter_sibling_whitespace(&mut children, ws_start, child_span);
+                let comment_end = comment.span().end;
+                self.lexer.set_position_for_astro(comment_end);
+                self.token = self.lexer.next_token();
+                // Resync bypasses the parser's bump bookkeeping; point `prev_token_end`
+                // at the comment end so the next iteration captures the whitespace
+                // between the comment and the following sibling.
+                self.prev_token_end = comment_end;
+                children.push(comment);
             } else {
                 self.rewind(checkpoint);
                 break;
@@ -534,6 +576,27 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
         Expression::JSXFragment(fragment)
     }
 
+    /// Push the source text in `[start, end)` as a `JSXText` child when it is
+    /// non-empty whitespace. Used to preserve whitespace between bare JSX
+    /// siblings in an expression, which the JS lexer would otherwise drop.
+    fn push_astro_inter_sibling_whitespace(
+        &self,
+        children: &mut Vec<'a, JSXChild<'a>>,
+        start: u32,
+        end: u32,
+    ) {
+        if end <= start {
+            return;
+        }
+        let span = Span::new(start, end);
+        let ws = span.source_text(self.source_text);
+        if !ws.bytes().all(|b| b.is_ascii_whitespace()) {
+            return;
+        }
+        let text = self.ast.alloc_jsx_text(span, Atom::from(ws), Some(Atom::from(ws)));
+        children.push(JSXChild::Text(text));
+    }
+
     // ==================== Astro-specific helpers ====================
 
     /// Try to parse Astro shorthand attribute `{prop}` -> `prop={prop}`
@@ -576,7 +639,10 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
 
         let expr_start = self.cur_token().span().start;
         let expr = self.parse_expr();
-        let name_span = Span::new(expr_start, self.prev_token_end);
+        // `parse_expr` can fail without advancing (e.g. the next token is `)`),
+        // leaving `prev_token_end < expr_start`. Clamp so the name span is never
+        // inverted; `source_text()` panics on an inverted span.
+        let name_span = Span::new(expr_start, self.prev_token_end.max(expr_start));
         self.expect(Kind::RCurly);
 
         let name = Atom::from(name_span.source_text(self.source_text).trim());
@@ -1092,10 +1158,14 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
     pub(crate) fn parse_html_comment_in_jsx(&mut self, span: u32) -> Option<JSXChild<'a>> {
         let start_pos = self.prev_token_end as usize;
 
+        // Search for the closing `-->` *after* the opening `!--` (offset >= 3).
+        // Otherwise an overlapping marker like `<!-->` would match `-->` at
+        // offset 1 and slice `rest[3..1]`, panicking the parser on malformed input.
         if let Some(rest) = self.source_text.get(start_pos..)
             && rest.starts_with("!--")
-            && let Some(end_offset) = rest.find("-->")
+            && let Some(rel) = rest[3..].find("-->")
         {
+            let end_offset = rel + 3;
             let comment_end = (start_pos + end_offset + 3) as u32;
             let comment_start = span;
 

crates/oxc_parser/src/astro/mod.rs

@@ -143,15 +143,14 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
         // Check if this is a comment (starts with `<!--`) or a doctype (starts with `<!doctype` or `<!DOCTYPE`)
         if let Some(rest) = self.source_text.get(start_pos..) {
             // Check for HTML comment `<!--`
-            if rest.starts_with("!--") {
-                // Find `-->` to close the comment
-                if let Some(end_offset) = rest.find("-->") {
-                    let comment_end = (start_pos + end_offset + 3) as u32;
+            if let Some(after_open) = rest.strip_prefix("!--") {
+                // Close must come after the open; an overlapping `<!-->` is not a
+                // real comment (and naive search would slice backwards and panic).
+                if let Some(content_len) = after_open.find("-->") {
+                    let comment_end = (start_pos + 3 + content_len + 3) as u32;
                     let comment_start = span; // `<` position
 
-                    // Extract comment content (between `<!--` and `-->`)
-                    // rest starts with "!--", so content starts at index 3
-                    let content = &rest[3..end_offset];
+                    let content = &after_open[..content_len];
                     let value = oxc_span::Atom::from(content);
 
                     // Create the AstroComment node
@@ -438,7 +437,8 @@ impl<'a, C: ParserConfig> ParserImpl<'a, C> {
 mod test {
     use oxc_allocator::Allocator;
     use oxc_ast::ast::{
-        JSXAttributeItem, JSXAttributeName, JSXChild, JSXElementName, JSXExpression, Statement,
+        Expression, JSXAttributeItem, JSXAttributeName, JSXChild, JSXElementName, JSXExpression,
+        Statement,
     };
     use oxc_span::SourceType;
 
@@ -5014,4 +5014,122 @@ console.log(msg);
             main.children.iter().filter(|c| matches!(c, JSXChild::Element(_))).count();
         assert_eq!(element_children, 2, "expected <div> and <p> as children of <main>");
     }
+
+    #[test]
+    fn parse_astro_second_script_in_expression_is_raw_text() {
+        // A non-first `<script>` sibling in a `{ ... }` expression must still parse
+        // its body as raw text, or a close tag in a template literal becomes a
+        // stray closing-tag error.
+        let allocator = Allocator::default();
+        let source_type = SourceType::astro();
+        let source = "{enabled && (\n  <script src=\"x.js\"></script>\n  <script>var a = `</article>`;</script>\n)}";
+        let ret = Parser::new(&allocator, source, source_type).parse_astro();
+        assert!(!ret.panicked);
+        assert!(ret.errors.is_empty(), "errors: {:?}", ret.errors);
+    }
+
+    #[test]
+    fn parse_astro_bare_siblings_in_expression_keep_whitespace() {
+        // Whitespace between bare JSX siblings in a `{ ... }` expression must
+        // survive as a JSXText child (the JS lexer skips it otherwise).
+        let allocator = Allocator::default();
+        let source_type = SourceType::astro();
+        let source = "{x && (<em>a</em>\n  <span>b</span>)}";
+        let ret = Parser::new(&allocator, source, source_type).parse_astro();
+        assert!(ret.errors.is_empty(), "errors: {:?}", ret.errors);
+
+        let JSXChild::ExpressionContainer(container) = &ret.root.body[0] else {
+            panic!("expected expression container");
+        };
+        let Some(Expression::LogicalExpression(logical)) = container.expression.as_expression()
+        else {
+            panic!("expected `&&` logical expression");
+        };
+        let right = match &logical.right {
+            Expression::ParenthesizedExpression(paren) => &paren.expression,
+            other => other,
+        };
+        let Expression::JSXFragment(fragment) = right else {
+            panic!("expected implicit fragment for bare siblings, got {right:?}");
+        };
+        let text_children = fragment
+            .children
+            .iter()
+            .filter(|c| matches!(c, JSXChild::Text(_)))
+            .count();
+        assert_eq!(text_children, 1, "inter-sibling whitespace should be a JSXText node");
+    }
+
+    #[test]
+    fn parse_astro_overlapping_comment_marker_does_not_panic() {
+        // `<!-->` overlaps the `-->` with the `<!--` open; the comment parsers must
+        // look for the close at offset >= 3, not slice `rest[3..1]` and panic.
+        let allocator = Allocator::default();
+        let source_type = SourceType::astro();
+        for source in ["{<a/><!-->}", "<!-->", "<div><!--></div>", "{x && (<a/><!-->)}"] {
+            // The contract is "no panic"; some of these are still errors.
+            let _ = Parser::new(&allocator, source, source_type).parse_astro();
+        }
+    }
+
+    #[test]
+    fn parse_astro_comment_between_bare_siblings() {
+        // An HTML comment in a run of bare JSX siblings is kept as an AstroComment
+        // child (matching Go), including the common case of the comment on its own
+        // line, which only works because the lexer no longer line-comments `<!--`.
+        let allocator = Allocator::default();
+        let source_type = SourceType::astro();
+        for source in [
+            "{x && (<a/><!--c--><b/>)}",
+            "{x && (\n  <a/>\n  <!-- c -->\n  <b/>\n)}",
+            "{x && (<a/><!--c-->)}",
+        ] {
+            let ret = Parser::new(&allocator, source, source_type).parse_astro();
+            assert!(ret.errors.is_empty(), "{source:?} errors: {:?}", ret.errors);
+
+            let JSXChild::ExpressionContainer(container) = &ret.root.body[0] else {
+                panic!("expected expression container for {source:?}");
+            };
+            let Some(Expression::LogicalExpression(logical)) =
+                container.expression.as_expression()
+            else {
+                panic!("expected `&&` logical expression for {source:?}");
+            };
+            let right = match &logical.right {
+                Expression::ParenthesizedExpression(paren) => &paren.expression,
+                other => other,
+            };
+            let Expression::JSXFragment(fragment) = right else {
+                panic!("expected implicit fragment for {source:?}, got {right:?}");
+            };
+            let comments =
+                fragment.children.iter().filter(|c| matches!(c, JSXChild::AstroComment(_))).count();
+            assert_eq!(comments, 1, "{source:?} should keep the comment as a child");
+        }
+    }
+
+    #[test]
+    fn parse_astro_jsx_lt_comparison_still_parses() {
+        // The comment heuristic only diverts when the left side is a JSX element, so
+        // an ordinary JS `< !--` (less-than, negated pre-decrement) is untouched even
+        // when a `-->` appears later. Must not be misread as a comment.
+        let allocator = Allocator::default();
+        let source_type = SourceType::astro();
+        for source in ["{(3 < !--n, m-->q)}", "{x && y < !--count}"] {
+            let ret = Parser::new(&allocator, source, source_type).parse_astro();
+            assert!(ret.errors.is_empty(), "{source:?} should parse: {:?}", ret.errors);
+        }
+    }
+
+    #[test]
+    fn parse_astro_shorthand_attribute_fatal_expr_does_not_panic() {
+        // Guards the inverted name-span panic when a shorthand-attribute `{…}`
+        // expression fails to parse without advancing the lexer.
+        let allocator = Allocator::default();
+        let source_type = SourceType::astro();
+        for source in ["<div><div{\n)</div>", "{x && (<a/>\n<div{\n)}", "<Comp {(\n} />"] {
+            // The contract is "no panic"; these are still errors.
+            let _ = Parser::new(&allocator, source, source_type).parse_astro();
+        }
+    }
 }

crates/oxc_parser/src/lexer/punctuation.rs

@@ -34,13 +34,16 @@ impl<C: Config> Lexer<'_, C> {
             }
             // `<!--` HTML comment (Annex B.1.1)
             Some(b'!') if self.remaining().starts_with("!--") => {
-                if self.source_type.is_module() {
+                if self.source_type.is_astro() {
+                    // In Astro, `<!--` is always an HTML comment for the template/JSX
+                    // parser (matched to `-->`), never the legacy JS line comment that
+                    // skips to EOL. Emit `<` so it reaches that parser regardless of
+                    // newline position.
+                    Some(Kind::LAngle)
+                } else if self.source_type.is_module() {
                     if self.token.is_on_new_line() {
-                        // In Astro files, HTML comments are valid even in module/expression context.
-                        if !self.source_type.is_astro() {
-                            let span = Span::new(self.token.start(), self.token.start() + 4);
-                            self.errors.push(diagnostics::html_comment_in_module(span));
-                        }
+                        let span = Span::new(self.token.start(), self.token.start() + 4);
+                        self.errors.push(diagnostics::html_comment_in_module(span));
                         None
                     } else {
                         // In middle of expression (e.g. `foo <!--bar`) - parse as `<`