fix dx fmt span attribute issue (#5132)

* fix: if expr spans causing invalid span locations

* cleanups

* clippy, typo

packages/autofmt/src/writer.rs

@@ -58,10 +58,10 @@ impl<'a> Writer<'a> {
     fn write_trailing_body_comments(&mut self, body: &CallBody) -> Result {
         if let Some(span) = body.span {
             self.out.indent_level += 1;
-            let comments = self.accumulate_comments(span.span().end());
+            let comments = self.accumulate_full_line_comments(span.span().end());
             if !comments.is_empty() {
                 self.out.new_line()?;
-                self.apply_comments(comments)?;
+                self.apply_line_comments(comments)?;
                 self.out.buf.pop(); // remove the trailing newline, forcing us to end at the end of the comment
             }
             self.out.indent_level -= 1;
@@ -425,9 +425,9 @@ impl<'a> Writer<'a> {
             ShortOptimization::NoOpt | ShortOptimization::PropsOnTop
         ) && self.leading_row_is_empty(brace.span.span().end())
         {
-            let comments = self.accumulate_comments(brace.span.span().end());
+            let comments = self.accumulate_full_line_comments(brace.span.span().end());
             if !comments.is_empty() {
-                self.apply_comments(comments)?;
+                self.apply_line_comments(comments)?;
                 self.out.tab()?;
             }
         }
@@ -487,7 +487,7 @@ impl<'a> Writer<'a> {
                     .comma
                     .as_ref()
                     .map(|c| c.span())
-                    .unwrap_or_else(|| self.final_span_of_attr(attr)),
+                    .unwrap_or_else(|| self.total_span_of_attr(attr)),
                 AttrType::Spread(attr) => attr.span(),
             };
 
@@ -565,7 +565,7 @@ impl<'a> Writer<'a> {
     }
 
     fn write_attribute_if_chain(&mut self, if_chain: &IfAttributeValue) -> Result {
-        let cond = self.unparse_expr(&if_chain.condition);
+        let cond = self.unparse_expr(&if_chain.if_expr.cond);
         write!(self.out, "if {cond} {{ ")?;
         self.write_attribute_value(&if_chain.then_value)?;
         write!(self.out, " }}")?;
@@ -619,13 +619,19 @@ impl<'a> Writer<'a> {
 
         whitespace = whitespace[offset..].trim();
 
+        // don't emit whitespace if the span is messed up for some reason
+        if final_span.line == 1 && final_span.column == 0 {
+            return Ok(());
+        };
+
         if whitespace.starts_with("//") {
             write!(self.out, " {whitespace}")?;
         }
 
         Ok(())
     }
-    fn accumulate_comments(&mut self, loc: LineColumn) -> VecDeque<usize> {
+
+    fn accumulate_full_line_comments(&mut self, loc: LineColumn) -> VecDeque<usize> {
         // collect all comments upwards
         // make sure we don't collect the comments of the node that we're currently under.
         let start = loc;
@@ -633,6 +639,11 @@ impl<'a> Writer<'a> {
 
         let mut comments = VecDeque::new();
 
+        // don't emit whitespace if the span is messed up for some reason
+        if loc.line == 1 && loc.column == 0 {
+            return comments;
+        };
+
         let Some(lines) = self.src.get(..line_start) else {
             return comments;
         };
@@ -649,7 +660,8 @@ impl<'a> Writer<'a> {
 
         comments
     }
-    fn apply_comments(&mut self, mut comments: VecDeque<usize>) -> Result {
+
+    fn apply_line_comments(&mut self, mut comments: VecDeque<usize>) -> Result {
         while let Some(comment_line) = comments.pop_front() {
             let Some(line) = self.src.get(comment_line) else {
                 continue;
@@ -668,16 +680,15 @@ impl<'a> Writer<'a> {
     }
 
     fn write_comments(&mut self, loc: LineColumn) -> Result {
-        let comments = self.accumulate_comments(loc);
+        let comments = self.accumulate_full_line_comments(loc);
-        self.apply_comments(comments)?;
+        self.apply_line_comments(comments)?;
-
         Ok(())
     }
 
     fn attr_value_len(&mut self, value: &AttributeValue) -> usize {
         match value {
             AttributeValue::IfExpr(if_chain) => {
-                let condition_len = self.retrieve_formatted_expr(&if_chain.condition).len();
+                let condition_len = self.retrieve_formatted_expr(&if_chain.if_expr.cond).len();
                 let value_len = self.attr_value_len(&if_chain.then_value);
                 let if_len = 2;
                 let brace_len = 2;
@@ -1069,17 +1080,13 @@ impl<'a> Writer<'a> {
         }
     }
 
-    fn final_span_of_attr(&self, attr: &Attribute) -> Span {
+    fn total_span_of_attr(&self, attr: &Attribute) -> Span {
         match &attr.value {
             AttributeValue::Shorthand(s) => s.span(),
             AttributeValue::AttrLiteral(l) => l.span(),
-            AttributeValue::EventTokens(closure) => closure.body.span(),
+            AttributeValue::EventTokens(closure) => closure.span(),
             AttributeValue::AttrExpr(exp) => exp.span(),
-            AttributeValue::IfExpr(ex) => ex
+            AttributeValue::IfExpr(ex) => ex.span(),
-                .else_value
-                .as_ref()
-                .map(|v| v.span())
-                .unwrap_or_else(|| ex.then_value.span()),
         }
     }
 

packages/autofmt/tests/samples.rs

@@ -61,4 +61,5 @@ twoway![
     prop_rsx,
     asset,
     collapse,
+    expr_on_conditional,
 ];

packages/autofmt/tests/samples/expr_on_conditional.rsx

@@ -0,0 +1,14 @@
+//! A comment about this file
+
+use dioxus::prelude::*;
+
+#[component]
+pub fn Sample() -> Element {
+    let is_active = use_signal(|| false);
+
+    rsx! {
+        div { class: if is_active() { "active" } else { "inactive" },
+            div { class: if is_active2() { "a" } else { "b" } }
+        }
+    }
+}

packages/rsx/src/attribute.rs

@@ -17,7 +17,7 @@
 use super::literal::HotLiteral;
 use crate::{innerlude::*, partial_closure::PartialClosure};
 
-use proc_macro2::TokenStream as TokenStream2;
+use proc_macro2::{Span, TokenStream as TokenStream2};
 use quote::{quote, quote_spanned, ToTokens, TokenStreamExt};
 use std::fmt::Display;
 use syn::{
@@ -173,7 +173,7 @@ impl Attribute {
             let AttributeValue::AttrExpr(expr) = &self.value else {
                 unreachable!("Spread attributes should always be expressions")
             };
-            return quote! { {#expr}.into_boxed_slice() };
+            return quote_spanned! { expr.span() => {#expr}.into_boxed_slice() };
         }
 
         let el_name = self
@@ -289,8 +289,9 @@ impl Attribute {
             }
         };
 
+        let attr_span = attribute.span();
         let completion_hints = self.completion_hints();
-        quote! {
+        quote_spanned! { attr_span =>
             Box::new([
                 {
                     #completion_hints
@@ -531,6 +532,7 @@ impl AttributeValue {
 /// A if else chain attribute value
 #[derive(PartialEq, Eq, Clone, Debug, Hash)]
 pub struct IfAttributeValue {
+    pub if_expr: ExprIf,
     pub condition: Expr,
     pub then_value: Box<AttributeValue>,
     pub else_value: Option<Box<AttributeValue>>,
@@ -560,7 +562,7 @@ impl IfAttributeValue {
                 return non_string_diagnostic(current_if_value.span());
             };
 
-            let condition = &current_if_value.condition;
+            let condition = &current_if_value.if_expr.cond;
             expression.extend(quote! {
                 if #condition {
                     #new.to_string()
@@ -599,8 +601,8 @@ impl IfAttributeValue {
         }
     }
 
-    fn span(&self) -> proc_macro2::Span {
+    fn span(&self) -> Span {
-        self.then_value.span()
+        self.if_expr.span()
     }
 
     fn is_terminated(&self) -> bool {
@@ -646,7 +648,7 @@ impl IfAttributeValue {
         match stmt {
             syn::Stmt::Expr(exp, None) => {
                 // Try parsing the statement as an IfmtInput by passing it through tokens
-                let value: Result<HotLiteral, syn::Error> = syn::parse2(quote! { #exp });
+                let value: Result<HotLiteral, syn::Error> = syn::parse2(exp.to_token_stream());
                 Ok(match value {
                     Ok(res) => Box::new(AttributeValue::AttrLiteral(res)),
                     Err(_) => Box::new(AttributeValue::AttrExpr(PartialExpr::from_expr(exp))),
@@ -663,9 +665,10 @@ impl IfAttributeValue {
         contains_expression: bool,
     ) {
         let IfAttributeValue {
-            condition,
+            if_expr,
             then_value,
             else_value,
+            ..
         } = self;
 
         // Quote an attribute value and convert the value to a string if it is formatted
@@ -712,21 +715,20 @@ impl IfAttributeValue {
             Some(other) => {
                 let other = quote_attribute_value_string(other, contains_expression);
                 if terminated {
-                    quote! { #other }
+                    other
                 } else {
-                    quote! { Some(#other) }
+                    quote_spanned! { other.span() => Some(#other) }
                 }
             }
             None => quote! { None },
         };
 
-        tokens.append_all(quote! {
+        let condition = &if_expr.cond;
-            {
+        tokens.append_all(quote_spanned! { if_expr.span()=>
-                if #condition {
+            if #condition {
-                    #then_value
+                #then_value
-                } else {
+            } else {
-                    #else_value
+                #else_value
-                }
             }
         });
     }
@@ -756,7 +758,7 @@ impl Parse for IfAttributeValue {
                     // If it is a block, then the else is terminated
                     Expr::Block(block) => Self::parse_attribute_value_from_block(&block.block)?,
                     // Otherwise try to parse it as an if expression
-                    _ => Box::new(syn::parse2(quote! { #else_branch })?),
+                    _ => Box::new(syn::parse2(else_branch.to_token_stream())?),
                 };
                 Some(attribute_value)
             }
@@ -764,7 +766,8 @@ impl Parse for IfAttributeValue {
         };
 
         Ok(Self {
-            condition: *if_expr.cond,
+            condition: *if_expr.cond.clone(),
+            if_expr,
             then_value,
             else_value,
         })