Introduce ReferenceStack::appendText

I feel this is a little better than before. It looks like we never need
to *replace* a text that existed before.

This depends on I4a156aa which fixes one of the last remaining trimming
issues. Outside of <references>, a <ref> </ref> with no other content
but some whitespace was already forbidden. But not inside of <references>.
This is relevant for appendText(). It should not be called with null, but
was because of the inconsistent behavior.

Change-Id: I38c9929f2fa6e69482e45919e2f8dbf823cb1c8b

src/Cite.php

@@ -326,23 +326,21 @@ class Cite {
 			} else {
 				$groupRefs = $this->referenceStack->getGroupRefs( $group );
 				if ( !isset( $groupRefs[$name]['text'] ) ) {
-					$this->referenceStack->setRefText( $group, $name, $text );
-				} else {
-					if ( $groupRefs[$name]['text'] !== $text ) {
-						// two refs with same key and different content
-						// adds error message to the original ref
-						// TODO: report these errors the same way as the others, rather than a
-						//  special case to append to the second one's content.
-						$this->referenceStack->setRefText(
-							$group,
-							$name,
-							$groupRefs[$name]['text'] . ' ' . $this->errorReporter->plain(
-								$parser,
-								'cite_error_references_duplicate_key',
-								$name
-							)
-						);
-					}
+					$this->referenceStack->appendText( $group, $name, $text );
+				} elseif ( $groupRefs[$name]['text'] !== $text ) {
+					// two refs with same key and different content
+					// adds error message to the original ref
+					// TODO: report these errors the same way as the others, rather than a
+					//  special case to append to the second one's content.
+					$this->referenceStack->appendText(
+						$group,
+						$name,
+						' ' . $this->errorReporter->plain(
+							$parser,
+							'cite_error_references_duplicate_key',
+							$name
+						)
+					);
 				}
 			}
 			return '';

src/ReferenceStack.php

@@ -2,6 +2,7 @@
 
 namespace Cite;
 
+use LogicException;
 use Parser;
 use StripState;
 
@@ -137,7 +138,7 @@ class ReferenceStack {
 		if ( $this->refs[$group][$follow] ?? false ) {
 			// We know the parent note already, so just perform the "follow" and bail out
 			// TODO: Separate `pushRef` from these side-effects.
-			$this->refs[$group][$follow]['text'] .= ' ' . $text;
+			$this->appendText( $group, $follow, ' ' . $text );
 			return null;
 		}
 
@@ -401,16 +402,20 @@ class ReferenceStack {
 	}
 
 	/**
-	 * Interface to set reference text from external code.  Ideally we can take over
-	 * responsibility for this logic.
-	 * @deprecated
-	 *
 	 * @param string $group
 	 * @param string $name
-	 * @param ?string $text
+	 * @param string $text
 	 */
-	public function setRefText( string $group, string $name, ?string $text ) {
-		$this->refs[$group][$name]['text'] = $text;
+	public function appendText( string $group, string $name, string $text ) {
+		if ( !isset( $this->refs[$group][$name] ) ) {
+			throw new LogicException( "Unknown ref \"$name\" in group \"$group\"" );
+		}
+
+		if ( isset( $this->refs[$group][$name]['text'] ) ) {
+			$this->refs[$group][$name]['text'] .= $text;
+		} else {
+			$this->refs[$group][$name]['text'] = $text;
+		}
 	}
 
 }

tests/phpunit/unit/CiteUnitTest.php

@@ -469,9 +469,9 @@ class CiteUnitTest extends \MediaWikiUnitTestCase {
 				];
 			}
 		);
-		$spy->referenceStack->method( 'setRefText' )->willReturnCallback(
+		$spy->referenceStack->method( 'appendText' )->willReturnCallback(
 			function ( $group, $name, $text ) use ( &$pushedRefs ) {
-				$pushedRefs[] = [ 'setRefText', $group, $name, $text ];
+				$pushedRefs[] = [ 'appendText', $group, $name, $text ];
 			}
 		);
 
@@ -519,7 +519,7 @@ class CiteUnitTest extends \MediaWikiUnitTestCase {
 				'',
 				[],
 				[
-					[ 'setRefText', 'foo', 'a', 'text' ]
+					[ 'appendText', 'foo', 'a', 'text' ]
 				]
 			],
 			'Successful ref' => [
@@ -561,7 +561,7 @@ class CiteUnitTest extends \MediaWikiUnitTestCase {
 				'',
 				[],
 				[
-					[ 'setRefText', '', 'a', 'text' ]
+					[ 'appendText', '', 'a', 'text' ]
 				]
 			],
 			'Mismatched text in references' => [
@@ -580,7 +580,7 @@ class CiteUnitTest extends \MediaWikiUnitTestCase {
 				'',
 				[],
 				[
-					[ 'setRefText', '', 'a', 'text-1 (cite_error_references_duplicate_key|a)' ]
+					[ 'appendText', '', 'a', ' (cite_error_references_duplicate_key|a)' ]
 				]
 			],
 		];

tests/phpunit/unit/ReferenceStackTest.php

@@ -4,6 +4,7 @@ namespace Cite\Tests\Unit;
 
 use Cite\ErrorReporter;
 use Cite\ReferenceStack;
+use LogicException;
 use Parser;
 use StripState;
 use Wikimedia\TestingAccessWrapper;
@@ -825,12 +826,6 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase {
 	 * @covers ::rollbackRefs
 	 * @covers ::rollbackRef
 	 * @dataProvider provideRollbackRefs
-	 *
-	 * @param array $initialCallStack
-	 * @param array $initialRefs
-	 * @param int $count
-	 * @param array $expectedRedo
-	 * @param array $expectedRefs
 	 */
 	public function testRollbackRefs(
 		array $initialCallStack,
@@ -1057,17 +1052,27 @@ class ReferenceStackTest extends \MediaWikiUnitTestCase {
 	}
 
 	/**
-	 * @covers ::setRefText
+	 * @covers ::appendText
 	 */
-	public function testSetRefText() {
+	public function testAppendText_failure() {
 		$stack = $this->newStack();
 
-		$stack->setRefText( 'group', 'name', 'the-text' );
+		$this->expectException( LogicException::class );
+		$stack->appendText( 'group', 'name', 'the-text' );
+	}
 
-		$this->assertSame(
-			[ 'group' => [ 'name' => [ 'text' => 'the-text' ] ] ],
-			$stack->refs
-		);
+	/**
+	 * @covers ::appendText
+	 */
+	public function testAppendText() {
+		$stack = $this->newStack();
+		$stack->refs = [ 'group' => [ 'name' => [] ] ];
+
+		$stack->appendText( 'group', 'name', 'set' );
+		$this->assertSame( [ 'text' => 'set' ], $stack->refs['group']['name'] );
+
+		$stack->appendText( 'group', 'name', ' and append' );
+		$this->assertSame( [ 'text' => 'set and append' ], $stack->refs['group']['name'] );
 	}
 
 	/**