Improve type safety of filter ids

Also fix a bug in FilterProfiler. It would attempt to reset
stats for global filters but we do not record them (yet?).

Change-Id: I0228d8c85dab146deb877dfce506f1e8e7711a9f

includes/AbuseFilter.php

@@ -138,6 +138,7 @@ class AbuseFilter {
 	 *
 	 * @param string|int $filter
 	 * @return array
+	 * @phan-return array{0:int,1:bool}
 	 * @throws InvalidArgumentException
 	 */
 	public static function splitGlobalName( $filter ) {

includes/FilterProfiler.php

@@ -91,9 +91,9 @@ class FilterProfiler {
 	}
 
 	/**
-	 * @param int|string $filter
+	 * @param int $filter
 	 */
-	public function resetFilterProfile( $filter ) : void {
+	public function resetFilterProfile( int $filter ) : void {
 		$profileKey = $this->filterProfileKey( $filter );
 		$this->objectStash->delete( $profileKey );
 	}
@@ -101,11 +101,11 @@ class FilterProfiler {
 	/**
 	 * Retrieve per-filter statistics.
 	 *
-	 * @param string|int $filter
+	 * @param int $filter
 	 * @return array See self::NULL_FILTER_PROFILE for the returned array structure
 	 * @phan-return array{count:int,matches:int,total-time:float,total-cond:int}
 	 */
-	public function getFilterProfile( $filter ) : array {
+	public function getFilterProfile( int $filter ) : array {
 		return $this->objectStash->get( $this->filterProfileKey( $filter ) )
 			?: self::NULL_FILTER_PROFILE;
 	}
@@ -164,7 +164,7 @@ class FilterProfiler {
 	 * disabled filters too, as their profiling data will be reset upon re-enabling anyway.
 	 *
 	 * @param string $group
-	 * @param array $allFilters
+	 * @param string[] $allFilters
 	 */
 	public function checkResetProfiling( string $group, array $allFilters ) : void {
 		$profile = $this->getGroupProfile( $group );
@@ -174,7 +174,10 @@ class FilterProfiler {
 			$profileKey = $this->filterProfileGroupKey( $group );
 			$this->objectStash->delete( $profileKey );
 			foreach ( $allFilters as $filter ) {
-				$this->resetFilterProfile( $filter );
+				list( $filterID, $global ) = AbuseFilter::splitGlobalName( $filter );
+				if ( $global === false ) {
+					$this->resetFilterProfile( $filterID );
+				}
 			}
 		}
 	}
@@ -304,10 +307,10 @@ class FilterProfiler {
 	/**
 	 * Get the memcache access key used to store per-filter profiling data.
 	 *
-	 * @param string|int $filter
+	 * @param int $filter
 	 * @return string
 	 */
-	private function filterProfileKey( $filter ) : string {
+	private function filterProfileKey( int $filter ) : string {
 		return $this->objectStash->makeKey( 'abusefilter-profile', 'v3', $filter );
 	}
 

includes/View/AbuseFilterViewEdit.php

@@ -101,11 +101,11 @@ class AbuseFilterViewEdit extends AbuseFilterView {
 		$out->setPageTitle( $this->msg( 'abusefilter-edit' ) );
 		$out->addHelpLink( 'Extension:AbuseFilter/Rules format' );
 
-		$filter = $this->filter;
-		if ( !is_numeric( $filter ) && $filter !== null ) {
+		if ( !is_numeric( $this->filter ) && $this->filter !== null ) {
 			$this->showUnrecoverableError( 'abusefilter-edit-badfilter' );
 			return;
 		}
+		$filter = $this->filter ? (int)$this->filter : null;
 		$history_id = $this->historyID;
 		if ( $this->historyID ) {
 			$dbr = wfGetDB( DB_REPLICA );

includes/Watcher/EmergencyWatcher.php

@@ -61,9 +61,9 @@ class EmergencyWatcher implements Watcher {
 	 * Determine which filters must be throttled, i.e. their potentially dangerous
 	 *  actions must be disabled.
 	 *
-	 * @param string[] $filters The filters to check
+	 * @param int[] $filters The filters to check
 	 * @param string $group Group the filters belong to
-	 * @return string[] Array of filters to be throttled
+	 * @return int[] Array of filters to be throttled
 	 */
 	public function getFiltersToThrottle( array $filters, string $group ) : array {
 		$groupProfile = $this->profiler->getGroupProfile( $group );
@@ -83,7 +83,7 @@ class EmergencyWatcher implements Watcher {
 
 		$throttleFilters = [];
 		foreach ( $filters as $filter ) {
-			$filterObj = $this->filterLookup->getFilter( (int)$filter, false );
+			$filterObj = $this->filterLookup->getFilter( $filter, false );
 			if ( $filterObj->isThrottled() ) {
 				continue;
 			}

includes/special/SpecialAbuseLog.php

@@ -1239,7 +1239,7 @@ class SpecialAbuseLog extends AbuseFilterSpecialPage {
 					$filterLink = $linkMsg->escaped();
 				}
 			} else {
-				$title = SpecialPage::getTitleFor( 'AbuseFilter', $filterID );
+				$title = SpecialPage::getTitleFor( 'AbuseFilter', (string)$filterID );
 				$linkText = $this->msg( 'abusefilter-log-detailedentry-local' )
 					->numParams( $filterID )->text();
 				$filterLink = $linkRenderer->makeKnownLink( $title, $linkText );

tests/phpunit/unit/EmergencyWatcherTest.php

@@ -173,11 +173,11 @@ class EmergencyWatcherTest extends MediaWikiUnitTestCase {
 			$this->getOptions()
 		);
 		$toThrottle = $watcher->getFiltersToThrottle(
-			[ '1' ],
+			[ 1 ],
 			$group
 		);
 		$this->assertSame(
-			$willThrottle ? [ '1' ] : [],
+			$willThrottle ? [ 1 ] : [],
 			$toThrottle
 		);
 	}