Add strictly typed RefGroupItem class to Parsoid code

This does the exact same as the previously used generic stdClass
object, just strictly typed. Turned out to be surprisingly
straightforward, as proven by the small size of this patch.

I'm intentionally not adding anything new in this patch. For
example, the new class is perfect to write longer documentation
for every field. But this is for a later patch.

Change-Id: Ibf696f6b5ef1bfdbe846b571fb7e9ded96693351
This commit is contained in:
thiemowmde 2024-03-05 16:54:19 +01:00
parent 52df9bd54f
commit 9e1c4645be
5 changed files with 76 additions and 64 deletions

View file

@ -3,7 +3,6 @@ declare( strict_types = 1 );
namespace Cite\Parsoid; namespace Cite\Parsoid;
use stdClass;
use Wikimedia\Parsoid\DOM\Document; use Wikimedia\Parsoid\DOM\Document;
use Wikimedia\Parsoid\DOM\Element; use Wikimedia\Parsoid\DOM\Element;
use Wikimedia\Parsoid\Ext\DOMUtils; use Wikimedia\Parsoid\Ext\DOMUtils;
@ -22,12 +21,12 @@ class RefGroup {
public $name; public $name;
/** /**
* @var stdClass[] * @var RefGroupItem[]
*/ */
public array $refs = []; public array $refs = [];
/** /**
* @var stdClass[] * @var RefGroupItem[]
*/ */
public array $indexByName = []; public array $indexByName = [];
@ -62,7 +61,7 @@ class RefGroup {
} }
public function renderLine( public function renderLine(
ParsoidExtensionAPI $extApi, Element $refsList, stdClass $ref ParsoidExtensionAPI $extApi, Element $refsList, RefGroupItem $ref
): void { ): void {
$ownerDoc = $refsList->ownerDocument; $ownerDoc = $refsList->ownerDocument;

View file

@ -0,0 +1,44 @@
<?php
declare( strict_types = 1 );
namespace Cite\Parsoid;
use Wikimedia\Parsoid\DOM\Element;
/**
* Individual item in a {@see RefGroup}.
*
* @license GPL-2.0-or-later
*/
class RefGroupItem {
/**
* Pointer to the contents of the ref, accessible with the
* {@see ParsoidExtensionAPI::getContentDOM}, to be used when serializing the references group.
* It gets set when extracting the ref from a node and not $missingContent. Note that that
* might not be the first one for named refs. Also, for named refs, it's used to detect
* multiple conflicting definitions.
*/
public ?string $contentId = null;
/** Just used for comparison when we have multiples */
public ?string $cachedHtml = null;
public string $dir = '';
public string $group = '';
public int $groupIndex = 1;
public int $index = 0;
public string $key;
public string $id;
/** @var array<int,string> */
public array $linkbacks = [];
public string $name = '';
public string $target;
/** @var Element[] */
public array $nodes = [];
/** @var string[] */
public array $embeddedNodes = [];
}

View file

@ -190,6 +190,7 @@ class References extends ExtensionTagHandler {
// HTML inline for all of them. // HTML inline for all of them.
if ( $ref->contentId ) { if ( $ref->contentId ) {
if ( $ref->cachedHtml === null ) { if ( $ref->cachedHtml === null ) {
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable False positive
$refContent = $extApi->getContentDOM( $ref->contentId )->firstChild; $refContent = $extApi->getContentDOM( $ref->contentId )->firstChild;
$ref->cachedHtml = $extApi->domToHtml( $refContent, true, false ); $ref->cachedHtml = $extApi->domToHtml( $refContent, true, false );
} }
@ -253,6 +254,7 @@ class References extends ExtensionTagHandler {
if ( $validFollow ) { if ( $validFollow ) {
// Migrate content from the follow to the ref // Migrate content from the follow to the ref
if ( $ref->contentId ) { if ( $ref->contentId ) {
// @phan-suppress-next-line PhanTypeMismatchArgumentNullable False positive
$refContent = $extApi->getContentDOM( $ref->contentId )->firstChild; $refContent = $extApi->getContentDOM( $ref->contentId )->firstChild;
DOMUtils::migrateChildren( $c, $refContent ); DOMUtils::migrateChildren( $c, $refContent );
} else { } else {
@ -495,7 +497,6 @@ class References extends ExtensionTagHandler {
self::addErrorsToNode( $node, $errs ); self::addErrorsToNode( $node, $errs );
} }
foreach ( $ref->embeddedNodes as $about ) { foreach ( $ref->embeddedNodes as $about ) {
'@phan-var string $about'; // $about is non-null
$refsData->embeddedErrors[$about] = $errs; $refsData->embeddedErrors[$about] = $errs;
} }
} }

View file

@ -3,7 +3,6 @@ declare( strict_types = 1 );
namespace Cite\Parsoid; namespace Cite\Parsoid;
use stdClass;
use Wikimedia\Parsoid\Core\Sanitizer; use Wikimedia\Parsoid\Core\Sanitizer;
use Wikimedia\Parsoid\Ext\ParsoidExtensionAPI; use Wikimedia\Parsoid\Ext\ParsoidExtensionAPI;
@ -79,7 +78,7 @@ class ReferencesData {
public function add( public function add(
ParsoidExtensionAPI $extApi, string $groupName, string $refName, string $refDir ParsoidExtensionAPI $extApi, string $groupName, string $refName, string $refDir
): stdClass { ): RefGroupItem {
$group = $this->getRefGroup( $groupName, true ); $group = $this->getRefGroup( $groupName, true );
$hasRefName = strlen( $refName ) > 0; $hasRefName = strlen( $refName ) > 0;
@ -99,28 +98,15 @@ class ReferencesData {
// bump index // bump index
$this->index += 1; $this->index += 1;
$ref = (object)[ $ref = new RefGroupItem();
// Pointer to the contents of the ref, accessible with the $ref->dir = $refDir;
// $extApi->getContentDOM(), to be used when serializing the $ref->group = $group->name;
// references group. It gets set when extracting the ref from a $ref->groupIndex = count( $group->refs ) + 1;
// node and not $missingContent. Note that that might not $ref->index = $n;
// be the first one for named refs. Also, for named refs, it's $ref->key = $refIdBase;
// used to detect multiple conflicting definitions. $ref->id = $hasRefName ? $refIdBase . '-0' : $refIdBase;
'contentId' => null, $ref->name = $refName;
// Just used for comparison when we have multiples $ref->target = $noteId;
'cachedHtml' => null,
'dir' => $refDir,
'group' => $group->name,
'groupIndex' => count( $group->refs ) + 1,
'index' => $n,
'key' => $refIdBase,
'id' => $hasRefName ? $refIdBase . '-0' : $refIdBase,
'linkbacks' => [],
'name' => $refName,
'target' => $noteId,
'nodes' => [],
'embeddedNodes' => [],
];
$group->refs[] = $ref; $group->refs[] = $ref;

View file

@ -4,6 +4,7 @@ namespace Cite\Tests\Unit;
use Cite\Cite; use Cite\Cite;
use Cite\Parsoid\ReferencesData; use Cite\Parsoid\ReferencesData;
use Cite\Parsoid\RefGroupItem;
use MediaWikiUnitTestCase; use MediaWikiUnitTestCase;
use Wikimedia\Parsoid\Ext\ParsoidExtensionAPI; use Wikimedia\Parsoid\Ext\ParsoidExtensionAPI;
@ -58,25 +59,14 @@ class ReferencesDataTest extends MediaWikiUnitTestCase {
'' ''
); );
$expected = [ $expected = new RefGroupItem();
'contentId' => null, $expected->key = 'cite_ref-1';
'cachedHtml' => null, $expected->id = 'cite_ref-1';
'dir' => '', $expected->target = 'cite_note-1';
'group' => '', $this->assertEquals( $expected, $ref );
'groupIndex' => 1,
'index' => 0,
'key' => 'cite_ref-1',
'id' => 'cite_ref-1',
'linkbacks' => [],
'name' => '',
'target' => 'cite_note-1',
'nodes' => [],
'embeddedNodes' => [],
];
$this->assertSame( $expected, (array)$ref );
$group = $data->getRefGroup( Cite::DEFAULT_GROUP ); $group = $data->getRefGroup( Cite::DEFAULT_GROUP );
$this->assertEquals( [ (object)$expected ], $group->refs ); $this->assertEquals( [ $expected ], $group->refs );
$this->assertSame( [], $group->indexByName ); $this->assertSame( [], $group->indexByName );
} }
@ -89,26 +79,18 @@ class ReferencesDataTest extends MediaWikiUnitTestCase {
'rtl' 'rtl'
); );
$expected = [ $expected = new RefGroupItem();
'contentId' => null, $expected->dir = 'rtl';
'cachedHtml' => null, $expected->group = 'note';
'dir' => 'rtl', $expected->key = 'cite_ref-wales_1';
'group' => 'note', $expected->id = 'cite_ref-wales_1-0';
'groupIndex' => 1, $expected->name = 'wales';
'index' => 0, $expected->target = 'cite_note-wales-1';
'key' => 'cite_ref-wales_1', $this->assertEquals( $expected, $ref );
'id' => 'cite_ref-wales_1-0',
'linkbacks' => [],
'name' => 'wales',
'target' => 'cite_note-wales-1',
'nodes' => [],
'embeddedNodes' => [],
];
$this->assertSame( $expected, (array)$ref );
$group = $data->getRefGroup( 'note' ); $group = $data->getRefGroup( 'note' );
$this->assertEquals( [ (object)$expected ], $group->refs ); $this->assertEquals( [ $expected ], $group->refs );
$this->assertEquals( [ 'wales' => (object)$expected ], $group->indexByName ); $this->assertEquals( [ 'wales' => $expected ], $group->indexByName );
} }
} }