mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(formatter): treat lone CR in JSX text and argument blank lines (#27780)
Also disables BOM and shebang variants.
This commit is contained in:
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,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.
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in new issue
Block a user