refactor(rome_formatter): `GroupElementsBuffer`
MODIFY
crates/rome_formatter/src/arguments.rs
crates/rome_formatter/src/arguments.rs
@@ -144,10 +144,10 @@ mod tests {
FormatElement::Space,
FormatElement::Token(Token::Static { text: "a" }),
FormatElement::Space,
- FormatElement::Group(Group::new(FormatElement::List(List::new(vec![
+ FormatElement::Group(Group::new(vec![
FormatElement::Token(Token::Static { text: "(" }),
FormatElement::Token(Token::Static { text: ")" }),
- ]))))
+ ]))
]))
);
}

MODIFY
crates/rome_formatter/src/builders.rs
crates/rome_formatter/src/builders.rs
use crate::prelude::*;
use crate::{
- format_element, write, Argument, Arguments, GroupId, PreambleBuffer, TextRange, TextSize,
+ format_element, write, Argument, Arguments, BufferSnapshot, FormatState, GroupId,
+ PreambleBuffer, TextRange, TextSize,
};
use crate::{Buffer, VecBuffer};
use rome_rowan::{Language, SyntaxNode, SyntaxToken, SyntaxTokenText, TextLen};
use std::borrow::Cow;
use std::cell::Cell;
use std::marker::PhantomData;
+use std::ops::Deref;
/// A line break that only gets printed if the enclosing `Group` doesn't fit on a single line.
/// It's omitted if the enclosing `Group` fits on a single line.
@@ -462,23 +464,30 @@ impl<Context> Format<Context> for LineSuffixBoundary {
/// ## Examples
///
/// ```
-/// use rome_formatter::{format, format_args};
+/// use rome_formatter::{format, write, format_args};
/// use rome_formatter::prelude::*;
///
/// let elements = format!(
/// SimpleFormatContext::default(),
/// [
/// group_elements(&format_args![
-/// comment(&empty_line()),
+/// comment(&format_args![token("// test"), hard_line_break()]),
+/// format_with(|f| {
+/// write!(f, [
+/// comment(&format_args![token("/* inline */"), hard_line_break()]).memoized(),
/// token("a"),
/// soft_line_break_or_space(),
-/// token("b")
+/// ])
+/// }).memoized(),
+/// token("b"),
+/// soft_line_break_or_space(),
+/// token("c")
/// ])
/// ]
/// ).unwrap();
///
/// assert_eq!(
-/// "\na b",
+/// "// test\n/* inline */\na b c",
/// elements.print().as_code()
/// );
/// ```
@@ -502,9 +511,9 @@ impl<Context> Format<Context> for FormatComment<'_, Context> {
let mut buffer = VecBuffer::new(f.state_mut());
buffer.write_fmt(Arguments::from(&self.content))?;
- let content = buffer.into_element();
+ let content = buffer.into_vec();
- f.write_element(FormatElement::Comment(Box::new(content)))
+ f.write_element(FormatElement::Comment(content.into_boxed_slice()))
}
}
@@ -926,24 +935,18 @@ impl<Context> GroupElements<'_, Context> {
impl<Context> Format<Context> for GroupElements<'_, Context> {
fn fmt(&self, f: &mut Formatter<Context>) -> FormatResult<()> {
- let mut buffer = VecBuffer::new(f.state_mut());
-
+ let mut buffer = GroupElementsBuffer::new(f);
buffer.write_fmt(Arguments::from(&self.content))?;
+ let content = buffer.into_vec();
- let content = buffer.into_element();
+ if content.is_empty() && self.group_id.is_none() {
+ return Ok(());
+ }
- let (leading, content, trailing) = content.split_trivia();
let group = Group::new(content).with_id(self.group_id);
- if !leading.is_empty() {
- f.write_element(leading)?;
- }
f.write_element(FormatElement::Group(group))?;
- if !trailing.is_empty() {
- f.write_element(trailing)?;
- }
-
Ok(())
}
}
@@ -957,6 +960,150 @@ impl<Context> std::fmt::Debug for GroupElements<'_, Context> {
}
}
+/// Custom buffer implementation for `GroupElements` that moves the leading comments out of the group
+/// to prevent that a leading line comment expands the token's enclosing group.
+///
+/// # Examples
+///
+/// ```javascript
+/// /* a comment */
+/// [1]
+/// ```
+///
+/// The `/* a comment */` belongs to the `[` group token that is part of a group wrapping the whole
+/// `[1]` expression. It's important that the comment `/* a comment */` gets moved out of the group element
+/// to avoid that the `[1]` group expands because of the line break inserted by the comment.
+struct GroupElementsBuffer<'inner, Context> {
+ inner: &'inner mut dyn Buffer<Context = Context>,
+
+ /// The group inner content
+ content: Vec<FormatElement>,
+}
+
+impl<'inner, Context> GroupElementsBuffer<'inner, Context> {
+ fn new(inner: &'inner mut dyn Buffer<Context = Context>) -> Self {
+ Self {
+ inner,
+ content: Vec::new(),
+ }
+ }
+
+ fn into_vec(self) -> Vec<FormatElement> {
+ self.content
+ }
+
+ fn write_interned(&mut self, interned: Interned) -> FormatResult<()> {
+ debug_assert!(self.content.is_empty());
+
+ match interned.deref() {
+ FormatElement::Comment(_) => {
+ self.inner.write_element(FormatElement::Interned(interned))
+ }
+ FormatElement::List(list) => {
+ let mut content_start = 0;
+
+ for element in list.iter() {
+ match element {
+ element @ FormatElement::Comment(_) => {
+ content_start += 1;
+ // Cloning comments should be alright as they are rarely nested
+ // and the case where all elements of an interned data structure are comments
+ // are rare
+ self.inner.write_element(element.clone())?;
+ }
+ FormatElement::Interned(interned) => {
+ self.write_interned(interned.clone())?;
+ content_start += 1;
+
+ if !self.content.is_empty() {
+ // Interned struct contained non-comment
+ break;
+ }
+ }
+ _ => {
+ // Found the first non-comment / nested interned element
+ break;
+ }
+ }
+ }
+
+ // No leading comments, this group has no comments
+ if content_start == 0 {
+ self.content.push(FormatElement::Interned(interned));
+ return Ok(());
+ }
+
+ let content = &list[content_start..];
+
+ // It is necessary to mutate the interned elements, write cloned elements
+ self.write_elements(content.iter().cloned())
+ }
+ FormatElement::Interned(interned) => self.write_interned(interned.clone()),
+ _ => {
+ self.content.push(FormatElement::Interned(interned));
+ Ok(())
+ }
+ }
+ }
+}
+
+impl<Context> Buffer for GroupElementsBuffer<'_, Context> {
+ type Context = Context;
+
+ fn write_element(&mut self, element: FormatElement) -> FormatResult<()> {
+ if self.content.is_empty() {
+ match element {
+ FormatElement::List(list) => {
+ self.write_elements(list.into_vec())?;
+ }
+ FormatElement::Interned(interned) => match Interned::try_unwrap(interned) {
+ Ok(owned) => self.write_element(owned)?,
+ Err(interned) => self.write_interned(interned)?,
+ },
+ comment @ FormatElement::Comment { .. } => {
+ self.inner.write_element(comment)?;
+ }
+ element => self.content.push(element),
+ }
+ } else {
+ match element {
+ FormatElement::List(list) => {
+ self.content.extend(list.into_vec());
+ }
+ element => self.content.push(element),
+ }
+ }
+
+ Ok(())
+ }
+
+ fn state(&self) -> &FormatState<Self::Context> {
+ self.inner.state()
+ }
+
+ fn state_mut(&mut self) -> &mut FormatState<Self::Context> {
+ self.inner.state_mut()
+ }
+
+ fn snapshot(&self) -> BufferSnapshot {
+ BufferSnapshot::Any(Box::new(GroupElementsBufferSnapshot {
+ inner: self.inner.snapshot(),
+ content_len: self.content.len(),
+ }))
+ }
+
+ fn restore_snapshot(&mut self, snapshot: BufferSnapshot) {
+ let snapshot = snapshot.unwrap_any::<GroupElementsBufferSnapshot>();
+ self.inner.restore_snapshot(snapshot.inner);
+ self.content.truncate(snapshot.content_len);
+ }
+}
+
+struct GroupElementsBufferSnapshot {
+ inner: BufferSnapshot,
+ content_len: usize,
+}
+
/// IR element that forces the parent group to print in expanded mode.
///
/// Has no effect if used outside of a group or element that introduce implicit groups (fill element).

