Skip to content

Commit

Permalink
fix: avoid stripping attr value braces when consecutive attr is a spread
Browse files Browse the repository at this point in the history
  • Loading branch information
@bram209
bram209 committed Jan 30, 2025
1 parent 47c43e0 commit f6c5027
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 17 deletions.
65 changes: 50 additions & 15 deletions formatter/src/formatter/attribute.rs
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
use rstml::node::{FnBinding, KeyedAttribute, KeyedAttributeValue, NodeAttribute};
use syn::{spanned::Spanned, Expr};
use rstml::node::{FnBinding, KeyedAttribute, KeyedAttributeValue, NodeAttribute, NodeBlock};
use syn::{spanned::Spanned, Expr, RangeLimits, Stmt};

use crate::{formatter::Formatter, AttributeValueBraceStyle as Braces};

use super::ExpressionFormatter;

impl Formatter<'_> {
pub fn attribute(&mut self, attribute: &NodeAttribute) {
pub fn attribute(&mut self, attribute: &NodeAttribute, next_attribute: Option<&NodeAttribute>) {
self.flush_comments(attribute.span().start().line - 1, false);
match attribute {
NodeAttribute::Attribute(k) => self.keyed_attribute(k),
NodeAttribute::Attribute(k) => self.keyed_attribute(k, next_attribute),
NodeAttribute::Block(b) => self.node_block(b),
}
}

pub fn keyed_attribute(&mut self, attribute: &KeyedAttribute) {
fn keyed_attribute(
&mut self,
attribute: &KeyedAttribute,
next_attribute: Option<&NodeAttribute>,
) {
self.node_name(&attribute.key);

match &attribute.possible_value {
Expand All @@ -28,7 +32,7 @@ impl Formatter<'_> {
.copied();

self.printer.word("=");
self.attribute_value(&expr.value, formatter);
self.attribute_value(&expr.value, formatter, next_attribute);
}
}
}
Expand All @@ -46,25 +50,56 @@ impl Formatter<'_> {
self.printer.word(")");
}

