Fix remaining PHPCS exclusions

Add some return type hints on private methods in OATHManage

Change-Id: I8580348f5460b59c21bfca07b7c4cb92ea6be43f

.phpcs.xml

@@ -1,9 +1,6 @@
 <?xml version="1.0"?>
 <ruleset name="MediaWiki">
-	<rule ref="./vendor/mediawiki/mediawiki-codesniffer/MediaWiki">
-		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationPrivate" />
-		<exclude name="MediaWiki.Commenting.PropertyDocumentation.MissingDocumentationPublic" />
-	</rule>
+	<rule ref="./vendor/mediawiki/mediawiki-codesniffer/MediaWiki" />
 	<file>.</file>
 	<arg name="encoding" value="UTF-8" />
 	<arg name="extensions" value="php,sample" />

src/Auth/TOTPAuthenticationRequest.php

@@ -27,6 +27,7 @@ use MediaWiki\Language\RawMessage;
  * by the server and the client.
  */
 class TOTPAuthenticationRequest extends AuthenticationRequest {
+	/** @var string */
 	public $OATHToken;
 
 	public function describeCredentials() {

src/Special/OATHManage.php

@@ -42,20 +42,11 @@ class OATHManage extends SpecialPage {
 	public const ACTION_ENABLE = 'enable';
 	public const ACTION_DISABLE = 'disable';
 
-	/**
-	 * @var OATHAuthModuleRegistry
-	 */
-	protected $moduleRegistry;
+	protected OATHAuthModuleRegistry $moduleRegistry;
 
-	/**
-	 * @var OATHUserRepository
-	 */
-	protected $userRepo;
+	protected OATHUserRepository $userRepo;
 
-	/**
-	 * @var OATHUser
-	 */
-	protected $authUser;
+	protected OATHUser $authUser;
 
 	/**
 	 * @var string
@@ -76,7 +67,7 @@ class OATHManage extends SpecialPage {
 	 * @throws ConfigException
 	 * @throws MWException
 	 */
-	public function __construct( $userRepo, OATHAuthModuleRegistry $moduleRegistry ) {
+	public function __construct( OATHUserRepository $userRepo, OATHAuthModuleRegistry $moduleRegistry ) {
 		parent::__construct( 'OATHManage', 'oathauth-enable' );
 
 		$this->userRepo = $userRepo;
@@ -148,39 +139,39 @@ class OATHManage extends SpecialPage {
 		}
 	}
 
-	private function setAction() {
+	private function setAction(): void {
 		$this->action = $this->getRequest()->getVal( 'action', '' );
 	}
 
-	private function setModule() {
+	private function setModule(): void {
 		$moduleKey = $this->getRequest()->getVal( 'module', '' );
 		$this->requestedModule = $this->moduleRegistry->getModuleByKey( $moduleKey );
 	}
 
-	private function hasEnabled() {
+	private function hasEnabled(): bool {
 		return $this->authUser->getModule() instanceof IModule;
 	}
 
-	private function getEnabled() {
+	private function getEnabled(): ?IModule {
 		return $this->hasEnabled() ? $this->authUser->getModule() : null;
 	}
 
-	private function addEnabledHTML() {
+	private function addEnabledHTML(): void {
 		$this->addHeading( $this->msg( 'oathauth-ui-enabled-module' ) );
 		$this->addModuleHTML( $this->getEnabled() );
 	}
 
-	private function addAlternativesHTML() {
+	private function addAlternativesHTML(): void {
 		$this->addHeading( $this->msg( 'oathauth-ui-not-enabled-modules' ) );
 		$this->addInactiveHTML();
 	}
 
-	private function nothingEnabled() {
+	private function nothingEnabled(): void {
 		$this->addHeading( $this->msg( 'oathauth-ui-available-modules' ) );
 		$this->addInactiveHTML();
 	}
 
-	private function addInactiveHTML() {
+	private function addInactiveHTML(): void {
 		foreach ( $this->moduleRegistry->getAllModules() as $module ) {
 			if ( $this->isModuleEnabled( $module ) ) {
 				continue;
@@ -189,20 +180,20 @@ class OATHManage extends SpecialPage {
 		}
 	}
 
-	private function addGeneralHelp() {
+	private function addGeneralHelp(): void {
 		$this->getOutput()->addHTML( $this->msg(
 			'oathauth-ui-general-help'
 		)->parseAsBlock() );
 	}
 
-	private function addModuleHTML( IModule $module ) {
-		if ( $this->isModuleRequested( $module ) ) {
+	private function addModuleHTML( ?IModule $module ): void {
+		if ( $module instanceof IModule && $this->isModuleRequested( $module ) ) {
 			$this->addCustomContent( $module );
 			return;
 		}
 
 		$panel = $this->getGenericContent( $module );
-		if ( $this->isModuleEnabled( $module ) ) {
+		if ( $module instanceof IModule && $this->isModuleEnabled( $module ) ) {
 			$this->addCustomContent( $module, $panel );
 		}
 
@@ -211,11 +202,8 @@ class OATHManage extends SpecialPage {
 
 	/**
 	 * Get the panel with generic content for a module
-	 *
-	 * @param IModule $module
-	 * @return PanelLayout
 	 */
-	private function getGenericContent( IModule $module ) {
+	private function getGenericContent( ?IModule $module ): PanelLayout {
 		$modulePanel = new PanelLayout( [
 			'framed' => true,
 			'expanded' => false,
@@ -249,11 +237,7 @@ class OATHManage extends SpecialPage {
 		return $modulePanel;
 	}
 
-	/**
-	 * @param IModule $module
-	 * @param PanelLayout|null $panel
-	 */
-	private function addCustomContent( IModule $module, $panel = null ) {
+	private function addCustomContent( IModule $module, PanelLayout $panel = null ): void {
 		$form = $module->getManageForm(
 			$this->action,
 			$this->authUser,
@@ -271,24 +255,27 @@ class OATHManage extends SpecialPage {
 		}
 	}
 
-	private function addHeading( Message $message ) {
+	private function addHeading( Message $message ): void {
 		$this->getOutput()->addHTML( Html::element( 'h2', [], $message->text() ) );
 	}
 
-	private function shouldShowGenericButtons() {
+	private function shouldShowGenericButtons(): bool {
 		return !$this->requestedModule instanceof IModule || !$this->isGenericAction();
 	}
 
-	private function isModuleRequested( IModule $module ) {
+	private function isModuleRequested( ?IModule $module ): bool {
 		return (
 			$this->requestedModule instanceof IModule
+			&& $module instanceof IModule
 			&& $this->requestedModule->getName() === $module->getName()
 		);
 	}
 
-	private function isModuleEnabled( IModule $module ) {
-		if ( $this->getEnabled() instanceof IModule ) {
-			return $this->getEnabled()->getName() === $module->getName();
+	private function isModuleEnabled( ?IModule $module ): bool {
+		$enabled = $this->getEnabled();
+		if ( $enabled instanceof IModule ) {
+			return $module instanceof IModule
+				&& $enabled->getName() === $module->getName();
 		}
 		return false;
 	}
@@ -299,7 +286,7 @@ class OATHManage extends SpecialPage {
 	 * @param mixed $form
 	 * @return bool
 	 */
-	private function isValidFormType( $form ) {
+	private function isValidFormType( $form ): bool {
 		if ( !( $form instanceof HTMLForm ) ) {
 			return false;
 		}
@@ -311,11 +298,7 @@ class OATHManage extends SpecialPage {
 		return true;
 	}
 
-	/**
-	 * @param IManageForm $form
-	 * @param IModule $module
-	 */
-	private function ensureRequiredFormFields( IManageForm $form, IModule $module ) {
+	private function ensureRequiredFormFields( IManageForm $form, IModule $module ): void {
 		if ( !$form->hasField( 'module' ) ) {
 			$form->addHiddenField( 'module', $module->getName() );
 		}
@@ -326,9 +309,9 @@ class OATHManage extends SpecialPage {
 
 	/**
 	 * When performing an action on a module (like enable/disable),
-	 * page should contain only the form for that action
+	 * page should contain only the form for that action.
 	 */
-	private function clearPage() {
+	private function clearPage(): void {
 		if ( $this->isGenericAction() ) {
 			$displayName = $this->requestedModule->getDisplayName();
 			$pageTitle = $this->isModuleEnabled( $this->requestedModule ) ?
@@ -343,14 +326,13 @@ class OATHManage extends SpecialPage {
 
 	/**
 	 * The enable and disable actions are generic, and all modules must
-	 * implement them, while all other actions are module-specific
-	 * @return bool
+	 * implement them, while all other actions are module-specific.
 	 */
-	private function isGenericAction() {
+	private function isGenericAction(): bool {
 		return in_array( $this->action, [ static::ACTION_ENABLE, static::ACTION_DISABLE ] );
 	}
 
-	private function hasAlternativeModules() {
+	private function hasAlternativeModules(): bool {
 		foreach ( $this->moduleRegistry->getAllModules() as $module ) {
 			if ( !$this->isModuleEnabled( $module ) ) {
 				return true;
@@ -359,13 +341,13 @@ class OATHManage extends SpecialPage {
 		return false;
 	}
 
-	private function shouldShowDisableWarning() {
+	private function shouldShowDisableWarning(): bool {
 		return $this->getRequest()->getBool( 'warn' ) &&
 			$this->requestedModule instanceof IModule &&
 			$this->getEnabled() instanceof IModule;
 	}
 
-	private function showDisableWarning() {
+	private function showDisableWarning(): void {
 		$panel = new PanelLayout( [
 			'padded' => true,
 			'framed' => true,
@@ -407,7 +389,7 @@ class OATHManage extends SpecialPage {
 		$this->getOutput()->addHTML( $panel->toString() );
 	}
 
-	private function isSwitch() {
+	private function isSwitch(): bool {
 		return $this->requestedModule instanceof IModule &&
 			$this->action === static::ACTION_ENABLE &&
 			$this->getEnabled() instanceof IModule;