MODIFY
crates/rome_formatter/src/format_element.rs
crates/rome_formatter/src/format_element.rs
@@ -55,10 +55,10 @@ pub enum FormatElement {
LineSuffixBoundary,
/// Special semantic element letting the printer and formatter know this is
- /// a trivia content, and it should only have a limited influence on the
+ /// a comment content, and it should only have a limited influence on the
/// formatting (for instance line breaks contained within will not cause
- /// the parent group to break if this element is at the start of it)
- Comment(Content),
+ /// the parent group to break if this element is at the start of it).
+ Comment(Box<[FormatElement]>),
/// A token that tracks tokens/nodes that are printed using [`format_verbatim`](crate::Formatter::format_verbatim) API
Verbatim(Verbatim),
@@ -67,9 +67,9 @@ pub enum FormatElement {
/// Line breaks inside of a best fitting don't propagate to parent groups.
BestFitting(BestFitting),
- /// Reference counted format element content. Useful when the same content must be emitted twice
- /// because it avoids deep cloning of the inner content and instead only requires bumping a ref counter.
- Rc(Rc<FormatElement>),
+ /// An interned format element. Useful when the same content must be emitted multiple times to avoid
+ /// deep cloning the IR when using the `best_fitting!` macro or `if_group_fits_on_line` and `if_group_breaks`.
+ Interned(Interned),
}
#[derive(Clone, Copy, Eq, PartialEq, Debug)]
@@ -149,7 +149,7 @@ impl Debug for FormatElement {
best_fitting.fmt(fmt)
}
FormatElement::ExpandParent => write!(fmt, "ExpandParent"),
- FormatElement::Rc(inner) => inner.fmt(fmt),
+ FormatElement::Interned(inner) => inner.fmt(fmt),
}
}
}
@@ -222,7 +222,7 @@ impl Fill {
/// but breaks the array cross multiple lines if it would exceed the specified `line_width`, if a child token is a hard line break or if a string contains a line break.
#[derive(Clone, PartialEq, Eq)]
pub struct Group {
- pub(crate) content: Content,
+ pub(crate) content: Box<[FormatElement]>,
pub(crate) id: Option<GroupId>,
}
@@ -240,9 +240,9 @@ impl Debug for Group {
}
impl Group {
- pub fn new(content: FormatElement) -> Self {
+ pub fn new(content: Vec<FormatElement>) -> Self {
Self {
- content: Box::new(content),
+ content: content.into_boxed_slice(),
id: None,
}
}
@@ -329,6 +329,29 @@ impl Debug for BestFitting {
}
}
+#[derive(Clone, Eq, PartialEq)]
+pub struct Interned(Rc<FormatElement>);
+
+impl Interned {
+ pub(crate) fn try_unwrap(this: Interned) -> Result<FormatElement, Interned> {
+ Rc::try_unwrap(this.0).map_err(Interned)
+ }
+}
+
+impl Debug for Interned {
+ fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+ self.0.fmt(f)
+ }
+}
+
+impl Deref for Interned {
+ type Target = FormatElement;
+
+ fn deref(&self) -> &Self::Target {
+ self.0.deref()
+ }
+}
+
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct ConditionalGroupContent {
pub(crate) content: Content,
@@ -470,7 +493,6 @@ impl FormatElement {
pub fn is_empty(&self) -> bool {
match self {
FormatElement::List(list) => list.is_empty(),
- FormatElement::Rc(inner) => inner.is_empty(),
_ => false,
}
}
@@ -487,73 +509,19 @@ impl FormatElement {
FormatElement::Space => false,
FormatElement::Line(line_mode) => matches!(line_mode, LineMode::Hard | LineMode::Empty),
FormatElement::Indent(content) => content.will_break(),
- FormatElement::Group(group) => group.content.will_break(),
+ FormatElement::Group(Group { content, .. }) | FormatElement::Comment(content) => {
+ content.iter().any(FormatElement::will_break)
+ }
FormatElement::ConditionalGroupContent(group) => group.content.will_break(),
FormatElement::List(list) => list.content.iter().any(FormatElement::will_break),
FormatElement::Fill(fill) => fill.list.content.iter().any(FormatElement::will_break),
FormatElement::Token(token) => token.contains('\n'),
FormatElement::LineSuffix(_) => false,
- FormatElement::Comment(content) => content.will_break(),
FormatElement::Verbatim(verbatim) => verbatim.element.will_break(),
FormatElement::BestFitting(_) => false,
FormatElement::LineSuffixBoundary => false,
FormatElement::ExpandParent => true,
- FormatElement::Rc(inner) => inner.will_break(),
- }
- }
-
- /// Splits off the leading and trailing trivias (comments) from this [FormatElement]
- ///
- /// For [FormatElement::HardGroup], the trailing trivia
- /// is automatically moved outside of the group. The group itself is then recreated around the
- /// content itself.
- pub fn split_trivia(self) -> (FormatElement, FormatElement, FormatElement) {
- match self {
- FormatElement::List(mut list) => {
- // Find the index of the first non-comment element in the list
- let content_start = list
- .content
- .iter()
- .position(|elem| !matches!(elem, FormatElement::Comment(_)));
-
- // List contains at least one non trivia element.
- if let Some(content_start) = content_start {
- let (leading, mut content) = if content_start > 0 {
- let content = list.content.split_off(content_start);
- (FormatElement::List(list), content)
- } else {
- // No leading trivia
- (FormatElement::List(List::default()), list.content)
- };
-
- let content_end = content
- .iter()
- .rposition(|elem| !matches!(elem, FormatElement::Comment(_)))
- .expect("List guaranteed to contain at least one non trivia element.");
- let trailing_start = content_end + 1;
-
- let trailing = if trailing_start < content.len() {
- FormatElement::List(List::new(content.split_off(trailing_start)))
- } else {
- FormatElement::List(List::default())
- };
-
- (leading, FormatElement::List(List::new(content)), trailing)
- } else {
- // All leading trivia
- (
- FormatElement::List(list),
- FormatElement::List(List::default()),
- FormatElement::List(List::default()),
- )
- }
- }
- // Non-list elements are returned directly
- _ => (
- FormatElement::List(List::default()),
- self,
- FormatElement::List(List::default()),
- ),
+ FormatElement::Interned(inner) => inner.0.will_break(),
}
}
@@ -573,11 +541,26 @@ impl FormatElement {
FormatElement::Line(_) | FormatElement::Comment(_) => None,
FormatElement::Indent(indent) => indent.last_element(),
- FormatElement::Group(group) => group.content.last_element(),
+ FormatElement::Group(group) => group
+ .content
+ .iter()
+ .rev()
+ .find_map(FormatElement::last_element),
_ => Some(self),
}
}
+
+ /// Interns a format element.
+ ///
+ /// Returns `self` for an empty list AND an already interned elements.
+ pub fn intern(self) -> FormatElement {
+ match self {
+ FormatElement::List(list) if list.is_empty() => list.into(),
+ element @ FormatElement::Interned(_) => element,
+ element => FormatElement::Interned(Interned(Rc::new(element))),
+ }
+ }
}
impl From<Token> for FormatElement {

MODIFY
crates/rome_formatter/src/format_extensions.rs
crates/rome_formatter/src/format_extensions.rs
use crate::prelude::*;
use std::cell::RefCell;
use std::marker::PhantomData;
-use std::rc::Rc;
use crate::{write, Buffer, VecBuffer};
@@ -145,7 +144,7 @@ impl<T, Context> MemoizeFormat<Context> for T where T: Format<Context> {}
#[derive(Debug)]
pub struct Memoized<F, Context> {
inner: F,
- memory: RefCell<Option<FormatResult<Rc<FormatElement>>>>,
+ memory: RefCell<Option<FormatResult<FormatElement>>>,
options: PhantomData<Context>,
}
@@ -171,7 +170,7 @@ where
if let Some(memory) = self.memory.borrow().as_ref() {
return match memory {
Ok(elements) => {
- f.write_element(FormatElement::Rc(elements.clone()))?;
+ f.write_element(elements.clone())?;
Ok(())
}
@@ -185,9 +184,11 @@ where
match result {
Ok(_) => {
let elements = buffer.into_element();
- let reference = Rc::new(elements);
- f.write_element(FormatElement::Rc(reference.clone()))?;
- *self.memory.borrow_mut() = Some(Ok(reference));
+ let interned = elements.intern();
+
+ f.write_element(interned.clone())?;
+
+ *self.memory.borrow_mut() = Some(Ok(interned));
Ok(())
}

MODIFY
crates/rome_formatter/src/printer/mod.rs
crates/rome_formatter/src/printer/mod.rs
@@ -129,7 +129,11 @@ impl<'a> Printer<'a> {
PrintMode::Flat if self.state.measured_group_fits => {
// A parent group has already verified that this group fits on a single line
// Thus, just continue in flat mode
- queue.enqueue(PrintElementCall::new(content.as_ref(), args));
+ queue.extend(
+ content
+ .iter()
+ .map(|element| PrintElementCall::new(element, args)),
+ );
PrintMode::Flat
}
// The printer is either in expanded mode or it's necessary to re-measure if the group fits
@@ -139,15 +143,16 @@ impl<'a> Printer<'a> {
// print the group in "flat" mode, otherwise continue in expanded mode
let flat_args = args.with_print_mode(PrintMode::Flat);
- if fits_on_line(&[content], flat_args, queue, self) {
- queue.enqueue(PrintElementCall::new(content, flat_args));
+ if fits_on_line(content.iter(), flat_args, queue, self) {
+ queue.extend(
+ content.iter().map(|e| PrintElementCall::new(e, flat_args)),
+ );
self.state.measured_group_fits = true;
PrintMode::Flat
} else {
- queue.enqueue(PrintElementCall::new(
- content,
- args.with_print_mode(PrintMode::Expanded),
- ));
+ queue.extend(content.iter().map(|e| {
+ PrintElementCall::new(e, args.with_print_mode(PrintMode::Expanded))
+ }));
PrintMode::Expanded
}
}
@@ -233,7 +238,7 @@ impl<'a> Printer<'a> {
}
FormatElement::Comment(content) => {
- queue.enqueue(PrintElementCall::new(content.as_ref(), args));
+ queue.extend(content.iter().map(|e| PrintElementCall::new(e, args)))
}
FormatElement::Verbatim(verbatim) => {
@@ -279,7 +284,7 @@ impl<'a> Printer<'a> {
PrintMode::Expanded
};
- if fits_on_line(&[variant], args.with_print_mode(mode), queue, self)
+ if fits_on_line([variant], args.with_print_mode(mode), queue, self)
{
self.state.measured_group_fits = true;
queue.enqueue(PrintElementCall::new(
@@ -293,7 +298,7 @@ impl<'a> Printer<'a> {
}
}
}
- FormatElement::Rc(content) => queue.enqueue(PrintElementCall::new(content, args)),
+ FormatElement::Interned(content) => queue.enqueue(PrintElementCall::new(content, args)),
}
}
@@ -362,7 +367,7 @@ impl<'a> Printer<'a> {
};
let mut current_fits = fits_on_line(
- &[current_content],
+ [current_content],
args.with_print_mode(PrintMode::Flat),
&empty_rest,
self,
@@ -384,7 +389,7 @@ impl<'a> Printer<'a> {
// otherwise see if both contents fit on the line.
let current_and_next_fit = current_fits
&& fits_on_line(
- &[separator, next_item],
+ [separator, next_item],
args.with_print_mode(PrintMode::Flat),
&empty_rest,
self,
@@ -406,7 +411,7 @@ impl<'a> Printer<'a> {
);
let next_fits = fits_on_line(
- &[next_item],
+ [next_item],
args.with_print_mode(PrintMode::Flat),
&empty_rest,
self,
@@ -626,12 +631,16 @@ impl<'a> ElementCallQueue<'a> {
/// Tests if it's possible to print the content of the queue up to the first hard line break
/// or the end of the document on a single line without exceeding the line width.
#[must_use = "Only determines if content fits on a single line but doesn't print it"]
-fn fits_on_line<'a>(
- elements: &[&'a FormatElement],
+fn fits_on_line<'a, I>(
+ elements: I,
args: PrintElementArgs,
queue: &ElementCallQueue<'a>,
printer: &mut Printer<'a>,
-) -> bool {
+) -> bool
+where
+ I: IntoIterator<Item = &'a FormatElement>,
+ I::IntoIter: DoubleEndedIterator,
+{
let shared_buffer = std::mem::take(&mut printer.state.measure_queue);
debug_assert!(shared_buffer.is_empty());
@@ -641,7 +650,7 @@ fn fits_on_line<'a>(
measure_queue.extend(
elements
- .iter()
+ .into_iter()
.map(|element| PrintElementCall::new(element, args)),
);
@@ -726,7 +735,9 @@ fn fits_element_on_line<'a, 'rest>(
args.with_incremented_indent(),
)),
- FormatElement::Group(group) => queue.enqueue(PrintElementCall::new(&group.content, args)),
+ FormatElement::Group(group) => {
+ queue.extend(group.content.iter().map(|e| PrintElementCall::new(e, args)))
+ }
FormatElement::ConditionalGroupContent(conditional) => {
if args.mode == conditional.mode {
@@ -745,6 +756,7 @@ fn fits_element_on_line<'a, 'rest>(
FormatElement::Token(token) => {
state.line_width += state.pending_indent as usize * options.indent_string.len();
+ state.pending_indent = 0;
if state.pending_space {
state.line_width += 1;
@@ -769,7 +781,6 @@ fn fits_element_on_line<'a, 'rest>(
}
state.pending_space = false;
- state.pending_indent = 0;
}
FormatElement::LineSuffix(_) => {
@@ -782,7 +793,9 @@ fn fits_element_on_line<'a, 'rest>(
}
}
- FormatElement::Comment(content) => queue.enqueue(PrintElementCall::new(content, args)),
+ FormatElement::Comment(content) => {
+ queue.extend(content.iter().map(|e| PrintElementCall::new(e, args)))
+ }
FormatElement::Verbatim(verbatim) => {
queue.enqueue(PrintElementCall::new(&verbatim.element, args))
@@ -800,7 +813,7 @@ fn fits_element_on_line<'a, 'rest>(
return Fits::No;
}
}
- FormatElement::Rc(content) => queue.enqueue(PrintElementCall::new(content, args)),
+ FormatElement::Interned(content) => queue.enqueue(PrintElementCall::new(content, args)),
}
Fits::Maybe
@@ -1164,7 +1177,7 @@ two lines`,
space_token(),
token("// trailing"),
space_token()
- ]))
+ ]),)
]);
assert_eq!(printed.as_code(), "[1, 2, 3]; // trailing")

MODIFY
crates/rome_formatter/src/token.rs
crates/rome_formatter/src/token.rs
@@ -901,7 +901,7 @@ where
// Write a space for all other cases
_ => space_token().fmt(f)?,
}
- comment.piece().fmt(f)?;
+ comment.piece().fmt(f)
} else {
write![
f,
@@ -909,10 +909,8 @@ where
line_suffix(&format_args![space_token(), comment.piece()]),
expand_parent()
]
- ]?;
+ ]
}
-
- Ok(())
});
crate::comment(&content).fmt(f)?;

refactor(rome_formatter): `GroupElementsBuffer` (#2724)