From 4963aa31184e230cb232feedcd6f84f379dca31c Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 16 Aug 2024 21:55:30 -0700 Subject: [PATCH] fix autofmt: don't panic when writing blocks out without a srcfile (#2854) * fix: don't panic when writing blocks out * also fix serialization for hotreload * fix windows line endings --- packages/autofmt/src/lib.rs | 2 +- packages/autofmt/src/writer.rs | 30 ++++++--- packages/autofmt/tests/srcless.rs | 21 +++++++ packages/autofmt/tests/srcless/basic_expr.rsx | 42 +++++++++++++ packages/core/src/hotreload_utils.rs | 20 +++--- packages/core/src/nodes.rs | 63 +++++++++++++------ packages/hot-reload/src/lib.rs | 2 - packages/hot-reload/src/ws_receiver.rs | 4 +- packages/web/src/hot_reload.rs | 4 +- 9 files changed, 142 insertions(+), 46 deletions(-) create mode 100644 packages/autofmt/tests/srcless.rs create mode 100644 packages/autofmt/tests/srcless/basic_expr.rsx diff --git a/packages/autofmt/src/lib.rs b/packages/autofmt/src/lib.rs index 2f5c0cba8..88515351d 100644 --- a/packages/autofmt/src/lib.rs +++ b/packages/autofmt/src/lib.rs @@ -93,7 +93,7 @@ pub fn try_fmt_file( writer.out.indent_level = writer .out .indent - .count_indents(writer.src[rsx_start.line - 1]); + .count_indents(writer.src.get(rsx_start.line - 1).unwrap_or(&"")); // TESTME // Writing *should* not fail but it's possible that it does diff --git a/packages/autofmt/src/writer.rs b/packages/autofmt/src/writer.rs index 33b859d70..0f53e2cd3 100644 --- a/packages/autofmt/src/writer.rs +++ b/packages/autofmt/src/writer.rs @@ -607,7 +607,11 @@ impl<'a> Writer<'a> { let mut comments = VecDeque::new(); - for (id, line) in self.src[..line_start].iter().enumerate().rev() { + let Some(lines) = self.src.get(..line_start) else { + return comments; + }; + + for (id, line) in lines.iter().enumerate().rev() { if line.trim().starts_with("//") || line.is_empty() && id != 0 { if id != 0 { comments.push_front(id); @@ -621,7 +625,11 @@ impl<'a> Writer<'a> { } fn apply_comments(&mut self, mut comments: VecDeque) -> Result { while let Some(comment_line) = comments.pop_front() { - let line = &self.src[comment_line].trim(); + let Some(line) = self.src.get(comment_line) else { + continue; + }; + + let line = &line.trim(); if line.is_empty() { self.out.new_line()?; @@ -690,13 +698,15 @@ impl<'a> Writer<'a> { for attr in attributes { if self.current_span_is_primary(attr.span().start()) { - 'line: for line in self.src[..attr.span().start().line - 1].iter().rev() { - match (line.trim().starts_with("//"), line.is_empty()) { - (true, _) => return 100000, - (_, true) => continue 'line, - _ => break 'line, + if let Some(lines) = self.src.get(..attr.span().start().line - 1) { + 'line: for line in lines.iter().rev() { + match (line.trim().starts_with("//"), line.is_empty()) { + (true, _) => return 100000, + (_, true) => continue 'line, + _ => break 'line, + } } - } + }; } let name_len = match &attr.name { @@ -738,7 +748,9 @@ impl<'a> Writer<'a> { writeln!(self.out)?; for idx in start.line..end.line { - let line = &self.src[idx]; + let Some(line) = self.src.get(idx) else { + continue; + }; if line.trim().starts_with("//") { for _ in 0..self.out.indent_level + 1 { write!(self.out, " ")? diff --git a/packages/autofmt/tests/srcless.rs b/packages/autofmt/tests/srcless.rs new file mode 100644 index 000000000..afddcff40 --- /dev/null +++ b/packages/autofmt/tests/srcless.rs @@ -0,0 +1,21 @@ +use dioxus_rsx::CallBody; +use proc_macro2::TokenStream as TokenStream2; + +/// Ensure we can write RSX blocks without a source file +/// +/// Useful in code generation use cases where we still want formatted code. +#[test] +fn write_block_out() { + let src = include_str!("./srcless/basic_expr.rsx"); + + let tokens: TokenStream2 = syn::parse_str(src).unwrap(); + let parsed: CallBody = syn::parse2(tokens).unwrap(); + + let block = dioxus_autofmt::write_block_out(&parsed).unwrap(); + + // normalize line endings for windows tests to pass + pretty_assertions::assert_eq!( + block.trim().lines().collect::>().join("\n"), + src.trim().lines().collect::>().join("\n") + ); +} diff --git a/packages/autofmt/tests/srcless/basic_expr.rsx b/packages/autofmt/tests/srcless/basic_expr.rsx new file mode 100644 index 000000000..1c1e21524 --- /dev/null +++ b/packages/autofmt/tests/srcless/basic_expr.rsx @@ -0,0 +1,42 @@ + div { + "hi" + {children} + } + Fragment { + Fragment { + Fragment { + Fragment { + Fragment { + div { "Finally have a real node!" } + } + } + } + } + } + div { class, "hello world" } + h1 { class, "hello world" } + h1 { class, {children} } + h1 { class, id, {children} } + h1 { class, + "hello world" + {children} + } + h1 { id, + "hello world" + {children} + } + Other { class, children } + Other { class, + "hello world" + {children} + } + div { + class: "asdasd", + onclick: move |_| { + let a = 10; + let b = 40; + let c = 50; + }, + "hi" + } + div { class: "asd", "Jon" } diff --git a/packages/core/src/hotreload_utils.rs b/packages/core/src/hotreload_utils.rs index 643b224f5..e94ce0e50 100644 --- a/packages/core/src/hotreload_utils.rs +++ b/packages/core/src/hotreload_utils.rs @@ -10,7 +10,6 @@ use crate::{ }; #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] #[doc(hidden)] #[derive(Debug, PartialEq, Clone)] pub struct HotreloadedLiteral { @@ -19,7 +18,6 @@ pub struct HotreloadedLiteral { } #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] #[doc(hidden)] #[derive(Debug, PartialEq, Clone)] pub enum HotReloadLiteral { @@ -71,7 +69,6 @@ impl Hash for HotReloadLiteral { } #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] #[doc(hidden)] #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub struct FmtedSegments { @@ -98,6 +95,8 @@ impl FmtedSegments { } } +type StaticStr = &'static str; + #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] #[doc(hidden)] #[derive(Debug, PartialEq, Eq, Clone, Hash)] @@ -107,7 +106,7 @@ pub enum FmtSegment { feature = "serialize", serde(deserialize_with = "deserialize_string_leaky") )] - value: &'static str, + value: StaticStr, }, Dynamic { id: usize, @@ -313,16 +312,16 @@ impl DynamicValuePool { #[doc(hidden)] #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] pub struct HotReloadTemplateWithLocation { pub location: String, pub template: HotReloadedTemplate, } +type StaticTemplateArray = &'static [TemplateNode]; + #[doc(hidden)] #[derive(Debug, PartialEq, Clone)] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] pub struct HotReloadedTemplate { pub key: Option, pub dynamic_nodes: Vec, @@ -332,7 +331,7 @@ pub struct HotReloadedTemplate { feature = "serialize", serde(deserialize_with = "crate::nodes::deserialize_leaky") )] - pub roots: &'static [TemplateNode], + pub roots: StaticTemplateArray, /// The template that is computed from the hot reload roots template: Template, } @@ -425,7 +424,6 @@ impl HotReloadedTemplate { #[doc(hidden)] #[derive(Debug, PartialEq, Clone, Hash)] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] pub enum HotReloadDynamicNode { Dynamic(usize), Formatted(FmtedSegments), @@ -434,7 +432,6 @@ pub enum HotReloadDynamicNode { #[doc(hidden)] #[derive(Debug, PartialEq, Clone, Hash)] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] pub enum HotReloadDynamicAttribute { Dynamic(usize), Named(NamedAttribute), @@ -449,13 +446,13 @@ pub struct NamedAttribute { feature = "serialize", serde(deserialize_with = "crate::nodes::deserialize_string_leaky") )] - name: &'static str, + name: StaticStr, /// The namespace of this attribute. Does not exist in the HTML spec #[cfg_attr( feature = "serialize", serde(deserialize_with = "crate::nodes::deserialize_option_leaky") )] - namespace: Option<&'static str>, + namespace: Option, value: HotReloadAttributeValue, } @@ -477,7 +474,6 @@ impl NamedAttribute { #[doc(hidden)] #[derive(Debug, PartialEq, Clone, Hash)] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] pub enum HotReloadAttributeValue { Literal(HotReloadLiteral), Dynamic(usize), diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 33c9d754d..aec3c08ca 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -288,6 +288,11 @@ impl VNode { } } +type StaticStr = &'static str; +type StaticPathArray = &'static [&'static [u8]]; +type StaticTemplateArray = &'static [TemplateNode]; +type StaticTemplateAttributeArray = &'static [TemplateAttribute]; + /// A static layout of a UI tree that describes a set of dynamic and static nodes. /// /// This is the core innovation in Dioxus. Most UIs are made of static nodes, yet participate in diffing like any @@ -297,14 +302,13 @@ impl VNode { /// For this to work properly, the [`Template::name`] *must* be unique across your entire project. This can be done via variety of /// ways, with the suggested approach being the unique code location (file, line, col, etc). #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "serialize", serde(bound(deserialize = "'de: 'static")))] #[derive(Debug, Clone, Copy, Eq, PartialOrd, Ord)] pub struct Template { /// The list of template nodes that make up the template /// /// Unlike react, calls to `rsx!` can have multiple roots. This list supports that paradigm. #[cfg_attr(feature = "serialize", serde(deserialize_with = "deserialize_leaky"))] - pub roots: &'static [TemplateNode], + pub roots: StaticTemplateArray, /// The paths of each node relative to the root of the template. /// @@ -314,7 +318,7 @@ pub struct Template { feature = "serialize", serde(deserialize_with = "deserialize_bytes_leaky") )] - pub node_paths: &'static [&'static [u8]], + pub node_paths: StaticPathArray, /// The paths of each dynamic attribute relative to the root of the template /// @@ -322,9 +326,9 @@ pub struct Template { /// topmost element, not the `roots` field. #[cfg_attr( feature = "serialize", - serde(deserialize_with = "deserialize_bytes_leaky") + serde(deserialize_with = "deserialize_bytes_leaky", bound = "") )] - pub attr_paths: &'static [&'static [u8]], + pub attr_paths: StaticPathArray, } impl std::hash::Hash for Template { @@ -344,7 +348,9 @@ impl PartialEq for Template { } #[cfg(feature = "serialize")] -pub(crate) fn deserialize_string_leaky<'a, 'de, D>(deserializer: D) -> Result<&'a str, D::Error> +pub(crate) fn deserialize_string_leaky<'a, 'de, D>( + deserializer: D, +) -> Result<&'static str, D::Error> where D: serde::Deserializer<'de>, { @@ -355,7 +361,9 @@ where } #[cfg(feature = "serialize")] -fn deserialize_bytes_leaky<'a, 'de, D>(deserializer: D) -> Result<&'a [&'a [u8]], D::Error> +fn deserialize_bytes_leaky<'a, 'de, D>( + deserializer: D, +) -> Result<&'static [&'static [u8]], D::Error> where D: serde::Deserializer<'de>, { @@ -370,7 +378,7 @@ where } #[cfg(feature = "serialize")] -pub(crate) fn deserialize_leaky<'a, 'de, T, D>(deserializer: D) -> Result<&'a [T], D::Error> +pub(crate) fn deserialize_leaky<'a, 'de, T, D>(deserializer: D) -> Result<&'static [T], D::Error> where T: serde::Deserialize<'de>, D: serde::Deserializer<'de>, @@ -421,7 +429,11 @@ pub enum TemplateNode { /// The name of the element /// /// IE for a div, it would be the string "div" - tag: &'static str, + #[cfg_attr( + feature = "serialize", + serde(deserialize_with = "deserialize_string_leaky") + )] + tag: StaticStr, /// The namespace of the element /// @@ -431,17 +443,20 @@ pub enum TemplateNode { feature = "serialize", serde(deserialize_with = "deserialize_option_leaky") )] - namespace: Option<&'static str>, + namespace: Option, /// A list of possibly dynamic attributes for this element /// /// An attribute on a DOM node, such as `id="my-thing"` or `href="https://example.com"`. - #[cfg_attr(feature = "serialize", serde(deserialize_with = "deserialize_leaky"))] - attrs: &'static [TemplateAttribute], + #[cfg_attr( + feature = "serialize", + serde(deserialize_with = "deserialize_leaky", bound = "") + )] + attrs: StaticTemplateAttributeArray, /// A list of template nodes that define another set of template nodes #[cfg_attr(feature = "serialize", serde(deserialize_with = "deserialize_leaky"))] - children: &'static [TemplateNode], + children: StaticTemplateArray, }, /// This template node is just a piece of static text @@ -449,9 +464,9 @@ pub enum TemplateNode { /// The actual text #[cfg_attr( feature = "serialize", - serde(deserialize_with = "deserialize_string_leaky") + serde(deserialize_with = "deserialize_string_leaky", bound = "") )] - text: &'static str, + text: StaticStr, }, /// This template node is unknown, and needs to be created at runtime. @@ -652,15 +667,27 @@ pub enum TemplateAttribute { /// The name of this attribute. /// /// For example, the `href` attribute in `href="https://example.com"`, would have the name "href" - name: &'static str, + #[cfg_attr( + feature = "serialize", + serde(deserialize_with = "deserialize_string_leaky", bound = "") + )] + name: StaticStr, /// The value of this attribute, known at compile time /// /// Currently this only accepts &str, so values, even if they're known at compile time, are not known - value: &'static str, + #[cfg_attr( + feature = "serialize", + serde(deserialize_with = "deserialize_string_leaky", bound = "") + )] + value: StaticStr, /// The namespace of this attribute. Does not exist in the HTML spec - namespace: Option<&'static str>, + #[cfg_attr( + feature = "serialize", + serde(deserialize_with = "deserialize_option_leaky", bound = "") + )] + namespace: Option, }, /// The attribute in this position is actually determined dynamically at runtime diff --git a/packages/hot-reload/src/lib.rs b/packages/hot-reload/src/lib.rs index 76a684fc3..1633d3a87 100644 --- a/packages/hot-reload/src/lib.rs +++ b/packages/hot-reload/src/lib.rs @@ -16,7 +16,6 @@ pub use ws_receiver::*; /// A message the hot reloading server sends to the client #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] -#[serde(bound(deserialize = "'de: 'static"))] pub enum DevserverMsg { /// Attempt a hotreload /// This includes all the templates/literals/assets/binary patches that have changed in one shot @@ -47,7 +46,6 @@ pub enum ClientMsg { } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq)] -#[serde(bound(deserialize = "'de: 'static"))] pub struct HotReloadMsg { pub templates: Vec, pub assets: Vec, diff --git a/packages/hot-reload/src/ws_receiver.rs b/packages/hot-reload/src/ws_receiver.rs index 2e64da5c4..b9346fd81 100644 --- a/packages/hot-reload/src/ws_receiver.rs +++ b/packages/hot-reload/src/ws_receiver.rs @@ -51,8 +51,8 @@ impl NativeReceiver { match res { Ok(res) => match res { Message::Text(text) => { - let leaked: &'static str = Box::leak(text.into_boxed_str()); - let msg = serde_json::from_str::(leaked); + // let leaked: &'static str = Box::leak(text.into_boxed_str()); + let msg = serde_json::from_str::(&text); if let Ok(msg) = msg { return Some(Ok(msg)); } diff --git a/packages/web/src/hot_reload.rs b/packages/web/src/hot_reload.rs index 1af362e29..a437d336d 100644 --- a/packages/web/src/hot_reload.rs +++ b/packages/web/src/hot_reload.rs @@ -57,9 +57,9 @@ fn make_ws(tx: UnboundedSender, poll_interval: i32, reload: bool) // The devserver messages have some &'static strs in them, so we need to leak the source string let string: String = text.into(); - let leaked: &'static str = Box::leak(Box::new(string)); + // let leaked: &'static str = Box::leak(Box::new(string)); - match serde_json::from_str::(leaked) { + match serde_json::from_str::(&string) { Ok(DevserverMsg::HotReload(hr)) => _ = tx_.unbounded_send(hr), // todo: we want to throw a screen here that shows the user that the devserver has disconnected