From f2080c1bb953e508524aa21014f5b7faf743c532 Mon Sep 17 00:00:00 2001 From: Reedy Date: Wed, 11 Oct 2023 00:20:01 +0100 Subject: [PATCH] Fix remaining PHPCS exclusions Add some return type hints on private methods in OATHManage Change-Id: I8580348f5460b59c21bfca07b7c4cb92ea6be43f --- .phpcs.xml | 5 +- src/Auth/TOTPAuthenticationRequest.php | 1 + src/Special/OATHManage.php | 92 +++++++++++--------------- 3 files changed, 39 insertions(+), 59 deletions(-) diff --git a/.phpcs.xml b/.phpcs.xml index b8f3b00e..b8daed89 100644 --- a/.phpcs.xml +++ b/.phpcs.xml @@ -1,9 +1,6 @@ - - - - + . diff --git a/src/Auth/TOTPAuthenticationRequest.php b/src/Auth/TOTPAuthenticationRequest.php index ced0ed3e..56a97e53 100644 --- a/src/Auth/TOTPAuthenticationRequest.php +++ b/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() { diff --git a/src/Special/OATHManage.php b/src/Special/OATHManage.php index bfeba2f3..1c1e1913 100644 --- a/src/Special/OATHManage.php +++ b/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;