From 79ada17cb5bb83694c7554d1f7ad6cf0472d8392 Mon Sep 17 00:00:00 2001 From: Harsh Kapse Date: Sat, 10 Oct 2026 19:55:39 +0530 Subject: [PATCH] fix(postprocess): enforce container provenance in stationary move check When code deletions shift lines up, elements in a different container can land on the exact row and column where an old element used to be. isStationaryMove previously checked only row and column coordinates within the enclosing scope, treating cross-container moves as stationary and dropping their move highlights. Require that dstParent's source mapping actually contained the node (and vice versa) before treating matching coordinates as stationary. - postprocess: check container provenance in isStationaryMove - tests: add regression test for cross-container coordinate collision --- internal/postprocess/normalize.go | 30 ++++++++++------ internal/postprocess/normalize_test.go | 47 ++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/internal/postprocess/normalize.go b/internal/postprocess/normalize.go index 3e3a75dc..7109e383 100644 --- a/internal/postprocess/normalize.go +++ b/internal/postprocess/normalize.go @@ -122,21 +122,12 @@ func computeMoveRetention(src, dst *treesitter.ASTNode, ms *engine.Mapping, r *r return effectiveSurviving / float64(nMax) } -// isStationaryMove reports whether a Move stayed in place across transparent -// wrappers (such as template_declaration or friend_declaration) or expression shifts. +// isStationaryMove reports whether a move stayed in place, accounting for transparent +// wrappers, expression shifts, and container provenance. func isStationaryMove(node, dstNode *treesitter.ASTNode, ms *engine.Mapping, r *rules.Rules) bool { if node == nil || dstNode == nil || node.Parent == nil || dstNode.Parent == nil { return false } - hasPos := (node.EndByte > 0 || node.StartRow > 0 || node.EndRow > 0) && - (dstNode.EndByte > 0 || dstNode.StartRow > 0 || dstNode.EndRow > 0) - if hasPos && node.StartRow == dstNode.StartRow && node.EndRow == dstNode.EndRow && - node.StartCol == dstNode.StartCol && node.EndCol == dstNode.EndCol { - if sameScopeDeclaration(node, dstNode, ms, r) { - return true - } - } - srcParent := node.Parent dstParent := dstNode.Parent @@ -149,6 +140,23 @@ func isStationaryMove(node, dstNode *treesitter.ASTNode, ms *engine.Mapping, r * return true } + hasPos := (node.EndByte > 0 || node.StartRow > 0 || node.EndRow > 0) && + (dstNode.EndByte > 0 || dstNode.StartRow > 0 || dstNode.EndRow > 0) + if hasPos && node.StartRow == dstNode.StartRow && node.EndRow == dstNode.EndRow && + node.StartCol == dstNode.StartCol && node.EndCol == dstNode.EndCol { + if sameScopeDeclaration(node, dstNode, ms, r) { + // Not stationary if moved across distinct containers, like when dstParent + // came from an old container that never held this node. + if mappedDstParent := ms.Dst()[dstParent]; mappedDstParent != nil && !mappedDstParent.Contains(node) { + return false + } + if mappedSrcParent := ms.Src()[srcParent]; mappedSrcParent != nil && !mappedSrcParent.Contains(dstNode) { + return false + } + return true + } + } + sameLine := node.StartRow == dstNode.StartRow // Walk up past transparent wrappers to find the enclosing container. diff --git a/internal/postprocess/normalize_test.go b/internal/postprocess/normalize_test.go index bbf63182..1b4b5116 100644 --- a/internal/postprocess/normalize_test.go +++ b/internal/postprocess/normalize_test.go @@ -3252,3 +3252,50 @@ func isDeclarationHeader(n *treesitter.ASTNode, r *rules.Rules) bool { t.Errorf("expected binary_expression move to be demoted after descendants were evicted, but found an active Move") } } + +func TestNormalizeStationaryMove_CrossContainerCoordinateCollisionPreservesMove(t *testing.T) { + // Don't drop a move as stationary if coordinates match only because an item moved + // into an adjacent container that landed on the item's old row and column. + src := []byte(`package rules + +var rules = &Rules{ + TagElements: []string{ + "jsx_element", + }, + Tags: []string{ + "jsx_opening_element", + "jsx_closing_element", + }, +} +`) + dst := []byte(`package rules + +var rules = &Rules{ + Tags: []string{ + "jsx_element", + "jsx_opening_element", + "jsx_closing_element", + }, +} +`) + + srcAST, err := treesitter.Parse(src, "rules.go") + if err != nil { + t.Fatalf("failed to parse src: %v", err) + } + dstAST, err := treesitter.Parse(dst, "rules.go") + if err != nil { + t.Fatalf("failed to parse dst: %v", err) + } + + matchResult := engine.Match(srcAST, dstAST, src, dst, nil) + script := actions.GenerateEditScript(srcAST, dstAST, matchResult.Mappings) + normalized := Run(script, matchResult.Mappings, srcAST, dstAST) + + hasMove := slices.ContainsFunc(normalized.Actions(), func(a actions.Action) bool { + return a.Type == actions.Move && a.Node != nil && (a.Node.Label == `"jsx_element"` || (len(a.Node.Children) > 0 && a.Node.Children[0].Label == `"jsx_element"`)) + }) + if !hasMove { + t.Errorf("expected Move action on \"jsx_element\" to be preserved across containers despite coordinate collision, but none found") + } +}