Merge "Fix and enable FunctionComment.MissingReturn PHPCS rule"

wikimedia · Oct 22, 2023 · 0bf1d2d · 0bf1d2d
2 parents 1e9e65f + d461c15
commit 0bf1d2d
Show file tree

Hide file tree

Showing 25 changed files with 47 additions and 123 deletions.
diff --git a/.phpcs.xml b/.phpcs.xml
@@ -5,7 +5,6 @@
 		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationProtected" />
 		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic" />
 		<exclude name="MediaWiki.Commenting.FunctionComment.MissingParamTag" />
-		<exclude name="MediaWiki.Commenting.FunctionComment.MissingReturn" />
 		<!-- Note: NotPunctuationReturnType can be enabled when the sniff accepts array shapes -->
 		<exclude name="MediaWiki.Commenting.FunctionComment.NotPunctuationReturnType" />
 		<exclude name="MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationPrivate" />

diff --git a/client/includes/MaintenanceShellStart.php b/client/includes/MaintenanceShellStart.php
@@ -4,6 +4,7 @@
 use Wikibase\Client\WikibaseClient;
 
 // phpcs:disable Squiz.Functions.GlobalFunction.Found
+// phpcs:disable MediaWiki.Commenting.FunctionComment.MissingReturn
 // phpcs:disable MediaWiki.NamingConventions.PrefixedGlobalFunctions.wfPrefix
 
 if ( !function_exists( 'mws' ) ) {

diff --git a/client/tests/phpunit/unit/includes/Hooks/SkinAfterBottomScriptsHandlerTest.php b/client/tests/phpunit/unit/includes/Hooks/SkinAfterBottomScriptsHandlerTest.php
@@ -134,6 +134,7 @@ private function mockTitle( $baseURL, $titleText ) {
 
 	/**
 	 * @param string|null $timestamp
+	 * @return RevisionLookup
 	 */
 	private function createMockRevisionLookup( $timestamp ) {
 		$revisionRecord = $this->createMock( RevisionRecord::class );

diff --git a/lib/packages/wikibase/data-model/tests/unit/NewItem.php b/lib/packages/wikibase/data-model/tests/unit/NewItem.php
@@ -31,7 +31,7 @@ class NewItem {
 	private $descriptions = [];
 
 	/**
-	 * @var array[] Indexed by language on the first level
+	 * @var array<string,string[]> Indexed by language code on the first level
 	 */
 	private $aliases = [];
 
@@ -86,15 +86,15 @@ public function build() {
 	}
 
 	/**
-	 * @see andId
+	 * @param ItemId|string $itemId
+	 * @return self
 	 */
 	public static function withId( $itemId ) {
 		return ( new self() )->andId( $itemId );
 	}
 
 	/**
 	 * @param ItemId|string $itemId
-	 *
 	 * @return self
 	 */
 	public function andId( $itemId ) {
@@ -107,7 +107,9 @@ public function andId( $itemId ) {
 	}
 
 	/**
-	 * @see andLabel
+	 * @param string $languageCode
+	 * @param string $label
+	 * @return self
 	 */
 	public static function withLabel( $languageCode, $label ) {
 		return ( new self() )->andLabel( $languageCode, $label );
@@ -116,7 +118,6 @@ public static function withLabel( $languageCode, $label ) {
 	/**
 	 * @param string $languageCode
 	 * @param string $label
-	 *
 	 * @return self
 	 */
 	public function andLabel( $languageCode, $label ) {
@@ -126,7 +127,9 @@ public function andLabel( $languageCode, $label ) {
 	}
 
 	/**
-	 * @see andDescription
+	 * @param string $languageCode
+	 * @param string $description
+	 * @return self
 	 */
 	public static function withDescription( $languageCode, $description ) {
 		return ( new self() )->andDescription( $languageCode, $description );
@@ -135,7 +138,6 @@ public static function withDescription( $languageCode, $description ) {
 	/**
 	 * @param string $languageCode
 	 * @param string $description
-	 *
 	 * @return self
 	 */
 	public function andDescription( $languageCode, $description ) {
@@ -145,7 +147,9 @@ public function andDescription( $languageCode, $description ) {
 	}
 
 	/**
-	 * @see andAliases
+	 * @param string $languageCode
+	 * @param string[]|string $aliases
+	 * @return self
 	 */
 	public static function withAliases( $languageCode, $aliases ) {
 		return ( new self() )->andAliases( $languageCode, $aliases );
@@ -154,7 +158,6 @@ public static function withAliases( $languageCode, $aliases ) {
 	/**
 	 * @param string $languageCode
 	 * @param string[]|string $aliases
-	 *
 	 * @return self
 	 */
 	public function andAliases( $languageCode, $aliases ) {
@@ -164,7 +167,11 @@ public function andAliases( $languageCode, $aliases ) {
 	}
 
 	/**
-	 * @see andSiteLink
+	 * @param string $siteId
+	 * @param string $pageName
+	 * @param ItemId[]|string[]|ItemId|string|null $badges Zero or more item ID references as either
+	 *  strings or ItemId objects. Can be an array or a single value.
+	 * @return self
 	 */
 	public static function withSiteLink( $siteId, $pageName, $badges = null ) {
 		return ( new self() )->andSiteLink( $siteId, $pageName, $badges );
@@ -175,7 +182,6 @@ public static function withSiteLink( $siteId, $pageName, $badges = null ) {
 	 * @param string $pageName
 	 * @param ItemId[]|string[]|ItemId|string|null $badges Zero or more item ID references as either
 	 *  strings or ItemId objects. Can be an array or a single value.
-	 *
 	 * @return self
 	 */
 	public function andSiteLink( $siteId, $pageName, $badges = null ) {
@@ -190,15 +196,15 @@ public function andSiteLink( $siteId, $pageName, $badges = null ) {
 	}
 
 	/**
-	 * @see andStatement
+	 * @param NewStatement|Statement|Snak $statement
+	 * @return self
 	 */
 	public static function withStatement( $statement ) {
 		return ( new self() )->andStatement( $statement );
 	}
 
 	/**
 	 * @param NewStatement|Statement|Snak $statement
-	 *
 	 * @return self
 	 */
 	public function andStatement( $statement ) {

diff --git a/lib/tests/phpunit/Formatters/ItemPropertyIdHtmlLinkFormatterTest.php b/lib/tests/phpunit/Formatters/ItemPropertyIdHtmlLinkFormatterTest.php
@@ -538,6 +538,8 @@ private function givenItemDoesNotExist( $itemId ) {
 
 	/**
 	 * @param string $itemId
+	 * phpcs:ignore MediaWiki.Commenting.FunctionComment.ObjectTypeHintReturn
+	 * @return object
 	 */
 	private function givenItemExists( $itemId ) {
 		$title = $this->createMock( Title::class );

diff --git a/repo/includes/ChangeOp/ChangeOpAliases.php b/repo/includes/ChangeOp/ChangeOpAliases.php
@@ -95,15 +95,7 @@ private function updateAliases( AliasGroupList $aliases ) {
 		$aliases->setAliasesForLanguage( $this->languageCode, $newAliases );
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof AliasesProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a AliasesProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpDescription.php b/repo/includes/ChangeOp/ChangeOpDescription.php
@@ -95,15 +95,7 @@ private function buildResult( EntityId $entityId = null, Term $oldDescription =
 		return new ChangeOpDescriptionResult( $entityId, $this->languageCode, $oldDescriptionText, $newDescriptionText, $isEntityChanged );
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof DescriptionsProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a DescriptionsProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpLabel.php b/repo/includes/ChangeOp/ChangeOpLabel.php
@@ -94,14 +94,7 @@ private function buildResult( EntityId $entityId = null, Term $oldLabel = null,
 		return new ChangeOpLabelResult( $entityId, $this->languageCode, $oldLabelText, $newLabelText, $isEntityChanged );
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof LabelsProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a LabelsProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpMainSnak.php b/repo/includes/ChangeOp/ChangeOpMainSnak.php
@@ -77,15 +77,10 @@ public function getStatementGuid() {
 	}
 
 	/**
-	 * @see ChangeOp::apply()
 	 * - a new claim with $snak as mainsnak gets added when $claimGuid is empty and $snak is set
 	 * - the claim's mainsnak gets set to $snak when $claimGuid and $snak are set
 	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
+	 * @inheritDoc
 	 */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {

diff --git a/repo/includes/ChangeOp/ChangeOpQualifier.php b/repo/includes/ChangeOp/ChangeOpQualifier.php
@@ -66,15 +66,10 @@ public function __construct( $statementGuid, Snak $snak, $snakHash, SnakValidato
 	}
 
 	/**
-	 * @see ChangeOp::apply()
 	 * - a new qualifier gets added when $snakHash is empty and $snak is set
 	 * - the qualifier gets set to $snak when $snakHash and $snak are set
 	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
+	 * @inheritDoc
 	 */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {

diff --git a/repo/includes/ChangeOp/ChangeOpQualifierRemove.php b/repo/includes/ChangeOp/ChangeOpQualifierRemove.php
@@ -49,15 +49,7 @@ public function __construct( $statementGuid, $snakHash ) {
 		$this->snakHash = $snakHash;
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a StatementListProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpReference.php b/repo/includes/ChangeOp/ChangeOpReference.php
@@ -84,15 +84,10 @@ public function __construct(
 	}
 
 	/**
-	 * @see ChangeOp::apply()
 	 * - a new reference gets added when $referenceHash is empty and $reference is set
 	 * - the reference gets set to $reference when $referenceHash and $reference are set
 	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
+	 * @inheritDoc
 	 */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {

diff --git a/repo/includes/ChangeOp/ChangeOpReferenceRemove.php b/repo/includes/ChangeOp/ChangeOpReferenceRemove.php
@@ -49,15 +49,7 @@ public function __construct( $statementGuid, $referenceHash ) {
 		$this->referenceHash = $referenceHash;
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a StatementListProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpRemoveStatement.php b/repo/includes/ChangeOp/ChangeOpRemoveStatement.php
@@ -45,15 +45,7 @@ public function __construct( $guid ) {
 		$this->guid = $guid;
 	}
 
-	/**
-	 * @see ChangeOp::apply
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a StatementListProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpStatement.php b/repo/includes/ChangeOp/ChangeOpStatement.php
@@ -86,15 +86,7 @@ public function __construct(
 		$this->index = $index;
 	}
 
-	/**
-	 * @see ChangeOp::apply
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a StatementListProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOpStatementRank.php b/repo/includes/ChangeOp/ChangeOpStatementRank.php
@@ -49,15 +49,7 @@ public function __construct( $statementGuid, $rank ) {
 		$this->rank = $rank;
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws InvalidArgumentException
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		if ( !( $entity instanceof StatementListProvider ) ) {
 			throw new InvalidArgumentException( '$entity must be a StatementListProvider' );

diff --git a/repo/includes/ChangeOp/ChangeOps.php b/repo/includes/ChangeOp/ChangeOps.php
@@ -62,15 +62,7 @@ public function getChangeOps() {
 		return $this->changeOps;
 	}
 
-	/**
-	 * @see ChangeOp::apply()
-	 * Applies all changes to the given entity
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary
-	 *
-	 * @throws ChangeOpException
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		$changeOpsResults = [];
 

diff --git a/repo/includes/ChangeOp/ChangeOpsMerge.php b/repo/includes/ChangeOp/ChangeOpsMerge.php
@@ -137,6 +137,7 @@ private function getSiteLinkChangeOpFactory() {
 
 	/**
 	 * @throws ChangeOpException
+	 * @return ChangeOpResult
 	 */
 	public function apply() {
 		// NOTE: we don't want to validate the ChangeOps individually, since they represent

diff --git a/repo/includes/ChangeOp/NullChangeOp.php b/repo/includes/ChangeOp/NullChangeOp.php
@@ -22,12 +22,7 @@ public function validate( EntityDocument $entity ) {
 		return Result::newSuccess();
 	}
 
-	/**
-	 * @see ChangeOp::apply
-	 *
-	 * @param EntityDocument $entity
-	 * @param Summary|null $summary Unused
-	 */
+	/** @inheritDoc */
 	public function apply( EntityDocument $entity, Summary $summary = null ) {
 		// no op
 

diff --git a/repo/includes/MaintenanceShellStart.php b/repo/includes/MaintenanceShellStart.php
@@ -4,6 +4,7 @@
 use Wikibase\Repo\WikibaseRepo;
 
 // phpcs:disable Squiz.Functions.GlobalFunction.Found
+// phpcs:disable MediaWiki.Commenting.FunctionComment.MissingReturn
 // phpcs:disable MediaWiki.NamingConventions.PrefixedGlobalFunctions.wfPrefix
 
 if ( !function_exists( 'mws' ) ) {

diff --git a/repo/includes/RepoHooks.php b/repo/includes/RepoHooks.php
@@ -159,8 +159,6 @@ public static function onSetupAfterCache() {
 				return $entityContentFactory->getContentHandlerForType( $entityType );
 			};
 		}
-
-		return true;
 	}
 
 	/**