Merge "Fix and disable some phpcs exclusions"

.phpcs.xml

@@ -6,11 +6,8 @@
 		<exclude name="MediaWiki.Files.ClassMatchesFilename.NotMatch" />
 		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationProtected" />
 		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic" />
-		<exclude name="MediaWiki.Commenting.FunctionComment.WrongStyle" />
 		<exclude name="MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName" />
 		<exclude name="MediaWiki.PHPUnit.AssertEmpty.AssertEmptyUsed" />
-		<exclude name="MediaWiki.Usage.ExtendClassUsage.FunctionConfigUsage" />
-		<exclude name="MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment" />
 	</rule>
 	<arg name="encoding" value="UTF-8" />
 	<arg name="extensions" value="php" />

includes/Api/ApiEchoNotifications.php

@@ -302,7 +302,8 @@ class ApiEchoNotifications extends ApiQueryBase {
 			}
 		} elseif ( in_array( 'read', $filter ) ) {
 			$notifs = $notifMapper->fetchReadByUser( $user, $limit + 1, $continue, $eventTypes, $titles );
-		} else { // = if ( in_array( '!read', $filter ) ) {
+		} else {
+			// = if ( in_array( '!read', $filter ) ) {
 			$notifs = $notifMapper->fetchUnreadByUser( $user, $limit + 1, $continue, $eventTypes, $titles );
 		}
 
@@ -592,8 +593,10 @@ class ApiEchoNotifications extends ApiQueryBase {
 				ParamValidator::PARAM_TYPE => [
 					'model',
 					'special',
-					'flyout', /* @deprecated */
-					'html', /* @deprecated */
+					// @deprecated
+					'flyout',
+					// @deprecated
+					'html',
 				],
 				ApiBase::PARAM_HELP_MSG_PER_VALUE => [],
 			],

includes/Controller/NotificationController.php

