diff options
| -rw-r--r-- | autoload.php | 1 | ||||
| -rw-r--r-- | includes/MediaWikiServices.php | 8 | ||||
| -rw-r--r-- | includes/ServiceWiring.php | 15 | ||||
| -rw-r--r-- | includes/parser/Parsoid/LintErrorChecker.php | 91 | ||||
| -rw-r--r-- | includes/preferences/SignatureValidator.php | 52 | ||||
| -rw-r--r-- | includes/preferences/SignatureValidatorFactory.php | 26 | ||||
| -rw-r--r-- | tests/phpunit/includes/parser/Parsoid/LintErrorCheckerTest.php | 107 | ||||
| -rw-r--r-- | tests/phpunit/includes/preferences/SignatureValidatorTest.php | 28 |
8 files changed, 232 insertions, 96 deletions
diff --git a/autoload.php b/autoload.php index 6baa2b851a8f..7bdfceb1c74d 100644 --- a/autoload.php +++ b/autoload.php @@ -1847,6 +1847,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Parser\\Parsoid\\HtmlToContentTransform' => __DIR__ . '/includes/parser/Parsoid/HtmlToContentTransform.php', 'MediaWiki\\Parser\\Parsoid\\HtmlTransformFactory' => __DIR__ . '/includes/parser/Parsoid/HtmlTransformFactory.php', 'MediaWiki\\Parser\\Parsoid\\LanguageVariantConverter' => __DIR__ . '/includes/parser/Parsoid/LanguageVariantConverter.php', + 'MediaWiki\\Parser\\Parsoid\\LintErrorChecker' => __DIR__ . '/includes/parser/Parsoid/LintErrorChecker.php', 'MediaWiki\\Parser\\Parsoid\\PageBundleJsonTrait' => __DIR__ . '/includes/parser/Parsoid/PageBundleJsonTrait.php', 'MediaWiki\\Parser\\Parsoid\\PageBundleParserOutputConverter' => __DIR__ . '/includes/parser/Parsoid/PageBundleParserOutputConverter.php', 'MediaWiki\\Parser\\Parsoid\\ParsoidOutputAccess' => __DIR__ . '/includes/parser/Parsoid/ParsoidOutputAccess.php', diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index 10c7c6c16c7e..2976551744a8 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -119,6 +119,7 @@ use MediaWiki\Parser\ParserCacheFactory; use MediaWiki\Parser\Parsoid\Config\PageConfigFactory; use MediaWiki\Parser\Parsoid\Config\SiteConfig; use MediaWiki\Parser\Parsoid\HtmlTransformFactory; +use MediaWiki\Parser\Parsoid\LintErrorChecker; use MediaWiki\Parser\Parsoid\ParsoidOutputAccess; use MediaWiki\Parser\Parsoid\ParsoidParserFactory; use MediaWiki\Password\PasswordFactory; @@ -1329,6 +1330,13 @@ class MediaWikiServices extends ServiceContainer { } /** + * @since 1.43 + */ + public function getLintErrorChecker(): LintErrorChecker { + return $this->getService( 'LintErrorChecker' ); + } + + /** * @since 1.34 */ public function getLocalisationCache(): LocalisationCache { diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 6cbbb17d4abf..aa01b827133e 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -154,6 +154,7 @@ use MediaWiki\Parser\Parsoid\Config\DataAccess as MWDataAccess; use MediaWiki\Parser\Parsoid\Config\PageConfigFactory as MWPageConfigFactory; use MediaWiki\Parser\Parsoid\Config\SiteConfig as MWSiteConfig; use MediaWiki\Parser\Parsoid\HtmlTransformFactory; +use MediaWiki\Parser\Parsoid\LintErrorChecker; use MediaWiki\Parser\Parsoid\ParsoidOutputAccess; use MediaWiki\Parser\Parsoid\ParsoidParserFactory; use MediaWiki\Password\PasswordFactory; @@ -1142,6 +1143,16 @@ return [ ); }, + 'LintErrorChecker' => static function ( MediaWikiServices $services ): LintErrorChecker { + return new LintErrorChecker( + $services->get( '_Parsoid' ), + $services->getParsoidPageConfigFactory(), + $services->getTitleFactory(), + ExtensionRegistry::getInstance(), + $services->getMainConfig(), + ); + }, + 'LocalisationCache' => static function ( MediaWikiServices $services ): LocalisationCache { $conf = $services->getMainConfig()->get( MainConfigNames::LocalisationCacheConf ); @@ -2073,12 +2084,10 @@ return [ return $services->getParserFactory(); }, static function () use ( $services ) { - return $services->get( '_Parsoid' ); + return $services->getLintErrorChecker(); }, - $services->getParsoidPageConfigFactory(), $services->getSpecialPageFactory(), $services->getTitleFactory(), - $services->getExtensionRegistry() ); }, diff --git a/includes/parser/Parsoid/LintErrorChecker.php b/includes/parser/Parsoid/LintErrorChecker.php new file mode 100644 index 000000000000..4d43557efcec --- /dev/null +++ b/includes/parser/Parsoid/LintErrorChecker.php @@ -0,0 +1,91 @@ +<?php +// SPDX-License-Identifier: GPL-2.0-or-later + +namespace MediaWiki\Parser\Parsoid; + +use ExtensionRegistry; +use MediaWiki\Config\Config; +use MediaWiki\Parser\Parsoid\Config\PageConfigFactory; +use MediaWiki\Revision\MutableRevisionRecord; +use MediaWiki\Revision\SlotRecord; +use MediaWiki\Title\TitleFactory; +use Wikimedia\Parsoid\Parsoid; +use WikitextContent; + +/** + * Check arbitrary wikitext for lint errors + * + * @since 1.43 + */ +class LintErrorChecker { + private Parsoid $parsoid; + private PageConfigFactory $pageConfigFactory; + private TitleFactory $titleFactory; + private ExtensionRegistry $extensionRegistry; + private Config $mainConfig; + + public function __construct( + Parsoid $parsoid, + PageConfigFactory $pageConfigFactory, + TitleFactory $titleFactory, + ExtensionRegistry $extensionRegistry, + Config $mainConfig + + ) { + $this->parsoid = $parsoid; + $this->pageConfigFactory = $pageConfigFactory; + $this->titleFactory = $titleFactory; + $this->extensionRegistry = $extensionRegistry; + $this->mainConfig = $mainConfig; + } + + private function linterOptions( array $disabled ): array { + // FIXME: We shouldn't be this interwined with an extension (T360809) + if ( $this->extensionRegistry->isLoaded( 'Linter' ) ) { + foreach ( $this->mainConfig->get( 'LinterCategories' ) as $name => $cat ) { + if ( $cat['priority'] === 'none' ) { + $disabled[] = $name; + } + } + } + $disabled = array_unique( $disabled ); + return [ 'linterOverrides' => [ 'disabled' => $disabled ] ]; + } + + /** + * Check the given wikitext for lint errors + * + * While not strictly required, you'll get better results if the wikitext has already gone through PST + * + * @param string $wikitext Wikitext after PST + * @return array Array of error objects returned by Parsoid's lint API (empty array for no errors) + */ + public function check( string $wikitext ): array { + return $this->checkSome( $wikitext, [] ); + } + + /** + * Check the given wikitext for lint errors against a subset of lint categories + * + * While not strictly required, you'll get better results if the wikitext has already gone through PST + * + * @param string $wikitext Wikitext after PST + * @param string[] $disabled Array of lint categories to disable + * @return array Array of error objects returned by Parsoid's lint API (empty array for no errors) + */ + public function checkSome( string $wikitext, array $disabled ): array { + $title = $this->titleFactory->newMainPage(); + $fakeRevision = new MutableRevisionRecord( $title ); + $fakeRevision->setSlot( + SlotRecord::newUnsaved( + SlotRecord::MAIN, + new WikitextContent( $wikitext ) + ) + ); + + return $this->parsoid->wikitext2lint( + $this->pageConfigFactory->create( $title, null, $fakeRevision ), + $this->linterOptions( $disabled ) + ); + } +} diff --git a/includes/preferences/SignatureValidator.php b/includes/preferences/SignatureValidator.php index 08067afff5f4..6bd874bd23c4 100644 --- a/includes/preferences/SignatureValidator.php +++ b/includes/preferences/SignatureValidator.php @@ -20,26 +20,19 @@ namespace MediaWiki\Preferences; -use ExtensionRegistry; use MediaWiki\Config\ServiceOptions; use MediaWiki\Html\Html; use MediaWiki\MainConfigNames; -use MediaWiki\MediaWikiServices; use MediaWiki\Parser\ParserOutputFlags; -use MediaWiki\Parser\Parsoid\Config\PageConfigFactory; -use MediaWiki\Revision\MutableRevisionRecord; -use MediaWiki\Revision\SlotRecord; +use MediaWiki\Parser\Parsoid\LintErrorChecker; use MediaWiki\SpecialPage\SpecialPage; use MediaWiki\SpecialPage\SpecialPageFactory; -use MediaWiki\Title\Title; use MediaWiki\Title\TitleFactory; use MediaWiki\User\UserIdentity; use MessageLocalizer; use OOUI\ButtonWidget; use ParserFactory; use ParserOptions; -use Wikimedia\Parsoid\Parsoid; -use WikitextContent; /** * @since 1.35 @@ -60,15 +53,13 @@ class SignatureValidator { private $popts; /** @var ParserFactory */ private $parserFactory; - private Parsoid $parsoid; - private PageConfigFactory $pageConfigFactory; + private LintErrorChecker $lintErrorChecker; /** @var ServiceOptions */ private $serviceOptions; /** @var SpecialPageFactory */ private $specialPageFactory; /** @var TitleFactory */ private $titleFactory; - private ExtensionRegistry $extensionRegistry; /** * @param ServiceOptions $options @@ -76,11 +67,9 @@ class SignatureValidator { * @param ?MessageLocalizer $localizer * @param ParserOptions $popts * @param ParserFactory $parserFactory - * @param Parsoid $parsoid - * @param PageConfigFactory $pageConfigFactory + * @param LintErrorChecker $lintErrorChecker * @param SpecialPageFactory $specialPageFactory * @param TitleFactory $titleFactory - * @param ExtensionRegistry $extensionRegistry */ public function __construct( ServiceOptions $options, @@ -88,25 +77,21 @@ class SignatureValidator { ?MessageLocalizer $localizer, ParserOptions $popts, ParserFactory $parserFactory, - Parsoid $parsoid, - PageConfigFactory $pageConfigFactory, + LintErrorChecker $lintErrorChecker, SpecialPageFactory $specialPageFactory, - TitleFactory $titleFactory, - ExtensionRegistry $extensionRegistry + TitleFactory $titleFactory ) { $this->user = $user; $this->localizer = $localizer; $this->popts = $popts; $this->parserFactory = $parserFactory; - $this->parsoid = $parsoid; - $this->pageConfigFactory = $pageConfigFactory; + $this->lintErrorChecker = $lintErrorChecker; // Configuration $this->serviceOptions = $options; $this->serviceOptions->assertRequiredOptions( self::CONSTRUCTOR_OPTIONS ); // TODO SpecialPage::getTitleFor should also be available via SpecialPageFactory $this->specialPageFactory = $specialPageFactory; $this->titleFactory = $titleFactory; - $this->extensionRegistry = $extensionRegistry; } /** @@ -278,30 +263,7 @@ class SignatureValidator { MainConfigNames::SignatureAllowedLintErrors ) ); - if ( $this->extensionRegistry->isLoaded( 'Linter' ) ) { // T360809 - $services = MediaWikiServices::getInstance(); - $linterCategories = $services->getMainConfig()->get( 'LinterCategories' ); - foreach ( $linterCategories as $name => $cat ) { - if ( $cat['priority'] === 'none' ) { - $disabled[] = $name; - } - } - } - $disabled = array_unique( $disabled ); - - $page = Title::newMainPage(); - $fakeRevision = new MutableRevisionRecord( $page ); - $fakeRevision->setSlot( - SlotRecord::newUnsaved( - SlotRecord::MAIN, - new WikitextContent( $signature ) - ) - ); - - return $this->parsoid->wikitext2lint( - $this->pageConfigFactory->create( $page, null, $fakeRevision ), - [ 'linterOverrides' => [ 'disabled' => $disabled ] ] - ); + return $this->lintErrorChecker->checkSome( $signature, $disabled ); } /** diff --git a/includes/preferences/SignatureValidatorFactory.php b/includes/preferences/SignatureValidatorFactory.php index 622e6adc9817..e1cc90be0715 100644 --- a/includes/preferences/SignatureValidatorFactory.php +++ b/includes/preferences/SignatureValidatorFactory.php @@ -21,9 +21,7 @@ namespace MediaWiki\Preferences; -use ExtensionRegistry; use MediaWiki\Config\ServiceOptions; -use MediaWiki\Parser\Parsoid\Config\PageConfigFactory; use MediaWiki\SpecialPage\SpecialPageFactory; use MediaWiki\Title\TitleFactory; use MediaWiki\User\UserIdentity; @@ -41,9 +39,7 @@ class SignatureValidatorFactory { private $parserFactoryClosure; /** @var callable */ - private $parsoidClosure; - - private PageConfigFactory $pageConfigFactory; + private $lintErrorCheckerClosure; /** @var SpecialPageFactory */ private $specialPageFactory; @@ -51,37 +47,29 @@ class SignatureValidatorFactory { /** @var TitleFactory */ private $titleFactory; - private ExtensionRegistry $extensionRegistry; - /** * @param ServiceOptions $options * @param callable $parserFactoryClosure A function which returns a ParserFactory. * We use this instead of an actual ParserFactory to avoid a circular dependency, * since Parser also needs a SignatureValidatorFactory for signature formatting. - * @param callable $parsoidClosure A function which returns a Parsoid, same as above. - * @param PageConfigFactory $pageConfigFactory + * @param callable $lintErrorCheckerClosure A function which returns a LintErrorChecker, same as above. * @param SpecialPageFactory $specialPageFactory * @param TitleFactory $titleFactory - * @param ExtensionRegistry $extensionRegistry */ public function __construct( ServiceOptions $options, callable $parserFactoryClosure, - callable $parsoidClosure, - PageConfigFactory $pageConfigFactory, + callable $lintErrorCheckerClosure, SpecialPageFactory $specialPageFactory, - TitleFactory $titleFactory, - ExtensionRegistry $extensionRegistry + TitleFactory $titleFactory ) { // Configuration $this->serviceOptions = $options; $this->serviceOptions->assertRequiredOptions( SignatureValidator::CONSTRUCTOR_OPTIONS ); $this->parserFactoryClosure = $parserFactoryClosure; - $this->parsoidClosure = $parsoidClosure; - $this->pageConfigFactory = $pageConfigFactory; + $this->lintErrorCheckerClosure = $lintErrorCheckerClosure; $this->specialPageFactory = $specialPageFactory; $this->titleFactory = $titleFactory; - $this->extensionRegistry = $extensionRegistry; } /** @@ -101,11 +89,9 @@ class SignatureValidatorFactory { $localizer, $popts, ( $this->parserFactoryClosure )(), - ( $this->parsoidClosure )(), - $this->pageConfigFactory, + ( $this->lintErrorCheckerClosure )(), $this->specialPageFactory, $this->titleFactory, - $this->extensionRegistry ); } } diff --git a/tests/phpunit/includes/parser/Parsoid/LintErrorCheckerTest.php b/tests/phpunit/includes/parser/Parsoid/LintErrorCheckerTest.php new file mode 100644 index 000000000000..1a47cb8a1cfb --- /dev/null +++ b/tests/phpunit/includes/parser/Parsoid/LintErrorCheckerTest.php @@ -0,0 +1,107 @@ +<?php +/** + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + */ + +use MediaWiki\MainConfigNames; +use MediaWiki\Parser\Parsoid\LintErrorChecker; + +/** + * @group Parser + * @group Database + * @covers \MediaWiki\Parser\Parsoid\LintErrorChecker + */ +class LintErrorCheckerTest extends MediaWikiIntegrationTestCase { + + protected function setUp(): void { + parent::setUp(); + $this->overrideConfigValue( MainConfigNames::ParsoidSettings, [ + 'linting' => true + ] ); + $this->overrideConfigValue( 'LinterCategories', [ + // No hidden categories in default set up + ] ); + } + + /** + * Get a basic LintErrorChecker for testing with. + * @return LintErrorChecker + */ + protected function getLintErrorChecker() { + $services = $this->getServiceContainer(); + $extReg = $this->createMock( ExtensionRegistry::class ); + $extReg->method( 'isLoaded' )->willReturnCallback( static function ( string $which ) { + return $which == 'Linter'; + } ); + + return new LintErrorChecker( + $services->get( '_Parsoid' ), + $services->getParsoidPageConfigFactory(), + $services->getTitleFactory(), + $extReg, + $services->getMainConfig(), + ); + } + + /** + * @dataProvider provideCheck + */ + public function testCheck( $wikitext, $expected ) { + $errors = $this->getLintErrorChecker()->check( $wikitext ); + $this->assertSame( $expected, $errors ); + } + + public static function provideCheck() { + yield 'Perfect' => [ '<strong>Foo</strong>', [] ]; + yield 'Unclosed tag' => [ + '<strong>Foo', + [ + [ + 'type' => 'missing-end-tag', + 'dsr' => [ 0, 11, 8, 0 ], + 'templateInfo' => null, + 'params' => [ + 'name' => 'strong', + 'inTable' => false, + ] + ] + ] + ]; + } + + public function testCheckSome() { + // Take the same "Unclosed tag" test from above but disable the category + $errors = $this->getLintErrorChecker()->checkSome( '<strong>Foo', [ 'missing-end-tag' ] ); + $this->assertSame( [], $errors ); + } + + /** Test when categories are diabled in $wgLinterCategories */ + public function testLinterCategory() { + $input = '<font color="red">RED</font>'; + $errors = $this->getLintErrorChecker()->check( $input ); + $this->assertEquals( 'obsolete-tag', $errors[0]['type'] ); + + // Now disable the category + $this->overrideConfigValue( 'LinterCategories', [ + 'obsolete-tag' => [ 'priority' => 'none' ], + ] ); + + $errors = $this->getLintErrorChecker()->check( $input ); + $this->assertSame( [], $errors ); + } +} diff --git a/tests/phpunit/includes/preferences/SignatureValidatorTest.php b/tests/phpunit/includes/preferences/SignatureValidatorTest.php index d2aba241762f..b2d938ac8b9f 100644 --- a/tests/phpunit/includes/preferences/SignatureValidatorTest.php +++ b/tests/phpunit/includes/preferences/SignatureValidatorTest.php @@ -177,27 +177,6 @@ class SignatureValidatorTest extends MediaWikiIntegrationTestCase { $this->assertSame( $expected, $result ); } - /** - * @covers \MediaWiki\Preferences\SignatureValidator::validateSignature() - * @dataProvider provideValidateSignature - */ - public function testValidateSignatureHidden( string $signature, $expected ) { - // For testing hidden category support in ::testValidateSignature - $this->overrideConfigValue( 'LinterCategories', [ - 'fostered' => [ 'priority' => 'medium' ], - // A hidden category, for testing. - 'wikilink-in-extlink' => [ 'priority' => 'none' ], - ] ); - $this->validator = $this->getSignatureValidator(); - $result = $this->validator->validateSignature( $signature ); - if ( $expected === 'hidden' ) { - $expected = false; - } elseif ( is_string( $expected ) ) { - $expected = true; - } - $this->assertSame( $expected, $result ); - } - public function provideValidateSignature() { yield 'Perfect' => [ '[[User:SignatureValidatorTest|Signature]] ([[User talk:SignatureValidatorTest|talk]])', @@ -214,13 +193,6 @@ class SignatureValidatorTest extends MediaWikiIntegrationTestCase { // This is allowed by SignatureAllowedLintErrors 'allowed' ]; - // Testing hidden category support; 'wikilink-in-extlink' has been - // made hidden. - yield 'Wikilink in Extlint (hidden)' => [ - '[http://example.com [[Foo]]!] [[User:SignatureValidatorTest|Signature]] ([[User talk:SignatureValidatorTest|talk]])', - // This is allowed because the category is hidden - 'hidden' - ]; } /** |