From 7a9fd40eb9339e859586572b55d192a809ba5615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Mon, 2 Aug 2021 15:07:41 +0200 Subject: [PATCH] Remove use of DOMXPath to remove Phan suppressions Use DOMCompat::querySelectorAll() instead. CommentModifier::isHtmlSigned() * Copied the CSS selector from the JS equivalent function. CommentUtils::unwrapParsoidSections() * Copied the CSS selector from the JS equivalent function (in VisualEditor). CommentItem::getMentions() * Trivial. This causes Phan to report some more issues, which are also fixed. Follow-up to 25272e7a4a437584dbf7493e4f8e2f20fd4da859. Change-Id: Iaf1222f7114916f2eca19942c3686168899486fd --- includes/CommentItem.php | 19 +++++++++---------- includes/CommentModifier.php | 19 ++++++++++--------- includes/CommentUtils.php | 14 ++++++-------- 3 files changed, 25 insertions(+), 27 deletions(-) diff --git a/includes/CommentItem.php b/includes/CommentItem.php index 753321174..384a333cf 100644 --- a/includes/CommentItem.php +++ b/includes/CommentItem.php @@ -2,7 +2,6 @@ namespace MediaWiki\Extension\DiscussionTools; -use DOMXPath; use MWException; use Title; use Wikimedia\Parsoid\DOM\DocumentFragment; @@ -107,17 +106,17 @@ class CommentItem extends ThreadItem { */ public function getMentions(): array { $fragment = $this->getBodyRange()->cloneContents(); - // XXX use DOMCompat::querySelectorAll('a[href]') perhaps - // @phan-suppress-next-line PhanTypeMismatchArgumentInternal Nonstandard DOM - $xPath = new DOMXPath( $fragment->ownerDocument ); - // @phan-suppress-next-line PhanTypeMismatchArgumentInternal Nonstandard DOM - $links = $xPath->query( './/a', $fragment ); + // Note: DOMCompat::getElementsByTagName() doesn't take a DocumentFragment argument + $links = DOMCompat::querySelectorAll( $fragment, 'a' ); $users = []; foreach ( $links as $link ) { - $title = CommentUtils::getTitleFromUrl( $link->getAttribute( 'href' ) ); - if ( $title && $title->getNamespace() === NS_USER ) { - // TODO: Consider returning User objects - $users[] = $title; + $href = $link->getAttribute( 'href' ); + if ( $href ) { + $title = CommentUtils::getTitleFromUrl( $href ); + if ( $title && $title->getNamespace() === NS_USER ) { + // TODO: Consider returning User objects + $users[] = $title; + } } } return array_unique( $users ); diff --git a/includes/CommentModifier.php b/includes/CommentModifier.php index 9a169a9c7..a7f01a905 100644 --- a/includes/CommentModifier.php +++ b/includes/CommentModifier.php @@ -2,7 +2,6 @@ namespace MediaWiki\Extension\DiscussionTools; -use DOMXPath; use MWException; use Wikimedia\Parsoid\DOM\Document; use Wikimedia\Parsoid\DOM\DocumentFragment; @@ -449,16 +448,18 @@ class CommentModifier { * @return bool */ public static function isHtmlSigned( Element $container ): bool { - // XXX use querySelectorAll - // @phan-suppress-next-line PhanTypeMismatchArgumentInternal Nonstandard DOM - $xpath = new DOMXPath( $container->ownerDocument ); // Good enough?… - // @phan-suppress-next-line PhanTypeMismatchArgumentInternal Nonstandard DOM - $matches = $xpath->query( './/span[@typeof="mw:Transclusion"][contains(@data-mw,"~~~~")]', $container ); - if ( $matches->length === 0 ) { + $matches = DOMCompat::querySelectorAll( $container, 'span[typeof="mw:Transclusion"][data-mw*="~~~~"]' ); + // Iterate to get the last item. We don't know if $matches is an array or some iterator, + // and there doesn't seem to be a nicer way to get just the last item. + foreach ( $matches as $match ) { + $lastSig = $match; + } + if ( !isset( $lastSig ) ) { + // List was empty return false; } - $lastSig = $matches->item( $matches->length - 1 ); + // Signature must be at the end of the comment - there must be no sibling following this node, or its parents $node = $lastSig; while ( $node ) { @@ -466,7 +467,7 @@ class CommentModifier { while ( $node->nextSibling && $node->nextSibling->nodeType === XML_TEXT_NODE && - CommentUtils::htmlTrim( $node->nextSibling->nodeValue ) === '' + CommentUtils::htmlTrim( $node->nextSibling->nodeValue ?? '' ) === '' ) { $node = $node->nextSibling; } diff --git a/includes/CommentUtils.php b/includes/CommentUtils.php index dc1b63f54..c5d9dd6fc 100644 --- a/includes/CommentUtils.php +++ b/includes/CommentUtils.php @@ -2,9 +2,9 @@ namespace MediaWiki\Extension\DiscussionTools; -use DOMXPath; use MediaWiki\MediaWikiServices; use Title; +use Wikimedia\Assert\Assert; use Wikimedia\Parsoid\DOM\Comment; use Wikimedia\Parsoid\DOM\Element; use Wikimedia\Parsoid\DOM\Node; @@ -404,17 +404,15 @@ class CommentUtils { public static function unwrapParsoidSections( Element $element, string $keepSection = null ): void { - // XXX use DOMCompat::querySelectorAll - // @phan-suppress-next-line PhanTypeMismatchArgumentInternal Nonstandard DOM - $xpath = new DOMXPath( $element->ownerDocument ); - // @phan-suppress-next-line PhanTypeMismatchArgumentInternal Nonstandard DOM - $sections = $xpath->query( '//section[@data-mw-section-id]', $element ); + $sections = DOMCompat::querySelectorAll( $element, 'section[data-mw-section-id]' ); foreach ( $sections as $section ) { $parent = $section->parentNode; $sectionId = $section->getAttribute( 'data-mw-section-id' ); // Copy section ID to first child (should be a heading) - if ( $sectionId !== '' && intval( $sectionId ) > 0 ) { - $section->firstChild->setAttribute( 'data-mw-section-id', $sectionId ); + if ( $sectionId !== null && $sectionId !== '' && intval( $sectionId ) > 0 ) { + $firstChild = $section->firstChild; + Assert::precondition( $firstChild instanceof Element, 'Section has a heading' ); + $firstChild->setAttribute( 'data-mw-section-id', $sectionId ); } if ( $keepSection !== null && $sectionId === $keepSection ) { return;