diff --git a/includes/Api/CheckSyntax.php b/includes/Api/CheckSyntax.php index 812bfa5ec..5573b087d 100644 --- a/includes/Api/CheckSyntax.php +++ b/includes/Api/CheckSyntax.php @@ -25,7 +25,7 @@ class CheckSyntax extends ApiBase { $warnings = []; foreach ( $result->getWarnings() as $warning ) { $warnings[] = [ - 'message' => $warning->getMessageObj()->text(), + 'message' => $this->msg( $warning->getMessageObj() )->text(), 'character' => $warning->getPosition() ]; } @@ -43,7 +43,7 @@ class CheckSyntax extends ApiBase { '@phan-var AFPUserVisibleException $excep'; $r = [ 'status' => 'error', - 'message' => $excep->getMessageObj()->text(), + 'message' => $this->msg( $excep->getMessageObj() )->text(), 'character' => $excep->getPosition(), ]; } diff --git a/includes/FilterRunner.php b/includes/FilterRunner.php index 12b8ac021..2e95baa53 100644 --- a/includes/FilterRunner.php +++ b/includes/FilterRunner.php @@ -462,7 +462,7 @@ class FilterRunner { $origExtraTime = LazyVariableComputer::$profilingExtraTime; $this->parser->setFilter( $filterName ); - $result = $this->parser->checkConditions( $filter->getRules(), true, $filterName )->getResult(); + $result = $this->parser->checkConditions( $filter->getRules(), $filterName )->getResult(); $actualExtra = LazyVariableComputer::$profilingExtraTime - $origExtraTime; $timeTaken = 1000 * ( microtime( true ) - $startTime - $actualExtra ); diff --git a/includes/FilterValidator.php b/includes/FilterValidator.php index aa5660632..dc256ace6 100644 --- a/includes/FilterValidator.php +++ b/includes/FilterValidator.php @@ -120,7 +120,7 @@ class FilterValidator { if ( $syntaxStatus->getResult() !== true ) { $excep = $syntaxStatus->getException(); $errMsg = $excep instanceof AFPUserVisibleException - ? $excep->getMessageObj()->text() + ? $excep->getMessageObj() : $excep->getMessage(); $ret->error( 'abusefilter-edit-badsyntax', $errMsg ); } diff --git a/includes/Parser/AFPUserVisibleException.php b/includes/Parser/AFPUserVisibleException.php index 907c047d0..53fa8bdd6 100644 --- a/includes/Parser/AFPUserVisibleException.php +++ b/includes/Parser/AFPUserVisibleException.php @@ -37,25 +37,18 @@ class AFPUserVisibleException extends AFPException { } /** - * Change the message of the exception to a localized version - */ - public function setLocalizedMessage() { - $this->message = $this->getMessageObj()->text(); - } - - /** - * Returns the error message in English for use in logs + * Returns the error message for use in logs * * @return string */ - public function getMessageForLogs() { - return $this->getMessageObj()->inLanguage( 'en' )->useDatabase( false )->text(); + public function getMessageForLogs() : string { + return "ID: {$this->mExceptionID}; position: {$this->mPosition}; params: " . implode( $this->mParams ); } /** * @return Message */ - public function getMessageObj() { + public function getMessageObj() : Message { // Give grep a chance to find the usages: // abusefilter-exception-unexpectedatend, abusefilter-exception-expectednotfound // abusefilter-exception-unrecognisedkeyword, abusefilter-exception-unexpectedtoken @@ -68,9 +61,9 @@ class AFPUserVisibleException extends AFPException { // abusefilter-exception-invalidiprange, abusefilter-exception-disabledvar // abusefilter-exception-variablevariable, abusefilter-exception-toomanyargs // abusefilter-exception-negativeoffset - return wfMessage( + return new Message( 'abusefilter-exception-' . $this->mExceptionID, - $this->mPosition, ...$this->mParams + array_merge( [ $this->mPosition ], $this->mParams ) ); } } diff --git a/includes/Parser/AbuseFilterParser.php b/includes/Parser/AbuseFilterParser.php index ce18dba72..2ca5dd25f 100644 --- a/includes/Parser/AbuseFilterParser.php +++ b/includes/Parser/AbuseFilterParser.php @@ -282,28 +282,21 @@ class AbuseFilterParser extends AFPTransitionBase { * be used to determine whether this method should throw. * * @param string $conds - * @param bool $ignoreError * @param string|null $filter The ID of the filter being parsed * @return ParserStatus - * @throws AFPException */ - public function checkConditions( string $conds, $ignoreError = true, $filter = null ) : ParserStatus { + public function checkConditions( string $conds, $filter = null ) : ParserStatus { $result = $this->parseDetailed( $conds ); $excep = $result->getException(); if ( $excep !== null ) { if ( $excep instanceof AFPUserVisibleException ) { $msg = $excep->getMessageForLogs(); - $excep->setLocalizedMessage(); } else { $msg = $excep->getMessage(); } $extraInfo = $filter !== null ? " for filter $filter" : ''; $this->logger->warning( "AbuseFilter parser error$extraInfo: $msg" ); - - if ( !$ignoreError ) { - throw $excep; - } } return $result; diff --git a/includes/Parser/UserVisibleWarning.php b/includes/Parser/UserVisibleWarning.php index b4aad2d3b..24e01f513 100644 --- a/includes/Parser/UserVisibleWarning.php +++ b/includes/Parser/UserVisibleWarning.php @@ -11,12 +11,12 @@ class UserVisibleWarning extends AFPUserVisibleException { /** * @return Message */ - public function getMessageObj() { + public function getMessageObj() : Message { // Give grep a chance to find the usages: // abusefilter-warning-match-empty-regex - return wfMessage( + return new Message( 'abusefilter-warning-' . $this->mExceptionID, - $this->mPosition, ...$this->mParams + array_merge( [ $this->mPosition ], $this->mParams ) ); } } diff --git a/tests/phpunit/unit/FilterValidatorTest.php b/tests/phpunit/unit/FilterValidatorTest.php index 317f5217f..b9303842b 100644 --- a/tests/phpunit/unit/FilterValidatorTest.php +++ b/tests/phpunit/unit/FilterValidatorTest.php @@ -115,7 +115,7 @@ class FilterValidatorTest extends MediaWikiUnitTestCase { $excMsg = $this->getMockMessage( $excText ); $excep = $this->createMock( AFPUserVisibleException::class ); $excep->method( 'getMessageObj' )->willReturn( $excMsg ); - yield 'invalid, user error' => [ false, $excep, 'abusefilter-edit-badsyntax', [ $excText ] ]; + yield 'invalid, user error' => [ false, $excep, 'abusefilter-edit-badsyntax', [ $excMsg ] ]; } /** diff --git a/tests/phpunit/unit/Parser/AFPUserVisibleExceptionTest.php b/tests/phpunit/unit/Parser/AFPUserVisibleExceptionTest.php new file mode 100644 index 000000000..a3be7dec2 --- /dev/null +++ b/tests/phpunit/unit/Parser/AFPUserVisibleExceptionTest.php @@ -0,0 +1,35 @@ +assertSame( $position, $exc->getPosition(), 'position' ); + $this->assertStringContainsString( $excID, $exc->getMessageForLogs(), 'ID in log message' ); + $this->assertStringContainsString( $position, $exc->getMessageForLogs(), 'position in logs message' ); + $message = $exc->getMessageObj(); + $this->assertSame( 'abusefilter-exception-' . $excID, $message->getKey(), 'msg key' ); + $this->assertArrayEquals( array_merge( [ $position ], $params ), $message->getParams(), 'msg params' ); + } +} diff --git a/tests/phpunit/unit/Parser/ParserFactoryTest.php b/tests/phpunit/unit/Parser/ParserFactoryTest.php new file mode 100644 index 000000000..ba2a6e302 --- /dev/null +++ b/tests/phpunit/unit/Parser/ParserFactoryTest.php @@ -0,0 +1,48 @@ + [ 'AbuseFilterParser', AbuseFilterParser::class ], + 'new' => [ 'AbuseFilterCachingParser', AbuseFilterCachingParser::class ] + ]; + } + + /** + * @covers ::__construct + * @covers ::newParser + * @dataProvider provideParserClass + */ + public function testNewParser( string $classOption, string $expClass ) { + $factory = new ParserFactory( + $this->createMock( Language::class ), + $this->createMock( BagOStuff::class ), + new NullLogger(), + $this->createMock( KeywordsManager::class ), + $this->createMock( VariablesManager::class ), + $classOption, + 1000 + ); + $this->assertInstanceOf( $expClass, $factory->newParser() ); + } +} diff --git a/tests/phpunit/unit/Parser/ParserStatusTest.php b/tests/phpunit/unit/Parser/ParserStatusTest.php new file mode 100644 index 000000000..a9b054710 --- /dev/null +++ b/tests/phpunit/unit/Parser/ParserStatusTest.php @@ -0,0 +1,37 @@ +assertSame( $result, $status->getResult() ); + $this->assertSame( $warm, $status->getWarmCache() ); + $this->assertSame( $exc, $status->getException() ); + $this->assertSame( $warnings, $status->getWarnings() ); + } +} diff --git a/tests/phpunit/unit/Parser/UserVisibleWarningTest.php b/tests/phpunit/unit/Parser/UserVisibleWarningTest.php new file mode 100644 index 000000000..67368aded --- /dev/null +++ b/tests/phpunit/unit/Parser/UserVisibleWarningTest.php @@ -0,0 +1,28 @@ +getMessageObj(); + $this->assertSame( 'abusefilter-warning-' . $excID, $message->getKey(), 'msg key' ); + $this->assertArrayEquals( array_merge( [ $position ], $params ), $message->getParams(), 'msg params' ); + } +}