Close the coverage gaps and gate at 98%
New tests cover the reversed insert-delete direction, arity conflicts attributed to branch-origin nodes, each node_satisfies rejection (doubled single-slot field, wrong kind in a field, loose child of the wrong kind), an end-to-end arity conflict (disjoint deletions emptying java's required throws list), and every self-check span-mapping arm. Two real fixes fell out: a child whose origin field the parent kind does not define now counts as loose so the children slot vets it, and infallible slot lookups became justified indexing so phantom uncovered regions disappear. Genuinely defensive lines carry cov-excl-line markers with reasoning; the gate sits at 98% because grcov also measures #[cfg(test)] failure paths a passing suite never takes. Assisted-by: Claude Fable 5 via Claude Code
diff --git a/justfile b/justfile
index 5d2c7b2..06da833 100644
--- a/justfile
+++ b/justfile
@@ -9,8 +9,12 @@ check:
clippy:
cargo clippy --workspace -- -D warnings
+# The gate is 98 rather than 100 because grcov measures #[cfg(test)]
+# modules too, and their failure paths — panic! arms and `?` error
+# branches — never run in a passing suite. Library code is either
+# covered or carries an explicit cov-excl-line with reasoning.
coverage:
- ./bin/coverage
+ COVERAGE_THRESHOLD=98 ./bin/coverage
mutants:
#!/usr/bin/env bash
diff --git a/src/diff.rs b/src/diff.rs
index 8661e5f..c34bad8 100644
--- a/src/diff.rs
+++ b/src/diff.rs
@@ -201,9 +201,10 @@ pub fn anchor(src: &Tree, dst: &Tree) -> Matching {
}
// Equal hashes virtually always mean equal structure, but a
// 64-bit collision would corrupt the matching; verify before
- // committing the subtree.
+ // committing the subtree. The rejection itself is pinned by
+ // structural_eq's own tests.
if !structural_eq(src, s, dst, d) {
- continue;
+ continue; // cov-excl-line: unreachable through real hashes.
}
match_subtree(&mut m, src, s, dst, d);
placed.push((s, d));
@@ -740,7 +741,7 @@ fn sibling_order_holds(
y: NodeId,
) -> bool {
let Some(parent) = tree.parent(x) else {
- return true;
+ return true; // cov-excl-line: candidates come from gaps, never the root.
};
let mut before = true;
for &sibling in tree.children(parent) {
diff --git a/src/merge.rs b/src/merge.rs
index 6480167..68f9b93 100644
--- a/src/merge.rs
+++ b/src/merge.rs
@@ -307,19 +307,19 @@ impl Builder<'_> {
self.graft_branch(Branch::B, yb, &base_position, &mut slots);
}
- // Materialize: slot grafts, then the base survivor they precede.
+ // Materialize: slot grafts, then the base survivor they
+ // precede. Slots are sized base.len()+1, so the indexing below
+ // stays in bounds.
let mut children: Vec<MergedId> = Vec::new();
+ #[allow(clippy::indexing_slicing)]
for (i, &c) in base.iter().enumerate() {
- if let Some(slot) = slots.get(i) {
- children.extend(slot.iter().map(|entry| entry.id));
- }
+ children.extend(slots[i].iter().map(|entry| entry.id));
if !self.consumed.contains(&c) {
children.push(self.build_survivor(c));
}
}
- if let Some(slot) = slots.last() {
- children.extend(slot.iter().map(|entry| entry.id));
- }
+ #[allow(clippy::indexing_slicing)]
+ children.extend(slots[base.len()].iter().map(|entry| entry.id));
self.push(MergedNode { origin, children })
}
@@ -354,8 +354,10 @@ impl Builder<'_> {
}
None => {
let hash = tree.hash(child);
+ // Cursor is at most base.len(); slots has one more.
+ #[allow(clippy::indexing_slicing)]
+ let slot = &mut slots[cursor];
if matches!(branch, Branch::B)
- && let Some(slot) = slots.get_mut(cursor)
&& let Some(twin) = slot
.iter_mut()
.find(|entry| entry.dedupes_b && entry.hash == hash)
@@ -364,13 +366,11 @@ impl Builder<'_> {
continue;
}
let id = self.graft(branch, child);
- if let Some(slot) = slots.get_mut(cursor) {
- slot.push(SlotEntry {
- hash,
- id,
- dedupes_b: matches!(branch, Branch::A),
- });
- }
+ slot.push(SlotEntry {
+ hash,
+ id,
+ dedupes_b: matches!(branch, Branch::A),
+ });
}
}
}
@@ -598,6 +598,35 @@ mod tests {
Ok(())
}
+ #[test]
+ fn insert_under_a_deleted_node_conflicts_reversed() -> Result<(), Error> {
+ // Same broken dependency with the branches swapped: B grows
+ // the body A deletes.
+ assert_conflicts(
+ "rust",
+ "fn a() { x(); }\nfn b() { y(); }",
+ "fn b() { y(); }",
+ "fn a() { x(); z(); }\nfn b() { y(); }",
+ "insert-delete",
+ )?;
+ Ok(())
+ }
+
+ #[test]
+ fn disjoint_deletions_emptying_required_children_conflict() -> Result<(), Error> {
+ // A drops E1, B drops E2 — individually fine, disjoint (so no
+ // delete-delete), but together they empty the throws list,
+ // whose children the grammar requires.
+ assert_conflicts(
+ "java",
+ "class C { void m() throws E1, E2 { } }",
+ "class C { void m() throws E2 { } }",
+ "class C { void m() throws E1 { } }",
+ "arity",
+ )?;
+ Ok(())
+ }
+
#[test]
fn insert_into_a_deleted_inner_class_conflicts() -> Result<(), Error> {
assert_conflicts(
@@ -662,6 +691,45 @@ mod tests {
Ok(())
}
+ #[test]
+ fn self_check_maps_each_violation_kind_to_its_span_slot() -> Result<(), Error> {
+ // Extra deletion: both branches kept 2, the fake output lost it.
+ let o = parse("[1, 2]", "json")?;
+ let same = parse("[1, 2]", "json")?;
+ let same2 = parse("[1, 2]", "json")?;
+ let result = merge_to_text_with(&o, &same, &same2, |_, _, _, _| "[1]".into())?;
+ let MergeResult::Conflicts(conflicts) = result else {
+ panic!("expected self-check conflicts");
+ };
+ assert!(conflicts.iter().any(|c| c.span_o.is_some()));
+
+ // Missed insertion from B: the fake output drops B's 3.
+ let b = parse("[1, 2, 3]", "json")?;
+ let o = parse("[1, 2]", "json")?;
+ let a = parse("[1, 2]", "json")?;
+ let result = merge_to_text_with(&o, &a, &b, |_, _, _, _| "[1, 2]".into())?;
+ let MergeResult::Conflicts(conflicts) = result else {
+ panic!("expected self-check conflicts");
+ };
+ assert!(conflicts.iter().any(|c| c.span_b.is_some()));
+
+ // Extra insertion: nobody wrote 9; the witness lives in M, so
+ // no span slot is filled.
+ let o = parse("[1]", "json")?;
+ let a = parse("[1]", "json")?;
+ let b = parse("[1]", "json")?;
+ let result = merge_to_text_with(&o, &a, &b, |_, _, _, _| "[1, 9]".into())?;
+ let MergeResult::Conflicts(conflicts) = result else {
+ panic!("expected self-check conflicts");
+ };
+ assert!(
+ conflicts
+ .iter()
+ .any(|c| c.span_o.is_none() && c.span_a.is_none() && c.span_b.is_none())
+ );
+ Ok(())
+ }
+
#[test]
fn a_dropped_node_becomes_a_self_check_conflict() -> Result<(), Error> {
let o = parse("[1, 2]", "json")?;
@@ -732,6 +800,119 @@ mod tests {
Ok(())
}
+ #[test]
+ fn arity_attributes_grafted_nodes_to_their_branch() -> Result<(), Error> {
+ // A branch-origin node emptied of its children is attributed
+ // to that branch's span slot.
+ let tree = parse("fn f() { if c { } }", "rust")?;
+ let if_node = tree.nodes().find(|&n| tree.kind(n) == "if_expression");
+ assert!(if_node.is_some());
+ let as_root = |origin: Origin| MergedTree {
+ nodes: vec![MergedNode {
+ origin,
+ children: Vec::new(),
+ }],
+ root: MergedId(0),
+ };
+
+ for (origin, expect_a, expect_b) in [
+ (Origin::A(if_node.unwrap_or(tree.root())), true, false),
+ (Origin::B(if_node.unwrap_or(tree.root())), false, true),
+ ] {
+ let conflicts = crate::rules::arity(&tree, &tree, &tree, &as_root(origin));
+ assert_eq!(conflicts.len(), 1);
+ let conflict = conflicts.first();
+ assert_eq!(conflict.is_some_and(|c| c.span_a.is_some()), expect_a);
+ assert_eq!(conflict.is_some_and(|c| c.span_b.is_some()), expect_b);
+ }
+ Ok(())
+ }
+
+ #[test]
+ fn arity_rejects_a_doubled_single_slot_field() -> Result<(), Error> {
+ // Two children in the non-multiple condition field.
+ let tree = parse("fn f() { if c { } }", "rust")?;
+ let lang = Lang::by_name("rust").ok_or(Error::UnknownLanguage {
+ path: "rust".into(),
+ })?;
+ let condition_field = lang.language().field_id_for_name("condition");
+ let condition = tree
+ .nodes()
+ .find(|&n| tree.field_id(n).is_some() && tree.field_id(n) == condition_field);
+ let if_node = tree.nodes().find(|&n| tree.kind(n) == "if_expression");
+ let (Some(condition), Some(if_node)) = (condition, if_node) else {
+ panic!("if expression with a condition parses");
+ };
+
+ // Clone the whole tree and add a second condition child, so
+ // every other slot stays valid and the multiplicity check is
+ // what fires.
+ let mut doubled = clone_skipping(&tree, None);
+ let extra = MergedId(doubled.nodes.len());
+ doubled.nodes.push(MergedNode {
+ origin: Origin::O(condition),
+ children: Vec::new(),
+ });
+ for node in &mut doubled.nodes {
+ if node.origin == Origin::O(if_node) {
+ node.children.push(extra);
+ }
+ }
+ let conflicts = crate::rules::arity(&tree, &tree, &tree, &doubled);
+ assert!(!conflicts.is_empty());
+ Ok(())
+ }
+
+ #[test]
+ fn arity_rejects_a_wrong_kind_in_a_field() -> Result<(), Error> {
+ // A struct's type_identifier fills the "name" field too, but
+ // function_item's name slot does not admit that kind.
+ // Clone the whole tree, then swap only the function's name for
+ // the struct's type_identifier, so every other field stays
+ // filled and the kind check is what fires.
+ let tree = parse("fn f() {}\nstruct S;", "rust")?;
+ let fn_name = tree.nodes().find(|&n| tree.label(n) == Some("f"));
+ let type_name = tree.nodes().find(|&n| tree.kind(n) == "type_identifier");
+ let (Some(fn_name), Some(type_name)) = (fn_name, type_name) else {
+ panic!("both items parse");
+ };
+ let mut wrong = clone_skipping(&tree, None);
+ for node in &mut wrong.nodes {
+ if node.origin == Origin::O(fn_name) {
+ node.origin = Origin::O(type_name);
+ }
+ }
+ let conflicts = crate::rules::arity(&tree, &tree, &tree, &wrong);
+ assert!(!conflicts.is_empty());
+ Ok(())
+ }
+
+ #[test]
+ fn arity_rejects_a_loose_child_of_the_wrong_kind() -> Result<(), Error> {
+ // A parameters node is not a declaration; source_file's
+ // children slot rejects it. Only the root's child list changes
+ // (a full clone plus one extra child), so this pins the
+ // loose-kind branch specifically.
+ let tree = parse("fn f() { x(); }", "rust")?;
+ let parameters = tree.nodes().find(|&n| tree.kind(n) == "parameters");
+ let Some(parameters) = parameters else {
+ panic!("the function's parameters parse");
+ };
+ let mut wrong = clone_skipping(&tree, None);
+ let extra = MergedId(wrong.nodes.len());
+ wrong.nodes.push(MergedNode {
+ origin: Origin::O(parameters),
+ children: Vec::new(),
+ });
+ let root = wrong.root;
+ if let Some(root_node) = wrong.nodes.get_mut(root.0) {
+ root_node.children.push(extra);
+ }
+ let conflicts = crate::rules::arity(&tree, &tree, &tree, &wrong);
+ assert!(!conflicts.is_empty());
+ Ok(())
+ }
+
#[test]
fn variable_arity_growth_does_not_fire_the_arity_rule() -> Result<(), Error> {
// Two statements inserted into one block is legal growth; the
diff --git a/src/rules.rs b/src/rules.rs
index f794092..c205fd1 100644
--- a/src/rules.rs
+++ b/src/rules.rs
@@ -252,7 +252,10 @@ fn node_satisfies(
let node_types = parent_tree.lang().node_types();
let language = parent_tree.lang().language();
- // Each merged child resolved to (field name, kind, named).
+ // Each merged child resolved to (field name, kind, named). A
+ // 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.
let members: Vec<(Option<&str>, &str, bool)> = merged
.children(id)
.iter()
@@ -260,7 +263,8 @@ fn node_satisfies(
let (tree, node) = resolve(o, a, b, merged.origin(child));
let field = tree
.field_id(node)
- .and_then(|fid| language.field_name_for_id(fid.get()));
+ .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))
})
.collect();
diff --git a/src/synth.rs b/src/synth.rs
index 30b02d7..a9b31ac 100644
--- a/src/synth.rs
+++ b/src/synth.rs
@@ -56,12 +56,11 @@ pub fn synthesize(merged: &MergedTree, o: &Tree, a: &Tree, b: &Tree) -> String {
let mut out = String::new();
let mut run: Option<Run> = None;
for (tree_index, node) in leaves {
- let (Some(&tree), Some(sequence)) = (trees.get(tree_index), sequences.get(tree_index))
- else {
- continue;
- };
+ // Tree indices are 0..3 by construction of the leaf list.
+ #[allow(clippy::indexing_slicing)]
+ let (tree, sequence) = (trees[tree_index], &sequences[tree_index]);
let Some(position) = sequence.position(node) else {
- continue;
+ continue; // cov-excl-line: merged leaves are origin leaves.
};
let span = tree.span(node);
@@ -104,22 +103,20 @@ pub fn synthesize(merged: &MergedTree, o: &Tree, a: &Tree, b: &Tree) -> String {
.map(|current| (current.tree_index, current.last_position));
flush(&mut out, run.take(), &trees);
let closed = last.is_some_and(|(tree_index, position)| {
- let (Some(&tree), Some(sequence)) = (trees.get(tree_index), sequences.get(tree_index))
- else {
- return false;
- };
+ #[allow(clippy::indexing_slicing)] // Tree indices are 0..3.
+ let (tree, sequence) = (trees[tree_index], &sequences[tree_index]);
if position.saturating_add(1) != sequence.len() {
- return false;
+ return false; // cov-excl-line: in practice the final merged leaf is its origin's closing token, which is that file's last leaf.
}
let Some(last_node) = sequence.node_at(position) else {
- return false;
+ return false; // cov-excl-line: the position was just bounds-checked.
};
let tail = tree.source_slice(tree.span(last_node).end..tree.source_len());
out.push_str(tail.unwrap_or(""));
true
});
if !closed && !out.is_empty() && !out.ends_with('\n') {
- out.push('\n');
+ out.push('\n'); // cov-excl-line: fallback for the unclosed case above.
}
out