Replace Psalm with PHPStan

Author: tvdijen

.gitattributes

@@ -5,8 +5,10 @@ codecov.yml export-ignore
 .editorconfig export-ignore
 .gitattributes export-ignore
 .gitignore export-ignore
-psalm.xml export-ignore
-psalm-dev.xml export-ignore
+phpstan.neon export-ignore
+phpstan-dev.neon export-ignore
+phpstan-baseline.neon export-ignore
+phpstan-baseline-dev.neon export-ignore
 phpcs.xml export-ignore
 phpunit.xml export-ignore
 .php_cs.dist export-ignore

.github/workflows/php.yml

@@ -170,9 +170,8 @@ jobs:
         with:
           # Should be the higest supported version, so we can use the newest tools
           php-version: '8.5'
-          tools: composer, composer-require-checker, composer-unused, psalm
-          # optional performance gain for psalm: opcache
-          extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, opcache, openssl, pcre, spl, xml \
+          tools: composer, composer-require-checker, composer-unused
+          extensions: ctype, date, dom, fileinfo, filter, hash, intl, ldap, mbstring, openssl, pcre, spl, xml \
             krb5-php/pecl-authentication-krb5@master
 
       - name: Setup problem matchers for PHP
@@ -205,27 +204,13 @@ jobs:
       - name: PHP Code Sniffer
         run: vendor/bin/phpcs
 
-      - name: Psalm
-        continue-on-error: true
-        run: |
-          psalm -c psalm.xml \
-          --show-info=true \
-          --shepherd \
-          --php-version=${{ steps.setup-php.outputs.php-version }}
-
-      - name: Psalm (testsuite)
+      - name: PHPStan
         run: |
-          psalm -c psalm-dev.xml \
-          --show-info=true \
-          --shepherd \
-          --php-version=${{ steps.setup-php.outputs.php-version }}
+          vendor/bin/phpstan analyze -c phpstan.neon --debug
 
-      - name: Psalter
+      - name: PHPStan (testsuite)
         run: |
-          psalm --alter \
-          --issues=UnnecessaryVarAnnotation \
-          --dry-run \
-          --php-version=${{ steps.setup-php.outputs.php-version }}
+          vendor/bin/phpstan analyze -c phpstan-dev.neon --debug
 
   security:
     name: Security checks

phpstan-dev.neon

@@ -0,0 +1,4 @@
+parameters:
+  level: 8
+  paths:
+    - tests

phpstan.neon

@@ -0,0 +1,4 @@
+parameters:
+  level: 6
+  paths:
+    - src

psalm-dev.xml

@@ -50,10 +50,10 @@ class Negotiate extends Auth\Source
     /** @var string|integer|null */
     protected $spn = null;
 
-    /** @var array|null */
+    /** @var string[]|null */
     protected ?array $subnet = null;
 
-    /** @var array */
+    /** @var string[] */
     private array $realms;
 
     /** @var string[] */
@@ -66,8 +66,8 @@ class Negotiate extends Auth\Source
     /**
      * Constructor for this authentication source.
      *
-     * @param array $info Information about this authentication source.
-     * @param array $config The configuration of the module
+     * @param array<mixed> $info Information about this authentication source.
+     * @param array<mixed> $config The configuration of the module
      *
      * @throws \Exception If the KRB5 extension is not installed or active.
      */
@@ -101,7 +101,7 @@ public function __construct(array $info, array $config)
      *
      * LDAP is used as a user metadata source.
      *
