diff --git a/crates/biome_formatter/src/format_element/document.rs b/crates/biome_formatter/src/format_element/document.rs index 75ad96e3d2f6..516dce46e094 100644 --- a/crates/biome_formatter/src/format_element/document.rs +++ b/crates/biome_formatter/src/format_element/document.rs @@ -72,10 +72,43 @@ impl Document { propagate_expands(variant, enclosing, checked_interned); } - // Best fitting acts as a boundary - expands = false; enclosing.pop(); - continue; + // BestFitting acts as a boundary, meaning there is no need to continue + // processing this element and we can move onto the next. However, we + // _don't_ set `expands = false`, because that ends up negating the + // expansion when processing `Interned` elements. + // + // Only interned lists are affected, because they cache the expansion value + // based on the value of `expands` at the end of iterating the children. If + // a `best_fitting` element occurs after the last expanding element, and we + // end up setting `expands = false` here, then the interned element ends up + // thinking that its content doesn't expand, even though it might. Example: + // group(1, + // interned 1 [ + // expand_parent, + // best_fitting, + // ] + // ) + // group(2, + // [ref interned 1] + // ) + // Here, `group(1)` gets expanded directly by the `expand_parent` element. + // This happens immediately, and then `expands = true` is set. The interned + // element continues processing, and encounters the `best_fitting`. If + // we set `expands = false` there, then the interned element's result ends + // up being `false`, even though it does actually expand. Then, when + // `group(2)` checks for expansion, it looks at the ref to `interned 1`, + // which thinks it doesn't expand, and so `group(2)` stays flat. + // + // By _not_ setting `expands = false`, even though `best_fitting` is a + // boundary for expansion, we ensure that any references to the interned + // element will get the correct value for whether the contained content + // actually expands, regardless of the order of elements within it. + // + // Instead, just returning false here enforces that `best_fitting` doesn't + // think it expands _itself_, but allows other sibling elements to still + // propagate their expansion. + false } FormatElement::StaticText { text } => text.contains('\n'), FormatElement::DynamicText { text, .. } => text.contains('\n'), @@ -746,6 +779,55 @@ mod tests { "]">, indent([]) > +]"# + ); + } + + #[test] + fn interned_best_fitting_allows_sibling_expand_propagation() { + use Tag::*; + + // An interned element containing something that expands the parent, + // and a best_fitting element after it. + // + // interned 1 [ + // expand_parent, + // best_fitting(...) + // ] + let interned = Interned::new(vec![FormatElement::ExpandParent, unsafe { + FormatElement::BestFitting(BestFittingElement::from_vec_unchecked(vec![ + Box::new([FormatElement::StaticText { text: "a" }]), + Box::new([FormatElement::StaticText { text: "b" }]), + ])) + }]); + + let mut document = Document::from(vec![ + // First group, processes the interned element for the first time. + FormatElement::Tag(StartGroup(tag::Group::new())), + FormatElement::Interned(interned.clone()), + FormatElement::Tag(EndGroup), + // Second group, references the already-processed interned element. + FormatElement::Tag(StartGroup(tag::Group::new())), + FormatElement::Interned(interned), + FormatElement::Tag(EndGroup), + ]); + + // After propagation, both groups _should_ be expanded. + document.propagate_expand(); + + assert_eq!( + &std::format!("{document}"), + r#"[ + group(expand: propagated, [ + [ + expand_parent, + best_fitting([ + ["a"] + ["b"] + ]) + ] + ]), + group(expand: propagated, []) ]"# ); }