Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_formatter): jsx punctuation
Browse files Browse the repository at this point in the history
  • Loading branch information
denbezrukov committed Nov 26, 2022
1 parent 2e14f31 commit 1c0e84d
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 38 deletions.
54 changes: 29 additions & 25 deletions crates/rome_js_formatter/src/jsx/lists/child_list.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::prelude::*;
use crate::utils::jsx::{
is_meaningful_jsx_text, is_whitespace_jsx_expression, jsx_split_children, JsxChild,
JsxRawSpace, JsxSpace,
JsxChildrenIterator, JsxRawSpace, JsxSpace,
};
use crate::JsFormatter;
use rome_formatter::format_element::tag::{GroupMode, Tag};
Expand Down Expand Up @@ -72,7 +72,6 @@ impl FormatJsxChildList {
let mut flat = FlatBuilder::new();
let mut multiline = MultilineBuilder::new(multiline_layout);

let mut last: Option<JsxChild> = None;
let mut force_multiline = layout.is_multiline();

let mut children = jsx_split_children(list, f.context().comments())?;
Expand All @@ -82,7 +81,8 @@ impl FormatJsxChildList {
children.pop();
}

let mut children_iter = children.into_iter().peekable();
let mut last: Option<&JsxChild> = None;
let mut children_iter = JsxChildrenIterator::new(children.iter());

// Trim leading new lines
if let Some(JsxChild::Newline | JsxChild::EmptyLine) = children_iter.peek() {
Expand Down Expand Up @@ -166,34 +166,38 @@ impl FormatJsxChildList {
// A new line between some JSX text and an element
JsxChild::Newline => {
let is_soft_break = {
// Here we handle the case when we have a newline between an ascii punctuation word and a jsx element
// We need to use the previous and the next element
// [JsxChild::Word, JsxChild::Newline, JsxChild::NonText]
// ```
// <div>
// <div>First</div>,
// <div>Second</div>
// </div>
// ```
if let Some(JsxChild::Word(word)) = last {
let is_next_element_self_closing = matches!(
children_iter.peek(),
Some(JsxChild::NonText(JsxAnyChild::JsxSelfClosingElement(_)))
);

// Here we handle the case when we have a newline between an ascii punctuation word and a jsx element
// We need to use the previous and the next element
// [JsxChild::Word, JsxChild::Newline, JsxChild::NonText]
// ```
// <div>
// <div>First</div>,
// <div>Second</div>
// </div>
// ```
!is_next_element_self_closing && word.is_ascii_punctuation()
} else if let Some(JsxChild::Word(next_word)) = children_iter.peek() {
// Here we handle the case when we have an ascii punctuation word between a new line and a jsx element
// Here we need to look ahead two elements
// [JsxChild::Newline, JsxChild::Word, JsxChild::NonText]
// ```
// <div>
// <div>First</div>
// ,<div>Second</div>
// </div>
// ```
!next_word.is_next_element_self_closing()
&& next_word.is_ascii_punctuation()
}
// Here we handle the case when we have an ascii punctuation word between a new line and a jsx element
// Here we need to look ahead two elements
// [JsxChild::Newline, JsxChild::Word, JsxChild::NonText]
// ```
// <div>
// <div>First</div>
// ,<div>Second</div>
// </div>
// ```
else if let Some(JsxChild::Word(next_word)) = children_iter.peek() {
let is_next_next_element_self_closing = matches!(
children_iter.peek_next(),
Some(JsxChild::NonText(JsxAnyChild::JsxSelfClosingElement(_)))
);

!is_next_next_element_self_closing && next_word.is_ascii_punctuation()
} else {
false
}
Expand Down
100 changes: 87 additions & 13 deletions crates/rome_js_formatter/src/utils/jsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,6 @@ impl JsxSplitChildrenBuilder {
self.buffer.push(child);
}
}
Some(JsxChild::Word(word)) => {
if let JsxChild::NonText(JsxAnyChild::JsxSelfClosingElement(_)) = child {
word.is_next_element_self_closing = true;
}
self.buffer.push(child)
}
_ => self.buffer.push(child),
}
}
Expand Down Expand Up @@ -409,22 +403,16 @@ impl JsxChild {
pub(crate) struct JsxWord {
text: SyntaxTokenText,
source_position: TextSize,
is_next_element_self_closing: bool,
}

impl JsxWord {
fn new(text: SyntaxTokenText, source_position: TextSize) -> Self {
JsxWord {
text,
source_position,
is_next_element_self_closing: false,
}
}

pub(crate) fn is_next_element_self_closing(&self) -> bool {
self.is_next_element_self_closing
}

pub(crate) fn is_ascii_punctuation(&self) -> bool {
self.text.chars().count() == 1
&& self
Expand Down Expand Up @@ -506,15 +494,101 @@ impl<'a> Iterator for JsxSplitChunksIterator<'a> {

impl FusedIterator for JsxSplitChunksIterator<'_> {}

/// An iterator adaptor that allows a lookahead of two tokens
///
/// # Examples
/// ```
/// use rome_js_formatter::utils::jsx::JsxChildrenIterator;
///
/// let buffer = vec![1, 2, 3, 4];
///
/// let mut iter = JsxChildrenIterator::new(buffer.iter());
///
/// assert_eq!(iter.peek(), Some(&&1));
/// assert_eq!(iter.peek_next(), Some(&&2));
/// assert_eq!(iter.next(), Some(&1));
/// assert_eq!(iter.next(), Some(&2));
/// ```
#[derive(Clone, Debug)]
pub struct JsxChildrenIterator<I: Iterator> {
iter: I,

peeked: Option<Option<I::Item>>,
peeked_next: Option<Option<I::Item>>,
}

impl<I: Iterator> JsxChildrenIterator<I> {
pub fn new(iter: I) -> Self {
Self {
iter,
peeked: None,
peeked_next: None,
}
}

pub fn peek(&mut self) -> Option<&I::Item> {
let iter = &mut self.iter;
self.peeked.get_or_insert_with(|| iter.next()).as_ref()
}

pub fn peek_next(&mut self) -> Option<&I::Item> {
let iter = &mut self.iter;
let peeked = &mut self.peeked;

self.peeked_next
.get_or_insert_with(|| {
peeked.get_or_insert_with(|| iter.next());
iter.next()
})
.as_ref()
}
}

impl<I: Iterator> Iterator for JsxChildrenIterator<I> {
type Item = I::Item;

fn next(&mut self) -> Option<Self::Item> {
match self.peeked.take() {
Some(peeked) => {
self.peeked = self.peeked_next.take();
peeked
}
None => self.iter.next(),
}
}
}

#[cfg(test)]
mod tests {
use crate::utils::jsx::{jsx_split_children, JsxChild, JsxSplitChunksIterator, JsxTextChunk};
use crate::utils::jsx::{
jsx_split_children, JsxChild, JsxChildrenIterator, JsxSplitChunksIterator, JsxTextChunk,
};
use rome_diagnostics::location::FileId;
use rome_formatter::comments::Comments;
use rome_js_parser::parse;
use rome_js_syntax::{JsxChildList, JsxText, SourceType};
use rome_rowan::{AstNode, TextSize};

#[test]
fn jsx_children_iterator_test() {
let buffer = vec![1, 2, 3, 4];

let mut iter = JsxChildrenIterator::new(buffer.iter());

assert_eq!(iter.peek(), Some(&&1));
assert_eq!(iter.peek(), Some(&&1));
assert_eq!(iter.peek_next(), Some(&&2));
assert_eq!(iter.peek_next(), Some(&&2));

assert_eq!(iter.next(), Some(&1));
assert_eq!(iter.next(), Some(&2));

assert_eq!(iter.peek_next(), Some(&&4));
assert_eq!(iter.peek_next(), Some(&&4));
assert_eq!(iter.peek(), Some(&&3));
assert_eq!(iter.peek(), Some(&&3));
}

fn assert_jsx_text_chunks(text: &str, expected_chunks: Vec<(TextSize, JsxTextChunk)>) {
let parse = parse(
&std::format!("<>{text}</>"),
Expand Down

0 comments on commit 1c0e84d

Please sign in to comment.