@@ -394,7 +394,8 @@ class NotificationController {
 
 		$userId = $user->getId();
 		if ( $userId === 0 ) {
-			return false; // anonymous user
+			// anonymous user
+			return false;
 		}
 
 		// Ensure we have a list of whitelists

includes/DiscussionParser.php

@@ -858,7 +858,8 @@ abstract class EchoDiscussionParser {
 		$sectionNum = count( $matches[0] );
 		$sections = [];
 
-		if ( $matches[0][0][1] > 1 ) { // is there text before the first headline?
+		// is there text before the first headline?
+		if ( $matches[0][0][1] > 1 ) {
 			$sections[] = [
 				'header' => false,
 				'content' => substr( $text, 0, $matches[0][0][1] - 1 )

includes/EchoDiffParser.php

@@ -185,7 +185,8 @@ class EchoDiffParser {
 		}
 
 		switch ( $op ) {
-			case '@': // metadata
+			case '@':
+				// metadata
 				if ( $change !== null ) {
 					$this->changeSet = array_merge( $this->changeSet, $change->getChangeSet() );
 					$change = null;
@@ -197,12 +198,13 @@ class EchoDiffParser {
 				$this->leftPos = (int)$this->leftPos;
 				$this->rightPos = (int)$this->rightPos;
 
-				// -1 because diff is 1 indexed and we are 0 indexed
+				// -1 because diff is 1 indexed, and we are 0 indexed
 				$this->leftPos--;
 				$this->rightPos--;
 				break;
 
-			case ' ': // No changes
+			case ' ':
+				// No changes
 				if ( $change !== null ) {
 					$this->changeSet = array_merge( $this->changeSet, $change->getChangeSet() );
 					$change = null;
@@ -211,7 +213,8 @@ class EchoDiffParser {
 				$this->rightPos++;
 				break;
 
-			case '-': // subtract
+			case '-':
+				// subtract
 				if ( $this->left[$this->leftPos] !== $line ) {
 					throw new MWException( 'Positional error: left' );
 				}
@@ -223,7 +226,8 @@ class EchoDiffParser {
 				$this->leftPos++;
 				break;
 
-			case '+': // add
+			case '+':
+				// add
 				if ( $this->right[$this->rightPos] !== $line ) {
 					throw new MWException( 'Positional error: right' );
 				}

includes/EmailFrequency.php

@@ -1,8 +1,23 @@
 <?php
 
 class EchoEmailFrequency {
-	public const NEVER = -1; // Never send email notifications
-	public const IMMEDIATELY = 0; // Send email notifications immediately as they come in
-	public const DAILY_DIGEST = 1; // Send daily email digests
-	public const WEEKLY_DIGEST = 7; // Send weekly email digests
+	/**
+	 * Never send email notifications
+	 */
+	public const NEVER = -1;
+
+	/**
+	 * Send email notifications immediately as they come in
+	 */
+	public const IMMEDIATELY = 0;
+
+	/**
+	 * Send daily email digests
+	 */
+	public const DAILY_DIGEST = 1;
+
+	/**
+	 * Send weekly email digests
+	 */
+	public const WEEKLY_DIGEST = 7;
 }

includes/Formatters/SpecialNotificationsFormatter.php

@@ -45,7 +45,8 @@ class SpecialNotificationsFormatter extends EchoEventFormatter {
 		$markAsReadButton = Html::rawElement(
 			'div',
 			[ 'class' => 'mw-echo-markAsReadButton' ],
-			$markAsReadForm->prepareForm()->getHTML( /* First submission attempt */ false )
+			// First submission attempt
+			$markAsReadForm->prepareForm()->getHTML( false )
 		);
 
 		$html = Xml::tags(

includes/Hooks.php

@@ -125,9 +125,9 @@ class Hooks implements
 		global $wgAllowHTMLEmail, $wgEchoNotificationCategories, $wgEchoEnablePush;
 
 		if ( $wgAllowHTMLEmail ) {
-			$defaults['echo-email-format'] = 'html'; /*EchoHooks::EMAIL_FORMAT_HTML*/
+			$defaults['echo-email-format'] = 'html';
 		} else {
-			$defaults['echo-email-format'] = 'plain-text'; /*EchoHooks::EMAIL_FORMAT_PLAIN_TEXT*/
+			$defaults['echo-email-format'] = 'plain-text';
 		}
 
 		$presets = [
@@ -1101,8 +1101,10 @@ class Hooks implements
 
 		$hasUnseen = false;
 		if (
-			$msgCount != 0 && // no unread notifications
-			$msgNotificationTimestamp !== false && // should already always be false if count === 0
+			// no unread notifications
+			$msgCount !== 0 &&
+			// should already always be false if count === 0
+			$msgNotificationTimestamp !== false &&
 			// there are no unseen notifications
 			( $seenMsgTime === null ||
 				$seenMsgTime < $msgNotificationTimestamp->getTimestamp( TS_ISO_8601 ) )
@@ -1118,8 +1120,10 @@ class Hooks implements
 		}
 
 		if (
-			$alertCount != 0 && // no unread notifications
-			$alertNotificationTimestamp !== false && // should already always be false if count === 0
+			// no unread notifications
+			$alertCount !== 0 &&
+			// should already always be false if count === 0
+			$alertNotificationTimestamp !== false &&
 			// all notifications have already been seen
 			( $seenAlertTime === null ||
 				$seenAlertTime < $alertNotificationTimestamp->getTimestamp( TS_ISO_8601 ) )
@@ -1334,8 +1338,10 @@ class Hooks implements
 
 		if (
 			$revertedUser &&
-			$revertedUser->getId() && // No notifications for anonymous users
-			!$oldRevision->hasSameContent( $newRevision ) // No notifications for null rollbacks
+			// No notifications for anonymous users
+			$revertedUser->isRegistered() &&
+			// No notifications for null rollbacks
+			!$oldRevision->hasSameContent( $newRevision )
 		) {
 			Event::create( [
 				'type' => 'reverted',

includes/Special/SpecialDisplayNotificationsConfiguration.php

@@ -76,11 +76,11 @@ class SpecialDisplayNotificationsConfiguration extends UnlistedSpecialPage {
 	}
 
 	public function execute( $subPage ) {
-		global $wgEchoNotifiers, $wgEchoNotifications;
-
 		$this->setHeaders();
 		$this->checkPermissions();
 
+		$config = $this->getConfig();
+
 		$internalCategoryNames = $this->attributeManager->getInternalCategoryNames();
 		$this->categoryNames = [];
 
@@ -106,13 +106,13 @@ class SpecialDisplayNotificationsConfiguration extends UnlistedSpecialPage {
 		$this->flippedCategoryNames = array_flip( $this->categoryNames );
 
 		$this->notifyTypes = [];
-		foreach ( $wgEchoNotifiers as $notifyType => $notifier ) {
+		foreach ( $config->get( 'EchoNotifiers' ) as $notifyType => $notifier ) {
 			$this->notifyTypes[$notifyType] = $this->msg( 'echo-pref-' . $notifyType )->escaped();
 		}
 
 		$this->flippedNotifyTypes = array_flip( $this->notifyTypes );
 
-		$notificationTypes = array_keys( $wgEchoNotifications );
+		$notificationTypes = array_keys( $config->get( 'EchoNotifications' ) );
 		$this->notificationTypeNames = array_combine(
 			array_map( 'htmlspecialchars', $notificationTypes ),
 			$notificationTypes

includes/Special/SpecialNotifications.php

@@ -140,7 +140,8 @@ class SpecialNotifications extends SpecialPage {
 				$markAllAsReadLabelIcon->toString()
 			);
 
-			$formHtml = $markAllAsReadForm->prepareForm()->getHTML( /* First submission attempt */ false );
+			// First submission attempt
+			$formHtml = $markAllAsReadForm->prepareForm()->getHTML( false );
 
 			$markAllAsReadFormWrapper = new OOUI\Tag();
 			$markAllAsReadFormWrapper
@@ -183,7 +184,8 @@ class SpecialNotifications extends SpecialPage {
 					$markAsReadLabelIcon->toString()
 				);
 
-				$formHtml = $markSectionAsReadForm->prepareForm()->getHTML( /* First submission attempt */ false );
+				// First submission attempt
+				$formHtml = $markSectionAsReadForm->prepareForm()->getHTML( false );
 
 				$formWrapper = new OOUI\Tag();
 				$formWrapper

maintenance/generateSampleNotifications.php

@@ -303,16 +303,20 @@ class GenerateSampleNotifications extends Maintenance {
 			$undoContent,
 			$undoContent,
 			$previousContent,
-			true // undoIsLatest
+			// undoIsLatest
+			true
 		);
 
 		$status = $page->doUserEditContent(
 			$content,
 			$agent,
 			'undo',
-			0, // $flags
-			false, // $originalRevId
-			[], // $tags
+			// $flags
+			0,
+			// $originalRevId
+			false,
+			// $tags
+			[],
 			$undoRev->getId()
 		);
 

maintenance/removeInvalidNotification.php

@@ -88,5 +88,5 @@ class RemoveInvalidNotification extends Maintenance {
 	}
 }
 
-$maintClass = RemoveInvalidNotification::class; // Tells it to run the class
+$maintClass = RemoveInvalidNotification::class;
 require_once RUN_MAINTENANCE_IF_MAIN;

maintenance/updateEchoSchemaForSuppression.php

@@ -73,5 +73,5 @@ class UpdateEchoSchemaForSuppression extends LoggedUpdateMaintenance {
 	}
 }
 
-$maintClass = UpdateEchoSchemaForSuppression::class; // Tells it to run the class
+$maintClass = UpdateEchoSchemaForSuppression::class;
 require_once RUN_MAINTENANCE_IF_MAIN;

tests/phpunit/ContainmentSetTest.php

@@ -23,7 +23,8 @@ class ContainmentSetTest extends MediaWikiIntegrationTestCase {
 	}
 
 	public function testCachedListInnerListIsOnlyCalledOnce() {
-		$innerCache = new HashBagOStuff; // simulate caching
+		// simulate caching
+		$innerCache = new HashBagOStuff;
 		$wanCache = new WANObjectCache( [ 'cache' => $innerCache ] );
 
 		$inner = [ 'bing', 'bang' ];

tests/phpunit/DiscussionParserTest.php

@@ -12,6 +12,10 @@ use Wikimedia\TestingAccessWrapper;
  * @covers \EchoDiscussionParser
  * @group Echo
  * @group Database
+ *
+ * TODO test cases for:
+ *  - stripHeader
+ *  - stripSignature
  */
 class EchoDiscussionParserTest extends MediaWikiIntegrationTestCase {
 
@@ -201,7 +205,8 @@ class EchoDiscussionParserTest extends MediaWikiIntegrationTestCase {
 					// pages expected to exist (e.g. templates to be expanded)
 					'Template:u' => '[[User:{{{1}}}|{{<includeonly>safesubst:</includeonly>#if:{{{2|}}}|{{{2}}}|{{{1}}}}}]]<noinclude>{{documentation}}</noinclude>',
 				],
-				'title' => 'UTPage', // can't remember, not important here
+				// can't remember, not important here
+				'title' => 'UTPage',
 				'expected' => [
 					// events expected to be fired going from old revision to new
 					[
@@ -225,7 +230,8 @@ class EchoDiscussionParserTest extends MediaWikiIntegrationTestCase {
 				'username' => 'Schnark',
 				'lang' => 'de',
 				'pages' => [],
-				'title' => 'UTPage', // can't remember, not important here
+				// can't remember, not important here
+				'title' => 'UTPage',
 				'expected' => [
 					[
 						'type' => 'mention',
@@ -242,7 +248,8 @@ class EchoDiscussionParserTest extends MediaWikiIntegrationTestCase {
 				'pages' => [
 					'Predefinição:U' => '[[User:{{{1|<noinclude>Exemplo</noinclude>}}}|{{{{{|safesubst:}}}#if:{{{2|}}}|{{{2}}}|{{{1|<noinclude>Exemplo</noinclude>}}}}}]]<noinclude>{{Atalho|Predefinição:U}}{{Documentação|Predefinição:Usuário/doc}}</noinclude>',
 				],
-				'title' => 'UTPage', // can't remember, not important here
+				// can't remember, not important here
+				'title' => 'UTPage',
 				'expected' => [
 					[
 						'type' => 'mention',
@@ -1012,10 +1019,6 @@ TEXT
 		$this->setTemporaryHook( 'BeforeEchoEventInsert', $callback );
 	}
 
-	// TODO test cases for:
-	// - stripHeader
-	// - stripSignature
-
 	public function testTimestampRegex() {
 		$exemplarTimestamp = self::EXEMPLAR_TIMESTAMP;
 		$timestampRegex = EchoDiscussionParser::getTimestampRegex();
@@ -1152,7 +1155,7 @@ TEXT
 				],
 			],
 			// when adding additional tests, make sure to add the non-anon users
-			// to EchoDiscussionParserTest::$testusers - the DiscussionParser
+			// to EchoDiscussionParserTest::$testUsers - the DiscussionParser
 			// needs the users to exist, because it'll generate a comparison
 			// signature, which is different when the user is considered anon
 		];
@@ -1502,7 +1505,8 @@ TEXT
 	}
 
 	public static function provider_detectSectionTitleAndText() {
-		$name = 'Werdna'; // See EchoDiscussionParserTest::$testusers
+		// See EchoDiscussionParserTest::$testUsers
+		$name = 'Werdna';
 		$comment = self::signedMessage( $name );
 
 		return [

tests/phpunit/Mapper/NotificationMapperTest.php

@@ -75,13 +75,6 @@ class NotificationMapperTest extends MediaWikiIntegrationTestCase {
 			]
 		];
 
-		$tpDbResult = [
-			(object)[
-				'etp_page' => 7, // pageid
-				'etp_event' => 1, // eventid
-			],
-		];
-
 		$notifMapper = new NotificationMapper( $this->mockMWEchoDbFactory( [ 'select' => $notifDbResult ] ) );
 		$res = $notifMapper->fetchByUser( $this->mockUser(), 10, '', [] );
 		$this->assertEmpty( $res );

tests/phpunit/UserLocatorTest.php

@@ -137,7 +137,7 @@ class EchoUserLocatorTest extends MediaWikiIntegrationTestCase {
 				// expected result user id's
 				[],
 				// event agent
-				User::newFromName( '4.5.6.7', /* $validate = */ false ),
+				User::newFromName( '4.5.6.7', false ),
 			],
 
 			[
@@ -235,7 +235,7 @@ class EchoUserLocatorTest extends MediaWikiIntegrationTestCase {
 		$event->method( 'getExtra' )
 			->willReturn( $extra );
 		$event->method( 'getExtraParam' )
-			->will( $this->returnValueMap( self::arrayToValueMap( $extra ) ) );
+			->willReturnMap( self::arrayToValueMap( $extra ) );
 
 		$users = EchoUserLocator::locateFromEventExtra( $event, $keys );
 		$this->assertEquals( $expect, array_keys( $users ), $message );

tests/phpunit/maintenance/SuppressionMaintenanceTest.php

@@ -27,13 +27,15 @@ class SuppressionMaintenanceTest extends MediaWikiIntegrationTestCase {
 
 			[
 				'Page title and namespace for non-existant page must move into event_extra',
-				[ // expected update
+				[
+					// expected update
 					'event_extra' => serialize( [
 						'page_title' => 'Yabba Dabba Do',
 						'page_namespace' => NS_MAIN
 					] ),
 				],
-				[ // input row
+				[
+					// input row
 					'event_page_title' => 'Yabba Dabba Do',
 					'event_page_namespace' => NS_MAIN,
 				] + $input,
@@ -41,10 +43,12 @@ class SuppressionMaintenanceTest extends MediaWikiIntegrationTestCase {
 
 			[
 				'Page title and namespace for existing page must be result in update to event_page_id',
-				[ // expected update
+				[
+					// expected update
 					'event_page_id' => 42,
 				],
-				[ // input row
+				[
+					// input row
 					'event_page_title' => 'Mount Rushmore',
 					'event_page_namespace' => NS_MAIN,
 				] + $input,
@@ -52,15 +56,17 @@ class SuppressionMaintenanceTest extends MediaWikiIntegrationTestCase {
 			],
 
 			[
-				'When updating non-existant page must keep old extra data',
-				[ // expected update
+				'When updating non-existent page must keep old extra data',
+				[
+					// expected update
 					'event_extra' => serialize( [
 						'foo' => 'bar',
 						'page_title' => 'Yabba Dabba Do',
 						'page_namespace' => NS_MAIN
 					] ),
 				],
-				[ // input row
+				[
+					// input row
 					'event_page_title' => 'Yabba Dabba Do',
 					'event_page_namespace' => NS_MAIN,
 					'event_extra' => serialize( [ 'foo' => 'bar' ] ),
@@ -69,10 +75,12 @@ class SuppressionMaintenanceTest extends MediaWikiIntegrationTestCase {
 
 			[
 				'Must update link-from-title/namespace to link-from-page-id for page-linked events',
-				[ // expected update
+				[
+					// expected update
 					'event_extra' => serialize( [ 'link-from-page-id' => 99 ] ),
 				],
-				[ // input row
+				[
+					// input row
 					'event_type' => 'page-linked',
 					'event_extra' => serialize( [
 						'link-from-title' => 'Horse',
@@ -84,11 +92,13 @@ class SuppressionMaintenanceTest extends MediaWikiIntegrationTestCase {
 
 			[
 				'Must perform both generic update and page-linked update at same time',
-				[ // expected update
+				[
+					// expected update
 					'event_extra' => serialize( [ 'link-from-page-id' => 8675309 ] ),
 					'event_page_id' => 8675309,
 				],
-				[ // input row
+				[
+					// input row
 					'event_type' => 'page-linked',
 					'event_extra' => serialize( [
 						'link-from-title' => 'Jenny',
@@ -110,7 +120,7 @@ class SuppressionMaintenanceTest extends MediaWikiIntegrationTestCase {
 			$title = $test->createMock( Title::class );
 			$title->expects( $test->any() )
 				->method( 'getArticleId' )
-				->will( $test->returnValue( $id ) );
+				->willReturn( $id );
 
 			$titles = [ $providedNamespace => [ $providedText => $title ] ];