Unified: Exclude trailing commas from locations

Trailing commas appear in a lot of places in the swift-syntax AST, and
are all attached to the individual values separated by said commas, not
the parent node. This means that the default heuristics thinks a call
like `foo(1, 2)` has arguments `1,` and `2`, which is undesirable.

Since this applies uniformly for a lot of different comma-separated
lists, I decided that adding a generic mechanism for ignoring certain
fields was the least invasive change. Thus, we now add `trailingComma`
to that list, and this means it's automatically excluded from the
location calculations, without need a bunch of specific changes to the
existing rules.

Author: tausbn

shared/yeast/doc/yeast.md

@@ -272,6 +272,12 @@ let synthetic = tree_at!(ctx, source, (synthetic_node child: (nested value: {chi
 let synthetic = tree_spanning!(ctx, nodes, (synthetic_node child: {child}));
 ```
 
+For input fields whose leading or trailing syntax should never belong to rule
+results, configure them once with
+`DesugaringConfig::with_ignored_location_fields(...)`. For example, ignoring
+`trailingComma` retains the rest of each matched list element without requiring
+every rule to capture or handle the comma.
+
 For literals, `ctx.literal_at_start_of(...)` creates an empty range at another
 node's start.
 

shared/yeast/src/lib.rs

@@ -597,6 +597,25 @@ impl Ast {
         self.nodes.get(id.0)
     }
 
+    fn source_range_ignoring_fields(
+        &self,
+        id: Id,
+        ignored_fields: &[&str],
+    ) -> Option<Range> {
+        let node = self.get_node(id)?;
+        let source_range = node.source_range()?;
+        let ignored_ranges = node
+            .fields
+            .iter()
+            .filter(|(field_id, _)| {
+                self.field_name_for_id(**field_id)
+                    .is_some_and(|name| ignored_fields.contains(&name))
+            })
+            .flat_map(|(_, children)| children)
+            .filter_map(|child| self.get_node(*child).and_then(Node::source_range));
+        Some(source_range.ignoring_boundary_ranges(ignored_ranges))
+    }
+
     pub fn print(&self, source: &str, root_id: Id) -> Value {
         let root = &self.nodes()[root_id.0];
         self.print_node(root, source)
@@ -1100,6 +1119,7 @@ pub struct Rule<C = ()> {
     query: QueryNode,
     guard: Option<Guard<C>>,
     transform: Transform<C>,
+    ignored_location_fields: Vec<&'static str>,
     /// If true, after this rule fires on a node the engine will try to
     /// re-apply this same rule on the result root. Defaults to false:
     /// each rule fires at most once on a given node, which prevents
@@ -1114,6 +1134,7 @@ impl<C> Rule<C> {
             query,
             guard: None,
             transform,
+            ignored_location_fields: Vec::new(),
             repeated: false,
         }
     }
@@ -1125,6 +1146,7 @@ impl<C> Rule<C> {
             query,
             guard: Some(guard),
             transform,
+            ignored_location_fields: Vec::new(),
             repeated: false,
         }
     }
@@ -1138,6 +1160,10 @@ impl<C> Rule<C> {
         self
     }
 
+    fn set_ignored_location_fields(&mut self, fields: &[&'static str]) {
+        self.ignored_location_fields = fields.to_vec();
+    }
+
     /// Attempt to match this rule's query against `node`, returning the raw
     /// captures on success. Does not evaluate the guard or invoke the
     /// transform.
@@ -1176,10 +1202,8 @@ impl<C> Rule<C> {
         translator: TranslatorHandle<'_, C>,
     ) -> Result<Vec<Id>, String> {
         fresh.next_scope();
-        let source_range = ast.get_node(node).and_then(|n| match n.content {
-            NodeContent::Range(r) => Some(r),
-            _ => n.source_range,
-        });
+        let source_range =
+            ast.source_range_ignoring_fields(node, &self.ignored_location_fields);
         (self.transform)(ast, captures, fresh, source_range, user_ctx, translator)
     }
 }
@@ -1465,6 +1489,9 @@ pub struct DesugaringConfig<C = ()> {
     /// node types are used (i.e. the desugared AST has the same node types
     /// as the tree-sitter grammar).
     pub output_node_types_yaml: Option<&'static str>,
+    /// Input field names whose boundary ranges are excluded from rule-result
+    /// locations.
+    pub ignored_location_fields: Vec<&'static str>,
 }
 
 // Manual `Default` impl so users with a custom `C` that doesn't implement
@@ -1474,6 +1501,7 @@ impl<C> Default for DesugaringConfig<C> {
         Self {
             phases: Vec::new(),
             output_node_types_yaml: None,
+            ignored_location_fields: Vec::new(),
         }
     }
 }
@@ -1490,12 +1518,30 @@ impl<C> DesugaringConfig<C> {
         mut self,
         name: impl Into<String>,
         kind: PhaseKind,
-        rules: Vec<Rule<C>>,
+        mut rules: Vec<Rule<C>>,
     ) -> Self {
+        for rule in &mut rules {
+            rule.set_ignored_location_fields(&self.ignored_location_fields);
+        }
         self.phases.push(Phase::new(name, kind, rules));
         self
     }
 
+    /// Ignore boundary syntax stored under any of these input field names when
+    /// calculating matched locations for rule results.
+    pub fn with_ignored_location_fields(
+        mut self,
+        fields: impl IntoIterator<Item = &'static str>,
+    ) -> Self {
+        self.ignored_location_fields = fields.into_iter().collect();
+        for phase in &mut self.phases {
+            for rule in &mut phase.rules {
+                rule.set_ignored_location_fields(&self.ignored_location_fields);
+            }
+        }
+        self
+    }
+
     pub fn with_output_node_types_yaml(mut self, yaml: &'static str) -> Self {
         self.output_node_types_yaml = Some(yaml);
         self

shared/yeast/src/range.rs

@@ -69,4 +69,31 @@ impl Range {
             ..self
         }
     }
+
+    pub(crate) fn ignoring_boundary_ranges(
+        mut self,
+        ignored: impl IntoIterator<Item = Self>,
+    ) -> Self {
+        let ignored: Vec<_> = ignored.into_iter().collect();
+        loop {
+            let previous = self;
+            for range in &ignored {
+                if *range == self {
+                    self = self.empty_at_start();
+                    continue;
+                }
+                if range.start_byte == self.start_byte && range.end_byte > range.start_byte {
+                    self.start_byte = range.end_byte;
+                    self.start_point = range.end_point;
+                }
+                if range.end_byte == self.end_byte && range.end_byte > range.start_byte {
+                    self.end_byte = range.start_byte;
+                    self.end_point = range.start_point;
+                }
+            }
+            if self == previous {
+                return self;
+            }
+        }
+    }
 }

shared/yeast/tests/test.rs

@@ -1787,6 +1787,30 @@ fn test_returned_capture_keeps_its_location() {
     assert_eq!(identifier.byte_range(), 4..7);
 }
 
+#[test]
+fn test_ignored_location_field_is_excluded_from_rule_result_location() {
+    let rule: Rule = rule!(
+        (assignment
+            left: (identifier) @left)
+        =>
+        (call method: {left})
+    );
+
+    let language: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into();
+    let config = DesugaringConfig::new()
+        .with_ignored_location_fields(["right"])
+        .add_phase("test", PhaseKind::Repeating, vec![rule]);
+    let runner: Runner = Runner::from_config(language, &config).unwrap();
+    let ast = runner.run("x = 1").unwrap();
+    let call = ast
+        .reachable_node_ids()
+        .into_iter()
+        .filter_map(|id| ast.get_node(id))
+        .find(|node| node.kind_name() == "call")
+        .expect("call exists");
+    assert_eq!(call.byte_range(), 0..4);
+}
+
 /// Nodes allocated by an explicit recursive translation belong to that nested
 /// rule invocation, even when the outer rule returns one directly.
 #[test]

unified/extractor/src/languages/swift/swift.rs

@@ -480,13 +480,13 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
         rule!(
             (enumCaseElement
                 name: @name
-                parameterClause: (enumCaseParameterClause parameters: _* @params)) @@element
+                parameterClause: (enumCaseParameterClause parameters: _* @params) @@clause)
             =>
             class_like_declaration {
                 let body = tree!((block));
-                let constructor = tree_at!(
+                let constructor = tree_spanning!(
                     ctx,
-                    element,
+                    [name, clause],
                     (constructor_declaration parameter: {params} body: {body})
                 );
                 tree!((class_like_declaration
@@ -1439,6 +1439,7 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
 
 pub fn language_spec(desugared_ast_schema: &'static str) -> desugaring::LanguageSpec {
     let config = DesugaringConfig::<SwiftContext>::new()
+        .with_ignored_location_fields(["trailingComma"])
         .add_phase("translate", PhaseKind::OneShot, translation_rules())
         .with_output_node_types_yaml(desugared_ast_schema);
     let desugarer =

unified/extractor/tests/corpus/swift/collections/tuple-literal.output

@@ -53,11 +53,11 @@ top_level source="⟨body⟩"
           modifier: modifier "let" source="let"
           pattern: identifier "t" source="t"
           value:
-            tuple_expr source="(⟨element⟩ ⟨element⟩ ⟨element⟩)"
+            tuple_expr source="(⟨element⟩, ⟨element⟩, ⟨element⟩)"
               element:
-                argument source="⟨value⟩,"
+                argument source="⟨value⟩"
                   value: int_literal "1" source="1"
-                argument source="⟨value⟩,"
+                argument source="⟨value⟩"
                   value: string_literal "\"two\"" source="\"two\""
                 argument source="⟨value⟩"
                   value: float_literal "3.0" source="3.0"

unified/extractor/tests/corpus/swift/control-flow/nested-enum-case-pattern.output

@@ -167,13 +167,13 @@ top_level source="⟨body⟩"
                 expr_pattern source="⟨modifier⟩ ⟨expr⟩"
                   modifier: modifier "let" source="let"
                   expr:
-                    call_expr source="⟨callee⟩(⟨argument⟩ ⟨argument⟩)"
+                    call_expr source="⟨callee⟩(⟨argument⟩, ⟨argument⟩)"
                       callee:
                         member_access_expr source="⟨base⟩⟨member_name_node⟩"
                           base: inferred_type_expr "." source="."
                           member_name_node: identifier "received" source="received"
                       argument:
-                        argument source="⟨value⟩,"
+                        argument source="⟨value⟩"
                           value:
                             call_expr source="⟨callee⟩(⟨argument⟩)"
                               callee:
@@ -188,10 +188,10 @@ top_level source="⟨body⟩"
               body:
                 block source="⟨stmt⟩"
                   stmt:
-                    call_expr source="⟨callee⟩(⟨argument⟩ ⟨argument⟩)"
+                    call_expr source="⟨callee⟩(⟨argument⟩, ⟨argument⟩)"
                       callee: identifier "print" source="print"
                       argument:
-                        argument source="⟨value⟩,"
+                        argument source="⟨value⟩"
                           value: identifier "value" source="value"
                         argument source="⟨value⟩"
                           value: identifier "timestamp" source="timestamp"

unified/extractor/tests/corpus/swift/control-flow/switch-case-item-where-clauses.output

@@ -179,9 +179,9 @@ top_level source="⟨body⟩"
                           value: string_literal "\"positive\"" source="\"positive\""
             switch_case source="case ⟨pattern⟩:\n  ⟨body⟩"
               pattern:
-                or_pattern source="⟨pattern⟩ ⟨pattern⟩"
+                or_pattern source="⟨pattern⟩, ⟨pattern⟩"
                   pattern:
-                    conditional_pattern source="⟨pattern⟩ where ⟨condition⟩,"
+                    conditional_pattern source="⟨pattern⟩ where ⟨condition⟩"
                       condition:
                         binary_expr source="⟨left⟩ ⟨operator⟩ ⟨right⟩"
                           left: identifier "y" source="y"

unified/extractor/tests/corpus/swift/control-flow/switch-expression-pattern.output

@@ -333,9 +333,9 @@ top_level source="⟨body⟩"
                     member_access_expr source="⟨base⟩⟨member_name_node⟩"
                       base: inferred_type_expr "." source="."
                       member_name_node: identifier "inferred" source="inferred"
-                    tuple_expr source="(⟨element⟩ ⟨element⟩)"
+                    tuple_expr source="(⟨element⟩, ⟨element⟩)"
                       element:
-                        argument source="⟨value⟩,"
+                        argument source="⟨value⟩"
                           value: identifier "value" source="value"
                         argument source="⟨value⟩"
                           value: identifier "offset" source="offset"

unified/extractor/tests/corpus/swift/control-flow/switch-with-labeled-case-pattern-arguments.output

@@ -151,13 +151,13 @@ top_level source="⟨body⟩"
                           value: string_literal "\"yes\"" source="\"yes\""
             switch_case source="case ⟨pattern⟩:\n  ⟨body⟩"
               pattern:
-                call_expr source="⟨callee⟩(⟨argument⟩ ⟨argument⟩)"
+                call_expr source="⟨callee⟩(⟨argument⟩, ⟨argument⟩)"
                   callee:
                     member_access_expr source="⟨base⟩⟨member_name_node⟩"
                       base: inferred_type_expr "." source="."
                       member_name_node: identifier "thread" source="thread"
                   argument:
-                    argument source="⟨name_node⟩: ⟨value⟩,"
+                    argument source="⟨name_node⟩: ⟨value⟩"
                       name_node: identifier "threadRowId" source="threadRowId"
                       value: identifier "_" source="_"
                     argument source="⟨value⟩"