From db23197925c4d79bc615f98942291a71232e1e61 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Thu, 25 Dec 2025 16:50:50 -0500 Subject: [PATCH] fix dx fmt span attribute issue (#5132) * fix: if expr spans causing invalid span locations * cleanups * clippy, typo --- packages/autofmt/src/writer.rs | 45 +++++++++++-------- packages/autofmt/tests/samples.rs | 1 + .../tests/samples/expr_on_conditional.rsx | 14 ++++++ packages/rsx/src/attribute.rs | 41 +++++++++-------- 4 files changed, 63 insertions(+), 38 deletions(-) create mode 100644 packages/autofmt/tests/samples/expr_on_conditional.rsx diff --git a/packages/autofmt/src/writer.rs b/packages/autofmt/src/writer.rs index 044802c8d..0a04256ab 100644 --- a/packages/autofmt/src/writer.rs +++ b/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 { + + fn accumulate_full_line_comments(&mut self, loc: LineColumn) -> VecDeque { // 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) -> Result { + + fn apply_line_comments(&mut self, mut comments: VecDeque) -> 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); - self.apply_comments(comments)?; - + let comments = self.accumulate_full_line_comments(loc); + 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 - .else_value - .as_ref() - .map(|v| v.span()) - .unwrap_or_else(|| ex.then_value.span()), + AttributeValue::IfExpr(ex) => ex.span(), } } diff --git a/packages/autofmt/tests/samples.rs b/packages/autofmt/tests/samples.rs index 2e7306fb9..9e05c1aa8 100644 --- a/packages/autofmt/tests/samples.rs +++ b/packages/autofmt/tests/samples.rs @@ -61,4 +61,5 @@ twoway![ prop_rsx, asset, collapse, + expr_on_conditional, ]; diff --git a/packages/autofmt/tests/samples/expr_on_conditional.rsx b/packages/autofmt/tests/samples/expr_on_conditional.rsx new file mode 100644 index 000000000..0415b7c37 --- /dev/null +++ b/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" } } + } + } +} diff --git a/packages/rsx/src/attribute.rs b/packages/rsx/src/attribute.rs index 9353febc5..001c4c27e 100644 --- a/packages/rsx/src/attribute.rs +++ 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, pub else_value: Option>, @@ -560,7 +562,7 @@ impl IfAttributeValue { return non_string_diagnostic(current_if_value.span()); }; - let condition = ¤t_if_value.condition; + let condition = ¤t_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 { - self.then_value.span() + fn span(&self) -> 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 = syn::parse2(quote! { #exp }); + let value: Result = 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! { - { - if #condition { - #then_value - } else { - #else_value - } + let condition = &if_expr.cond; + tokens.append_all(quote_spanned! { if_expr.span()=> + if #condition { + #then_value + } else { + #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, })