fix(formatter): treat lone CR in JSX text and argument blank lines (#27780)

Also disables BOM and shebang variants.
This commit is contained in:
leaysgur committed 2026-10-16 04:04:07 +00:00
1 parent 78b6f0260c
commit 6fe4c1b911
8 files changed
+32 -116

No files matched your search

@@ -26,6 +26,7 @@ use crate::{
function::FormatFunction,
parameters::has_only_simple_parameters,
},
source_text::line_breaks,
utils::{
call_expression::is_test_call_expression,
expression::as_call_expression_without_chain_wrappers, is_long_curried_call,
@@ -102,12 +103,8 @@ impl<'a> Format<'a, JsFormatContext<'a>> for AstNode<'a, ArenaVec<'a, Argument<'
let has_empty_line = arguments.windows(2).any(|window| {
let (cur_arg, next_arg) = (&window[0], &window[1]);
// Count newlines between arguments, short-circuiting at 2 for performance
// Check if there are at least two newlines between arguments
f.source_text()
.bytes_range(cur_arg.span().end, next_arg.span().start)
.iter()
.filter(|&&b| b == b'\n')
// Check if there are at least two line breaks between arguments
line_breaks(f.source_text().slice_range(cur_arg.span().end, next_arg.span().start))
.nth(1)
.is_some()
});
@@ -63,7 +63,10 @@ fn is_multiline_string_literal_attribute(attribute: &JSXAttributeItem<'_>) -> bo
let JSXAttributeItem::Attribute(attr) = attribute else {
return false;
};
attr.value.as_ref().is_some_and(|value| matches!(value, JSXAttributeValue::StringLiteral(string) if string.value.as_str().is_some_and(|value| value.contains('\n'))))
attr.value.as_ref().is_some_and(|value| matches!(
value,
JSXAttributeValue::StringLiteral(s) if s.value.as_str().is_some_and(|v| v.contains(['\n', '\r']))
))
}
impl<'a> Format<'a, JsFormatContext<'a>> for FormatOpeningElement<'a, '_> {
@@ -158,8 +161,9 @@ pub enum OpeningElementLayout {
/// Returns `true` if this is an attribute with a string literal initializer that does not contain any new line characters.
fn is_single_line_string_literal_attribute(attribute: &JSXAttributeItem) -> bool {
as_string_literal_attribute_value(attribute)
.is_some_and(|string| !string.value.as_str().is_some_and(|value| value.contains('\n')))
as_string_literal_attribute_value(attribute).is_some_and(|string| {
!string.value.as_str().is_some_and(|value| value.contains(['\n', '\r']))
})
}
/// Returns `Some` if the initializer value of this attribute is a string literal.
+12
View File
@@ -12,6 +12,18 @@ use oxc_syntax::{
line_terminator::{CR, LF, is_line_terminator},
};
/// The line terminators in `text`, with `\r\n` as one.
///
/// `oxc_formatter_json`'s `count_newlines` counts the same set over bytes; keep them in sync.
pub fn line_breaks(text: &str) -> impl Iterator<Item = char> + '_ {
let mut prev = '\0';
text.chars().filter(move |&c| {
let is_break = is_line_terminator(c) && !(c == LF && prev == CR);
prev = c;
is_break
})
}
/// JS/TS-specific source scanning, layered on the core mechanical [`SourceText`].
pub trait SourceTextExt {
/// Check if span contains line terminators.
+6 -6
View File
@@ -9,6 +9,7 @@ use oxc_ast::ast::*;
use crate::QuoteStyle;
use crate::formatter::Comments;
use crate::source_text::line_breaks;
use crate::{ast_nodes::AstNode, format_args, formatter::prelude::*, write};
pub static JSX_WHITESPACE_CHARS: [u8; 4] = [b' ', b'\n', b'\t', b'\r'];
@@ -20,7 +21,7 @@ pub static JSX_WHITESPACE_CHARS: [u8; 4] = [b' ', b'\n', b'\t', b'\r'];
/// ```text
/// use oxc_formatter::utils::jsx::is_meaningful_jsx_text;
///
/// assert_eq!(is_meaningful_jsx_text(" \t\r "), true);
/// assert_eq!(is_meaningful_jsx_text(" \t\r "), false);
/// assert_eq!(is_meaningful_jsx_text(" \n\r "), false);
/// assert_eq!(is_meaningful_jsx_text(" Alien "), true);
/// assert_eq!(is_meaningful_jsx_text("\n Alien "), true);
@@ -33,7 +34,7 @@ pub fn is_meaningful_jsx_text(text: &str) -> bool {
// If there is a non-whitespace character
if !JSX_WHITESPACE_CHARS.contains(&byte) {
return true;
} else if byte == b'\n' {
} else if matches!(byte, b'\n' | b'\r') {
has_newline = true;
}
}
@@ -296,12 +297,11 @@ pub fn jsx_split_children<'a, 'b>(
if let Some(JsxTextChunk::Whitespace(_whitespace)) = chunks.peek() {
match chunks.next() {
Some(JsxTextChunk::Whitespace(whitespace)) => {
if whitespace.contains('\n') {
if whitespace.contains(['\n', '\r']) {
if chunks.peek().is_none() {
// A text only consisting of whitespace that also contains a new line isn't considered meaningful text.
// It can be entirely removed from the content without changing the semantics.
let newlines =
whitespace.bytes().filter(|b| *b == b'\n').count();
let newlines = line_breaks(whitespace).count();
// Keep up to one blank line between tags/expressions and text.
// ```javascript
@@ -331,7 +331,7 @@ pub fn jsx_split_children<'a, 'b>(
JsxTextChunk::Whitespace(whitespace) => {
// Only handle trailing whitespace. Words must always be joined by new lines
if chunks.peek().is_none() {
if whitespace.contains('\n') {
if whitespace.contains(['\n', '\r']) {
builder.entry(JsxChild::Newline);
} else {
builder.entry(JsxChild::Whitespace);
@@ -95,60 +95,3 @@ js compatibility: 761/811 (93.83%), 23 files skipped
- js/object-prop-break-in/short-keys.js
- jsx/comments/in-attributes.js
- jsx/spread/child.js
# Input variants differ
- js/arrows/call.js (CR)
- js/binary-expressions/jsx_parent.js (CR)
- js/break-calls/react.js (CR)
- js/comments/issue-3532.js (CR)
- js/comments/issues.js (CR)
- js/comments/jsdoc.js (CR)
- js/comments/jsx.js (CR)
- js/comments/trailing_space.js (BOM)
- js/conditional/new-ternary-examples.js (CR)
- js/conditional/new-ternary-spec.js (CR)
- js/last-argument-expansion/edge_case.js (CR)
- js/last-argument-expansion/issue-7518.js (CR)
- js/last-argument-expansion/jsx.js (CR)
- js/line-suffix-boundary/boundary.js (CR)
- js/method-chain/pr-7889.js (CR)
- js/preserve-line/argument-list.js (CR)
- js/shebang/shebang-newline.js (BOM)
- js/shebang/shebang.js (BOM)
- js/ternaries/nested-in-condition.js (CR)
- js/ternaries/nested.js (CR)
- jsx/comments/in-tags.js (CR)
- jsx/comments/like-a-comment-in-jsx-text.js (CR)
- jsx/deprecated-jsx-bracket-same-line-option/jsx.js (CR)
- jsx/escape/nbsp.js (CR)
- jsx/fbt/test.js (CR)
- jsx/fragment/fragment.js (CR)
- jsx/ignore/jsx_ignore.js (CR)
- jsx/ignore/spread.js (CR)
- jsx/jsx/array-iter.js (CR)
- jsx/jsx/arrow.js (CR)
- jsx/jsx/await.js (CR)
- jsx/jsx/conditional-expression.js (CR)
- jsx/jsx/expression.js (CR)
- jsx/jsx/flow_fix_me.js (CR)
- jsx/jsx/hug.js (CR)
- jsx/jsx/logical-expression.js (CR)
- jsx/jsx/quotes.js (CR)
- jsx/jsx/return-statement.js (CR)
- jsx/jsx/spacing.js (CR)
- jsx/jsx/template-literal-in-attr.js (CR)
- jsx/last-line/last_line.js (CR)
- jsx/last-line/single_prop_multiline_string.js (CR)
- jsx/multiline-assign/test.js (CR)
- jsx/newlines/test.js (CR)
- jsx/newlines/windows.js (CR)
- jsx/optional-chaining/optional-chaining.jsx (CR)
- jsx/parentheses/argument.js (CR)
- jsx/significant-space/comments.js (CR)
- jsx/significant-space/test.js (CR)
- jsx/single-attribute-per-line/single-attribute-per-line.js (CR)
- jsx/split-attrs/test.js (CR)
- jsx/stateless-arrow-fn/test.js (CR)
- jsx/text-wrap/issue-16897.js (CR)
- jsx/text-wrap/test.js (CR)
@@ -76,46 +76,3 @@ ts compatibility: 618/660 (93.64%), 16 files skipped
- jsx/spread/child.js
- typescript/as/comments/17407.ts
- typescript/method-chain/object/issue-17239.ts
# Input variants differ
- jsx/comments/in-tags.js (CR)
- jsx/comments/like-a-comment-in-jsx-text.js (CR)
- jsx/deprecated-jsx-bracket-same-line-option/jsx.js (CR)
- jsx/escape/nbsp.js (CR)
- jsx/fbt/test.js (CR)
- jsx/fragment/fragment.js (CR)
- jsx/ignore/jsx_ignore.js (CR)
- jsx/ignore/spread.js (CR)
- jsx/jsx/array-iter.js (CR)
- jsx/jsx/arrow.js (CR)
- jsx/jsx/await.js (CR)
- jsx/jsx/conditional-expression.js (CR)
- jsx/jsx/expression.js (CR)
- jsx/jsx/flow_fix_me.js (CR)
- jsx/jsx/hug.js (CR)
- jsx/jsx/logical-expression.js (CR)
- jsx/jsx/quotes.js (CR)
- jsx/jsx/return-statement.js (CR)
- jsx/jsx/spacing.js (CR)
- jsx/jsx/template-literal-in-attr.js (CR)
- jsx/last-line/last_line.js (CR)
- jsx/last-line/single_prop_multiline_string.js (CR)
- jsx/multiline-assign/test.js (CR)
- jsx/newlines/test.js (CR)
- jsx/newlines/windows.js (CR)
- jsx/optional-chaining/optional-chaining.jsx (CR)
- jsx/parentheses/argument.js (CR)
- jsx/significant-space/comments.js (CR)
- jsx/significant-space/test.js (CR)
- jsx/single-attribute-per-line/single-attribute-per-line.js (CR)
- jsx/split-attrs/test.js (CR)
- jsx/stateless-arrow-fn/test.js (CR)
- jsx/text-wrap/issue-16897.js (CR)
- jsx/text-wrap/test.js (CR)
- typescript/comments/after-jsx-generic.tsx (CR)
- typescript/comments/jsx.tsx (CR)
- typescript/last-argument-expansion/decorated-function.tsx (CR)
- typescript/tsx/member-expression.tsx (CR)
- typescript/tsx/optional-chaining.tsx (CR)
- typescript/tsx/react.tsx (CR)
@@ -168,6 +168,8 @@ pub fn write_hole_comments(comma: u32, f: &mut JsonFormatter<'_, '_>) {
/// this crate parses every variant leniently as JS, so the lexer accepts a bare LS/PS in an inter-entry gap regardless of variant.
/// A spec-strict JSON document keeps them inside string literals, so the branch rarely fires for `json` / `jsonc`.
/// But when it does, treating it as a break matches Prettier (every variant goes through its JS printer).
///
/// `oxc_formatter`'s `line_breaks` (`source_text.rs`) counts the same set over a `&str`; keep them in sync.
pub fn count_newlines(slice: &[u8]) -> usize {
let mut count = 0;
let mut i = 0;
@@ -427,7 +427,8 @@ where
variants.push(("CRLF", source_text.cow_replace('\n', "\r\n").into_owned(), actual.clone()));
variants.push(("CR", source_text.cow_replace('\n', "\r").into_owned(), actual.clone()));
}
if !source_text.starts_with('\u{feff}') {
// BOM + hashbang is a syntax error (Prettier strips the BOM first)
if !source_text.starts_with('\u{feff}') && !source_text.starts_with("#!") {
variants.push(("BOM", format!("\u{feff}{source_text}"), format!("\u{feff}{actual}")));
}
for (name, input, expected_output) in variants {