From 6fe4c1b9117a35561fb40b843da92a3bdbe2bff3 Mon Sep 17 00:00:00 2001 From: leaysgur <6259812+leaysgur@users.noreply.github.com> Date: Fri, 16 Oct 2026 04:04:07 +0000 Subject: [PATCH] fix(formatter): treat lone CR in JSX text and argument blank lines (#27780) Also disables BOM and shebang variants. --- .../print/call_like_expression/arguments.rs | 9 +-- .../src/print/jsx/opening_element.rs | 10 +++- crates/oxc_formatter/src/source_text.rs | 12 ++++ crates/oxc_formatter/src/utils/jsx.rs | 12 ++-- .../snapshots/conformance__prettier-js.snap | 57 ------------------- .../snapshots/conformance__prettier-ts.snap | 43 -------------- crates/oxc_formatter_json/src/comments.rs | 2 + crates/oxc_formatter_tests/src/conformance.rs | 3 +- 8 files changed, 32 insertions(+), 116 deletions(-) diff --git a/crates/oxc_formatter/src/print/call_like_expression/arguments.rs b/crates/oxc_formatter/src/print/call_like_expression/arguments.rs index 79014c5a7b..42586e4f80 100644 --- a/crates/oxc_formatter/src/print/call_like_expression/arguments.rs +++ b/crates/oxc_formatter/src/print/call_like_expression/arguments.rs @@ -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() }); diff --git a/crates/oxc_formatter/src/print/jsx/opening_element.rs b/crates/oxc_formatter/src/print/jsx/opening_element.rs index f62f105795..6688cc0132 100644 --- a/crates/oxc_formatter/src/print/jsx/opening_element.rs +++ b/crates/oxc_formatter/src/print/jsx/opening_element.rs @@ -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. diff --git a/crates/oxc_formatter/src/source_text.rs b/crates/oxc_formatter/src/source_text.rs index d780de5851..f1f88124c6 100644 --- a/crates/oxc_formatter/src/source_text.rs +++ b/crates/oxc_formatter/src/source_text.rs @@ -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 + '_ { + 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. diff --git a/crates/oxc_formatter/src/utils/jsx.rs b/crates/oxc_formatter/src/utils/jsx.rs index a04e403df8..e86e6d9c21 100644 --- a/crates/oxc_formatter/src/utils/jsx.rs +++ b/crates/oxc_formatter/src/utils/jsx.rs @@ -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); diff --git a/crates/oxc_formatter/tests/snapshots/conformance__prettier-js.snap b/crates/oxc_formatter/tests/snapshots/conformance__prettier-js.snap index fb860e5726..3f4f2a7ca7 100644 --- a/crates/oxc_formatter/tests/snapshots/conformance__prettier-js.snap +++ b/crates/oxc_formatter/tests/snapshots/conformance__prettier-js.snap @@ -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) diff --git a/crates/oxc_formatter/tests/snapshots/conformance__prettier-ts.snap b/crates/oxc_formatter/tests/snapshots/conformance__prettier-ts.snap index 615206d2c7..0a3959cd73 100644 --- a/crates/oxc_formatter/tests/snapshots/conformance__prettier-ts.snap +++ b/crates/oxc_formatter/tests/snapshots/conformance__prettier-ts.snap @@ -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) diff --git a/crates/oxc_formatter_json/src/comments.rs b/crates/oxc_formatter_json/src/comments.rs index 0d0106f5d3..fc7882d6c6 100644 --- a/crates/oxc_formatter_json/src/comments.rs +++ b/crates/oxc_formatter_json/src/comments.rs @@ -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; diff --git a/crates/oxc_formatter_tests/src/conformance.rs b/crates/oxc_formatter_tests/src/conformance.rs index 86a7ea003c..c187c37867 100644 --- a/crates/oxc_formatter_tests/src/conformance.rs +++ b/crates/oxc_formatter_tests/src/conformance.rs @@ -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 {