Refactor ParserStatus

ParserStatus is now more lightweight, and doesn't know about "result"
and "from cache". Instead, it has an isValid() method which is merely a
shorthand for checking whether getException() is null.

Introduce a child class, RuleCheckerStatus, which knows about result and
cache and can be (un)serialized.

This removes the ambiguity of the $result field, and helps the
transition to a new RuleChecker class.

Change-Id: I0dac7ab4febbfdabe72596631db630411d967ab5

includes/Api/CheckMatch.php

@@ -127,7 +127,7 @@ class CheckMatch extends ApiBase {
 		}
 
 		$ruleChecker = $this->ruleCheckerFactory->newRuleChecker( $vars );
-		if ( !$ruleChecker->checkSyntax( $params['filter'] )->getResult() ) {
+		if ( !$ruleChecker->checkSyntax( $params['filter'] )->isValid() ) {
 			$this->dieWithError( 'apierror-abusefilter-badsyntax', 'badsyntax' );
 		}
 

includes/Api/CheckSyntax.php

@@ -59,7 +59,7 @@ class CheckSyntax extends ApiBase {
 			$r['warnings'] = $warnings;
 		}
 
-		if ( $result->getResult() ) {
+		if ( $result->isValid() ) {
 			// Everything went better than expected :)
 			$r['status'] = 'ok';
 		} else {

includes/Api/EvalExpression.php

@@ -73,7 +73,7 @@ class EvalExpression extends ApiBase {
 	 */
 	private function evaluateExpression( string $expr ): Status {
 		$ruleChecker = $this->ruleCheckerFactory->newRuleChecker();
-		if ( !$ruleChecker->checkSyntax( $expr )->getResult() ) {
+		if ( !$ruleChecker->checkSyntax( $expr )->isValid() ) {
 			return Status::newFatal( 'abusefilter-tools-syntax-error' );
 		}
 

includes/EditStashCache.php

@@ -15,7 +15,7 @@ use Psr\Log\LoggerInterface;
  */
 class EditStashCache {
 
-	private const CACHE_VERSION = 'v4';
+	private const CACHE_VERSION = 'v5';
 
 	/** @var BagOStuff */
 	private $cache;

includes/FilterRunner.php

@@ -11,8 +11,6 @@ use MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesExecutorFactory;
 use MediaWiki\Extension\AbuseFilter\Filter\ExistingFilter;
 use MediaWiki\Extension\AbuseFilter\Hooks\AbuseFilterHookRunner;
 use MediaWiki\Extension\AbuseFilter\Parser\FilterEvaluator;
-// phpcs:ignore MediaWiki.Classes.UnusedUseStatement.UnusedUse
-use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
 use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerFactory;
 use MediaWiki\Extension\AbuseFilter\VariableGenerator\VariableGeneratorFactory;
 use MediaWiki\Extension\AbuseFilter\Variables\LazyVariableComputer;
@@ -356,7 +354,7 @@ class FilterRunner {
 	 * @param ExistingFilter $filter
 	 * @param bool $global
 	 * @return array [ status, time taken ]
-	 * @phan-return array{0:ParserStatus,1:float}
+	 * @phan-return array{0:\MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus,1:float}
 	 */
 	protected function checkFilter( ExistingFilter $filter, bool $global = false ): array {
 		$filterName = GlobalNameUtils::buildGlobalName( $filter->getID(), $global );

includes/FilterValidator.php

@@ -132,7 +132,7 @@ class FilterValidator {
 		$ret = Status::newGood();
 		$ruleChecker = $this->ruleCheckerFactory->newRuleChecker();
 		$syntaxStatus = $ruleChecker->checkSyntax( $filter->getRules() );
-		if ( !$syntaxStatus->getResult() ) {
+		if ( !$syntaxStatus->isValid() ) {
 			$excep = $syntaxStatus->getException();
 			$errMsg = $excep instanceof UserVisibleException
 				? $excep->getMessageObj()

includes/Parser/FilterEvaluator.php

@@ -313,19 +313,16 @@ class FilterEvaluator {
 	 * Check the syntax of $filter, without throwing
 	 *
 	 * @param string $filter
-	 * @return ParserStatus The result indicates whether the syntax is valid
+	 * @return ParserStatus
 	 */
 	public function checkSyntax( string $filter ): ParserStatus {
 		$initialConds = $this->mCondCount;
 		try {
-			$valid = $this->checkSyntaxThrow( $filter );
+			$this->checkSyntaxThrow( $filter );
 		} catch ( UserVisibleException $excep ) {
-			$valid = false;
 		}
 
 		return new ParserStatus(
-			$valid,
-			$this->fromCache,
 			$excep ?? null,
 			$this->warnings,
 			$this->mCondCount - $initialConds
@@ -338,9 +335,9 @@ class FilterEvaluator {
 	 *
 	 * @param string $conds
 	 * @param string|null $filter The ID of the filter being parsed
-	 * @return ParserStatus
+	 * @return RuleCheckerStatus
 	 */
-	public function checkConditions( string $conds, $filter = null ): ParserStatus {
+	public function checkConditions( string $conds, $filter = null ): RuleCheckerStatus {
 		$this->mFilter = $filter;
 		$excep = null;
 		$initialConds = $this->mCondCount;
@@ -351,7 +348,7 @@ class FilterEvaluator {
 			$res = false;
 		}
 		$this->statsd->timing( 'abusefilter_cachingParser_full', microtime( true ) - $startTime );
-		$result = new ParserStatus(
+		$result = new RuleCheckerStatus(
 			$res,
 			$this->fromCache,
 			$excep,

includes/Parser/ParserStatus.php

@@ -6,52 +6,28 @@ use MediaWiki\Extension\AbuseFilter\Parser\Exception\ExceptionBase;
 use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleWarning;
 
 class ParserStatus {
-	/** @var bool */
-	private $result;
-	/** @var bool */
-	private $warmCache;
 	/** @var ExceptionBase|null */
-	private $excep;
+	protected $excep;
 	/** @var UserVisibleWarning[] */
-	private $warnings;
+	protected $warnings;
 	/** @var int */
-	private $condsUsed;
+	protected $condsUsed;
 
 	/**
-	 * @param bool $result A generic operation result
-	 * @param bool $warmCache Whether we retrieved the AST from cache
 	 * @param ExceptionBase|null $excep An exception thrown while parsing, or null if it parsed correctly
 	 * @param UserVisibleWarning[] $warnings
 	 * @param int $condsUsed
 	 */
 	public function __construct(
-		bool $result,
-		bool $warmCache,
 		?ExceptionBase $excep,
 		array $warnings,
 		int $condsUsed
 	) {
-		$this->result = $result;
-		$this->warmCache = $warmCache;
 		$this->excep = $excep;
 		$this->warnings = $warnings;
 		$this->condsUsed = $condsUsed;
 	}
 
-	/**
-	 * @return bool
-	 */
-	public function getResult(): bool {
-		return $this->result;
-	}
-
-	/**
-	 * @return bool
-	 */
-	public function getWarmCache(): bool {
-		return $this->warmCache;
-	}
-
 	/**
 	 * @return ExceptionBase|null
 	 */
@@ -74,38 +50,10 @@ class ParserStatus {
 	}
 
 	/**
-	 * Serialize data for edit stash
-	 * @return array
+	 * Whether the parsing/evaluation happened successfully.
+	 * @return bool
 	 */
-	public function toArray(): array {
-		return [
-			'result' => $this->result,
-			'warmCache' => $this->warmCache,
-			'exception' => $this->excep ? $this->excep->toArray() : null,
-			'warnings' => array_map(
-				static function ( $warn ) {
-					return $warn->toArray();
-				},
-				$this->warnings
-			),
-			'condsUsed' => $this->condsUsed,
-		];
+	public function isValid(): bool {
+		return !$this->excep;
 	}
-
-	/**
-	 * Deserialize data from edit stash
-	 * @param array $value
-	 * @return self
-	 */
-	public static function fromArray( array $value ): self {
-		$excClass = $value['exception']['class'] ?? null;
-		return new self(
-			$value['result'],
-			$value['warmCache'],
-			$excClass !== null ? call_user_func( [ $excClass, 'fromArray' ], $value['exception'] ) : null,
-			array_map( [ UserVisibleWarning::class, 'fromArray' ], $value['warnings'] ),
-			$value['condsUsed']
-		);
-	}
-
 }

includes/Parser/RuleCheckerStatus.php

@@ -0,0 +1,81 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter\Parser;
+
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\ExceptionBase;
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleWarning;
+
+class RuleCheckerStatus extends ParserStatus {
+	/** @var bool */
+	private $result;
+	/** @var bool */
+	private $warmCache;
+
+	/**
+	 * @param bool $result Whether the rule matched
+	 * @param bool $warmCache Whether we retrieved the AST from cache
+	 * @param ExceptionBase|null $excep An exception thrown while parsing, or null if it parsed correctly
+	 * @param UserVisibleWarning[] $warnings
+	 * @param int $condsUsed
+	 */
+	public function __construct(
+		bool $result,
+		bool $warmCache,
+		?ExceptionBase $excep,
+		array $warnings,
+		int $condsUsed
+	) {
+		parent::__construct( $excep, $warnings, $condsUsed );
+		$this->result = $result;
+		$this->warmCache = $warmCache;
+	}
+
+	/**
+	 * @return bool
+	 */
+	public function getResult(): bool {
+		return $this->result;
+	}
+
+	/**
+	 * @return bool
+	 */
+	public function getWarmCache(): bool {
+		return $this->warmCache;
+	}
+
+	/**
+	 * Serialize data for edit stash
+	 * @return array
+	 */
+	public function toArray(): array {
+		return [
+			'result' => $this->result,
+			'warmCache' => $this->warmCache,
+			'exception' => $this->excep ? $this->excep->toArray() : null,
+			'warnings' => array_map(
+				static function ( $warn ) {
+					return $warn->toArray();
+				},
+				$this->warnings
+			),
+			'condsUsed' => $this->condsUsed,
+		];
+	}
+
+	/**
+	 * Deserialize data from edit stash
+	 * @param array $value
+	 * @return self
+	 */
+	public static function fromArray( array $value ): self {
+		$excClass = $value['exception']['class'] ?? null;
+		return new self(
+			$value['result'],
+			$value['warmCache'],
+			$excClass !== null ? call_user_func( [ $excClass, 'fromArray' ], $value['exception'] ) : null,
+			array_map( [ UserVisibleWarning::class, 'fromArray' ], $value['warnings'] ),
+			$value['condsUsed']
+		);
+	}
+}

includes/RunnerData.php

@@ -3,7 +3,7 @@
 namespace MediaWiki\Extension\AbuseFilter;
 
 use LogicException;
-use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
+use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus;
 
 /**
  * Mutable value class storing and accumulating information about filter matches and runtime
@@ -11,8 +11,7 @@ use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
 class RunnerData {
 
 	/**
-	 * @var ParserStatus[]
-	 * @phan-var array<string,ParserStatus>
+	 * @var array<string,RuleCheckerStatus>
 	 */
 	private $matchedFilters;
 
@@ -29,7 +28,7 @@ class RunnerData {
 	private $totalConditions;
 
 	/**
-	 * @param ParserStatus[] $matchedFilters
+	 * @param RuleCheckerStatus[] $matchedFilters
 	 * @param array[] $profilingData
 	 * @param float $totalRuntime
 	 * @param int $totalConditions
@@ -51,10 +50,10 @@ class RunnerData {
 	 *
 	 * @param int $filterID
 	 * @param bool $global
-	 * @param ParserStatus $status
+	 * @param RuleCheckerStatus $status
 	 * @param float $timeTaken
 	 */
-	public function record( int $filterID, bool $global, ParserStatus $status, float $timeTaken ): void {
+	public function record( int $filterID, bool $global, RuleCheckerStatus $status, float $timeTaken ): void {
 		$key = GlobalNameUtils::buildGlobalName( $filterID, $global );
 		if ( array_key_exists( $key, $this->matchedFilters ) ) {
 			throw new LogicException( "Filter '$key' has already been recorded" );
@@ -144,7 +143,7 @@ class RunnerData {
 	 */
 	public static function fromArray( array $value ): self {
 		return new self(
-			array_map( [ ParserStatus::class, 'fromArray' ], $value['matches'] ),
+			array_map( [ RuleCheckerStatus::class, 'fromArray' ], $value['matches'] ),
 			$value['profiling'],
 			$value['runtime'],
 			$value['condCount']

includes/View/AbuseFilterViewTestBatch.php

@@ -216,7 +216,7 @@ class AbuseFilterViewTestBatch extends AbuseFilterView {
 		$out = $this->getOutput();
 		$ruleChecker = $this->ruleCheckerFactory->newRuleChecker();
 
-		if ( !$ruleChecker->checkSyntax( $this->testPattern )->getResult() ) {
+		if ( !$ruleChecker->checkSyntax( $this->testPattern )->isValid() ) {
 			$out->addWikiMsg( 'abusefilter-test-syntaxerr' );
 			return;
 		}

tests/phpunit/integration/Api/CheckMatchTest.php

@@ -4,9 +4,11 @@ namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Api;
 
 use ApiTestCase;
 use FormatJson;
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\InternalException;
 use MediaWiki\Extension\AbuseFilter\Parser\FilterEvaluator;
 use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
 use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerFactory;
+use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus;
 
 /**
  * @coversDefaultClass \MediaWiki\Extension\AbuseFilter\Api\CheckMatch
@@ -44,8 +46,8 @@ class CheckMatchTest extends ApiTestCase {
 	 */
 	public function testExecute_Ok( bool $expected ) {
 		$filter = 'sampleFilter';
-		$checkStatus = new ParserStatus( true, false, null, [], 1 );
-		$resultStatus = new ParserStatus( $expected, false, null, [], 1 );
+		$checkStatus = new ParserStatus( null, [], 1 );
+		$resultStatus = new RuleCheckerStatus( $expected, false, null, [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->expects( $this->once() )
 			->method( 'checkSyntax' )->with( $filter )
@@ -79,7 +81,7 @@ class CheckMatchTest extends ApiTestCase {
 	public function testExecute_error() {
 		$this->setExpectedApiException( 'apierror-abusefilter-badsyntax', 'badsyntax' );
 		$filter = 'sampleFilter';
-		$status = new ParserStatus( false, false, null, [], 1 );
+		$status = new ParserStatus( $this->createMock( InternalException::class ), [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->expects( $this->once() )
 			->method( 'checkSyntax' )->with( $filter )

tests/phpunit/integration/Api/CheckSyntaxTest.php

@@ -36,7 +36,7 @@ class CheckSyntaxTest extends ApiTestCase {
 	 */
 	public function testExecute_Ok() {
 		$input = 'sampleFilter';
-		$status = new ParserStatus( true, false, null, [], 1 );
+		$status = new ParserStatus( null, [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->method( 'checkSyntax' )->with( $input )
 			->willReturn( $status );
@@ -64,7 +64,7 @@ class CheckSyntaxTest extends ApiTestCase {
 			new UserVisibleWarning( 'exception-1', 3, [] ),
 			new UserVisibleWarning( 'exception-2', 8, [ 'param' ] ),
 		];
-		$status = new ParserStatus( true, false, null, $warnings, 1 );
+		$status = new ParserStatus( null, $warnings, 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->method( 'checkSyntax' )->with( $input )
 			->willReturn( $status );
@@ -110,7 +110,7 @@ class CheckSyntaxTest extends ApiTestCase {
 	public function testExecute_error() {
 		$input = 'sampleFilter';
 		$exception = new UserVisibleException( 'error-id', 4, [] );
-		$status = new ParserStatus( false, false, $exception, [], 1 );
+		$status = new ParserStatus( $exception, [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->method( 'checkSyntax' )->with( $input )
 			->willReturn( $status );

tests/phpunit/integration/Api/EvalExpressionTest.php

@@ -3,6 +3,7 @@
 namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Api;
 
 use ApiTestCase;
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\InternalException;
 use MediaWiki\Extension\AbuseFilter\Parser\FilterEvaluator;
 use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
 use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerFactory;
@@ -36,7 +37,7 @@ class EvalExpressionTest extends ApiTestCase {
 	public function testExecute_error() {
 		$this->setExpectedApiException( 'abusefilter-tools-syntax-error' );
 		$expression = 'sampleExpression';
-		$status = new ParserStatus( false, false, null, [], 1 );
+		$status = new ParserStatus( $this->createMock( InternalException::class ), [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->method( 'checkSyntax' )->with( $expression )
 			->willReturn( $status );
@@ -54,7 +55,7 @@ class EvalExpressionTest extends ApiTestCase {
 	 */
 	public function testExecute_Ok() {
 		$expression = 'sampleExpression';
-		$status = new ParserStatus( true, false, null, [], 1 );
+		$status = new ParserStatus( null, [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->method( 'checkSyntax' )->with( $expression )
 			->willReturn( $status );
@@ -86,7 +87,7 @@ class EvalExpressionTest extends ApiTestCase {
 	 */
 	public function testExecute_OkAndPrettyPrint() {
 		$expression = 'sampleExpression';
-		$status = new ParserStatus( true, false, null, [], 1 );
+		$status = new ParserStatus( null, [], 1 );
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
 		$ruleChecker->method( 'checkSyntax' )->with( $expression )
 			->willReturn( $status );

tests/phpunit/unit/FilterValidatorTest.php

@@ -43,7 +43,7 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
 		if ( !$ruleChecker ) {
 			$ruleChecker = $this->createMock( FilterEvaluator::class );
 			$ruleChecker->method( 'checkSyntax' )->willReturn(
-				new ParserStatus( true, true, null, [], 1 )
+				new ParserStatus( null, [], 1 )
 			);
 		}
 		$checkerFactory = $this->createMock( RuleCheckerFactory::class );
@@ -93,16 +93,15 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
 	}
 
 	/**
-	 * @param bool $valid
 	 * @param ExceptionBase|null $excep
 	 * @param string|null $expected
 	 * @param array|null $expParams
 	 * @covers ::checkValidSyntax
 	 * @dataProvider provideSyntax
 	 */
-	public function testCheckValidSyntax( bool $valid, ?ExceptionBase $excep, ?string $expected, ?array $expParams ) {
+	public function testCheckValidSyntax( ?ExceptionBase $excep, ?string $expected, ?array $expParams ) {
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
-		$syntaxStatus = new ParserStatus( $valid, true, $excep, [], 1 );
+		$syntaxStatus = new ParserStatus( $excep, [], 1 );
 		$ruleChecker->method( 'checkSyntax' )->willReturn( $syntaxStatus );
 		$validator = $this->getFilterValidator( null, $ruleChecker );
 
@@ -114,10 +113,9 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
 	}
 
 	public function provideSyntax(): Generator {
-		yield 'valid' => [ true, null, null, null ];
+		yield 'valid' => [ null, null, null ];
 		$excText = 'Internal error text';
 		yield 'invalid, internal error' => [
-			false,
 			new InternalException( $excText ),
 			'abusefilter-edit-badsyntax',
 			[ $excText ]
@@ -125,7 +123,7 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
 		$excMsg = $this->getMockMessage( $excText );
 		$excep = $this->createMock( UserVisibleException::class );
 		$excep->method( 'getMessageObj' )->willReturn( $excMsg );
-		yield 'invalid, user error' => [ false, $excep, 'abusefilter-edit-badsyntax', [ $excMsg ] ];
+		yield 'invalid, user error' => [ $excep, 'abusefilter-edit-badsyntax', [ $excMsg ] ];
 	}
 
 	/**
@@ -390,7 +388,7 @@ class FilterValidatorTest extends MediaWikiUnitTestCase {
 		$noopFilter->method( 'isEnabled' )->willReturn( true );
 
 		$ruleChecker = $this->createMock( FilterEvaluator::class );
-		$syntaxStatus = new ParserStatus( false, true, $this->createMock( UserVisibleException::class ), [], 1 );
+		$syntaxStatus = new ParserStatus( $this->createMock( UserVisibleException::class ), [], 1 );
 		$ruleChecker->method( 'checkSyntax' )->willReturn( $syntaxStatus );
 		yield 'invalid syntax' => [ $noopFilter, 'abusefilter-edit-badsyntax', null, $ruleChecker ];
 

tests/phpunit/unit/Parser/ParserStatusTest.php

@@ -2,8 +2,7 @@
 
 namespace MediaWiki\Extension\AbuseFilter\Tests\Unit\Parser;
 
-use MediaWiki\Extension\AbuseFilter\Parser\Exception\ExceptionBase;
-use MediaWiki\Extension\AbuseFilter\Parser\Exception\InternalException;
+use Generator;
 use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleException;
 use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleWarning;
 use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
@@ -20,53 +19,30 @@ class ParserStatusTest extends MediaWikiUnitTestCase {
 
 	/**
 	 * @covers ::__construct
-	 * @covers ::getResult
-	 * @covers ::getWarmCache
 	 * @covers ::getException
 	 * @covers ::getWarnings
 	 * @covers ::getCondsUsed
 	 */
 	public function testGetters() {
-		$result = true;
-		$warm = false;
 		$exc = $this->createMock( UserVisibleException::class );
 		$warnings = [ new UserVisibleWarning( 'foo', 1, [] ) ];
 		$condsUsed = 42;
-		$status = new ParserStatus( $result, $warm, $exc, $warnings, $condsUsed );
-		$this->assertSame( $result, $status->getResult() );
-		$this->assertSame( $warm, $status->getWarmCache() );
+		$status = new ParserStatus( $exc, $warnings, $condsUsed );
 		$this->assertSame( $exc, $status->getException() );
 		$this->assertSame( $warnings, $status->getWarnings() );
 		$this->assertSame( $condsUsed, $status->getCondsUsed() );
 	}
 
-	public function provideToArrayException() {
-		yield 'exception instance' => [ new InternalException() ];
-		yield 'null' => [ null ];
+	/**
+	 * @covers ::isValid
+	 * @dataProvider provideIsValid
+	 */
+	public function testIsValid( ParserStatus $status, bool $expected ) {
+		$this->assertSame( $expected, $status->isValid() );
 	}
 
-	/**
-	 * @dataProvider provideToArrayException
-	 * @covers ::toArray
-	 * @covers ::fromArray
-	 */
-	public function testToArrayRoundTrip( ?ExceptionBase $exception ) {
-		$status = new ParserStatus(
-			true,
-			false,
-			$exception,
-			[ new UserVisibleWarning( 'foo', 1, [] ) ],
-			42
-		);
-		$newStatus = ParserStatus::fromArray( $status->toArray() );
-		$this->assertSame( $status->getResult(), $newStatus->getResult() );
-		$this->assertSame( $status->getWarmCache(), $newStatus->getWarmCache() );
-		if ( $exception !== null ) {
-			$this->assertInstanceOf( get_class( $exception ), $newStatus->getException() );
-		} else {
-			$this->assertNull( $newStatus->getException() );
-		}
-		$this->assertContainsOnlyInstancesOf( UserVisibleWarning::class, $newStatus->getWarnings() );
-		$this->assertSame( $status->getCondsUsed(), $newStatus->getCondsUsed() );
+	public function provideIsValid(): Generator {
+		yield 'valid' => [ new ParserStatus( null, [], 42 ), true ];
+		yield 'invalid' => [ new ParserStatus( $this->createMock( UserVisibleException::class ), [], 42 ), false ];
 	}
 }

tests/phpunit/unit/Parser/ParserTest.php

@@ -933,7 +933,7 @@ class ParserTest extends ParserTestCase {
 		// Note that some of the data sets will actually match at runtime, even if the variable
 		// they refer to is not set, due to the parser using GET_BC rather than GET_STRICT.
 		// TODO: Once T230256 is done, this can be changed to $this->assertFalse( $this->getParser()->parse( $code ) )
-		$this->assertTrue( $this->getParser()->checkSyntax( $code )->getResult() );
+		$this->assertTrue( $this->getParser()->checkSyntax( $code )->isValid() );
 	}
 
 	/**
@@ -987,7 +987,7 @@ class ParserTest extends ParserTestCase {
 	 * @dataProvider provideBuiltinArrays
 	 */
 	public function testBuiltinArrays( string $code ) {
-		$this->assertTrue( $this->getParser()->checkSyntax( $code )->getResult() );
+		$this->assertTrue( $this->getParser()->checkSyntax( $code )->isValid() );
 		$this->exceptionTest( 'notarray', $code, '' );
 	}
 

tests/phpunit/unit/Parser/RuleCheckerStatusTest.php

@@ -0,0 +1,72 @@
+<?php
+
+namespace MediaWiki\Extension\AbuseFilter\Tests\Unit\Parser;
+
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\ExceptionBase;
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\InternalException;
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleException;
+use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleWarning;
+use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus;
+use MediaWikiUnitTestCase;
+
+/**
+ * @group Test
+ * @group AbuseFilter
+ * @group AbuseFilterParser
+ *
+ * @coversDefaultClass \MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus
+ */
+class RuleCheckerStatusTest extends MediaWikiUnitTestCase {
+
+	/**
+	 * @covers ::__construct
+	 * @covers ::getResult
+	 * @covers ::getWarmCache
+	 * @covers ::getException
+	 * @covers ::getWarnings
+	 * @covers ::getCondsUsed
+	 */
+	public function testGetters() {
+		$result = true;
+		$warm = false;
+		$exc = $this->createMock( UserVisibleException::class );
+		$warnings = [ new UserVisibleWarning( 'foo', 1, [] ) ];
+		$condsUsed = 42;
+		$status = new RuleCheckerStatus( $result, $warm, $exc, $warnings, $condsUsed );
+		$this->assertSame( $result, $status->getResult() );
+		$this->assertSame( $warm, $status->getWarmCache() );
+		$this->assertSame( $exc, $status->getException() );
+		$this->assertSame( $warnings, $status->getWarnings() );
+		$this->assertSame( $condsUsed, $status->getCondsUsed() );
+	}
+
+	public function provideToArrayException() {
+		yield 'exception instance' => [ new InternalException() ];
+		yield 'null' => [ null ];
+	}
+
+	/**
+	 * @dataProvider provideToArrayException
+	 * @covers ::toArray
+	 * @covers ::fromArray
+	 */
+	public function testToArrayRoundTrip( ?ExceptionBase $exception ) {
+		$status = new RuleCheckerStatus(
+			true,
+			false,
+			$exception,
+			[ new UserVisibleWarning( 'foo', 1, [] ) ],
+			42
+		);
+		$newStatus = RuleCheckerStatus::fromArray( $status->toArray() );
+		$this->assertSame( $status->getResult(), $newStatus->getResult() );
+		$this->assertSame( $status->getWarmCache(), $newStatus->getWarmCache() );
+		if ( $exception !== null ) {
+			$this->assertInstanceOf( get_class( $exception ), $newStatus->getException() );
+		} else {
+			$this->assertNull( $newStatus->getException() );
+		}
+		$this->assertContainsOnlyInstancesOf( UserVisibleWarning::class, $newStatus->getWarnings() );
+		$this->assertSame( $status->getCondsUsed(), $newStatus->getCondsUsed() );
+	}
+}

tests/phpunit/unit/RunnerDataTest.php

@@ -4,7 +4,7 @@ namespace MediaWiki\Extension\AbuseFilter\Tests\Unit;
 
 use LogicException;
 use MediaWiki\Extension\AbuseFilter\Parser\Exception\UserVisibleWarning;
-use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
+use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus;
 use MediaWiki\Extension\AbuseFilter\RunnerData;
 use MediaWikiUnitTestCase;
 
@@ -39,12 +39,12 @@ class RunnerDataTest extends MediaWikiUnitTestCase {
 		$runnerData = new RunnerData();
 		$runnerData->record(
 			1, false,
-			new ParserStatus( true, false, null, [], 7 ),
+			new RuleCheckerStatus( true, false, null, [], 7 ),
 			12.3
 		);
 		$runnerData->record(
 			1, true,
-			new ParserStatus( false, false, null, [], 5 ),
+			new RuleCheckerStatus( false, false, null, [], 5 ),
 			23.4
 		);
 
@@ -74,13 +74,13 @@ class RunnerDataTest extends MediaWikiUnitTestCase {
 		$runnerData = new RunnerData();
 		$runnerData->record(
 			1, false,
-			new ParserStatus( true, false, null, [], 7 ),
+			new RuleCheckerStatus( true, false, null, [], 7 ),
 			12.3
 		);
 		$this->expectException( LogicException::class );
 		$runnerData->record(
 			1, false,
-			new ParserStatus( false, false, null, [], 5 ),
+			new RuleCheckerStatus( false, false, null, [], 5 ),
 			23.4
 		);
 	}
@@ -94,7 +94,7 @@ class RunnerDataTest extends MediaWikiUnitTestCase {
 		$runnerData = new RunnerData();
 		$runnerData->record(
 			1, false,
-			new ParserStatus(
+			new RuleCheckerStatus(
 				true,
 				false,
 				null,
@@ -105,7 +105,7 @@ class RunnerDataTest extends MediaWikiUnitTestCase {
 		);
 		$runnerData->record(
 			1, true,
-			new ParserStatus( false, false, null, [], 5 ),
+			new RuleCheckerStatus( false, false, null, [], 5 ),
 			23.4
 		);
 		$newData = RunnerData::fromArray( $runnerData->toArray() );
@@ -123,22 +123,22 @@ class RunnerDataTest extends MediaWikiUnitTestCase {
 		$runnerData = new RunnerData();
 		$runnerData->record(
 			1, false,
-			new ParserStatus( true, false, null, [], 7 ),
+			new RuleCheckerStatus( true, false, null, [], 7 ),
 			12.3
 		);
 		$runnerData->record(
 			1, true,
-			new ParserStatus( false, false, null, [], 5 ),
+			new RuleCheckerStatus( false, false, null, [], 5 ),
 			23.4
 		);
 		$runnerData->record(
 			3, false,
-			new ParserStatus( false, false, null, [], 7 ),
+			new RuleCheckerStatus( false, false, null, [], 7 ),
 			12.3
 		);
 		$runnerData->record(
 			3, true,
-			new ParserStatus( true, false, null, [], 5 ),
+			new RuleCheckerStatus( true, false, null, [], 5 ),
 			23.4
 		);