yeast: Evaluate fields and locations in order

Instead of filtering out the location overrides and doing them all at
the end, we now evaluate them as we encounter them, gathering up all of
the node Ids (if any) in a list. This list is then traversed (trivially
so, if there are no entries) after we construct the node, to get the
final location (or None if no overrides were present), and the newly
constructed node's location is extended with this location information.

Author: tausbn

shared/yeast-macros/src/lower.rs

@@ -319,7 +319,7 @@ impl Node {
         let ctx = Ident::new("ctx", Span::call_site());
         let kind_str = self.kind.to_string();
 
-        if let Some(content) = &self.content {
+        let node_init = if let Some(content) = &self.content {
             let node = match content {
                 Content::Static(lit) => quote! { #ctx.literal(#kind_str, #lit) },
                 Content::Computed(block) => {
@@ -362,97 +362,96 @@ impl Node {
                     }
                 }
             };
-            return self.with_location(node, &ctx);
-        }
+            quote! { let __node: yeast::Id = #node; }
+        } else {
+            TokenStream::new()
+        };
 
         let mut stmts = Vec::new();
         let mut field_args = Vec::new();
 
         for (field_counter, field) in self.fields.iter().enumerate() {
-            let Field::Named {
-                name,
-                value,
-                optional,
-            } = field
-            else {
-                continue;
-            };
-            let field_name = name.to_string();
-            let field_str = field_name
-                .strip_prefix("r#")
-                .unwrap_or(&field_name)
-                .to_string();
-            let temp = Ident::new(
-                &format!("__field_{field_str}_{field_counter}"),
-                Span::call_site(),
-            );
-
-            match value {
-                Template::Splice(block) => {
-                    debug_assert!(!optional);
+            match field {
+                Field::Location { block } => {
                     let block = rust_block(block);
                     stmts.push(quote! {
-                        let mut #temp: Vec<yeast::Id> = Vec::new();
-                        yeast::IntoFieldIds::extend_into(#block, &mut #temp);
-                    });
-                    field_args.push(quote! {
-                        if !#temp.is_empty() {
-                            __fields.push((#field_str, #temp));
-                        }
+                        yeast::IntoFieldIds::extend_into(#block, &mut __location_ids);
                     });
                 }
-                Template::Node(child) if *optional => {
-                    let label = fresh_fallible_label();
-                    let value = child.lower(&LoweringContext {
-                        fallible_scope: Some(&label),
-                    });
-                    stmts.push(quote! {
-                        #[allow(unused_labels)]
-                        let #temp: ::std::option::Option<yeast::Id> = #label: {
-                            ::std::option::Option::Some(#value)
-                        };
-                    });
-                    field_args.push(quote! {
-                        if let ::std::option::Option::Some(__id) = #temp {
-                            __fields.push((#field_str, vec![__id]));
+                Field::Named {
+                    name,
+                    value,
+                    optional,
+                } => {
+                    let field_name = name.to_string();
+                    let field_str = field_name
+                        .strip_prefix("r#")
+                        .unwrap_or(&field_name)
+                        .to_string();
+                    let temp = Ident::new(
+                        &format!("__field_{field_str}_{field_counter}"),
+                        Span::call_site(),
+                    );
+
+                    match value {
+                        Template::Splice(block) => {
+                            debug_assert!(!optional);
+                            let block = rust_block(block);
+                            stmts.push(quote! {
+                                let mut #temp: Vec<yeast::Id> = Vec::new();
+                                yeast::IntoFieldIds::extend_into(#block, &mut #temp);
+                            });
+                            field_args.push(quote! {
+                                if !#temp.is_empty() {
+                                    __fields.push((#field_str, #temp));
+                                }
+                            });
                         }
-                    });
-                }
-                Template::Node(child) => {
-                    let value = child.lower(context);
-                    stmts.push(quote! { let #temp: yeast::Id = #value; });
-                    field_args.push(quote! {
-                        __fields.push((#field_str, vec![#temp]));
-                    });
+                        Template::Node(child) if *optional => {
+                            let label = fresh_fallible_label();
+                            let value = child.lower(&LoweringContext {
+                                fallible_scope: Some(&label),
+                            });
+                            stmts.push(quote! {
+                                #[allow(unused_labels)]
+                                let #temp: ::std::option::Option<yeast::Id> = #label: {
+                                    ::std::option::Option::Some(#value)
+                                };
+                            });
+                            field_args.push(quote! {
+                                if let ::std::option::Option::Some(__id) = #temp {
+                                    __fields.push((#field_str, vec![__id]));
+                                }
+                            });
+                        }
+                        Template::Node(child) => {
+                            let value = child.lower(context);
+                            stmts.push(quote! { let #temp: yeast::Id = #value; });
+                            field_args.push(quote! {
+                                __fields.push((#field_str, vec![#temp]));
+                            });
+                        }
+                    }
                 }
             }
         }
 
-        let node = quote! {
-            {
-                #(#stmts)*
+        let build_node = if self.content.is_none() {
+            quote! {
                 let mut __fields: Vec<(&str, Vec<yeast::Id>)> = Vec::new();
                 #(#field_args)*
-                #ctx.node(#kind_str, __fields)
+                let __node: yeast::Id = #ctx.node(#kind_str, __fields);
             }
+        } else {
+            TokenStream::new()
         };
-        self.with_location(node, &ctx)
-    }
-
-    fn with_location(&self, node: TokenStream, ctx: &Ident) -> TokenStream {
-        let locations = self.fields.iter().filter_map(|field| match field {
-            Field::Named { .. } => None,
-            Field::Location { block } => Some(rust_block(block)),
-        });
-        let locations: Vec<_> = locations.collect();
-        if locations.is_empty() {
-            return node;
-        }
 
         quote! {
             {
                 let mut __location_ids: Vec<yeast::Id> = Vec::new();
-                #(yeast::IntoFieldIds::extend_into(#locations, &mut __location_ids);)*
+                #node_init
+                #(#stmts)*
+                #build_node
                 let __source_range = __location_ids
                     .into_iter()
                     .filter_map(|__source| {
@@ -461,7 +460,6 @@ impl Node {
                             .and_then(|__node| __node.source_range())
                     })
                     .reduce(yeast::Range::union);
-                let __node: yeast::Id = #node;
                 #ctx.set_node_source_range(__node, __source_range)
             }
         }

shared/yeast/tests/test.rs

@@ -1756,6 +1756,31 @@ fn test_multiple_location_fields_combine_iterable_locations() {
     assert_eq!(arguments.byte_range(), 0..5);
 }
 
+#[test]
+fn test_location_and_named_fields_are_evaluated_left_to_right() {
+    let rule: Rule = rule!(
+        (assignment
+            left: (identifier) @left
+            right: (integer) @right)
+        =>
+        call {
+            let mut order = Vec::new();
+            let call = tree!((call
+                location: {order.push("first location"); left}
+                method: {order.push("method"); left}
+                location: {order.push("second location"); right}
+                arguments: {order.push("arguments"); right}));
+            assert_eq!(
+                order,
+                ["first location", "method", "second location", "arguments"]
+            );
+            call
+        }
+    );
+
+    run_and_ast("x = 1", vec![rule]);
+}
+
 #[test]
 fn test_location_field_is_available_on_literal_nodes() {
     let rule: Rule = rule!(