fn attribute_value(&mut self, value: &Expr, formatter: Option<ExpressionFormatter>) {
match (self.settings.attr_value_brace_style, value) {
(Braces::Always, syn::Expr::Block(_)) => {
fn attribute_value(
&mut self,
value: &Expr,
formatter: Option<ExpressionFormatter>,
next_attribute: Option<&NodeAttribute>,
) {
match (self.settings.attr_value_brace_style, value, next_attribute) {
(Braces::WhenRequired, syn::Expr::Block(_), Some(next))
if is_spread_attribute(next) =>
{
// If the next attribute is a spread attribute, make sure that the braces are not stripped from the expression
// to avoid an ambiguity in the parser (i.e. `foo=bar {..}` could be interpreted as initialization of a struct called `bar`, instead of two separate attributes)
self.node_value_expr(value, false, true, formatter)
}
(Braces::Always, syn::Expr::Block(_), _) => {
self.node_value_expr(value, false, false, formatter)
}
(Braces::AlwaysUnlessLit, syn::Expr::Block(_) | syn::Expr::Lit(_)) => {
(Braces::AlwaysUnlessLit, syn::Expr::Block(_) | syn::Expr::Lit(_), _) => {
self.node_value_expr(value, false, true, formatter)
}
(Braces::Always | Braces::AlwaysUnlessLit, _) => {
(Braces::Always | Braces::AlwaysUnlessLit, _, _) => {
self.printer.word("{");
self.node_value_expr(value, false, false, formatter);
self.printer.word("}");
}
(Braces::WhenRequired, _) => self.node_value_expr(value, true, true, formatter),
(Braces::Preserve, _) => self.node_value_expr(value, false, false, formatter),
(Braces::WhenRequired, _, _) => self.node_value_expr(value, true, true, formatter),
(Braces::Preserve, _, _) => self.node_value_expr(value, false, false, formatter),
}
}
}

fn is_spread_attribute(attr: &NodeAttribute) -> bool {
if let NodeAttribute::Block(NodeBlock::ValidBlock(block)) = attr {
if let [Stmt::Expr(
Expr::Range(syn::ExprRange {
start: None,
limits: RangeLimits::HalfOpen(..),
end,
..
}),
None,
)] = block.stmts.as_slice()
{
return matches!(end.as_deref(), None | Some(Expr::Path(_)));
}
}

false
}

#[cfg(test)]
mod tests {
use insta::assert_snapshot;
Expand All @@ -78,7 +113,7 @@ mod tests {
($($tt:tt)*) => {{
let attr = attribute! { $($tt)* };
format_with(FormatterSettings::default(), |formatter| {
formatter.attribute(&attr);
formatter.attribute(&attr, None);
})
}};
}
Expand All @@ -92,7 +127,7 @@ mod tests {
};

format_with(settings, |formatter| {
formatter.attribute(&attr);
formatter.attribute(&attr, None);
})
}};
}
Expand Down
32 changes: 30 additions & 2 deletions formatter/src/formatter/element.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Formatter<'_> {
[attribute] => {
self.printer.cbox(0);
self.printer.nbsp();
self.attribute(attribute);
self.attribute(attribute, None);

if trailing_space {
self.printer.nbsp();
Expand All @@ -61,7 +61,7 @@ impl Formatter<'_> {

let mut iter = attributes.iter().peekable();
while let Some(attr) = iter.next() {
self.attribute(attr);
self.attribute(attr, iter.peek().copied());

if iter.peek().is_some() {
self.printer.space()
Expand Down Expand Up @@ -168,11 +168,13 @@ fn is_self_closing(element: &NodeElement, name: &str, closing_tag_style: Closing

#[cfg(test)]
mod tests {

use indoc::indoc;

use crate::{
formatter::{ClosingTagStyle, FormatterSettings},
test_helpers::{element, format_element_from_string, format_with},
AttributeValueBraceStyle,
};

macro_rules! format_element {
Expand All @@ -181,6 +183,15 @@ mod tests {
}};
}

macro_rules! format_element_with_brace_style {
($style:expr, $($tt:tt)*) => {{
format_element_with!(FormatterSettings {
attr_value_brace_style: $style,
..Default::default()
}, $($tt)*)
}};
}

macro_rules! format_element_with_closing_style {
($style:expr, $($tt:tt)*) => {{
format_element_with!(FormatterSettings {
Expand Down Expand Up @@ -564,4 +575,21 @@ mod tests {
insta::assert_snapshot!(self_closing_formatted, @"<div />");
insta::assert_snapshot!(non_self_closing_formatted, @"<div></div>");
}

#[test]
fn preserve_braces_before_spreading() {
// Note: This is a special case where the braces are preserved before the spreading, to avoid an ambiguity in the parser
// (i.e. `foo=bar {..}` could be interpreted as a struct 'bar' initialization)
let when_required = format_element_with_brace_style! { AttributeValueBraceStyle::WhenRequired, <div foo={bar} {..} a={b} /> };
let when_required_with_named_props = format_element_with_brace_style! { AttributeValueBraceStyle::WhenRequired, <div foo={bar} {..other_props} /> };
let when_required_with_lit = format_element_with_brace_style! { AttributeValueBraceStyle::WhenRequired, <div foo={12} {..} a={"test"} {..} /> };
let unless_lit = format_element_with_brace_style! { AttributeValueBraceStyle::AlwaysUnlessLit, <div foo={12} {..} a=||{} {..} /> };
let always = format_element_with_brace_style! { AttributeValueBraceStyle::Always, <div foo={bar} {..} a=12 {..} /> };

insta::assert_snapshot!(when_required, @"<div foo={bar} {..} a=b />");
insta::assert_snapshot!(when_required_with_named_props, @"<div foo={bar} {..other_props} />");
insta::assert_snapshot!(when_required_with_lit, @"<div foo=12 {..} a=\"test\" {..} />");
insta::assert_snapshot!(unless_lit, @"<div foo=12 {..} a={|| {}} {..} />");
insta::assert_snapshot!(always, @"<div foo={bar} {..} a={12} {..} />");
}
}

0 comments on commit f6c5027

Please sign in to comment.