diff --git a/packages/autofmt/src/writer.rs b/packages/autofmt/src/writer.rs index afd31df36..16555b650 100644 --- a/packages/autofmt/src/writer.rs +++ b/packages/autofmt/src/writer.rs @@ -82,7 +82,8 @@ impl<'a> Writer<'a> { write!(self.out, "{name} {{")?; - self.write_rsx_block(attributes, spreads, children, brace)?; + let brace = brace.unwrap_or_default(); + self.write_rsx_block(attributes, spreads, children, &brace)?; write!(self.out, "}}")?; diff --git a/packages/core/tests/create_fragments.rs b/packages/core/tests/create_fragments.rs index c39ab2e86..9890d7009 100644 --- a/packages/core/tests/create_fragments.rs +++ b/packages/core/tests/create_fragments.rs @@ -77,7 +77,7 @@ fn fragments_across_components() { fn demo_child() -> Element { let world = "world"; - rsx! { "hellO!", {world} } + rsx! { "hellO!" {world} } } assert_eq!( diff --git a/packages/core/tests/create_passthru.rs b/packages/core/tests/create_passthru.rs index 31e19cfe4..2df55de04 100644 --- a/packages/core/tests/create_passthru.rs +++ b/packages/core/tests/create_passthru.rs @@ -81,7 +81,7 @@ fn dynamic_node_as_root() { fn app() -> Element { let a = 123; let b = 456; - rsx! { "{a}", "{b}" } + rsx! { "{a}" "{b}" } } let mut dom = VirtualDom::new(app); diff --git a/packages/router/examples/simple_routes.rs b/packages/router/examples/simple_routes.rs index b6425cf94..53c82dbeb 100644 --- a/packages/router/examples/simple_routes.rs +++ b/packages/router/examples/simple_routes.rs @@ -92,7 +92,7 @@ fn Route1(user_id: usize, dynamic: usize, query: String) -> Element { fn Route2(user_id: usize) -> Element { rsx! { pre { "Route2{{\n\tuser_id:{user_id}\n}}" } - {(0..user_id).map(|i| rsx!{ p { "{i}" } })}, + {(0..user_id).map(|i| rsx!{ p { "{i}" } })} p { "Footer" } Link { to: Route::Route3 { diff --git a/packages/rsx/src/element.rs b/packages/rsx/src/element.rs index b9095ba2b..2c2dd8df4 100644 --- a/packages/rsx/src/element.rs +++ b/packages/rsx/src/element.rs @@ -35,7 +35,7 @@ pub struct Element { pub children: Vec, /// the brace of the `div { }` - pub brace: Brace, + pub brace: Option, /// A list of diagnostics that were generated during parsing. This element might be a valid rsx_block /// but not technically a valid element - these diagnostics tell us what's wrong and then are used @@ -47,32 +47,51 @@ impl Parse for Element { fn parse(stream: ParseStream) -> Result { let name = stream.parse::()?; - let RsxBlock { - attributes: mut fields, - children, - brace, - spreads, - diagnostics, - } = stream.parse::()?; + // We very liberally parse elements - they might not even have a brace! + // This is designed such that we can throw a compile error but still give autocomplete + // ... partial completions mean we do some weird parsing to get the right completions + let mut brace = None; + let mut block = RsxBlock::default(); + + match stream.peek(Brace) { + // If the element is followed by a brace, it is complete. Parse the body + true => { + block = stream.parse::()?; + brace = Some(block.brace); + } + + // Otherwise, it is incomplete. Add a diagnostic + false => block.diagnostics.push( + name.span() + .error("Elements must be followed by braces") + .help("Did you forget a brace?"), + ), + } // Make sure these attributes have an el_name set for completions and Template generation - for attr in fields.iter_mut() { + for attr in block.attributes.iter_mut() { attr.el_name = Some(name.clone()); } + // Assemble the new element from the contents of the block let mut element = Element { - name, - raw_attributes: fields, - children, brace, - diagnostics, - spreads: spreads.clone(), + name, + raw_attributes: block.attributes, + children: block.children, + diagnostics: block.diagnostics, + spreads: block.spreads.clone(), merged_attributes: Vec::new(), }; + // And then merge the various attributes together + // The original raw_attributes are kept for lossless parsing used by hotreload/autofmt element.merge_attributes(); - for spread in spreads.iter() { + // And then merge the spreads *after* the attributes are merged. This ensures walking the + // merged attributes in path order stops before we hit the spreads, but spreads are still + // counted as dynamic attributes + for spread in block.spreads.iter() { element.merged_attributes.push(Attribute { name: AttributeName::Spread(spread.dots), colon: None, @@ -171,10 +190,13 @@ impl ToTokens for Element { let ns = ns(quote!(NAME_SPACE)); let el_name = el_name.tag_name(); let diagnostics = &el.diagnostics; + let completion_hints = &el.completion_hints(); // todo: generate less code if there's no diagnostics by not including the curlies tokens.append_all(quote! { { + #completion_hints + #diagnostics dioxus_core::TemplateNode::Element { @@ -294,6 +316,29 @@ impl Element { None } + + fn completion_hints(&self) -> TokenStream2 { + // If there is already a brace, we don't need any completion hints + if self.brace.is_some() { + return quote! {}; + } + + let ElementName::Ident(name) = &self.name else { + return quote! {}; + }; + + quote! { + { + #[allow(dead_code)] + #[doc(hidden)] + mod __completions { + fn ignore() { + super::dioxus_elements::elements::completions::CompleteWithBraces::#name + } + } + } + } + } } #[derive(PartialEq, Eq, Clone, Debug, Hash)] diff --git a/packages/rsx/src/node.rs b/packages/rsx/src/node.rs index 1f18588e0..43a2e93d1 100644 --- a/packages/rsx/src/node.rs +++ b/packages/rsx/src/node.rs @@ -7,7 +7,7 @@ use quote::ToTokens; use syn::{ parse::{Parse, ParseStream}, spanned::Spanned, - token::{self, Brace}, + token::{self}, Ident, LitStr, Result, Token, }; @@ -76,16 +76,17 @@ impl Parse for BodyNode { return Ok(BodyNode::Element(stream.parse::()?)); } - // this is an Element if path match of: + // this is an Element if the path is: // // - one ident - // - followed by `{` // - 1st char is lowercase // - no underscores (reserved for components) + // And it is not: + // - the start of a path with components // // example: // div {} - if stream.peek(Ident) && stream.peek2(Brace) { + if stream.peek(Ident) && !stream.peek2(Token![::]) { let ident = stream.fork().parse::().unwrap(); let el_name = ident.to_string(); let first_char = el_name.chars().next().unwrap(); diff --git a/packages/rsx/src/rsx_block.rs b/packages/rsx/src/rsx_block.rs index 5aedd834e..b15c36b4d 100644 --- a/packages/rsx/src/rsx_block.rs +++ b/packages/rsx/src/rsx_block.rs @@ -11,7 +11,7 @@ use proc_macro2::Span; use proc_macro2_diagnostics::SpanDiagnosticExt; use syn::{ ext::IdentExt, - parse::{Parse, ParseBuffer}, + parse::{Parse, ParseBuffer, ParseStream}, spanned::Spanned, token::{self, Brace}, Expr, Ident, LitStr, Token, @@ -31,7 +31,7 @@ use syn::{ /// The name of the block is expected to be parsed by the parent parser. It will accept items out of /// order if possible and then bubble up diagnostics to the parent. This lets us give better errors /// and autocomplete -#[derive(PartialEq, Eq, Clone, Debug)] +#[derive(PartialEq, Eq, Clone, Debug, Default)] pub struct RsxBlock { pub brace: token::Brace, pub attributes: Vec, @@ -49,16 +49,37 @@ impl Parse for RsxBlock { } impl RsxBlock { + /// Only parse the children of the block - all others will be rejected + pub fn parse_children(content: &ParseBuffer) -> syn::Result { + let mut nodes = vec![]; + let mut diagnostics = Diagnostics::new(); + while !content.is_empty() { + nodes.push(Self::parse_body_node_with_comma_diagnostics( + content, + &mut diagnostics, + )?); + } + Ok(Self { + children: nodes, + diagnostics, + ..Default::default() + }) + } + pub fn parse_inner(content: &ParseBuffer, brace: token::Brace) -> syn::Result { let mut items = vec![]; let mut diagnostics = Diagnostics::new(); + // If we are after attributes, we can try to provide better completions and diagnostics + // by parsing the following nodes as body nodes if they are ambiguous, we can parse them as body nodes + let mut after_attributes = false; + // Lots of manual parsing but it's important to do it all here to give the best diagnostics possible // We can do things like lookaheads, peeking, etc. to give better errors and autocomplete // We allow parsing in any order but complain if its done out of order. // Autofmt will fortunately fix this for us in most cases // - // Weo do this by parsing the unambiguous cases first and then do some clever lookahead to parse the rest + // We do this by parsing the unambiguous cases first and then do some clever lookahead to parse the rest while !content.is_empty() { // Parse spread attributes if content.peek(Token![..]) { @@ -89,6 +110,7 @@ impl RsxBlock { ); } items.push(RsxItem::Spread(attr)); + after_attributes = true; continue; } @@ -113,7 +135,7 @@ impl RsxBlock { continue; } - // Eagerly match on children, generally + // Eagerly match on completed children, generally if content.peek(LitStr) | content.peek(Token![for]) | content.peek(Token![if]) @@ -122,10 +144,13 @@ impl RsxBlock { // web components | (content.peek(Ident::peek_any) && content.peek2(Token![-])) // elements - | (content.peek(Ident::peek_any) && content.peek2(token::Brace)) - // todo: eager parse components? + | (content.peek(Ident::peek_any) && (after_attributes || content.peek2(token::Brace))) + // components + | (content.peek(Ident::peek_any) && (after_attributes || content.peek2(token::Brace) || content.peek2(Token![::]))) { - items.push(RsxItem::Child(content.parse::()?)); + items.push(RsxItem::Child( + Self::parse_body_node_with_comma_diagnostics(content, &mut diagnostics)?, + )); if !content.is_empty() && content.peek(Token![,]) { let comma = content.parse::()?; diagnostics.push( @@ -134,30 +159,20 @@ impl RsxBlock { ), ); } + after_attributes = true; continue; } // Parse shorthand attributes // todo: this might cause complications with partial expansion... think more about the cases // where we can imagine expansion and what better diagnostics we can provide - let looks_like_attribute = match content.fork().parse::() { - // If it is an ident, it is only a shorthand attribute if it starts with a lowercase letter - // Otherwise this is the start of a component - Ok(ident) => ident - .to_string() - .chars() - .next() - .unwrap() - .is_ascii_lowercase(), - Err(_) => false, - }; - if looks_like_attribute - && !content.peek2(Brace) - && !content.peek2(Token![:]) // regular attributes / components with generics - && !content.peek2(Token![-]) // web components - && !content.peek2(Token![<]) // generics on components - // generics on components - && !content.peek2(Token![::]) + if Self::peek_lowercase_ident(&content) + && !content.peek2(Brace) + && !content.peek2(Token![:]) // regular attributes / components with generics + && !content.peek2(Token![-]) // web components + && !content.peek2(Token![<]) // generics on components + // generics on components + && !content.peek2(Token![::]) { let attribute = content.parse::()?; @@ -176,7 +191,9 @@ impl RsxBlock { } // Finally just attempt a bodynode parse - items.push(RsxItem::Child(content.parse::()?)) + items.push(RsxItem::Child( + Self::parse_body_node_with_comma_diagnostics(content, &mut diagnostics)?, + )) } // Validate the order of the items @@ -204,6 +221,36 @@ impl RsxBlock { }) } + // Parse a body node with diagnostics for unnecessary trailing commas + fn parse_body_node_with_comma_diagnostics( + content: &ParseBuffer, + diagnostics: &mut Diagnostics, + ) -> syn::Result { + let body_node = content.parse::()?; + if !content.is_empty() && content.peek(Token![,]) { + let comma = content.parse::()?; + diagnostics.push( + comma + .span() + .warning("Elements and text nodes do not need to be separated by commas."), + ); + } + Ok(body_node) + } + + fn peek_lowercase_ident(stream: &ParseStream) -> bool { + let Ok(ident) = stream.fork().parse::() else { + return false; + }; + + ident + .to_string() + .chars() + .next() + .unwrap() + .is_ascii_lowercase() + } + /// Ensure the ordering of the items is correct /// - Attributes must come before children /// - Spreads must come before children @@ -551,4 +598,27 @@ mod tests { let _parsed: RsxBlock = syn::parse2(input).unwrap(); } + + #[test] + fn incomplete_root_elements() { + use syn::parse::Parser; + + let input = quote::quote! { + di + }; + + let parsed = RsxBlock::parse_children.parse2(input).unwrap(); + let children = parsed.children; + + assert_eq!(children.len(), 1); + if let BodyNode::Element(parsed) = &children[0] { + assert_eq!( + parsed.name, + ElementName::Ident(Ident::new("di", Span::call_site())) + ); + } else { + panic!("expected element, got {:?}", children); + } + assert!(parsed.diagnostics.is_empty()); + } } diff --git a/packages/rsx/src/template_body.rs b/packages/rsx/src/template_body.rs index 92bd73cd0..e3cc6d8ad 100644 --- a/packages/rsx/src/template_body.rs +++ b/packages/rsx/src/template_body.rs @@ -58,8 +58,6 @@ use self::location::DynIdx; use crate::innerlude::Attribute; use crate::*; use proc_macro2::TokenStream as TokenStream2; -use proc_macro2_diagnostics::SpanDiagnosticExt; -use syn::token::Brace; #[cfg(feature = "hot_reload")] use dioxus_core::prelude::Template; @@ -83,39 +81,19 @@ pub struct TemplateBody { pub template_idx: DynIdx, pub node_paths: Vec, pub attr_paths: Vec<(AttributePath, usize)>, - pub diagnostic: Diagnostics, + pub diagnostics: Diagnostics, current_path: Vec, } impl Parse for TemplateBody { /// Parse the nodes of the callbody as `Body`. fn parse(input: ParseStream) -> Result { - let brace = Brace::default(); - - let RsxBlock { - brace: _, - attributes, - spreads, - children, - diagnostics: _, // we don't care about the diagnostics here - ours might be different - } = RsxBlock::parse_inner(input, brace)?; - - let mut template = Self::new(children); - for spread in spreads { - template.diagnostic.push( - spread - .span() - .error("Spreads are only allowed in elements and components"), - ); - } - for attr in attributes { - template.diagnostic.push( - attr.span() - .error("Attributes are only allowed in elements and components"), - ); - } - - Ok(template) + let children = RsxBlock::parse_children(input)?; + let mut myself = Self::new(children.children); + myself + .diagnostics + .extend(children.diagnostics.into_diagnostics()); + Ok(myself) } } @@ -183,6 +161,8 @@ impl ToTokens for TemplateBody { let index = self.template_idx.get(); + let diagnostics = &self.diagnostics; + tokens.append_all(quote! { dioxus_core::Element::Ok({ #[doc(hidden)] // vscode please stop showing these in symbol search @@ -197,6 +177,8 @@ impl ToTokens for TemplateBody { attr_paths: &[ #( #attr_paths ),* ], }; + #diagnostics + { // NOTE: Allocating a temporary is important to make reads within rsx drop before the value is returned #[allow(clippy::let_and_return)] @@ -208,6 +190,7 @@ impl ToTokens for TemplateBody { ); __vnodes } + }) }); } @@ -225,7 +208,7 @@ impl TemplateBody { node_paths: Vec::new(), attr_paths: Vec::new(), current_path: Vec::new(), - diagnostic: Diagnostics::new(), + diagnostics: Diagnostics::new(), }; // Assign paths to all nodes in the template