diff --git a/src/ReferenceStack.php b/src/ReferenceStack.php index 1e0388130..6eedd75bd 100644 --- a/src/ReferenceStack.php +++ b/src/ReferenceStack.php @@ -31,8 +31,6 @@ class ReferenceStack { /** @var int[] Counter for the number of refs in each group */ private array $groupRefSequence = []; - /** @var int[][] */ - private array $extendsCount = []; /** * call stack @@ -100,8 +98,7 @@ class ReferenceStack { $incomplete = clone $ref; $incomplete->follow = $follow; $this->refs[$group][] = $incomplete; - $this->refCallStack[] = [ self::ACTION_NEW, $this->refSequence, $group, $name, $extends, $text, - $argv ]; + $this->refCallStack[] = [ self::ACTION_NEW, $this->refSequence, $group, $name, $text, $argv ]; } elseif ( $text !== null ) { // We know the parent already, so just perform the follow="…" and bail out $this->resolveFollow( $group, $follow, $text ); @@ -121,6 +118,7 @@ class ReferenceStack { $action = self::ACTION_NEW; } elseif ( $this->refs[$group][$name]->placeholder ) { // Populate a placeholder. + $ref->extendsCount = $this->refs[$group][$name]->extendsCount; $ref->number = $this->refs[$group][$name]->number; $this->refs[$group][$name] =& $ref; $action = self::ACTION_NEW_FROM_PLACEHOLDER; @@ -170,18 +168,15 @@ class ReferenceStack { // Roll back the group sequence number. --$this->groupRefSequence[$group]; } - - $this->extendsCount[$group][$extends] = - ( $this->extendsCount[$group][$extends] ?? 0 ) + 1; - + $parentRef->extendsCount ??= 0; $ref->extends = $extends; - $ref->extendsIndex = $this->extendsCount[$group][$extends]; + $ref->extendsIndex = ++$parentRef->extendsCount; } elseif ( $extends && $ref->extends !== $extends ) { // TODO: Change the error message to talk about "conflicting content or parent"? $ref->warnings[] = [ 'cite_error_references_duplicate_key', $name ]; } - $this->refCallStack[] = [ $action, $ref->key, $group, $name, $extends, $text, $argv ]; + $this->refCallStack[] = [ $action, $ref->key, $group, $name, $text, $argv ]; return $ref; } @@ -228,7 +223,6 @@ class ReferenceStack { * @param int $key Autoincrement counter for this ref. * @param string $group * @param ?string $name The name attribute passed in the ref tag. - * @param ?string $extends * @param ?string $text * @param array $argv * @@ -239,7 +233,6 @@ class ReferenceStack { int $key, string $group, ?string $name, - ?string $extends, ?string $text, array $argv ): array { @@ -269,10 +262,6 @@ class ReferenceStack { } $ref =& $this->refs[$group][$lookup]; - if ( $extends ) { - $this->extendsCount[$group][$extends]--; - } - switch ( $action ) { case self::ACTION_NEW: // Rollback the addition of new elements to the stack @@ -282,7 +271,9 @@ class ReferenceStack { } elseif ( isset( $this->groupRefSequence[$group] ) ) { $this->groupRefSequence[$group]--; } - // TODO: Don't we need to rollback extendsCount as well? + if ( $ref->extends ) { + $this->refs[$group][$ref->extends]->extendsCount--; + } break; case self::ACTION_NEW_FROM_PLACEHOLDER: $ref->placeholder = true; @@ -314,7 +305,6 @@ class ReferenceStack { $refs = $this->getGroupRefs( $group ); unset( $this->refs[$group] ); unset( $this->groupRefSequence[$group] ); - unset( $this->extendsCount[$group] ); return $refs; } diff --git a/src/ReferenceStackItem.php b/src/ReferenceStackItem.php index 15f5e52d9..7e30ee533 100644 --- a/src/ReferenceStackItem.php +++ b/src/ReferenceStackItem.php @@ -22,6 +22,11 @@ class ReferenceStackItem { * @var ?string Marks a sub-reference. Points to the parent reference by name. */ public ?string $extends = null; + /** + * @var ?int Count how many subreferences point to a parent. Corresponds to + * the last {@see extendsIndex} but this field belongs to the parent. + */ + public ?int $extendsCount = null; /** * @var ?int Sequence number for sub-references with the same extends * attribute value, starting from 1. {@see number} and this extends index are diff --git a/tests/phpunit/unit/ReferenceStackTest.php b/tests/phpunit/unit/ReferenceStackTest.php index bca51dcac..5d759fbd3 100644 --- a/tests/phpunit/unit/ReferenceStackTest.php +++ b/tests/phpunit/unit/ReferenceStackTest.php @@ -83,7 +83,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, '', null, null, 'text', [] ], + [ 'new', 1, '', null, 'text', [] ], ] ], 'Anonymous ref in named group' => [ @@ -115,7 +115,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', null, null, 'text', [] ], + [ 'new', 1, 'foo', null, 'text', [] ], ] ], 'Ref with text' => [ @@ -147,7 +147,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', null, null, 'text', [] ], + [ 'new', 1, 'foo', null, 'text', [] ], ] ], 'Named ref with text' => [ @@ -179,7 +179,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'name', null, 'text', [] ], + [ 'new', 1, 'foo', 'name', 'text', [] ], ] ], 'Follow after base' => [ @@ -213,7 +213,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-a', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], ] ], 'Follow with no base' => [ @@ -237,7 +237,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', null, null, 'text', [] ], + [ 'new', 1, 'foo', null, 'text', [] ], ] ], 'Follow pointing to later ref' => [ @@ -299,9 +299,9 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-a', [] ], - [ 'new', 2, 'foo', null, null, 'text-b', [] ], - [ 'new', 3, 'foo', 'c', null, 'text-c', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], + [ 'new', 2, 'foo', null, 'text-b', [] ], + [ 'new', 3, 'foo', 'c', 'text-c', [] ], ] ], 'Repeated ref, text in first tag' => [ @@ -343,8 +343,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text', [] ], - [ 'increment', 1, 'foo', 'a', null, null, [] ], + [ 'new', 1, 'foo', 'a', 'text', [] ], + [ 'increment', 1, 'foo', 'a', null, [] ], ] ], 'Repeated ref, text in second tag' => [ @@ -386,8 +386,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, null, [] ], - [ 'assign', 1, 'foo', 'a', null, 'text', [] ], + [ 'new', 1, 'foo', 'a', null, [] ], + [ 'assign', 1, 'foo', 'a', 'text', [] ], ] ], 'Repeated ref, mismatched text' => [ @@ -431,8 +431,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-1', [] ], - [ 'increment', 1, 'foo', 'a', null, 'text-2', [] ], + [ 'new', 1, 'foo', 'a', 'text-1', [] ], + [ 'increment', 1, 'foo', 'a', 'text-2', [] ], ] ], 'Named extends with no parent' => [ @@ -467,6 +467,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'b' => [ 'count' => 0, + 'extendsCount' => 1, 'name' => 'b', 'number' => 1, 'placeholder' => true, @@ -474,7 +475,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', 'b', 'text-a', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], ] ], 'Named extends before parent' => [ @@ -497,6 +498,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 2, 'group' => 'foo', 'name' => 'b', @@ -520,6 +522,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { 'b' => [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 2, 'group' => 'foo', 'name' => 'b', @@ -529,8 +532,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', 'b', 'text-a', [] ], - [ 'new-from-placeholder', 2, 'foo', 'b', null, 'text-b', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], + [ 'new-from-placeholder', 2, 'foo', 'b', 'text-b', [] ], ] ], 'Named extends after parent' => [ @@ -565,6 +568,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { 'a' => [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 1, 'group' => 'foo', 'name' => 'a', @@ -585,8 +589,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-a', [] ], - [ 'new', 2, 'foo', 'b', 'a', 'text-b', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], + [ 'new', 2, 'foo', 'b', 'text-b', [] ], ] ], 'Anonymous extends with no parent' => [ @@ -621,6 +625,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'b' => [ 'count' => 0, + 'extendsCount' => 1, 'name' => 'b', 'number' => 1, 'placeholder' => true, @@ -628,7 +633,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], ], 'finalCallStack' => [ - [ 'new', 1, 'foo', null, 'b', 'text-a', [] ], + [ 'new', 1, 'foo', null, 'text-a', [] ], ] ], 'Anonymous extends before parent' => [ @@ -651,6 +656,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 2, 'group' => 'foo', 'name' => 'b', @@ -674,6 +680,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { 'b' => [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 2, 'group' => 'foo', 'name' => 'b', @@ -683,8 +690,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', null, 'b', 'text-a', [] ], - [ 'new-from-placeholder', 2, 'foo', 'b', null, 'text-b', [] ], + [ 'new', 1, 'foo', null, 'text-a', [] ], + [ 'new-from-placeholder', 2, 'foo', 'b', 'text-b', [] ], ] ], 'Anonymous extends after parent' => [ @@ -719,6 +726,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { 'a' => [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 1, 'group' => 'foo', 'name' => 'a', @@ -739,8 +747,8 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-a', [] ], - [ 'new', 2, 'foo', null, 'a', 'text-b', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], + [ 'new', 2, 'foo', null, 'text-b', [] ], ] ], 'Normal after extends' => [ @@ -785,6 +793,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { 'a' => [ 'count' => 1, 'dir' => 'rtl', + 'extendsCount' => 1, 'key' => 1, 'group' => 'foo', 'name' => 'a', @@ -814,9 +823,9 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-a', [] ], - [ 'new', 2, 'foo', null, 'a', 'text-b', [] ], - [ 'new', 3, 'foo', 'c', null, 'text-c', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], + [ 'new', 2, 'foo', null, 'text-b', [] ], + [ 'new', 3, 'foo', 'c', 'text-c', [] ], ] ], 'Two incomplete follows' => [ @@ -870,9 +879,9 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ] ], 'finalCallStack' => [ - [ 'new', 1, 'foo', 'a', null, 'text-a', [] ], - [ 'new', 2, 'foo', null, null, 'text-b', [] ], - [ 'new', 3, 'foo', null, null, 'text-c', [] ], + [ 'new', 1, 'foo', 'a', 'text-a', [] ], + [ 'new', 2, 'foo', null, 'text-b', [] ], + [ 'new', 3, 'foo', null, 'text-c', [] ], ] ], ]; @@ -927,7 +936,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'Missing group' => [ 'initialCallStack' => [ - [ 'new', 1, 'foo', null, null, 'text', [] ], + [ 'new', 1, 'foo', null, 'text', [] ], ], 'initialRefs' => [], 'rollbackCount' => 1, @@ -935,7 +944,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'Find anonymous ref by key' => [ 'initialCallStack' => [ - [ 'new', 1, 'foo', null, null, 'text', [] ], + [ 'new', 1, 'foo', null, 'text', [] ], ], 'initialRefs' => [ 'foo' => [ [ @@ -950,7 +959,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'Missing anonymous ref' => [ 'initialCallStack' => [ - [ 'new', 1, 'foo', null, null, 'text', [] ], + [ 'new', 1, 'foo', null, 'text', [] ], ], 'initialRefs' => [ 'foo' => [ [ @@ -962,7 +971,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'Assign text' => [ 'initialCallStack' => [ - [ 'assign', 1, 'foo', null, null, 'text-2', [] ], + [ 'assign', 1, 'foo', null, 'text-2', [] ], ], 'initialRefs' => [ 'foo' => [ [ @@ -985,7 +994,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'Increment' => [ 'initialCallStack' => [ - [ 'increment', 1, 'foo', null, null, null, [] ], + [ 'increment', 1, 'foo', null, null, [] ], ], 'initialRefs' => [ 'foo' => [ [ @@ -1006,7 +1015,7 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { ], 'Safely ignore placeholder' => [ 'initialCallStack' => [ - [ 'increment', 2, 'foo', null, null, null, [] ], + [ 'increment', 2, 'foo', null, null, [] ], ], 'initialRefs' => [ 'foo' => [ [ @@ -1050,11 +1059,18 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase { 'text', [], 'foo', null, 'a', null, 'rtl' ); - $this->assertSame( 1, $stack->extendsCount['foo']['a'] ); - $stack->rollbackRefs( 1 ); - - $this->assertSame( 0, $stack->extendsCount['foo']['a'] ); + $this->assertEquals( + TestUtils::refFromArray( [ + 'count' => 0, + 'extendsCount' => 0, + 'name' => 'a', + 'number' => 1, + 'placeholder' => true, + 'warnings' => [] + ] ), + $stack->refs['foo']['a'] + ); } public function testRemovals() {