Merge pull request #18488 from kamil-tekiela/Add-never-type

MauricioFauth · web-flow · commit a52667ee28b1 · 2023-06-09T16:13:33.000-03:00
Add never return type
diff --git a/libraries/classes/Controllers/Setup/ShowConfigController.php b/libraries/classes/Controllers/Setup/ShowConfigController.php
@@ -36,8 +36,6 @@ public function __invoke(ServerRequest $request): void
             $GLOBALS['ConfigFile']->resetConfigData();
             // drop post data
             $response->generateHeader303('../setup/index.php' . Url::getCommonRaw(['route' => '/setup']));
-
-            return;
         }
 
         /** @var mixed $submitDownload */
diff --git a/libraries/classes/Plugins/Auth/AuthenticationConfig.php b/libraries/classes/Plugins/Auth/AuthenticationConfig.php
@@ -28,20 +28,18 @@ class AuthenticationConfig extends AuthenticationPlugin
 {
     /**
      * Displays authentication form
-     *
-     * @return bool always true
      */
-    public function showLoginForm(): bool
+    public function showLoginForm(): void
     {
         $response = ResponseRenderer::getInstance();
-        if ($response->isAjax()) {
-            $response->setRequestStatus(false);
-            // reload_flag removes the token parameter from the URL and reloads
-            $response->addJSON('reload_flag', '1');
-            $response->callExit();
+        if (! $response->isAjax()) {
+            return;
         }
 
-        return true;
+        $response->setRequestStatus(false);
+        // reload_flag removes the token parameter from the URL and reloads
+        $response->addJSON('reload_flag', '1');
+        $response->callExit();
     }
 
     /**
@@ -66,7 +64,7 @@ public function readCredentials(): bool
      *
      * @param string $failure String describing why authentication has failed
      */
-    public function showFailure(string $failure): void
+    public function showFailure(string $failure): never
     {
         parent::showFailure($failure);
 
diff --git a/libraries/classes/Plugins/Auth/AuthenticationCookie.php b/libraries/classes/Plugins/Auth/AuthenticationCookie.php
@@ -63,7 +63,7 @@ class AuthenticationCookie extends AuthenticationPlugin
      *
      * @global string $conn_error the last connection error
      */
-    public function showLoginForm(): bool
+    public function showLoginForm(): never
     {
         $GLOBALS['conn_error'] ??= null;
 
@@ -379,7 +379,6 @@ public function readCredentials(): bool
             SessionCache::remove('proc_priv');
 
             $this->showFailure('no-activity');
-            ResponseRenderer::getInstance()->callExit();
         }
 
         // check password cookie
@@ -552,12 +551,9 @@ public function storePasswordCookie(string $password): void
      * prepares error message and switches to showLoginForm() which display the error
      * and the login form
      *
-     * this function MUST exit/quit the application,
-     * currently done by call to showLoginForm()
-     *
      * @param string $failure String describing why authentication has failed
      */
-    public function showFailure(string $failure): void
+    public function showFailure(string $failure): never
     {
         $GLOBALS['conn_error'] ??= null;
 
diff --git a/libraries/classes/Plugins/Auth/AuthenticationHttp.php b/libraries/classes/Plugins/Auth/AuthenticationHttp.php
@@ -30,10 +30,8 @@ class AuthenticationHttp extends AuthenticationPlugin
 {
     /**
      * Displays authentication form and redirect as necessary
-     *
-     * @return bool always true (no return indeed)
      */
-    public function showLoginForm(): bool
+    public function showLoginForm(): never
     {
         $response = ResponseRenderer::getInstance();
         if ($response->isAjax()) {
@@ -43,13 +41,13 @@ public function showLoginForm(): bool
             $response->callExit();
         }
 
-        return $this->authForm();
+        $this->authForm();
     }
 
     /**
      * Displays authentication form
      */
-    public function authForm(): bool
+    public function authForm(): never
     {
         if (empty($GLOBALS['cfg']['Server']['auth_http_realm'])) {
             if (empty($GLOBALS['cfg']['Server']['verbose'])) {
@@ -179,7 +177,7 @@ public function readCredentials(): bool
      *
      * @param string $failure String describing why authentication has failed
      */
-    public function showFailure(string $failure): void
+    public function showFailure(string $failure): never
     {
         parent::showFailure($failure);
 
diff --git a/libraries/classes/Plugins/Auth/AuthenticationSignon.php b/libraries/classes/Plugins/Auth/AuthenticationSignon.php
@@ -32,7 +32,7 @@ class AuthenticationSignon extends AuthenticationPlugin
     /**
      * Displays authentication form
      */
-    public function showLoginForm(): bool
+    public function showLoginForm(): never
     {
         $response = ResponseRenderer::getInstance();
         $response->disable();
@@ -244,7 +244,7 @@ public function readCredentials(): bool
      *
      * @param string $failure String describing why authentication has failed
      */
-    public function showFailure(string $failure): void
+    public function showFailure(string $failure): never
     {
         parent::showFailure($failure);
 
diff --git a/libraries/classes/Plugins/AuthenticationPlugin.php b/libraries/classes/Plugins/AuthenticationPlugin.php
@@ -61,7 +61,7 @@ public function __construct()
     /**
      * Displays authentication form
      */
-    abstract public function showLoginForm(): bool;
+    abstract public function showLoginForm(): void;
 
     /**
      * Gets authentication credentials
diff --git a/libraries/classes/ResponseRenderer.php b/libraries/classes/ResponseRenderer.php
@@ -439,7 +439,7 @@ public function setHttpResponseCode(int $responseCode): void
      *
      * @param string $location will set location to redirect.
      */
-    public function generateHeader303(string $location): void
+    public function generateHeader303(string $location): never
     {
         $this->setHttpResponseCode(303);
         $this->header('Location: ' . $location);
diff --git a/libraries/classes/Setup/FormProcessing.php b/libraries/classes/Setup/FormProcessing.php
@@ -48,8 +48,6 @@ public static function process(FormDisplay $formDisplay): void
             $response = ResponseRenderer::getInstance();
             $response->disable();
             $response->generateHeader303('../setup/index.php' . Url::getCommonRaw(['route' => '/setup']));
-
-            return;
         }
 
         // form has errors, show warning
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -8177,9 +8177,6 @@
     <InvalidArrayOffset>
       <code><![CDATA[$GLOBALS['allowDeny_forbidden']]]></code>
     </InvalidArrayOffset>
-    <PossiblyUnusedReturnValue>
-      <code>bool</code>
-    </PossiblyUnusedReturnValue>
   </file>
   <file src="libraries/classes/Plugins/Auth/AuthenticationCookie.php">
     <DocblockTypeContradiction>
@@ -8243,7 +8240,6 @@
     </PossiblyNullReference>
     <PossiblyUnusedReturnValue>
       <code>bool</code>
-      <code>bool</code>
     </PossiblyUnusedReturnValue>
     <RedundantCast>
       <code><![CDATA[(int) $GLOBALS['cfg']['LoginCookieStore']]]></code>
@@ -8271,9 +8267,6 @@
     <PossiblyInvalidCast>
       <code>$oldUser</code>
     </PossiblyInvalidCast>
-    <PossiblyUnusedReturnValue>
-      <code>bool</code>
-    </PossiblyUnusedReturnValue>
   </file>
   <file src="libraries/classes/Plugins/Auth/AuthenticationSignon.php">
     <MixedArgument>
@@ -8297,9 +8290,6 @@
       <code><![CDATA[$this->password]]></code>
       <code><![CDATA[$this->user]]></code>
     </MixedAssignment>
-    <PossiblyUnusedReturnValue>
-      <code>bool</code>
-    </PossiblyUnusedReturnValue>
     <RedundantCast>
       <code><![CDATA[(array) $GLOBALS['cfg']['Server']['SignonCookieParams']]]></code>
     </RedundantCast>
@@ -8326,9 +8316,6 @@
     <PossiblyNullReference>
       <code>issetCookie</code>
     </PossiblyNullReference>
-    <PossiblyUnusedReturnValue>
-      <code>bool</code>
-    </PossiblyUnusedReturnValue>
   </file>
   <file src="libraries/classes/Plugins/AuthenticationPluginFactory.php">
     <RedundantFunctionCall>
diff --git a/test/classes/Plugins/Auth/AuthenticationConfigTest.php b/test/classes/Plugins/Auth/AuthenticationConfigTest.php
@@ -94,7 +94,7 @@ public function testAuthFails(): void
 
         $html = ob_get_clean();
 
-        $this->assertInstanceOf(ExitException::class, $throwable ?? null);
+        $this->assertInstanceOf(ExitException::class, $throwable);
 
         $this->assertIsString($html);
 
diff --git a/test/classes/Plugins/Auth/AuthenticationCookieTest.php b/test/classes/Plugins/Auth/AuthenticationCookieTest.php
@@ -73,7 +73,7 @@ protected function tearDown(): void
     }
 
     #[Group('medium')]
-    public function testAuthErrorAJAX(): void
+    public function testAuthErrorAJAX(): never
     {
         $mockResponse = $this->mockResponse();
 
@@ -191,7 +191,7 @@ public function testAuthError(): void
 
         $result = ob_get_clean();
 
-        $this->assertInstanceOf(ExitException::class, $throwable ?? null);
+        $this->assertInstanceOf(ExitException::class, $throwable);
 
         $this->assertIsString($result);
 
@@ -276,7 +276,7 @@ public function testAuthCaptcha(): void
 
         $result = ob_get_clean();
 
-        $this->assertInstanceOf(ExitException::class, $throwable ?? null);
+        $this->assertInstanceOf(ExitException::class, $throwable);
 
         $this->assertIsString($result);
 
@@ -355,7 +355,7 @@ public function testAuthCaptchaCheckbox(): void
 
         $result = ob_get_clean();
 
-        $this->assertInstanceOf(ExitException::class, $throwable ?? null);
+        $this->assertInstanceOf(ExitException::class, $throwable);
 
         $this->assertIsString($result);
 
@@ -643,7 +643,8 @@ public function testAuthCheckAuthFails(): void
             ->will($this->returnValue('testBF'));
 
         $this->object->expects($this->once())
-            ->method('showFailure');
+            ->method('showFailure')
+            ->willThrowException(new ExitException());
 
         $this->expectException(ExitException::class);
         $this->object->readCredentials();
@@ -711,14 +712,21 @@ public function testAuthFailsNoPass(): void
             ->onlyMethods(['showLoginForm'])
             ->getMock();
 
+        $this->object->expects($this->exactly(1))
+            ->method('showLoginForm')
+            ->willThrowException(new ExitException());
+
         $GLOBALS['server'] = 2;
         $_COOKIE['pmaAuth-2'] = 'pass';
 
         $this->mockResponse(
             ['Cache-Control: no-store, no-cache, must-revalidate'],
             ['Pragma: no-cache'],
         );
-        $this->object->showFailure('empty-denied');
+        try {
+            $this->object->showFailure('empty-denied');
+        } catch (ExitException) {
+        }
 
         $this->assertEquals(
             $GLOBALS['conn_error'],
@@ -773,14 +781,21 @@ public function testAuthFailsDeny(): void
             ->onlyMethods(['showLoginForm'])
             ->getMock();
 
+        $this->object->expects($this->exactly(1))
+            ->method('showLoginForm')
+            ->willThrowException(new ExitException());
+
         $GLOBALS['server'] = 2;
         $_COOKIE['pmaAuth-2'] = 'pass';
 
         $this->mockResponse(
             ['Cache-Control: no-store, no-cache, must-revalidate'],
             ['Pragma: no-cache'],
         );
-        $this->object->showFailure('allow-denied');
+        try {
+            $this->object->showFailure('allow-denied');
+        } catch (ExitException) {
+        }
 
         $this->assertEquals($GLOBALS['conn_error'], 'Access denied!');
     }
@@ -792,6 +807,10 @@ public function testAuthFailsActivity(): void
             ->onlyMethods(['showLoginForm'])
             ->getMock();
 
+        $this->object->expects($this->exactly(1))
+            ->method('showLoginForm')
+            ->willThrowException(new ExitException());
+
         $GLOBALS['server'] = 2;
         $_COOKIE['pmaAuth-2'] = 'pass';
 
@@ -802,7 +821,10 @@ public function testAuthFailsActivity(): void
             ['Cache-Control: no-store, no-cache, must-revalidate'],
             ['Pragma: no-cache'],
         );
-        $this->object->showFailure('no-activity');
+        try {
+            $this->object->showFailure('no-activity');
+        } catch (ExitException) {
+        }
 
         $this->assertEquals(
             $GLOBALS['conn_error'],
@@ -818,6 +840,10 @@ public function testAuthFailsDBI(): void
             ->onlyMethods(['showLoginForm'])
             ->getMock();
 
+        $this->object->expects($this->exactly(1))
+            ->method('showLoginForm')
+            ->willThrowException(new ExitException());
+
         $GLOBALS['server'] = 2;
         $_COOKIE['pmaAuth-2'] = 'pass';
 
@@ -836,7 +862,10 @@ public function testAuthFailsDBI(): void
             ['Cache-Control: no-store, no-cache, must-revalidate'],
             ['Pragma: no-cache'],
         );
-        $this->object->showFailure('');
+        try {
+            $this->object->showFailure('');
+        } catch (ExitException) {
+        }
 
         $this->assertEquals($GLOBALS['conn_error'], '#42 Cannot log in to the MySQL server');
     }
@@ -848,6 +877,10 @@ public function testAuthFailsErrno(): void
             ->onlyMethods(['showLoginForm'])
             ->getMock();
 
+        $this->object->expects($this->exactly(1))
+            ->method('showLoginForm')
+            ->willThrowException(new ExitException());
+
         $dbi = $this->getMockBuilder(DatabaseInterface::class)
             ->disableOriginalConstructor()
             ->getMock();
@@ -866,7 +899,10 @@ public function testAuthFailsErrno(): void
             ['Cache-Control: no-store, no-cache, must-revalidate'],
             ['Pragma: no-cache'],
         );
-        $this->object->showFailure('');
+        try {
+            $this->object->showFailure('');
+        } catch (ExitException) {
+        }
 
         $this->assertEquals($GLOBALS['conn_error'], 'Cannot log in to the MySQL server');
     }
diff --git a/test/classes/Plugins/Auth/AuthenticationHttpTest.php b/test/classes/Plugins/Auth/AuthenticationHttpTest.php
diff --git a/test/classes/Plugins/Auth/AuthenticationSignonTest.php b/test/classes/Plugins/Auth/AuthenticationSignonTest.php

Original file line number	Diff line number	Diff line change
`@@ -36,8 +36,6 @@ public function __invoke(ServerRequest $request): void`
`36`	`36`	`$GLOBALS['ConfigFile']->resetConfigData();`
`37`	`37`	`// drop post data`
`38`	`38`	`$response->generateHeader303('../setup/index.php' . Url::getCommonRaw(['route' => '/setup']));`
`39`		`-`
`40`		`- return;`
`41`	`39`	`}`
`42`	`40`
`43`	`41`	`/** @var mixed $submitDownload */`
Original file line number	Diff line number	Diff line change
`@@ -28,20 +28,18 @@ class AuthenticationConfig extends AuthenticationPlugin`
`28`	`28`	`{`
`29`	`29`	`/**`
`30`	`30`	`* Displays authentication form`
`31`		`- *`
`32`		`- * @return bool always true`
`33`	`31`	`*/`
`34`		`- public function showLoginForm(): bool`
	`32`	`+ public function showLoginForm(): void`
`35`	`33`	`{`
`36`	`34`	`$response = ResponseRenderer::getInstance();`
`37`		`- if ($response->isAjax()) {`
`38`		`- $response->setRequestStatus(false);`
`39`		`- // reload_flag removes the token parameter from the URL and reloads`
`40`		`- $response->addJSON('reload_flag', '1');`
`41`		`- $response->callExit();`
	`35`	`+ if (! $response->isAjax()) {`
	`36`	`+ return;`
`42`	`37`	`}`
`43`	`38`
`44`		`- return true;`
	`39`	`+ $response->setRequestStatus(false);`
	`40`	`+ // reload_flag removes the token parameter from the URL and reloads`
	`41`	`+ $response->addJSON('reload_flag', '1');`
	`42`	`+ $response->callExit();`
`45`	`43`	`}`
`46`	`44`
`47`	`45`	`/**`
`@@ -66,7 +64,7 @@ public function readCredentials(): bool`
`66`	`64`	`*`
`67`	`65`	`* @param string $failure String describing why authentication has failed`
`68`	`66`	`*/`
`69`		`- public function showFailure(string $failure): void`
	`67`	`+ public function showFailure(string $failure): never`
`70`	`68`	`{`
`71`	`69`	`parent::showFailure($failure);`
`72`	`70`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ class AuthenticationCookie extends AuthenticationPlugin`
`63`	`63`	`*`
`64`	`64`	`* @global string $conn_error the last connection error`
`65`	`65`	`*/`
`66`		`- public function showLoginForm(): bool`
	`66`	`+ public function showLoginForm(): never`
`67`	`67`	`{`
`68`	`68`	`$GLOBALS['conn_error'] ??= null;`
`69`	`69`
`@@ -379,7 +379,6 @@ public function readCredentials(): bool`
`379`	`379`	`SessionCache::remove('proc_priv');`
`380`	`380`
`381`	`381`	`$this->showFailure('no-activity');`
`382`		`- ResponseRenderer::getInstance()->callExit();`
`383`	`382`	`}`
`384`	`383`
`385`	`384`	`// check password cookie`
`@@ -552,12 +551,9 @@ public function storePasswordCookie(string $password): void`
`552`	`551`	`* prepares error message and switches to showLoginForm() which display the error`
`553`	`552`	`* and the login form`
`554`	`553`	`*`
`555`		`- * this function MUST exit/quit the application,`
`556`		`- * currently done by call to showLoginForm()`
`557`		`- *`
`558`	`554`	`* @param string $failure String describing why authentication has failed`
`559`	`555`	`*/`
`560`		`- public function showFailure(string $failure): void`
	`556`	`+ public function showFailure(string $failure): never`
`561`	`557`	`{`
`562`	`558`	`$GLOBALS['conn_error'] ??= null;`
`563`	`559`
Original file line number	Diff line number	Diff line change
`@@ -30,10 +30,8 @@ class AuthenticationHttp extends AuthenticationPlugin`
`30`	`30`	`{`
`31`	`31`	`/**`
`32`	`32`	`* Displays authentication form and redirect as necessary`
`33`		`- *`
`34`		`- * @return bool always true (no return indeed)`
`35`	`33`	`*/`
`36`		`- public function showLoginForm(): bool`
	`34`	`+ public function showLoginForm(): never`
`37`	`35`	`{`
`38`	`36`	`$response = ResponseRenderer::getInstance();`
`39`	`37`	`if ($response->isAjax()) {`
`@@ -43,13 +41,13 @@ public function showLoginForm(): bool`
`43`	`41`	`$response->callExit();`
`44`	`42`	`}`
`45`	`43`
`46`		`- return $this->authForm();`
	`44`	`+ $this->authForm();`
`47`	`45`	`}`
`48`	`46`
`49`	`47`	`/**`
`50`	`48`	`* Displays authentication form`
`51`	`49`	`*/`
`52`		`- public function authForm(): bool`
	`50`	`+ public function authForm(): never`
`53`	`51`	`{`
`54`	`52`	`if (empty($GLOBALS['cfg']['Server']['auth_http_realm'])) {`
`55`	`53`	`if (empty($GLOBALS['cfg']['Server']['verbose'])) {`
`@@ -179,7 +177,7 @@ public function readCredentials(): bool`
`179`	`177`	`*`
`180`	`178`	`* @param string $failure String describing why authentication has failed`
`181`	`179`	`*/`
`182`		`- public function showFailure(string $failure): void`
	`180`	`+ public function showFailure(string $failure): never`
`183`	`181`	`{`
`184`	`182`	`parent::showFailure($failure);`
`185`	`183`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ class AuthenticationSignon extends AuthenticationPlugin`
`32`	`32`	`/**`
`33`	`33`	`* Displays authentication form`
`34`	`34`	`*/`
`35`		`- public function showLoginForm(): bool`
	`35`	`+ public function showLoginForm(): never`
`36`	`36`	`{`
`37`	`37`	`$response = ResponseRenderer::getInstance();`
`38`	`38`	`$response->disable();`
`@@ -244,7 +244,7 @@ public function readCredentials(): bool`
`244`	`244`	`*`
`245`	`245`	`* @param string $failure String describing why authentication has failed`
`246`	`246`	`*/`
`247`		`- public function showFailure(string $failure): void`
	`247`	`+ public function showFailure(string $failure): never`
`248`	`248`	`{`
`249`	`249`	`parent::showFailure($failure);`
`250`	`250`
Original file line number	Diff line number	Diff line change
`@@ -439,7 +439,7 @@ public function setHttpResponseCode(int $responseCode): void`
`439`	`439`	`*`
`440`	`440`	`* @param string $location will set location to redirect.`
`441`	`441`	`*/`
`442`		`- public function generateHeader303(string $location): void`
	`442`	`+ public function generateHeader303(string $location): never`
`443`	`443`	`{`
`444`	`444`	`$this->setHttpResponseCode(303);`
`445`	`445`	`$this->header('Location: ' . $location);`
Original file line number	Diff line number	Diff line change
`@@ -48,8 +48,6 @@ public static function process(FormDisplay $formDisplay): void`
`48`	`48`	`$response = ResponseRenderer::getInstance();`
`49`	`49`	`$response->disable();`
`50`	`50`	`$response->generateHeader303('../setup/index.php' . Url::getCommonRaw(['route' => '/setup']));`
`51`		`-`
`52`		`- return;`
`53`	`51`	`}`
`54`	`52`
`55`	`53`	`// form has errors, show warning`