Move parser tests to /unit

IMHO these can be considered unit tests; they were already fast, but now
they're executed in an instant.
This requires several changes: 1 - delay retrieving messages in
AFPUserVisibleException, to avoid having to deal with i18n whenever we
want to test exceptions; 2 - Use some DI for Parser and Tokenizer.
Equivset-dependend tests are also moved to a new class, thus helping to
fix the AF part of T189560.

Change-Id: If4585bf9bb696857005cf40a0d6985c36ac7e7a8

extension.json

@@ -171,7 +171,7 @@
 		"ApiAbuseLogPrivateDetails": "includes/api/ApiAbuseLogPrivateDetails.php",
 		"NormalizeThrottleParameters": "maintenance/normalizeThrottleParameters.php",
 		"AbuseFilterConsequencesTest": "tests/phpunit/AbuseFilterConsequencesTest.php",
-		"AbuseFilterParserTestCase": "tests/phpunit/AbuseFilterParserTestCase.php",
+		"AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php",
 		"FixOldLogEntries": "maintenance/fixOldLogEntries.php"
 	},
 	"ResourceModules": {
@@ -293,7 +293,7 @@
 		},
 		"AbuseFilterParserClass": {
 			"value": "AbuseFilterParser",
-			"description": "Class of the parser to use"
+			"description": "Class of the parser to use. The only possible values are 'AbuseFilterParser' and 'AbuseFilterCachingParser' (experimental). The code should only use the wrapper AbuseFilter::getDefaultParser."
 		},
 		"AbuseFilterEmergencyDisableThreshold": {
 			"value": {

includes/AbuseFilter.php

@@ -425,12 +425,7 @@ class AbuseFilter {
 	 *  and character position of the syntax error
 	 */
 	public static function checkSyntax( $filter ) {
-		global $wgAbuseFilterParserClass;
-
-		/** @var $parser AbuseFilterParser */
-		$parser = new $wgAbuseFilterParserClass;
-
-		return $parser->checkSyntax( $filter );
+		return self::getDefaultParser()->checkSyntax( $filter );
 	}
 
 	/**
@@ -438,8 +433,6 @@ class AbuseFilter {
 	 * @return string
 	 */
 	public static function evaluateExpression( $expr ) {
-		global $wgAbuseFilterParserClass;
-
 		if ( self::checkSyntax( $expr ) !== true ) {
 			return 'BADSYNTAX';
 		}
@@ -447,8 +440,7 @@ class AbuseFilter {
 		// Static vars are the only ones available
 		$vars = self::generateStaticVars();
 		$vars->setVar( 'timestamp', wfTimestamp( TS_UNIX ) );
-		/** @var $parser AbuseFilterParser */
-		$parser = new $wgAbuseFilterParserClass( $vars );
+		$parser = self::getDefaultParser( $vars );
 
 		return $parser->evaluateExpression( $expr );
 	}
@@ -469,9 +461,16 @@ class AbuseFilter {
 		} catch ( Exception $excep ) {
 			$result = false;
 
+			if ( $excep instanceof AFPUserVisibleException ) {
+				$msg = $excep->getMessageForLogs();
+				$excep->setLocalizedMessage();
+			} else {
+				$msg = $excep->getMessage();
+			}
+
 			$logger = LoggerFactory::getInstance( 'AbuseFilter' );
 			$extraInfo = $filter !== null ? " for filter $filter" : '';
-			$logger->warning( "AbuseFilter parser error$extraInfo: " . $excep->getMessage() );
+			$logger->warning( "AbuseFilter parser error$extraInfo: $msg" );
 
 			if ( !$ignoreError ) {
 				throw $excep;
@@ -497,10 +496,7 @@ class AbuseFilter {
 		$group = 'default',
 		$mode = 'execute'
 	) {
-		global $wgAbuseFilterParserClass;
-
-		/** @var $parser AbuseFilterParser */
-		$parser = new $wgAbuseFilterParserClass( $vars );
+		$parser = self::getDefaultParser( $vars );
 		$user = RequestContext::getMain()->getUser();
 
 		$runner = new AbuseFilterRunner( $user, $title, $vars, $group );
@@ -2237,4 +2233,31 @@ class AbuseFilter {
 	public static function canViewPrivate( User $user ) {
 		return $user->isAllowedAny( 'abusefilter-modify', 'abusefilter-view-private' );
 	}
+
+	/**
+	 * Get a parser instance using default options. This should mostly be intended as a wrapper
+	 * around $wgAbuseFilterParserClass and for choosing the right type of cache. It also has the
+	 * benefit of typehinting the return value, thus making IDEs and static analysis tools happier.
+	 *
+	 * @param AbuseFilterVariableHolder|null $vars
+	 * @return AbuseFilterParser
+	 * @throws InvalidArgumentException if $wgAbuseFilterParserClass is not valid
+	 */
+	public static function getDefaultParser(
+		AbuseFilterVariableHolder $vars = null
+	) : AbuseFilterParser {
+		global $wgAbuseFilterParserClass;
+
+		$allowedValues = [ AbuseFilterParser::class, AbuseFilterCachingParser::class ];
+		if ( !in_array( $wgAbuseFilterParserClass, $allowedValues ) ) {
+			throw new InvalidArgumentException(
+				"Invalid value $wgAbuseFilterParserClass for \$wgAbuseFilterParserClass."
+			);
+		}
+
+		$contLang = MediaWikiServices::getInstance()->getContentLanguage();
+		$cache = ObjectCache::getLocalServerInstance( 'hash' );
+		$logger = LoggerFactory::getInstance( 'AbuseFilter' );
+		return new $wgAbuseFilterParserClass( $contLang, $cache, $logger, $vars );
+	}
 }

includes/AbuseFilterRunner.php

@@ -93,11 +93,11 @@ class AbuseFilterRunner {
 	}
 
 	/**
+	 * Shortcut method, so that it can be overridden in mocks.
 	 * @return AbuseFilterParser
 	 */
 	protected function getParser() : AbuseFilterParser {
-		global $wgAbuseFilterParserClass;
-		return new $wgAbuseFilterParserClass( $this->vars );
+		return AbuseFilter::getDefaultParser( $this->vars );
 	}
 
 	/**

includes/Views/AbuseFilterViewTestBatch.php

@@ -215,9 +215,7 @@ class AbuseFilterViewTestBatch extends AbuseFilterView {
 				continue;
 			}
 
-			$parserClass = $this->getConfig()->get( 'AbuseFilterParserClass' );
-			/** @var AbuseFilterParser $parser */
-			$parser = new $parserClass( $vars );
+			$parser = AbuseFilter::getDefaultParser( $vars );
 			$parser->toggleConditionLimit( false );
 			$result = AbuseFilter::checkConditions( $this->mFilter, $parser );
 

includes/api/ApiAbuseFilterCheckMatch.php

@@ -54,9 +54,7 @@ class ApiAbuseFilterCheckMatch extends ApiBase {
 			$this->dieWithError( 'apierror-abusefilter-badsyntax', 'badsyntax' );
 		}
 
-		$parserClass = $this->getConfig()->get( 'AbuseFilterParserClass' );
-		/** @var AbuseFilterParser $parser */
-		$parser = new $parserClass( $vars );
+		$parser = AbuseFilter::getDefaultParser( $vars );
 		$result = [
 			ApiResult::META_BC_BOOLS => [ 'result' ],
 			'result' => AbuseFilter::checkConditions( $params['filter'], $parser ),

includes/parser/AFPTreeParser.php

@@ -7,7 +7,7 @@
  * @file
  */
 
-use MediaWiki\Logger\LoggerFactory;
+use Psr\Log\LoggerInterface;
 
 /**
  * A parser that transforms the text of the filter into a parse tree.
@@ -38,9 +38,22 @@ class AFPTreeParser {
 	private $variablesNames;
 
 	/**
-	 * Create a new instance
+	 * @var BagOStuff Used to cache tokens
 	 */
-	public function __construct() {
+	protected $cache;
+
+	/**
+	 * @var LoggerInterface Used for debugging
+	 */
+	protected $logger;
+
+	/**
+	 * @param BagOStuff $cache
+	 * @param LoggerInterface $logger Used for debugging
+	 */
+	public function __construct( BagOStuff $cache, LoggerInterface $logger ) {
+		$this->cache = $cache;
+		$this->logger = $logger;
 		$this->resetState();
 	}
 
@@ -107,7 +120,8 @@ class AFPTreeParser {
 	 * @return AFPSyntaxTree
 	 */
 	public function parse( $code ) : AFPSyntaxTree {
-		$this->mTokens = AbuseFilterTokenizer::getTokens( $code );
+		$tokenizer = new AbuseFilterTokenizer( $this->cache );
+		$this->mTokens = $tokenizer->getTokens( $code );
 		$this->mPos = 0;
 
 		return $this->buildSyntaxTree();
@@ -703,7 +717,6 @@ class AFPTreeParser {
 	 * should be avoided when merging the parsers.
 	 */
 	protected function checkArgCount( $args, $func ) {
-		$logger = LoggerFactory::getInstance( 'AbuseFilter' );
 		if ( !array_key_exists( $func, AbuseFilterParser::$funcArgCount ) ) {
 			throw new InvalidArgumentException( "$func is not a valid function." );
 		}
@@ -715,7 +728,7 @@ class AFPTreeParser {
 				[ $func, $min, count( $args ) ]
 			);
 		} elseif ( count( $args ) > $max ) {
-			$logger->warning(
+			$this->logger->warning(
 				"Too many params to $func for filter: " . ( $this->mFilter ?? 'unavailable' )
 			);
 			/*

includes/parser/AFPUserVisibleException.php

@@ -20,9 +20,23 @@ class AFPUserVisibleException extends AFPException {
 		$this->mPosition = $position;
 		$this->mParams = $params;
 
-		// Exception message text for logs should be in English.
-		$msg = $this->getMessageObj()->inLanguage( 'en' )->useDatabase( false )->text();
-		parent::__construct( $msg );
+		parent::__construct( $exception_id );
+	}
+
+	/**
+	 * 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
+	 *
+	 * @return string
+	 */
+	public function getMessageForLogs() {
+		return $this->getMessageObj()->inLanguage( 'en' )->useDatabase( false )->text();
 	}
 
 	/**

includes/parser/AbuseFilterCachingParser.php

@@ -67,20 +67,15 @@ class AbuseFilterCachingParser extends AbuseFilterParser {
 	 * @return AFPSyntaxTree
 	 */
 	private function getTree( $code ) : AFPSyntaxTree {
-		static $cache = null;
-		if ( !$cache ) {
-			$cache = ObjectCache::getLocalServerInstance( 'hash' );
-		}
-
-		return $cache->getWithSetCallback(
-			$cache->makeGlobalKey(
+		return $this->cache->getWithSetCallback(
+			$this->cache->makeGlobalKey(
 				__CLASS__,
 				self::getCacheVersion(),
 				hash( 'sha256', $code )
 			),
-			$cache::TTL_DAY,
+			BagOStuff::TTL_DAY,
 			function () use ( $code ) {
-				$parser = new AFPTreeParser();
+				$parser = new AFPTreeParser( $this->cache, $this->logger );
 				$parser->setFilter( $this->mFilter );
 				return $parser->parse( $code );
 			}

includes/parser/AbuseFilterParser.php

@@ -1,8 +1,7 @@
 <?php
 
+use Psr\Log\LoggerInterface;
 use Wikimedia\Equivset\Equivset;
-use MediaWiki\Logger\LoggerFactory;
-use MediaWiki\MediaWikiServices;
 
 class AbuseFilterParser {
 	/**
@@ -45,6 +44,19 @@ class AbuseFilterParser {
 	 */
 	protected $mFilter;
 
+	/**
+	 * @var BagOStuff Used to cache the AST (in CachingParser) and the tokens
+	 */
+	protected $cache;
+	/**
+	 * @var LoggerInterface Used for debugging
+	 */
+	protected $logger;
+	/**
+	 * @var Language Content language, used for language-dependent functions
+	 */
+	protected $contLang;
+
 	public static $mFunctions = [
 		'lcase' => 'funcLc',
 		'ucase' => 'funcUc',
@@ -144,10 +156,21 @@ class AbuseFilterParser {
 	/**
 	 * Create a new instance
 	 *
+	 * @param Language $contLang Content language, used for language-dependent function
+	 * @param BagOStuff $cache Used to cache the AST (in CachingParser) and the tokens
+	 * @param LoggerInterface $logger Used for debugging
 	 * @param AbuseFilterVariableHolder|null $vars
 	 */
-	public function __construct( AbuseFilterVariableHolder $vars = null ) {
+	public function __construct(
+		Language $contLang,
+		BagOStuff $cache,
+		LoggerInterface $logger,
+		AbuseFilterVariableHolder $vars = null
+	) {
 		$this->resetState();
+		$this->contLang = $contLang;
+		$this->cache = $cache;
+		$this->logger = $logger;
 		if ( $vars ) {
 			$this->mVariables = $vars;
 		}
@@ -160,6 +183,20 @@ class AbuseFilterParser {
 		$this->mFilter = $filter;
 	}
 
+	/**
+	 * @param BagOStuff $cache
+	 */
+	public function setCache( BagOStuff $cache ) {
+		$this->cache = $cache;
+	}
+
+	/**
+	 * @param LoggerInterface $logger
+	 */
+	public function setLogger( LoggerInterface $logger ) {
+		$this->logger = $logger;
+	}
+
 	/**
 	 * @return int
 	 */
@@ -355,7 +392,8 @@ class AbuseFilterParser {
 	 */
 	public function intEval( $code ) {
 		// Reset all class members to their default value
-		$this->mTokens = AbuseFilterTokenizer::getTokens( $code );
+		$tokenizer = new AbuseFilterTokenizer( $this->cache );
+		$this->mTokens = $tokenizer->getTokens( $code );
 		$this->mPos = 0;
 		$this->mShortCircuit = false;
 
@@ -1101,8 +1139,7 @@ class AbuseFilterParser {
 		$deprecatedVars = AbuseFilter::getDeprecatedVariables();
 
 		if ( array_key_exists( $var, $deprecatedVars ) ) {
-			$logger = LoggerFactory::getInstance( 'AbuseFilter' );
-			$logger->debug( "AbuseFilter: deprecated variable $var used." );
+			$this->logger->debug( "AbuseFilter: deprecated variable $var used." );
 			$var = $deprecatedVars[$var];
 		}
 		if ( !$this->varExists( $var ) ) {
@@ -1159,7 +1196,6 @@ class AbuseFilterParser {
 	 * @throws AFPUserVisibleException
 	 */
 	protected function checkArgCount( $args, $func ) {
-		$logger = LoggerFactory::getInstance( 'AbuseFilter' );
 		if ( !array_key_exists( $func, self::$funcArgCount ) ) {
 			throw new InvalidArgumentException( "$func is not a valid function." );
 		}
@@ -1171,7 +1207,7 @@ class AbuseFilterParser {
 				[ $func, $min, count( $args ) ]
 			);
 		} elseif ( count( $args ) > $max ) {
-			$logger->warning(
+			$this->logger->warning(
 				"Too many params to $func for filter: " . ( $this->mFilter ?? 'unavailable' )
 			);
 			/*
@@ -1238,10 +1274,9 @@ class AbuseFilterParser {
 	 * @return AFPData
 	 */
 	protected function funcLc( $args ) {
-		$contLang = MediaWikiServices::getInstance()->getContentLanguage();
 		$s = $args[0]->toString();
 
-		return new AFPData( AFPData::DSTRING, $contLang->lc( $s ) );
+		return new AFPData( AFPData::DSTRING, $this->contLang->lc( $s ) );
 	}
 
 	/**
@@ -1249,10 +1284,9 @@ class AbuseFilterParser {
 	 * @return AFPData
 	 */
 	protected function funcUc( $args ) {
-		$contLang = MediaWikiServices::getInstance()->getContentLanguage();
 		$s = $args[0]->toString();
 
-		return new AFPData( AFPData::DSTRING, $contLang->uc( $s ) );
+		return new AFPData( AFPData::DSTRING, $this->contLang->uc( $s ) );
 	}
 
 	/**
@@ -1862,8 +1896,7 @@ class AbuseFilterParser {
 	 * @param string $fname Method where the empty operand is found
 	 */
 	protected function logEmptyOperand( $type, $fname ) {
-		$logger = LoggerFactory::getInstance( 'AbuseFilter' );
-		$logger->info(
+		$this->logger->info(
 			"Empty operand of type {type} at method {fname}. Filter: {filter}",
 			[
 				'type' => $type,

includes/parser/AbuseFilterTokenizer.php

@@ -66,16 +66,27 @@ class AbuseFilterTokenizer {
 		'rlike', 'irlike', 'regex', 'if', 'then', 'else', 'end',
 	];
 
+	/**
+	 * @var BagOStuff
+	 */
+	private $cache;
+
+	/**
+	 * @param BagOStuff $cache
+	 */
+	public function __construct( BagOStuff $cache ) {
+		$this->cache = $cache;
+	}
+
 	/**
 	 * Get a cache key used to store the tokenized code
 	 *
-	 * @param BagOStuff $cache
 	 * @param string $code Not yet tokenized
 	 * @return string
 	 * @internal
 	 */
-	public static function getCacheKey( BagOStuff $cache, $code ) {
-		return $cache->makeGlobalKey( __CLASS__, self::CACHE_VERSION, crc32( $code ) );
+	public function getCacheKey( $code ) {
+		return $this->cache->makeGlobalKey( __CLASS__, self::CACHE_VERSION, crc32( $code ) );
 	}
 
 	/**
@@ -84,12 +95,10 @@ class AbuseFilterTokenizer {
 	 * @param string $code
 	 * @return array[]
 	 */
-	public static function getTokens( $code ) {
-		$cache = ObjectCache::getLocalServerInstance( 'hash' );
-
-		$tokens = $cache->getWithSetCallback(
-			self::getCacheKey( $cache, $code ),
-			$cache::TTL_DAY,
+	public function getTokens( $code ) {
+		$tokens = $this->cache->getWithSetCallback(
+			$this->getCacheKey( $code ),
+			BagOStuff::TTL_DAY,
 			function () use ( $code ) {
 				return self::tokenize( $code );
 			}
@@ -122,7 +131,7 @@ class AbuseFilterTokenizer {
 	 * @throws AFPException
 	 * @throws AFPUserVisibleException
 	 */
-	protected static function nextToken( $code, &$offset ) {
+	private static function nextToken( $code, &$offset ) {
 		$matches = [];
 		$start = $offset;
 
@@ -219,7 +228,7 @@ class AbuseFilterTokenizer {
 	 * @throws AFPException
 	 * @throws AFPUserVisibleException
 	 */
-	protected static function readStringLiteral( $code, &$offset, $start ) {
+	private static function readStringLiteral( $code, &$offset, $start ) {
 		$type = $code[$offset];
 		$offset++;
 		$length = strlen( $code );

tests/phpunit/AbuseFilterParserEquivsetTest.php

@@ -0,0 +1,120 @@
+<?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
+ *
+ * @license GPL-2.0-or-later
+ */
+
+/**
+ * Tests that require Equivset, separated from the parser unit tests.
+ *
+ * @covers AbuseFilterCachingParser
+ * @covers AFPTreeParser
+ * @covers AFPTreeNode
+ * @covers AFPParserState
+ * @covers AbuseFilterParser
+ * @covers AbuseFilterTokenizer
+ * @covers AFPToken
+ * @covers AFPData
+ */
+class AbuseFilterParserEquivsetTest extends MediaWikiIntegrationTestCase {
+	/**
+	 * @see AbuseFilterParserTestCase::getParsers() - we cannot reuse that due to inheritance
+	 * @return AbuseFilterParser[]
+	 */
+	protected function getParsers() {
+		static $parsers = null;
+		if ( !$parsers ) {
+			// We're not interested in caching or logging; tests should call respectively setCache
+			// and setLogger if they want to test any of those.
+			$contLang = new LanguageEn();
+			$cache = new EmptyBagOStuff();
+			$logger = new \Psr\Log\NullLogger();
+
+			$parser = new AbuseFilterParser( $contLang, $cache, $logger );
+			$parser->toggleConditionLimit( false );
+			$cachingParser = new AbuseFilterCachingParser( $contLang, $cache, $logger );
+			$cachingParser->toggleConditionLimit( false );
+			$parsers = [ $parser, $cachingParser ];
+		} else {
+			// Reset so that already executed tests don't influence new ones
+			$parsers[0]->resetState();
+			$parsers[0]->clearFuncCache();
+			$parsers[1]->resetState();
+			$parsers[1]->clearFuncCache();
+		}
+		return $parsers;
+	}
+
+	/**
+	 * @param string $rule The rule to parse
+	 * @dataProvider provideGenericTests
+	 */
+	public function testGeneric( $rule ) {
+		if ( !class_exists( 'Wikimedia\Equivset\Equivset' ) ) {
+			$this->markTestSkipped( 'Equivset is not installed' );
+		}
+		foreach ( $this->getParsers() as $parser ) {
+			$this->assertTrue( $parser->parse( $rule ), 'Parser used: ' . get_class( $parser ) );
+		}
+	}
+
+	/**
+	 * @return Generator|array
+	 */
+	public function provideGenericTests() {
+		$testPath = __DIR__ . "/../parserTestsEquivset";
+		$testFiles = glob( $testPath . "/*.t" );
+
+		foreach ( $testFiles as $testFile ) {
+			$testName = basename( substr( $testFile, 0, -2 ) );
+			$rule = trim( file_get_contents( $testFile ) );
+
+			yield $testName => [ $rule ];
+		}
+	}
+
+	/**
+	 * @param string $func
+	 * @see AbuseFilterParserTest::testVariadicFuncsArbitraryArgsAllowed()
+	 * @dataProvider variadicFuncs
+	 */
+	public function testVariadicFuncsArbitraryArgsAllowed( $func ) {
+		$argsList = str_repeat( ', "arg"', 50 );
+		$code = "$func( 'arg' $argsList )";
+		foreach ( self::getParsers() as $parser ) {
+			$pname = get_class( $parser );
+			try {
+				$parser->parse( $code );
+				$this->assertTrue( true );
+			} catch ( AFPException $e ) {
+				$this->fail( "Got exception with parser $pname.\n$e" );
+			}
+		}
+	}
+
+	/**
+	 * @return array
+	 */
+	public function variadicFuncs() {
+		return [
+			[ 'ccnorm_contains_any' ],
+			[ 'ccnorm_contains_all' ],
+		];
+	}
+}

tests/phpunit/unit/AbuseFilterParserTest.php

@@ -21,6 +21,8 @@
  * @author Marius Hoch < hoo@online.de >
  */
 
+use Psr\Log\NullLogger;
+
 /**
  * @group Test
  * @group AbuseFilter
@@ -45,7 +47,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider readTests
 	 */
 	public function testParser( $rule ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$this->assertTrue( $parser->parse( $rule ), 'Parser used: ' . get_class( $parser ) );
 		}
 	}
@@ -54,7 +56,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @return Generator|array
 	 */
 	public function readTests() {
-		$testPath = __DIR__ . "/../parserTests";
+		$testPath = __DIR__ . "/../../parserTests";
 		$testFiles = glob( $testPath . "/*.t" );
 
 		foreach ( $testFiles as $testFile ) {
@@ -73,7 +75,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider provideExpressions
 	 */
 	public function testEvaluateExpression( $expr, $expected ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$actual = $parser->evaluateExpression( $expr );
 			$this->assertEquals( $expected, $actual );
 		}
@@ -102,7 +104,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider provideEmptySyntax
 	 */
 	public function testEmptySyntax( $code ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$this->assertFalse( $parser->parse( $code ) );
 		}
 	}
@@ -153,7 +155,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider condCountCases
 	 */
 	public function testCondCount( $rule, $expected ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$parserClass = get_class( $parser );
 			$countBefore = $parser->getCondCount();
 			$parser->parse( $rule );
@@ -190,7 +192,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	public function testArrayShortcircuit() {
 		$code = 'a := [false, false]; b := [false, false]; c := 42; d := [0,1];' .
 			'a[0] != false & b[1] != false & (b[5**2/(5*(4+1))] !== a[43-c] | a[d[0]] === b[d[c-41]])';
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$this->assertFalse( $parser->parse( $code ), 'Parser: ' . get_class( $parser ) );
 		}
 	}
@@ -662,8 +664,6 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 			[ 'contains_any' ],
 			[ 'contains_all' ],
 			[ 'equals_to_any' ],
-			[ 'ccnorm_contains_any' ],
-			[ 'ccnorm_contains_all' ],
 		];
 	}
 
@@ -678,7 +678,12 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	public function testCheckArgCountInConditional( $funcCode, $exceptionCode ) {
 		$code = "if ( 1==1 ) then ( 1 ) else ( $funcCode ) end;";
 		// AbuseFilterParser skips the parentheses altogether, so this is not supposed to work
-		$parser = new AbuseFilterCachingParser();
+		$parser = new AbuseFilterCachingParser(
+			new LanguageEn(),
+			new EmptyBagOStuff(),
+			new NullLogger()
+		);
+		$parser->toggleConditionLimit( false );
 		try {
 			$parser->parse( $code );
 			$this->fail( 'No exception was thrown.' );
@@ -709,16 +714,15 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider provideDeprecatedVars
 	 */
 	public function testDeprecatedVars( $old, $new ) {
-		$loggerMock = new TestLogger();
-		$loggerMock->setCollect( true );
-		$this->setLogger( 'AbuseFilter', $loggerMock );
-
-		$vars = new AbuseFilterVariableHolder();
 		// Set it under the new name, and check that the old name points to it
-		$vars->setVar( $new, 'Some value' );
+		$vars = AbuseFilterVariableHolder::newFromArray( [ $new => 'value' ] );
 
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$pname = get_class( $parser );
+			$loggerMock = new TestLogger();
+			$loggerMock->setCollect( true );
+			$parser->setLogger( $loggerMock );
+
 			$parser->setVariables( $vars );
 			$actual = $parser->parse( "$old === $new" );
 
@@ -759,7 +763,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider provideConsecutiveComparisons
 	 */
 	public function testDisallowConsecutiveComparisons( $code, $valid ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$pname = get_class( $parser );
 			$actuallyValid = true;
 			try {
@@ -815,7 +819,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider provideVarDeclarationInSkippedBlock
 	 */
 	public function testVarDeclarationInSkippedBlock( $code ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$pname = get_class( $parser );
 			try {
 				$this->assertFalse(
@@ -862,7 +866,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	 * @dataProvider provideDUNDEFINED
 	 */
 	public function testDUNDEFINED( $code ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$pname = get_class( $parser );
 			try {
 				$this->assertFalse(
@@ -920,6 +924,9 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 	public function testEmptyOperands( $code, $operandType ) {
 		/** @var PHPUnit\Framework\MockObject\MockObject|AbuseFilterParser $mock */
 		$mock = $this->getMockBuilder( AbuseFilterParser::class )
+			->setConstructorArgs(
+				[ new LanguageEn(), new EmptyBagOStuff(), new NullLogger() ]
+			)
 			->setMethods( [ 'logEmptyOperand' ] )
 			->getMock();
 
@@ -927,6 +934,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
 			->method( 'logEmptyOperand' )
 			->with( $operandType );
 
+		$mock->toggleConditionLimit( false );
 		$mock->parse( $code );
 	}
 

tests/phpunit/unit/AbuseFilterParserTestCase.php

@@ -23,17 +23,24 @@
 /**
  * Helper for parser-related tests
  */
-abstract class AbuseFilterParserTestCase extends MediaWikiTestCase {
+abstract class AbuseFilterParserTestCase extends MediaWikiUnitTestCase {
 	/**
 	 * @return AbuseFilterParser[]
 	 */
-	public static function getParsers() {
+	protected function getParsers() {
 		static $parsers = null;
 		if ( !$parsers ) {
-			$parsers = [
-				new AbuseFilterParser(),
-				new AbuseFilterCachingParser()
-			];
+			// We're not interested in caching or logging; tests should call respectively setCache
+			// and setLogger if they want to test any of those.
+			$contLang = new LanguageEn();
+			$cache = new EmptyBagOStuff();
+			$logger = new \Psr\Log\NullLogger();
+
+			$parser = new AbuseFilterParser( $contLang, $cache, $logger );
+			$parser->toggleConditionLimit( false );
+			$cachingParser = new AbuseFilterCachingParser( $contLang, $cache, $logger );
+			$cachingParser->toggleConditionLimit( false );
+			$parsers = [ $parser, $cachingParser ];
 		} else {
 			// Reset so that already executed tests don't influence new ones
 			$parsers[0]->resetState();
@@ -54,7 +61,7 @@ abstract class AbuseFilterParserTestCase extends MediaWikiTestCase {
 	 *  just used for debugging purposes.
 	 */
 	protected function exceptionTest( $excep, $expr, $caller ) {
-		foreach ( self::getParsers() as $parser ) {
+		foreach ( $this->getParsers() as $parser ) {
 			$pname = get_class( $parser );
 			try {
 				$parser->parse( $expr );

tests/phpunit/unit/AbuseFilterTokenizerTest.php

@@ -111,20 +111,19 @@ class AbuseFilterTokenizerTest extends AbuseFilterParserTestCase {
 	}
 
 	/**
-	 * Test that tokenized code is saved in cache
+	 * Test that tokenized code is saved in cache.
 	 *
 	 * @param string $code To be tokenized
 	 * @dataProvider provideCode
+	 * @covers AbuseFilterTokenizer::getTokens
 	 */
 	public function testCaching( $code ) {
 		$cache = new HashBagOStuff();
-		$this->setService( 'LocalServerObjectCache', $cache );
+		$tokenizer = new AbuseFilterTokenizer( $cache );
 
-		$key = AbuseFilterTokenizer::getCacheKey( $cache, $code );
+		$key = $tokenizer->getCacheKey( $code );
 
-		// Other tests may have already cached the same code.
-		$cache->delete( $key );
-		AbuseFilterTokenizer::getTokens( $code );
+		$tokenizer->getTokens( $code );
 		$this->assertNotFalse( $cache->get( $key ) );
 	}