Fix rsx autocomplete and diagnostics in the root; provide better completions after attributes are finished (#2656)

* fix rsx autocomplete and diagnostics in the root; provide better completions after attributes are finished

* clean up pr to use internal child parsing

* Fix peek_lowercase name

* fix comma diagnostics in the root

* Fix parsing components with a path

* remove simple routes trailing comma

* Fix incomplete_root_elements test

* Remove trailing commas from root body node in tests

---------

Co-authored-by: Jonathan Kelley <jkelleyrtp@gmail.com>

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, "}}")?;
 

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!(

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);

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 {

packages/rsx/src/element.rs

@@ -35,7 +35,7 @@ pub struct Element {
     pub children: Vec<BodyNode>,
 
     /// the brace of the `div { }`
-    pub brace: Brace,
+    pub brace: Option<Brace>,
 
     /// 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<Self> {
         let name = stream.parse::<ElementName>()?;
 
-        let RsxBlock {
+        // We very liberally parse elements - they might not even have a brace!
-            attributes: mut fields,
+        // This is designed such that we can throw a compile error but still give autocomplete
-            children,
+        // ... partial completions mean we do some weird parsing to get the right completions
-            brace,
+        let mut brace = None;
-            spreads,
+        let mut block = RsxBlock::default();
-            diagnostics,
+
-        } = stream.parse::<RsxBlock>()?;
+        match stream.peek(Brace) {
+            // If the element is followed by a brace, it is complete. Parse the body
+            true => {
+                block = stream.parse::<RsxBlock>()?;
+                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,
+            name,
-            spreads: spreads.clone(),
+            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)]

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::<Element>()?));
         }
 
-        // 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::<Ident>().unwrap();
             let el_name = ident.to_string();
             let first_char = el_name.chars().next().unwrap();

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<Attribute>,
@@ -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<Self> {
+        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<Self> {
         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))
+                | (content.peek(Ident::peek_any) && (after_attributes || content.peek2(token::Brace)))
-            // todo: eager parse components?
+                // components
+                | (content.peek(Ident::peek_any) && (after_attributes || content.peek2(token::Brace) || content.peek2(Token![::])))
             {
-                items.push(RsxItem::Child(content.parse::<BodyNode>()?));
+                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::<Token![,]>()?;
                     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::<Ident>() {
+            if Self::peek_lowercase_ident(&content)
-                // If it is an ident, it is only a shorthand attribute if it starts with a lowercase letter
+                    && !content.peek2(Brace)
-                // Otherwise this is the start of a component
+                    && !content.peek2(Token![:]) // regular attributes / components with generics
-                Ok(ident) => ident
+                    && !content.peek2(Token![-]) // web components
-                    .to_string()
+                    && !content.peek2(Token![<]) // generics on components
-                    .chars()
+                    // generics on components
-                    .next()
+                    && !content.peek2(Token![::])
-                    .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![::])
             {
                 let attribute = content.parse::<Attribute>()?;
 
@@ -176,7 +191,9 @@ impl RsxBlock {
             }
 
             // Finally just attempt a bodynode parse
-            items.push(RsxItem::Child(content.parse::<BodyNode>()?))
+            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<BodyNode> {
+        let body_node = content.parse::<BodyNode>()?;
+        if !content.is_empty() && content.peek(Token![,]) {
+            let comma = content.parse::<Token![,]>()?;
+            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::<Ident>() 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());
+    }
 }

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<NodePath>,
     pub attr_paths: Vec<(AttributePath, usize)>,
-    pub diagnostic: Diagnostics,
+    pub diagnostics: Diagnostics,
     current_path: Vec<u8>,
 }
 
 impl Parse for TemplateBody {
     /// Parse the nodes of the callbody as `Body`.
     fn parse(input: ParseStream) -> Result<Self> {
-        let brace = Brace::default();
+        let children = RsxBlock::parse_children(input)?;
-
+        let mut myself = Self::new(children.children);
-        let RsxBlock {
+        myself
-            brace: _,
+            .diagnostics
-            attributes,
+            .extend(children.diagnostics.into_diagnostics());
-            spreads,
+        Ok(myself)
-            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)
     }
 }
 
@@ -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