Remove unused code and fix missing/broken documentation

General code cleanup as reported by the PHPStorm static code
analysis. I hope it's not a problem that I made a lot of very
different (but all very tiny) changes in a single patch. If you
want to merge this but you think it's better to split it into
several patches first, please tell me.

Change-Id: I2e2c4bb47f8d20e038d28e236e2ff813b30504af

Hooks.php

@@ -99,6 +99,7 @@ class EchoHooks {
 
 	/**
 	 * Handler for EchoGetBundleRule hook, which defines the bundle rule for each notification
+	 *
 	 * @param $event EchoEvent
 	 * @param $bundleString string Determines how the notification should be bundled, for example,
 	 * talk page notification is bundled based on namespace and title, the bundle string would be
@@ -106,6 +107,7 @@ class EchoHooks {
 	 * a key to identify bundle-able event.  For web bundling, we bundle further based on user's
 	 * visit to the overlay, we would generate a display hash based on the hash of $bundleString
 	 *
+	 * @return bool
 	 */
 	public static function onEchoGetBundleRules( $event, &$bundleString ) {
 		switch ( $event->getType() ) {
@@ -231,7 +233,6 @@ class EchoHooks {
 	 * @return bool
 	 */
 	public static function getNotificationTypes( $user, $event, &$notifyTypes ) {
-		$type = $event->getType();
 		if ( !$user->getOption( 'enotifminoredits' ) ) {
 			$extra = $event->getExtra();
 			if ( !empty( $extra['revid'] ) ) {
@@ -248,14 +249,17 @@ class EchoHooks {
 	/**
 	 * Handler for GetPreferences hook.
 	 * @see http://www.mediawiki.org/wiki/Manual:Hooks/GetPreferences
+	 *
 	 * @param $user User to get preferences for
 	 * @param &$preferences Preferences array
+	 *
+	 * @throws MWException
 	 * @return bool true in all cases
 	 */
 	public static function getPreferences( $user, &$preferences ) {
 		global $wgEchoDefaultNotificationTypes, $wgAuth, $wgEchoEnableEmailBatch,
 			$wgEchoNotifiers, $wgEchoNotificationCategories, $wgEchoNotifications,
-			$wgEchoHelpPage, $wgEchoNewMsgAlert, $wgAllowHTMLEmail;
+			$wgEchoNewMsgAlert, $wgAllowHTMLEmail;
 
 		// Don't show echo preference page if echo is disabled for this user
 		if ( self::isEchoDisabled( $user ) ) {
@@ -538,9 +542,12 @@ class EchoHooks {
 	/**
 	 * Handler for UserRights hook.
 	 * @see http://www.mediawiki.org/wiki/Manual:Hooks/UserRights
+	 *
 	 * @param $user User User object that was changed
 	 * @param $add array Array of strings corresponding to groups added
 	 * @param $remove array Array of strings corresponding to groups removed
+	 *
+	 * @return bool
 	 */
 	public static function onUserRights( &$user, $add, $remove ) {
 		global $wgUser;
@@ -597,7 +604,7 @@ class EchoHooks {
 		$max = 10;
 		// Only create notifications for links to content namespace pages
 		// @Todo - use one big insert instead of individual insert inside foreach loop
-		foreach ( $insertions as $key => $page ) {
+		foreach ( $insertions as $page ) {
 			if ( MWNamespace::isContent( $page['pl_namespace'] ) ) {
 				$title = Title::makeTitle( $page['pl_namespace'], $page['pl_title'] );
 				if ( $title->isRedirect() ) {
@@ -753,7 +760,6 @@ class EchoHooks {
 
 		// Provide info for the Overlay
 
-		$timestamp = new MWTimestamp( wfTimestampNow() );
 		if ( ! $user->isAnon() ) {
 			$vars['wgEchoOverlayConfiguration'] = array(
 				'notification-count' => MWEchoNotifUser::newFromUser( $user )->getFormattedNotificationCount(),

Notifier.php

@@ -10,8 +10,6 @@ class EchoNotifier {
 	 * @param $event EchoEvent to notify about.
 	 */
 	public static function notifyWithNotification( $user, $event ) {
-		global $wgEchoConfig, $wgEchoNotifications;
-
 		// Only create the notification if the user wants to recieve that type
 		// of notification and they are eligible to recieve it. See bug 47664.
 		$userWebNotifications = EchoNotificationController::getUserEnabledEvents( $user, 'web' );

api/ApiEchoNotifications.php

@@ -60,9 +60,10 @@ class ApiEchoNotifications extends ApiQueryBase {
 	 * Get a list of notifications based on the passed parameters
 	 *
 	 * @param $user User the user to get notifications for
-	 * @param $format string/bool false to not format any notifications, string to a specific output format
+	 * @param $format string|bool false to not format any notifications, string to a specific output format
 	 * @param $limit int The maximum number of notifications to return
 	 * @param $continue string Used for offset
+	 *
 	 * @return array
 	 */
 	public static function getNotifications( $user, $format = false, $limit = 20, $continue = null ) {
@@ -152,9 +153,12 @@ class ApiEchoNotifications extends ApiQueryBase {
 
 	/**
 	 * Internal helper function for converting UTC timezone to a user's timezone
+	 *
 	 * @param $user User
 	 * @param $ts string
-	 * @param $format output format
+	 * @param $format int output format
+	 *
+	 * @return string
 	 */
 	private static function getUserLocalTime( $user, $ts, $format = TS_MW ) {
 		$timestamp = new MWTimestamp( $ts );

formatters/BasicFormatter.php

@@ -524,8 +524,10 @@ class EchoBasicFormatter extends EchoNotificationFormatter {
 	/**
 	 * Plain text email in some mail client is misinterpreting the ending
 	 * punctuation, this function would encode the last character
+	 *
 	 * @param $url string
-	 * @param string
+	 *
+	 * @return string
 	 */
 	public function sanitizeEmailLink( $url ) {
 		// $url should contain all ascii characters now, it's safe to use substr()
@@ -580,8 +582,6 @@ class EchoBasicFormatter extends EchoNotificationFormatter {
 	 * @throws MWException
 	 */
 	protected function generateBundleData( $event, $user, $type ) {
-		global $wgEchoMaxNotificationCount;
-
 		$data = $this->getRawBundleData( $event, $user, $type );
 
 		// Default the last raw data to false, which means there is no
@@ -872,7 +872,10 @@ class EchoBasicFormatter extends EchoNotificationFormatter {
 
 	/**
 	 * Getter method
+	 *
 	 * @param $key string
+	 *
+	 * @throws MWException
 	 * @return mixed
 	 */
 	public function getValue( $key ) {

formatters/EditUserTalkFormatter.php

@@ -11,8 +11,6 @@ class EchoEditUserTalkFormatter extends EchoEditFormatter {
 	protected function applyChangeBeforeFormatting( EchoEvent $event, User $user, $type ) {
 		parent::applyChangeBeforeFormatting( $event, $user, $type );
 
-		$extra = $event->getExtra();
-
 		// Replace default generic notification message with 'Someone left a message
 		// on your talk page in "xxxx"' if 
 		// * the message is not bundled and

includes/BatchRowUpdate.php

@@ -108,6 +108,8 @@ class EchoBatchRowUpdate {
 	 * string status updates
 	 *
 	 * @param callable $output A callback taking a single string parameter to output
+	 *
+	 * @throws MWException
 	 */
 	public function setOutput( $output ) {
 		if ( !is_callable( $output ) ) {
@@ -174,7 +176,7 @@ class EchoBatchRowWriter {
 	/**
 	 * @param DatabaseBase $db          The database to write to
 	 * @param string       $table       The name of the table to update
-	 * @param string       $clusterName A cluster name valid for use with LBFactory
+	 * @param string|bool  $clusterName A cluster name valid for use with LBFactory
 	 */
 	public function __construct( DatabaseBase $db, $table, $clusterName = false ) {
 		$this->db = $db;
@@ -190,7 +192,7 @@ class EchoBatchRowWriter {
 	public function write( array $updates ) {
 		$this->db->begin();
 
-		foreach ( $updates as $id => $update ) {
+		foreach ( $updates as $update ) {
 			//echo "Updating: ";var_dump( $update['primaryKey'] );
 			//echo "With values: ";var_dump( $update['changes'] );
 			$this->db->update(
@@ -263,6 +265,8 @@ class EchoBatchRowIterator implements Iterator {
 	 * @param string       $table      The name of the table to read from
 	 * @param string|array $primaryKey The name or names of the primary key columns
 	 * @param integer      $batchSize  The number of rows to fetch per iteration
+	 *
+	 * @throws MWException
 	 */
 	public function __construct( DatabaseBase $db, $table, $primaryKey, $batchSize ) {
 		if ( $batchSize < 1 ) {

includes/ContainmentSet.php

@@ -64,6 +64,8 @@ class EchoContainmentSet {
 	 * @param $title          string    The title of the page containing the list.
 	 * @param $cache          BagOStuff An object to cache the page with or null for no cache.
 	 * @param $cacheKeyPrefix string    A prefix to be combined with the pages latest revision id and used as a cache key.
+	 *
+	 * @throws MWException
 	 */
 	public function addOnWiki( $namespace, $title, BagOStuff $cache = null, $cacheKeyPrefix = '' ) {
 		$list = new EchoOnWikiList( $namespace, $title );

includes/DbEmailBatch.php

@@ -138,8 +138,11 @@ class MWDbEchoEmailBatch extends MWEchoEmailBatch {
 
 	/**
 	 * Get a list of users to be notified for the batch
+	 *
 	 * @param $startUserId int
 	 * @param $batchSize int
+	 *
+	 * @return ResultWrapper|bool
 	 */
 	public static function actuallyGetUsersToNotify( $startUserId, $batchSize ) {
 		$dbr = MWEchoDbFactory::getDB( DB_SLAVE );

includes/DiffParser.php

@@ -57,7 +57,7 @@ class EchoDiffParser {
 	protected $rightPos;
 
 	/**
-	 * @var array $changeSet Set of add, subtract, or change operations within the diff
+	 * @var array[] $changeSet Set of add, subtract, or change operations within the diff
 	 */
 	protected $changeSet;
 
@@ -66,7 +66,7 @@ class EchoDiffParser {
 	 *
 	 * @param string $leftText The left, or old, revision of the text
 	 * @param string $rightText The right, or new, revision of the text
-	 * @return array Array of arrays containing changes to individual groups of lines within the text
+	 * @return array[] Array of arrays containing changes to individual groups of lines within the text
 	 * Each change consists of:
 	 * An 'action', one of:
 	 * - add
@@ -115,6 +115,8 @@ class EchoDiffParser {
 	 * @param string $diff The unified diff output
 	 * @param string $left The left side of the diff used for sanity checks
 	 * @param string $right The right side of the diff used for sanity checks
+	 *
+	 * @return array[]
 	 */
 	protected function parse( $diff, $left, $right ) {
 		$this->left = explode( "\n", $left );
@@ -148,6 +150,8 @@ class EchoDiffParser {
 	 *
 	 * @param string $line The next line of the unified diff
 	 * @param EchoDiffGroup $change Changes the the immediately previous lines
+	 *
+	 * @throws MWException
 	 * @return EchoDiffGroup Changes to this line and any changed lines immediately previous
 	 */
 	protected function parseLine( $line, EchoDiffGroup $change = null ) {
@@ -169,7 +173,7 @@ class EchoDiffParser {
 				$change = null;
 			}
 			// @@ -start,numLines +start,numLines @@
-			list( $at, $left, $right, $at ) = explode( ' ', $line );
+			list( , $left, $right ) = explode( ' ', $line );
 			list( $this->leftPos ) = explode( ',', substr( $left, 1 ) );
 			list( $this->rightPos ) = explode( ',', substr( $right, 1 ) );
 
@@ -263,7 +267,7 @@ class EchoDiffGroup {
 	}
 
 	/**
-	 * @return array set of changes
+	 * @return array[] set of changes
 	 * Each change consists of:
 	 * An 'action', one of:
 	 *   - add
@@ -309,4 +313,3 @@ class EchoDiffGroup {
 		return $changeSet;
 	}
 }
-

includes/DiscussionParser.php

@@ -558,7 +558,7 @@ abstract class EchoDiscussionParser {
 			return true;
 		}
 
-		list( $signaturePos, $foundUser ) = $userData;
+		list( , $foundUser ) = $userData;
 
 		return User::getCanonicalName( $foundUser, false ) === User::getCanonicalName( $user, false );
 	}
@@ -618,8 +618,6 @@ abstract class EchoDiscussionParser {
 	 */
 	static function extractSignatures( $text ) {
 		$lines = explode( "\n", $text );
-		$timestampRegex = self::getTimestampRegex();
-		$endOfLine = self::getLineEndingRegex();
 
 		$output = array();
 

includes/EchoBackend.php

@@ -27,8 +27,11 @@ abstract class MWEchoBackend {
 
 	/**
 	 * Extract the offset used for notification list
+	 *
 	 * @param $continue String Used for offset
-	 * @param @return array
+	 *
+	 * @throws MWException
+	 * @return int[]
 	 */
 	protected function extractQueryOffset( $continue ) {
 		$offset = array (

includes/EmailBatch.php

@@ -228,10 +228,13 @@ abstract class MWEchoEmailBatch {
 
 	/**
 	 * Insert notification event into email queue
+	 *
 	 * @param $userId int
 	 * @param $eventId int
 	 * @param $priority int
 	 * @param $hash string
+	 *
+	 * @throws MWException
 	 */
 	public static function addToQueue( $userId, $eventId, $priority, $hash ) {
 		$batchClassName = self::getEmailBatchClass();
@@ -245,8 +248,12 @@ abstract class MWEchoEmailBatch {
 
 	/**
 	 * Get a list of users to be notified for the batch
+	 *
 	 * @param $startUserId int
 	 * @param $batchSize int
+	 *
+	 * @throws MWException
+	 * @return ResultWrapper|bool
 	 */
 	public static function getUsersToNotify( $startUserId, $batchSize ) {
 		$batchClassName = self::getEmailBatchClass();

includes/EmailBundler.php

@@ -30,7 +30,7 @@ abstract class MWEchoEmailBundler {
 	protected $timestamp;
 
 	/**
-	 * @var Event
+	 * @var EchoEvent
 	 */
 	protected $baseEvent;
 
@@ -92,6 +92,10 @@ abstract class MWEchoEmailBundler {
 	 * Check if a new notification should be added to the batch queue
 	 * true  - added to the queue for bundling email
 	 * false - not added, the client should send single email
+	 *
+	 * @param int $eventId
+	 * @param int $eventPriority
+	 *
 	 * @return bool
 	 */
 	public function addToEmailBatch( $eventId, $eventPriority ) {

includes/EmailFormatter.php

@@ -616,7 +616,7 @@ interface EchoEmailDecorator {
 
 	/**
 	 * Decorate a revision snippet
-	 * @param $snippet the raw revision snippet
+	 * @param string $snippet the raw revision snippet
 	 * @return string
 	 */
 	public function decorateRevisionSnippet( $snippet );

includes/schemaUpdate.php

@@ -8,7 +8,7 @@
 class EchoSuppressionRowUpdateGenerator implements EchoRowUpdateGenerator
 {
 	/**
-	 * @param callable Hack to allow replacing Title::newFromText in tests
+	 * @var callable Hack to allow replacing Title::newFromText in tests
 	 */
 	protected $newTitleFromText = array( 'Title', 'newFromText' );
 
@@ -79,6 +79,8 @@ class EchoSuppressionRowUpdateGenerator implements EchoRowUpdateGenerator
 	 * rather than the namespace+title combo.
 	 *
 	 * @param $row stdClass A row from the database
+	 * @param $update array
+	 *
 	 * @return array All updates required for this row
 	 */
 	protected function updatePageLinkedExtraData( $row, array $update ) {

model/Event.php

@@ -194,7 +194,7 @@ class EchoEvent {
 	/**
 	 * Loads data from the provided $row into this object.
 	 *
-	 * @param $row Database row object from echo_event
+	 * @param $row stdClass row object from echo_event
 	 */
 	public function loadFromRow( $row ) {
 		$this->id = $row->event_id;
@@ -243,7 +243,7 @@ class EchoEvent {
 	/**
 	 * Creates an EchoEvent from a row object
 	 *
-	 * @param $row Database row object from echo_event
+	 * @param $row stdClass row object from echo_event
 	 * @return EchoEvent object.
 	 */
 	public static function newFromRow( $row ) {
@@ -295,7 +295,7 @@ class EchoEvent {
 	/**
 	 * Check if the event is dismissable for the given distribution type
 	 *
-	 * @param $distribution notification distribution web/email
+	 * @param string $distribution notification distribution web/email
 	 * @return bool
 	 */
 	public function isDismissable( $distribution ) {

modules/base/ext.echo.base.js

@@ -7,9 +7,9 @@
 
 		/**
 		 * Set up event logging for individual notification
-		 * @param {JQuery} notification JQuery representing a single notification
+		 * @param {jQuery} notification JQuery representing a single notification
 		 * @param {string} context 'flyout'/'archive'
-		 * @param {boolean} mobile True if interaction was on a mobile device
+		 * @param {boolean} [mobile] True if interaction was on a mobile device
 		 */
 		setupNotificationLogging: function ( notification, context, mobile ) {
 			var eventId = +notification.attr( 'data-notification-event' ),
@@ -29,11 +29,11 @@
 
 		/**
 		 * Log all Echo interaction related events
-		 * @param {string} clickAction The interaction
-		 * @param {string} context 'flyout'/'archive' or undefined for the badge
-		 * @param {int} eventId Notification event id
-		 * @param {string} eventType notification type
-		 * @param {boolean} mobile True if interaction was on a mobile device
+		 * @param {string} action The interaction
+		 * @param {string} [context] 'flyout'/'archive' or undefined for the badge
+		 * @param {int} [eventId] Notification event id
+		 * @param {string} [eventType] notification type
+		 * @param {boolean} [mobile] True if interaction was on a mobile device
 		 */
 		logInteraction: function ( action, context, eventId, eventType, mobile ) {
 			// Check if Schema:EchoInteraction is enabled

modules/overlay/ext.echo.overlay.css

@@ -9,7 +9,7 @@
 	padding: 0;
 	color: #6D6D6D;
 	z-index: 100;
-	box-shadow: 0px 3px 8px rgba(50, 50, 50, 0.35);
+	box-shadow: 0 3px 8px rgba(50, 50, 50, 0.35);
 }
 .mw-echo-overlay-pokey {
 	/* @embed */
@@ -82,7 +82,7 @@
 }
 #mw-echo-overlay-moreinfo-link {
 	display: inline-block;
-	margin: 0px 2px -1px 4px;
+	margin: 0 2px -1px 4px;
 	height: 13px;
 	width: 13px;
 	/* @embed */
@@ -98,7 +98,7 @@
 }
 
 #mw-echo-overlay-footer {
-	padding: 0px;
+	padding: 0;
 	border-top: 1px solid #DDDDDD;
 	display: table;
 	width: 100%;

modules/special/ext.echo.special.css

@@ -35,7 +35,7 @@
 }
 #firstHeading #mw-echo-moreinfo-link {
 	display: inline-block;
-	margin: 0px 3px;
+	margin: 0 3px;
 	/* @embed */
 	background-image: url(Help.png);
 	background-repeat: no-repeat;

tests/ContainmentSetTest.php

@@ -3,7 +3,7 @@
 class ContainmentSetTest extends MediaWikiTestCase {
 
 	public function testGenericContains() {
-		$list = new EchoContainmentSet;
+		$list = new EchoContainmentSet();
 
 		$list->addArray( array( 'foo', 'bar' ) );
 		$this->assertTrue( $list->contains( 'foo' ) );

tests/DiscussionParserTest.php

@@ -415,7 +415,6 @@ TEXT
 
 	static public function provider_detectSectionTitleAndText() {
 		$name = 'TestUser';
-		$mention = 'Someone';
 		$comment = self::signedMessage( $name );
 
 		return array(

tests/NotificationFormatterTest.php

@@ -207,8 +207,6 @@ class EchoNotificationFormatterTest extends MediaWikiTestCase {
 	}
 
 	protected function format( EchoEvent $event, $format, $user = false, $type = 'web' ) {
-		global $wgEchoNotifications;
-
 		if ( $user === false ) {
 			$user = User::newFromName('Notification-formatter-test');
 		}