yeast: Handle empty locations correctly

The location 0..0 was being treated as "no useable location", which at
the beginning of a file could cause problems. We now represent this as
None instead, making Some(0..0) a valid empty location.

Author: tausbn

shared/yeast/src/lib.rs

@@ -144,10 +144,7 @@ impl YeastDisplay for Id {
 
 impl YeastSourceRange for Id {
     fn yeast_source_range(&self, ast: &Ast) -> Option<Range> {
-        ast.get_node(*self).and_then(|n| match &n.content {
-            NodeContent::Range(r) => Some(*r),
-            _ => n.source_range,
-        })
+        ast.get_node(*self).and_then(Node::source_range)
     }
 }
 
@@ -598,7 +595,7 @@ impl Ast {
                 .union_source_range_of_children(&fields)
                 .map_or(source_range, |child_range| {
                     Some(match source_range {
-                        Some(source_range) => union_source_ranges(child_range, source_range),
+                        Some(source_range) => child_range.union(source_range),
                         None => child_range,
                     })
                 }),
@@ -631,7 +628,7 @@ impl Ast {
             panic!("extend_source_range: cannot modify a parsed node");
         }
         node.source_range = Some(match node.source_range {
-            Some(existing) => union_source_ranges(existing, source_range),
+            Some(existing) => existing.union(source_range),
             None => source_range,
         });
     }
@@ -673,35 +670,30 @@ impl Ast {
                 let Some(child) = self.get_node(child_id) else {
                     continue;
                 };
-
-                let child_start_byte = child.start_byte();
-                let child_end_byte = child.end_byte();
-
-                // Skip children that carry no usable location.
-                if child_start_byte == 0 && child_end_byte == 0 {
+                let Some(child_range) = child.source_range() else {
                     continue;
-                }
+                };
 
                 match start_byte {
                     None => {
-                        start_byte = Some(child_start_byte);
-                        start_point = child.start_position();
+                        start_byte = Some(child_range.start_byte);
+                        start_point = child_range.start_point;
                     }
-                    Some(current_start) if child_start_byte < current_start => {
-                        start_byte = Some(child_start_byte);
-                        start_point = child.start_position();
+                    Some(current_start) if child_range.start_byte < current_start => {
+                        start_byte = Some(child_range.start_byte);
+                        start_point = child_range.start_point;
                     }
                     _ => {}
                 }
 
                 match end_byte {
                     None => {
-                        end_byte = Some(child_end_byte);
-                        end_point = child.end_position();
+                        end_byte = Some(child_range.end_byte);
+                        end_point = child_range.end_point;
                     }
-                    Some(current_end) if child_end_byte > current_end => {
-                        end_byte = Some(child_end_byte);
-                        end_point = child.end_position();
+                    Some(current_end) if child_range.end_byte > current_end => {
+                        end_byte = Some(child_range.end_byte);
+                        end_point = child_range.end_point;
                     }
                     _ => {}
                 }
@@ -810,25 +802,6 @@ impl Ast {
     }
 }
 
-fn union_source_ranges(first: Range, second: Range) -> Range {
-    let (start_byte, start_point) = if first.start_byte <= second.start_byte {
-        (first.start_byte, first.start_point)
-    } else {
-        (second.start_byte, second.start_point)
-    };
-    let (end_byte, end_point) = if first.end_byte >= second.end_byte {
-        (first.end_byte, first.end_point)
-    } else {
-        (second.end_byte, second.end_point)
-    };
-    Range {
-        start_byte,
-        end_byte,
-        start_point,
-        end_point,
-    }
-}
-
 /// A node in our AST
 #[derive(PartialEq, Eq, Debug, Clone, Serialize)]
 pub struct Node {
@@ -871,36 +844,29 @@ impl Node {
         Point { row: 0, column: 0 }
     }
 
-    pub fn start_position(&self) -> Point {
+    pub fn source_range(&self) -> Option<Range> {
         match self.content {
-            NodeContent::Range(range) => range.start_point,
-            _ => self
-                .source_range
-                .map_or_else(|| self.fake_point(), |r| r.start_point),
+            NodeContent::Range(range) => Some(range),
+            _ => self.source_range,
         }
     }
 
+    pub fn start_position(&self) -> Point {
+        self.source_range()
+            .map_or_else(|| self.fake_point(), |range| range.start_point)
+    }
+
     pub fn end_position(&self) -> Point {
-        match self.content {
-            NodeContent::Range(range) => range.end_point,
-            _ => self
-                .source_range
-                .map_or_else(|| self.fake_point(), |r| r.end_point),
-        }
+        self.source_range()
+            .map_or_else(|| self.fake_point(), |range| range.end_point)
     }
 
     pub fn start_byte(&self) -> usize {
-        match self.content {
-            NodeContent::Range(range) => range.start_byte,
-            _ => self.source_range.map_or(0, |r| r.start_byte),
-        }
+        self.source_range().map_or(0, |range| range.start_byte)
     }
 
     pub fn end_byte(&self) -> usize {
-        match self.content {
-            NodeContent::Range(range) => range.end_byte,
-            _ => self.source_range.map_or(0, |r| r.end_byte),
-        }
+        self.source_range().map_or(0, |range| range.end_byte)
     }
 
     pub fn byte_range(&self) -> std::ops::Range<usize> {

shared/yeast/src/range.rs

@@ -30,3 +30,43 @@ pub struct Range {
     pub start_point: Point,
     pub end_point: Point,
 }
+
+impl Range {
+    /// Return the smallest range containing both ranges.
+    pub fn union(self, other: Self) -> Self {
+        let (start_byte, start_point) = if self.start_byte <= other.start_byte {
+            (self.start_byte, self.start_point)
+        } else {
+            (other.start_byte, other.start_point)
+        };
+        let (end_byte, end_point) = if self.end_byte >= other.end_byte {
+            (self.end_byte, self.end_point)
+        } else {
+            (other.end_byte, other.end_point)
+        };
+        Self {
+            start_byte,
+            end_byte,
+            start_point,
+            end_point,
+        }
+    }
+
+    /// Return an empty range anchored at this range's start.
+    pub fn empty_at_start(self) -> Self {
+        Self {
+            end_byte: self.start_byte,
+            end_point: self.start_point,
+            ..self
+        }
+    }
+
+    /// Return an empty range anchored at this range's end.
+    pub fn empty_at_end(self) -> Self {
+        Self {
+            start_byte: self.end_byte,
+            start_point: self.end_point,
+            ..self
+        }
+    }
+}

shared/yeast/tests/test.rs

@@ -1709,6 +1709,36 @@ fn test_nested_synthetic_node_uses_child_location() {
     assert_eq!(arguments.byte_range(), 0..3);
 }
 
+/// An explicit empty range at byte zero is a real location, not the sentinel
+/// for an absent location, and therefore contributes to parent ranges.
+#[test]
+fn test_empty_range_at_file_start_contributes_to_parent() {
+    use std::collections::BTreeMap;
+
+    let lang: tree_sitter::Language = tree_sitter_ruby::LANGUAGE.into();
+    let schema =
+        yeast::node_types_yaml::schema_from_yaml_with_language(OUTPUT_SCHEMA_YAML, &lang).unwrap();
+    let mut ast = Ast::with_schema(schema);
+    let empty = Range {
+        start_byte: 0,
+        end_byte: 0,
+        start_point: Point::new(0, 0),
+        end_point: Point::new(0, 0),
+    };
+    let child =
+        ast.create_named_token_with_range("identifier", "synthetic".to_owned(), Some(empty));
+    let fields = BTreeMap::from([(ast.field_id_for_name("method").unwrap(), vec![child])]);
+    let parent = ast.create_node_with_range(
+        ast.id_for_node_kind("call").unwrap(),
+        NodeContent::DynamicString(String::new()),
+        fields,
+        true,
+        None,
+    );
+
+    assert_eq!(ast.get_node(parent).unwrap().source_range(), Some(empty));
+}
+
 /// A rule that only unwraps and returns a translated capture must not widen
 /// that capture to the wrapper's source range.
 #[test]

unified/extractor/tests/location_tests.rs

@@ -37,7 +37,8 @@ fn ranges(ast: &Ast, kind: &str, content: Option<&str>) -> Vec<std::ops::Range<u
             {
                 return None;
             }
-            Some(node.byte_range())
+            node.source_range()
+                .map(|range| range.start_byte..range.end_byte)
         })
         .collect()
 }
@@ -50,16 +51,6 @@ fn assert_has_span(ast: &Ast, source: &str, kind: &str, content: Option<&str>, e
     );
 }
 
-fn assert_has_empty_span(ast: &Ast, kind: &str, content: Option<&str>, expected_offset: usize) {
-    let ranges = ranges(ast, kind, content);
-    assert!(
-        ranges
-            .iter()
-            .any(|range| range.start == expected_offset && range.end == expected_offset),
-        "expected {kind} {content:?} to have an empty span at {expected_offset}, got {ranges:?}"
-    );
-}
-
 #[test]
 fn generic_type_children_have_local_ranges() {
     let source = "let x = C<Foo>()";
@@ -96,5 +87,4 @@ fn import_member_chain_excludes_import_keyword() {
         None,
         "import Foundation.Networking.URLSession",
     );
-    assert_has_empty_span(&ast, "bulk_importing_pattern", None, 0);
 }