From 9e1c4645be46d3f1027e70961b5b7df0b18156d2 Mon Sep 17 00:00:00 2001 From: thiemowmde Date: Tue, 5 Mar 2024 16:54:19 +0100 Subject: [PATCH] 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 --- src/Parsoid/RefGroup.php | 7 ++- src/Parsoid/RefGroupItem.php | 44 ++++++++++++++++ src/Parsoid/References.php | 3 +- src/Parsoid/ReferencesData.php | 34 ++++-------- .../unit/Parsoid/ReferencesDataTest.php | 52 ++++++------------- 5 files changed, 76 insertions(+), 64 deletions(-) create mode 100644 src/Parsoid/RefGroupItem.php diff --git a/src/Parsoid/RefGroup.php b/src/Parsoid/RefGroup.php index 28211d5e5..c4eaefa74 100644 --- a/src/Parsoid/RefGroup.php +++ b/src/Parsoid/RefGroup.php @@ -3,7 +3,6 @@ declare( strict_types = 1 ); namespace Cite\Parsoid; -use stdClass; use Wikimedia\Parsoid\DOM\Document; use Wikimedia\Parsoid\DOM\Element; use Wikimedia\Parsoid\Ext\DOMUtils; @@ -22,12 +21,12 @@ class RefGroup { public $name; /** - * @var stdClass[] + * @var RefGroupItem[] */ public array $refs = []; /** - * @var stdClass[] + * @var RefGroupItem[] */ public array $indexByName = []; @@ -62,7 +61,7 @@ class RefGroup { } public function renderLine( - ParsoidExtensionAPI $extApi, Element $refsList, stdClass $ref + ParsoidExtensionAPI $extApi, Element $refsList, RefGroupItem $ref ): void { $ownerDoc = $refsList->ownerDocument; diff --git a/src/Parsoid/RefGroupItem.php b/src/Parsoid/RefGroupItem.php new file mode 100644 index 000000000..be042be6d --- /dev/null +++ b/src/Parsoid/RefGroupItem.php @@ -0,0 +1,44 @@ + */ + public array $linkbacks = []; + public string $name = ''; + public string $target; + + /** @var Element[] */ + public array $nodes = []; + + /** @var string[] */ + public array $embeddedNodes = []; + +} diff --git a/src/Parsoid/References.php b/src/Parsoid/References.php index d6a01dc16..a15644beb 100644 --- a/src/Parsoid/References.php +++ b/src/Parsoid/References.php @@ -190,6 +190,7 @@ class References extends ExtensionTagHandler { // HTML inline for all of them. if ( $ref->contentId ) { if ( $ref->cachedHtml === null ) { + // @phan-suppress-next-line PhanTypeMismatchArgumentNullable False positive $refContent = $extApi->getContentDOM( $ref->contentId )->firstChild; $ref->cachedHtml = $extApi->domToHtml( $refContent, true, false ); } @@ -253,6 +254,7 @@ class References extends ExtensionTagHandler { if ( $validFollow ) { // Migrate content from the follow to the ref if ( $ref->contentId ) { + // @phan-suppress-next-line PhanTypeMismatchArgumentNullable False positive $refContent = $extApi->getContentDOM( $ref->contentId )->firstChild; DOMUtils::migrateChildren( $c, $refContent ); } else { @@ -495,7 +497,6 @@ class References extends ExtensionTagHandler { self::addErrorsToNode( $node, $errs ); } foreach ( $ref->embeddedNodes as $about ) { - '@phan-var string $about'; // $about is non-null $refsData->embeddedErrors[$about] = $errs; } } diff --git a/src/Parsoid/ReferencesData.php b/src/Parsoid/ReferencesData.php index e5e640a6a..b36db3060 100644 --- a/src/Parsoid/ReferencesData.php +++ b/src/Parsoid/ReferencesData.php @@ -3,7 +3,6 @@ declare( strict_types = 1 ); namespace Cite\Parsoid; -use stdClass; use Wikimedia\Parsoid\Core\Sanitizer; use Wikimedia\Parsoid\Ext\ParsoidExtensionAPI; @@ -79,7 +78,7 @@ class ReferencesData { public function add( ParsoidExtensionAPI $extApi, string $groupName, string $refName, string $refDir - ): stdClass { + ): RefGroupItem { $group = $this->getRefGroup( $groupName, true ); $hasRefName = strlen( $refName ) > 0; @@ -99,28 +98,15 @@ class ReferencesData { // bump index $this->index += 1; - $ref = (object)[ - // Pointer to the contents of the ref, accessible with the - // $extApi->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. - 'contentId' => null, - // Just used for comparison when we have multiples - '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' => [], - ]; + $ref = new RefGroupItem(); + $ref->dir = $refDir; + $ref->group = $group->name; + $ref->groupIndex = count( $group->refs ) + 1; + $ref->index = $n; + $ref->key = $refIdBase; + $ref->id = $hasRefName ? $refIdBase . '-0' : $refIdBase; + $ref->name = $refName; + $ref->target = $noteId; $group->refs[] = $ref; diff --git a/tests/phpunit/unit/Parsoid/ReferencesDataTest.php b/tests/phpunit/unit/Parsoid/ReferencesDataTest.php index ad854f922..7d8388638 100644 --- a/tests/phpunit/unit/Parsoid/ReferencesDataTest.php +++ b/tests/phpunit/unit/Parsoid/ReferencesDataTest.php @@ -4,6 +4,7 @@ namespace Cite\Tests\Unit; use Cite\Cite; use Cite\Parsoid\ReferencesData; +use Cite\Parsoid\RefGroupItem; use MediaWikiUnitTestCase; use Wikimedia\Parsoid\Ext\ParsoidExtensionAPI; @@ -58,25 +59,14 @@ class ReferencesDataTest extends MediaWikiUnitTestCase { '' ); - $expected = [ - 'contentId' => null, - 'cachedHtml' => null, - 'dir' => '', - 'group' => '', - '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 ); + $expected = new RefGroupItem(); + $expected->key = 'cite_ref-1'; + $expected->id = 'cite_ref-1'; + $expected->target = 'cite_note-1'; + $this->assertEquals( $expected, $ref ); $group = $data->getRefGroup( Cite::DEFAULT_GROUP ); - $this->assertEquals( [ (object)$expected ], $group->refs ); + $this->assertEquals( [ $expected ], $group->refs ); $this->assertSame( [], $group->indexByName ); } @@ -89,26 +79,18 @@ class ReferencesDataTest extends MediaWikiUnitTestCase { 'rtl' ); - $expected = [ - 'contentId' => null, - 'cachedHtml' => null, - 'dir' => 'rtl', - 'group' => 'note', - 'groupIndex' => 1, - 'index' => 0, - 'key' => 'cite_ref-wales_1', - 'id' => 'cite_ref-wales_1-0', - 'linkbacks' => [], - 'name' => 'wales', - 'target' => 'cite_note-wales-1', - 'nodes' => [], - 'embeddedNodes' => [], - ]; - $this->assertSame( $expected, (array)$ref ); + $expected = new RefGroupItem(); + $expected->dir = 'rtl'; + $expected->group = 'note'; + $expected->key = 'cite_ref-wales_1'; + $expected->id = 'cite_ref-wales_1-0'; + $expected->name = 'wales'; + $expected->target = 'cite_note-wales-1'; + $this->assertEquals( $expected, $ref ); $group = $data->getRefGroup( 'note' ); - $this->assertEquals( [ (object)$expected ], $group->refs ); - $this->assertEquals( [ 'wales' => (object)$expected ], $group->indexByName ); + $this->assertEquals( [ $expected ], $group->refs ); + $this->assertEquals( [ 'wales' => $expected ], $group->indexByName ); } }