Lift comments as opaque leaf nodes
Comments were tree-sitter extras, skipped at lift and carried only as
inter-leaf trivia by synthesis, so a comment edit was invisible to the
diff: a branch rewriting a comment produced no tree edit and the merge
silently kept whichever bytes won. Keeping the extra node but not its
children makes every comment a labeled leaf whose label is its full
source text — an edit becomes an ordinary relabel, so relabel-relabel
and relabel-delete now cover concurrent comment edits for free.
Opacity, rather than lifting comment interiors, is what makes this
uniform: a plain rust comment body lives in no CST leaf (line_comment
holds just the `//` token), so two different comments would otherwise
be structurally identical. The arity rule skips extra children — they
fill no grammar slot and node-types.json admits them nowhere, so a
changed node holding a comment would false-conflict.
Assisted-by: Claude Fable 5 via Claude Code
diff --git a/src/diff.rs b/src/diff.rs
index 70523f8..4a7fd12 100644
--- a/src/diff.rs
+++ b/src/diff.rs
@@ -1093,6 +1093,24 @@ mod tests {
Ok(())
}
+ #[test]
+ fn a_comment_edit_produces_exactly_one_relabel() -> Result<(), Error> {
+ // Comments are opaque labeled leaves, so editing one is a
+ // relabel like any rename — the edit the old trivia-only
+ // representation could not see.
+ let o = parse_rust("// old\nfn a() {}")?;
+ let a = parse_rust("// new\nfn a() {}")?;
+ let m = diff(&o, &a);
+ let script = edits(&o, &a, &m);
+ assert_eq!(script.len(), 1);
+ let relabels_comment = script.iter().any(|e| match e {
+ Edit::Relabel(s, d) => o.label(*s) == Some("// old") && a.label(*d) == Some("// new"),
+ _ => false,
+ });
+ assert!(relabels_comment, "{script:?}");
+ Ok(())
+ }
+
#[test]
fn a_rename_produces_exactly_one_relabel() -> Result<(), Error> {
let o = parse_rust("fn a() {}")?;
diff --git a/src/merge.rs b/src/merge.rs
index 3ae5bf6..8e82525 100644
--- a/src/merge.rs
+++ b/src/merge.rs
@@ -1495,6 +1495,71 @@ mod tests {
Ok(())
}
+ #[test]
+ fn a_comment_edit_merges_with_a_code_edit() -> Result<(), Error> {
+ // A rewrites the comment, B renames the call: disjoint edits,
+ // both land — the merge the trivia representation silently
+ // dropped to one side.
+ let o = parse("// call the thing\nfn f() { x(); }\n", "rust")?;
+ let a = parse("// call the improved thing\nfn f() { x(); }\n", "rust")?;
+ let b = parse("// call the thing\nfn f() { y(); }\n", "rust")?;
+ match merge_to_text(&o, &a, &b)? {
+ MergeResult::Merged(text) => {
+ assert_eq!(text, "// call the improved thing\nfn f() { y(); }\n");
+ }
+ MergeResult::Conflicts(conflicts) => panic!("unexpected conflicts: {conflicts:?}"),
+ }
+ Ok(())
+ }
+
+ #[test]
+ fn concurrent_distinct_comment_edits_conflict() -> Result<(), Error> {
+ assert_conflicts(
+ "rust",
+ "// v1\nfn f() {}",
+ "// v2\nfn f() {}",
+ "// v3\nfn f() {}",
+ "relabel-relabel",
+ )?;
+ // The identical edit made twice still merges.
+ assert_merges(
+ "rust",
+ "// v1\nfn f() {}",
+ "// v2\nfn f() {}",
+ "// v2\nfn f() {}",
+ "// v2\nfn f() {}",
+ )
+ }
+
+ #[test]
+ fn a_comment_edit_under_a_deletion_conflicts() -> Result<(), Error> {
+ // A deletes fn b and its comment; B rewrites that comment.
+ // Deletion-wins would silently drop B's edit — relabel-delete
+ // now sees the comment like any other node.
+ assert_conflicts(
+ "rust",
+ "fn a() {}\n// about b\nfn b() {}\n",
+ "fn a() {}\n",
+ "fn a() {}\n// all about b\nfn b() {}\n",
+ "relabel-delete",
+ )?;
+ Ok(())
+ }
+
+ #[test]
+ fn an_inserted_comment_does_not_fire_the_arity_rule() -> Result<(), Error> {
+ // Extras fill no grammar slot and node-types.json admits them
+ // nowhere, so a changed node holding a comment child must be
+ // vetted with the comment skipped, not rejected.
+ assert_merges(
+ "rust",
+ "fn f() { x(); }",
+ "fn f() { /* note */ x(); }",
+ "fn f() { x(); }",
+ "fn f() { /* note */ x(); }",
+ )
+ }
+
#[test]
fn a_wrap_pulls_the_survivor_into_the_graft() -> Result<(), Error> {
// A wraps the array one level deeper; B leaves it alone.
diff --git a/src/rules.rs b/src/rules.rs
index 6d15da2..4132cf8 100644
--- a/src/rules.rs
+++ b/src/rules.rs
@@ -286,16 +286,21 @@ fn node_satisfies(
// child's field id travels with its origin node; a field this
// parent kind does not define (a graft that came from a different
// context) counts as loose so the children slot still vets it.
+ // Extras (comments) fill no grammar slot and node-types.json
+ // admits them nowhere, so they are skipped rather than vetted.
let members: Vec<(Option<&str>, &str, bool)> = merged
.children(id)
.iter()
- .map(|&child| {
+ .filter_map(|&child| {
let (tree, node) = resolve(o, a, b, merged.origin(child));
+ if tree.is_extra(node) {
+ return None;
+ }
let field = tree
.field_id(node)
.and_then(|fid| language.field_name_for_id(fid.get()))
.filter(|name| node_type.fields.contains_key(*name));
- (field, tree.kind(node), tree.is_named(node))
+ Some((field, tree.kind(node), tree.is_named(node)))
})
.collect();
diff --git a/src/tree.rs b/src/tree.rs
index deaf054..17076c7 100644
--- a/src/tree.rs
+++ b/src/tree.rs
@@ -31,9 +31,14 @@ impl NodeId {
///
/// Nodes live in an arena in pre-order (the root is index 0). The lift
/// keeps named and anonymous nodes — anonymous tokens like `+` carry
-/// meaning through their kind — but drops `extra` nodes (comments), and
-/// parsing rejects sources whose CST contains error or missing nodes:
-/// structural merge requires syntactically valid inputs.
+/// meaning through their kind. `extra` nodes (comments) are lifted as
+/// opaque leaves: the node stays, its children do not, so its label is
+/// its full source text. Opacity is what makes comment edits visible
+/// uniformly — a rust `// plain` comment's body text lives in no CST
+/// leaf, so lifting comment interiors would leave two different
+/// comments structurally identical. Parsing rejects sources whose CST
+/// contains error or missing nodes: structural merge requires
+/// syntactically valid inputs.
#[derive(Debug)]
pub struct Tree {
lang: &'static Lang,
@@ -47,6 +52,7 @@ struct NodeData {
kind: &'static str,
kind_id: u16,
named: bool,
+ extra: bool,
span: Range<usize>,
parent: Option<NodeId>,
children: Vec<NodeId>,
@@ -112,6 +118,12 @@ impl Tree {
self.node(id).named
}
+ /// Whether the node is an `extra` (a comment): valid anywhere,
+ /// filling no grammar slot. Extras are lifted as opaque leaves.
+ pub fn is_extra(&self, id: NodeId) -> bool {
+ self.node(id).extra
+ }
+
/// All nodes in pre-order.
pub fn nodes(&self) -> impl Iterator<Item = NodeId> + '_ {
(0..self.nodes.len()).map(NodeId)
@@ -212,7 +224,8 @@ fn has_error_or_missing(root: tree_sitter::Node) -> bool {
}
}
-/// Lifts the CST into arena nodes, pre-order, skipping `extra` nodes.
+/// Lifts the CST into arena nodes, pre-order, keeping `extra` nodes as
+/// opaque leaves (their children are not descended into).
///
/// Iterative with an explicit stack — recursing per CST level blows the
/// call stack on deeply nested sources.
@@ -226,6 +239,7 @@ fn lift(root: tree_sitter::Node) -> Vec<NodeData> {
kind: node.kind(),
kind_id: node.kind_id(),
named: node.is_named(),
+ extra: node.is_extra(),
span: node.byte_range(),
parent,
children: Vec::new(),
@@ -240,16 +254,16 @@ fn lift(root: tree_sitter::Node) -> Vec<NodeData> {
.children
.push(id);
}
+ if node.is_extra() {
+ continue;
+ }
// Children go on the stack reversed so the leftmost child pops
// first, giving pre-order arena indices and in-order `children`.
let mut cursor = node.walk();
let mut kids = Vec::new();
if cursor.goto_first_child() {
loop {
- let child = cursor.node();
- if !child.is_extra() {
- kids.push((child, Some(id), cursor.field_id()));
- }
+ kids.push((cursor.node(), Some(id), cursor.field_id()));
if !cursor.goto_next_sibling() {
break;
}
@@ -295,9 +309,45 @@ mod tests {
}
#[test]
- fn comments_are_excluded() -> Result<(), Error> {
+ fn comments_are_opaque_labeled_leaves() -> Result<(), Error> {
+ // A plain rust comment's body text lives in no CST leaf (the
+ // line_comment's only child is the `//` token), so the lift
+ // must keep the extra node itself, childless, labeled with the
+ // full comment text.
let t = parse_rust("// hello\nfn main() {}")?;
- assert!(t.nodes().all(|n| t.kind(n) != "line_comment"));
+ let comment = t.nodes().find(|&n| t.kind(n) == "line_comment");
+ let Some(comment) = comment else {
+ panic!("the comment is lifted");
+ };
+ assert!(t.is_extra(comment));
+ assert!(t.children(comment).is_empty());
+ assert_eq!(t.label(comment), Some("// hello"));
+ assert!(!t.is_extra(t.root()));
+ Ok(())
+ }
+
+ #[test]
+ fn doc_comment_interiors_are_not_lifted() -> Result<(), Error> {
+ // `/// doc` does carry an inner doc_comment leaf, but lifting
+ // it for doc comments only would split the representation;
+ // opacity applies to every extra.
+ let t = parse_rust("/// doc\nfn main() {}")?;
+ let comment = t.nodes().find(|&n| t.kind(n) == "line_comment");
+ assert!(comment.is_some_and(|n| t.children(n).is_empty()));
+ assert!(t.nodes().all(|n| t.kind(n) != "doc_comment"));
+ Ok(())
+ }
+
+ #[test]
+ fn comments_are_lifted_in_java_and_json() -> Result<(), Error> {
+ let t = Tree::parse("// c\nclass A { }", lang("java")?)?;
+ let comment = t.nodes().find(|&n| t.kind(n) == "line_comment");
+ assert!(comment.is_some_and(|n| t.is_extra(n)));
+ assert_eq!(comment.and_then(|n| t.label(n)), Some("// c"));
+
+ let t = Tree::parse("// c\n[1]", lang("json")?)?;
+ let comment = t.nodes().find(|&n| t.kind(n) == "comment");
+ assert!(comment.is_some_and(|n| t.is_extra(n)));
Ok(())
}