-     * @param array &$state Information about the current authentication.
+     * @param array<mixed> &$state Information about the current authentication.
      */
     public function authenticate(array &$state): void
     {
@@ -151,7 +151,7 @@ public function authenticate(array &$state): void
                 Logger::debug('Negotiate - authenticate(): No "Negotiate" found. Skipping.');
             } else {
                 // attempt Kerberos authentication
-                $reply = null;
+                $reply = $auth = null;
 
                 try {
                     if (version_compare(phpversion('krb5'), '1.1.6', '<')) {
@@ -178,7 +178,7 @@ public function authenticate(array &$state): void
                             }
                         }
 
-                        if (!$auth->isChannelBound()) {
+                        if ($auth === null || !$auth->isChannelBound()) {
                             throw new Error\Exception(
                                 'Negotiate - authenticate(): Failed to perform channel binding using '
                                 . 'any of the configured certificate hashes.',
@@ -189,15 +189,13 @@ public function authenticate(array &$state): void
                     Logger::error('Negotiate - authenticate(): doAuthentication() exception: ' . $e->getMessage());
                 }
 
-                if ($reply) {
+                if ($reply && $auth !== null) {
                     // success! krb TGS received
-                    /** @psalm-var \KRB5NegotiateAuth $auth */
                     $userPrincipalName = $auth->getAuthenticatedUser();
                     Logger::info('Negotiate - authenticate(): ' . $userPrincipalName . ' authenticated.');
 
                     // Search for the corresponding realm and set current variables
                     @list($uid, $realmName) = preg_split('/@/', $userPrincipalName, 2);
-                    /** @psalm-var string $realmName */
                     Assert::notNull($realmName);
 
                     // Use the correct realm
@@ -279,7 +277,7 @@ private function doAuthentication(KRB5NegotiateAuth $auth, ?string $hash = null)
 
 
     /**
-     * @param array $spMetadata
+     * @param array<mixed> $spMetadata
      * @return bool
      */
     public function spDisabledInMetadata(array $spMetadata): bool
@@ -330,7 +328,7 @@ public function checkMask(): bool
      * Send the actual headers and body of the 401. Embedded in the body is a post that is triggered by JS if the client
      * wants to show the 401 message.
      *
-     * @param array $params additional parameters to the URL in the URL in the body.
+     * @param array<mixed> $params additional parameters to the URL in the URL in the body.
      */
     protected function sendNegotiate(array $params): void
     {
@@ -351,7 +349,7 @@ protected function sendNegotiate(array $params): void
     /**
      * Passes control of the login process to a different module.
      *
-     * @param array $state Information about the current authentication.
+     * @param array<mixed> $state Information about the current authentication.
      *
      * @throws \SimpleSAML\Error\Error If couldn't determine the auth source.
      * @throws \SimpleSAML\Error\Exception
@@ -364,7 +362,6 @@ public static function fallBack(array &$state): void // never
             throw new Error\Error([500, "Unable to determine auth source."]);
         }
 
-        /** @psalm-var \SimpleSAML\Auth\Source|null $source */
         $source = Auth\Source::getById($authId);
         if ($source === null) {
             throw new Exception('Could not find authentication source with id ' . $state[self::AUTHID]);
@@ -390,13 +387,12 @@ public static function fallBack(array &$state): void // never
      *
      * @param string $uid The user identifier.
      *
-     * @return array|null The attributes for the user or NULL if not found.
+     * @return array<mixed>|null The attributes for the user or NULL if not found.
      */
     protected function lookupUserData(string $uid): ?array
     {
         /**
          * @var \SimpleSAML\Module\ldap\Auth\Source\Ldap|null $source
-         * @psalm-var string $this->backend - We only reach this method when $this->backend is set
          */
         $source = Auth\Source::getById($this->backend);
         if ($source === null) {
@@ -418,7 +414,7 @@ protected function lookupUserData(string $uid): ?array
      * This method either logs the user out from Negotiate or passes the
      * logout call to the fallback module.
      *
-     * @param array &$state Information about the current logout operation.
+     * @param array<mixed> &$state Information about the current logout operation.
      */
     public function logout(array &$state): void
     {
@@ -431,7 +427,6 @@ public function logout(array &$state): void
             $session->setData('negotiate:disable', 'session', true, 24 * 60 * 60);
             parent::logout($state);
         } else {
-            /** @psalm-var \SimpleSAML\Module\negotiate\Auth\Source\Negotiate|null $source */
             $source = Auth\Source::getById($authId);
             if ($source === null) {
                 throw new Exception('Could not find authentication source with id ' . $state[self::AUTHID]);

psalm.xml

@@ -81,7 +81,7 @@ public function testEnable(): void
         $this->assertTrue($response->isSuccessful());
 
         // Validate cookie
-        /** @var non-empty-array $cookies */
+        /** @var non-empty-array<mixed> $cookies */
         $cookies = $response->headers->getCookies();
         foreach ($cookies as $cookie) {
             if ($cookie->getName() === 'NEGOTIATE_AUTOLOGIN_DISABLE_PERMANENT') {
@@ -118,7 +118,7 @@ public function testDisable(): void
         $this->assertTrue($response->isSuccessful());
 
         // Validate cookie
-        /** @var non-empty-array $cookies */
+        /** @var non-empty-array<mixed> $cookies */
         $cookies = $response->headers->getCookies();
         foreach ($cookies as $cookie) {
             if ($cookie->getName() === 'NEGOTIATE_AUTOLOGIN_DISABLE_PERMANENT') {
@@ -151,7 +151,8 @@ public function testRetry(): void
         $c = new Controller\NegotiateController($this->config, $this->session);
         $c->setLogger($this->logger);
         $c->setAuthState(new class () extends State {
-            public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
+            /** @return array<mixed> */
+            public static function loadState(string $id, string $stage, bool $allowMissing = false): array
             {
                 return [
                     'LogoutState' => [
@@ -174,11 +175,14 @@ final public function __construct()
                 // stub
             }
 
+
+            /** @param array<mixed> $state */
             public function authenticate(array &$state): void
             {
                 // stub
             }
 
+
             public static function getById(string $authId, ?string $type = null): ?Source
             {
                 return new static();
@@ -205,7 +209,8 @@ public function testRetryInvalidMetadataThrowsException(): void
         $c = new Controller\NegotiateController($this->config, $this->session);
         $c->setLogger($this->logger);
         $c->setAuthState(new class () extends State {
-            public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
+            /** @return array<mixed> */
+            public static function loadState(string $id, string $stage, bool $allowMissing = false): array
             {
                 return [
                     'LogoutState' => [
@@ -243,7 +248,8 @@ public function testRetryInvalidAuthSourceThrowsException(): void
         $c = new Controller\NegotiateController($this->config, $this->session);
         $c->setLogger($this->logger);
         $c->setAuthState(new class () extends State {
-            public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
+            /** @return array<mixed> */
+            public static function loadState(string $id, string $stage, bool $allowMissing = false): array
             {
                 return [
                     'LogoutState' => [
@@ -266,6 +272,8 @@ public function __construct()
                 // stub
             }
 
+
+            /** @param array<mixed> $state */
             public function authenticate(array &$state): void
             {
                 // stub
@@ -322,7 +330,8 @@ public function testBackend(): void
         $c = new Controller\NegotiateController($this->config, $this->session);
         $c->setLogger($this->logger);
         $c->setAuthState(new class () extends State {
-            public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
+            /** @return array<mixed> */
+            public static function loadState(string $id, string $stage, bool $allowMissing = false): array
             {
                 return [
                     'LogoutState' => [