From d616706c17261e320c631ba62df363903022aba2 Mon Sep 17 00:00:00 2001 From: Ralf Lang Date: Thu, 18 Jun 2026 22:28:28 +0200 Subject: [PATCH] fix(hordeymlfile): preserve RC2 changelog sort on save --- src/ChangelogYmlFile.php | 98 ++++++++++++++++++++++++++++ test/unit/RoundTripChangelogTest.php | 36 ++++++++++ 2 files changed, 134 insertions(+) diff --git a/src/ChangelogYmlFile.php b/src/ChangelogYmlFile.php index bc654f2..55feebe 100644 --- a/src/ChangelogYmlFile.php +++ b/src/ChangelogYmlFile.php @@ -272,6 +272,104 @@ private function syncStreamFromChangelogYml(): void } $existing->setValueInternal($this->buildNode($value) ?? new MapNode()); } + + // Reorder entry-runs to match the stdClass property order. + // Each entry's preceding standalone trivia (comments and + // blank lines, until the previous entry or the start of the + // children list) travels with the entry. The leading group + // (trivia before the first entry) stays at file-leading. + $this->reorderEntries($root, $stdKeys); + } + + /** + * Permute the root MapNode's children so the entries appear in + * the order given by $stdKeys. Each entry's preceding + * standalone trivia run (CommentNode + BlankLineNode siblings + * since the previous entry) moves with the entry. The trivia + * preceding the first entry (file-leading banner) stays at the + * top of the children list, ahead of every entry-run. + * + * @param list $stdKeys + */ + private function reorderEntries(MapNode $root, array $stdKeys): void + { + $children = &$this->mapChildrenRef($root); + if ($children === []) { + unset($children); + return; + } + + // Split the children list into groups. Each group is either + // [trivia..., MapEntry] or, for the leading run if no + // entry follows, [trivia...]. The first group is always the + // file-leading run (may be empty). + $leading = []; + $groups = []; // indexed by entry key + $current = []; + foreach ($children as $child) { + $current[] = $child; + if ($child instanceof MapEntry) { + $keyNode = $child->getKey(); + $key = $keyNode instanceof ScalarNode ? $keyNode->getValue() : null; + if ($key === null) { + // Compound or non-scalar key: keep this group in + // its original position by anchoring it under a + // unique pseudo-key. Entry-run reorder skips it. + $key = "\0compound-" . spl_object_id($child); + } + $groups[(string) $key] = $current; + $current = []; + } + } + // Anything left in $current is trailing trivia after the + // last entry. Carry it forward as a final orphan group. + $trailing = $current; + + // Emit the leading run unchanged. (No entry preceded it, so + // it is not bound to any specific entry.) + $newOrder = $leading; + + // Emit entry-runs in stdClass order. Keys not in stdKeys + // (only happens if the AST has compound keys we couldn't + // sort) emit afterwards in their original order. + $emittedKeys = []; + foreach ($stdKeys as $key) { + $key = (string) $key; + if (isset($groups[$key])) { + $newOrder = array_merge($newOrder, $groups[$key]); + $emittedKeys[$key] = true; + } + } + foreach ($groups as $key => $group) { + if (!isset($emittedKeys[$key])) { + $newOrder = array_merge($newOrder, $group); + } + } + + $newOrder = array_merge($newOrder, $trailing); + $children = $newOrder; + unset($children); + } + + /** + * Reach into a MapNode's private $children array via a Closure + * bound to the MapNode class so the syncer can permute the + * children list while preserving each child's parent link. + * MapNode does not expose a public reorder method. + * + * @return array + */ + private function &mapChildrenRef(MapNode $map): array + { + $accessor = \Closure::bind( + function & () { + return $this->children; + }, + $map, + MapNode::class, + ); + $array = &$accessor(); + return $array; } /** diff --git a/test/unit/RoundTripChangelogTest.php b/test/unit/RoundTripChangelogTest.php index e8c788b..c3a6966 100644 --- a/test/unit/RoundTripChangelogTest.php +++ b/test/unit/RoundTripChangelogTest.php @@ -93,4 +93,40 @@ public function testNewVersionLandsAtVersionCompareSortedPosition(): void $this->assertLessThan($pos300, $pos310); $this->assertLessThan($pos210, $pos300); } + + /** + * Regression: when the file on disk is not in version-compare + * order, add multiple new versions, the emitted YAML must end + * up sorted by version_compare. This catches the case where a + * naive insert-before-next-existing-key respects stale AST + * positions and produces wrong order. + */ + public function testReorderEntriesWhenSourceIsUnsortedAndNewVersionsAdded(): void + { + $path = tempnam(sys_get_temp_dir(), 'cl_') . '.yml'; + // Source is intentionally unsorted: 2.0.0 before 3.0.0. + file_put_contents($path, "---\n2.0.0:\n notes: old\n3.0.0:\n notes: middle\n"); + + try { + $file = new ChangelogYmlFile($path); + $file->addVersionEntry('2.5.0', ['notes' => 'between']); + $file->addVersionEntry('4.0.0', ['notes' => 'newest']); + $output = (string) $file; + + $pos400 = strpos($output, '4.0.0:'); + $pos300 = strpos($output, '3.0.0:'); + $pos250 = strpos($output, '2.5.0:'); + $pos200 = strpos($output, '2.0.0:'); + + $this->assertNotFalse($pos400); + $this->assertNotFalse($pos300); + $this->assertNotFalse($pos250); + $this->assertNotFalse($pos200); + $this->assertLessThan($pos300, $pos400); + $this->assertLessThan($pos250, $pos300); + $this->assertLessThan($pos200, $pos250); + } finally { + @unlink($path); + } + } }