Merge pull request #8 from simplesamlphp/feature/phpstan

Replace Psalm with PHPstan

Author: tvdijen

.github/workflows/php.yml

@@ -162,9 +162,8 @@ jobs:
         with:
           # Should be the higest supported version, so we can use the newest tools
           php-version: '8.4'
-          tools: composer, composer-require-checker, composer-unused, phpcs, psalm
-          # optional performance gain for psalm: opcache
-          extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, opcache, openssl, pcre, spl, xml
+          tools: composer, composer-require-checker, composer-unused, phpcs
+          extensions: ctype, date, dom, fileinfo, filter, hash, intl, mbstring, openssl, pcre, spl, xml
 
       - name: Setup problem matchers for PHP
         run: echo "::add-matcher::${{ runner.tool_cache }}/php.json"
@@ -196,27 +195,13 @@ jobs:
       - name: PHP Code Sniffer
         run: 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
 
-      - 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
 
   security:
     name: Security checks

composer.json

@@ -19,7 +19,8 @@
             "composer/package-versions-deprecated": true,
             "simplesamlphp/composer-module-installer": true,
             "dealerdirect/phpcodesniffer-composer-installer": true,
-            "phpstan/extension-installer": true
+            "phpstan/extension-installer": true,
+            "simplesamlphp/composer-xmlprovider-installer": true
         }
     },
     "autoload": {
@@ -35,7 +36,7 @@
     "require": {
         "php": "^8.1",
         "simplesamlphp/composer-module-installer": "^1.3.4",
-        "simplesamlphp/simplesamlphp": "^3@dev",
+        "simplesamlphp/simplesamlphp": "~2.4.0",
         "simplesamlphp/simplesamlphp-module-ldap": "~1.2",
         "symfony/http-foundation": "^6.4"
     },

phpstan-dev.neon

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

phpstan.neon

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

psalm-dev.xml

@@ -41,12 +41,12 @@ class CAS extends Auth\Source
     public const AUTHID = '\SimpleSAML\Module\cas\Auth\Source\CAS.AuthId';
 
     /**
-     * @var array with ldap configuration
+     * @var array<mixed> with ldap configuration
      */
     private array $ldapConfig;
 
     /**
-     * @var array cas configuration
+     * @var array<mixed> cas configuration
      */
     private array $casConfig;
 
@@ -64,8 +64,8 @@ class CAS extends Auth\Source
     /**
      * Constructor for this authentication source.
      *
-     * @param array $info  Information about this authentication source.
-     * @param array $config  Configuration.
+     * @param array<mixed> $info  Information about this authentication source.
+     * @param array<mixed> $config  Configuration.
      */
     public function __construct(array $info, array $config)
     {
@@ -105,7 +105,7 @@ public function __construct(array $info, array $config)
      * @param string $ticket
      * @param string $service
      *
-     * @return array username and attributes
+     * @return array<mixed> username and attributes
      */
     private function casValidate(string $ticket, string $service): array
     {
@@ -114,9 +114,11 @@ private function casValidate(string $ticket, string $service): array
             'ticket' => $ticket,
             'service' => $service,
         ]);
-        $result = $httpUtils->fetch($url);
 
         /** @var string $result */
+        $result = $httpUtils->fetch($url);
+
+        /** @var string $res */
         $res = preg_split("/\r?\n/", $result);
 
         if (strcmp($res[0], "yes") == 0) {
@@ -133,7 +135,7 @@ private function casValidate(string $ticket, string $service): array
      * @param string $ticket
      * @param string $service
      *
-     * @return array username and attributes
+     * @return array<mixed> username and attributes
      */
     private function casServiceValidate(string $ticket, string $service): array
     {
@@ -151,15 +153,17 @@ private function casServiceValidate(string $ticket, string $service): array
         $dom = DOMDocumentFactory::fromString($result);
         $xPath = new DOMXpath($dom);
         $xPath->registerNamespace("cas", 'http://www.yale.edu/tp/cas');
+
         $success = $xPath->query("/cas:serviceResponse/cas:authenticationSuccess/cas:user");
-        if ($success->length == 0) {
+        if ($success === false || $success->length === 0) {
             $failure = $xPath->evaluate("/cas:serviceResponse/cas:authenticationFailure");
             throw new Exception("Error when validating CAS service ticket: " . $failure->item(0)->textContent);
         } else {
             $attributes = [];
             if ($casattributes = $this->casConfig['attributes']) {
                 // Some has attributes in the xml - attributes is a list of XPath expressions to get them
                 foreach ($casattributes as $name => $query) {
+                    /** @var \DOMNodeList<\DOMNode> $attrs */
                     $attrs = $xPath->query($query);
                     foreach ($attrs as $attrvalue) {
                         $attributes[$name][] = $attrvalue->textContent;
@@ -184,7 +188,7 @@ private function casServiceValidate(string $ticket, string $service): array
      *
      * @param string $ticket
      * @param string $service
-     * @return array username and attributes
+     * @return array<mixed> username and attributes
      */
     protected function casValidation(string $ticket, string $service): array
     {
@@ -201,7 +205,7 @@ protected function casValidation(string $ticket, string $service): array
 
     /**
      * Called by linkback, to finish validate/ finish logging in.
-     * @param array $state
+     * @param array<mixed> $state
      */
     public function finalStep(array &$state): void
     {
@@ -237,7 +241,7 @@ public function finalStep(array &$state): void
     /**
      * Log-in using cas
      *
-     * @param array &$state  Information about the current authentication.
+     * @param array<mixed> &$state  Information about the current authentication.
      */
     public function authenticate(array &$state): void
     {
@@ -264,7 +268,7 @@ public function authenticate(array &$state): void
      * should be called with the state. If this operation can be completed without
      * showing the user a page, or redirecting, this function should return.
      *
-     * @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
     {

psalm.xml

@@ -84,14 +84,14 @@ public function linkback(Request $request): RunnableResponse
             throw new Error\BadRequest('Missing StateId parameter.');
         }
 
-        $stateId = $request->query->get('stateId');
+        $stateId = $request->query->getString('stateId');
         $state = $this->authState::loadState($stateId, CASSource::STAGE_INIT);
 
         if (!$request->query->has('ticket')) {
             throw new Error\BadRequest('Missing ticket parameter.');
         }
 
-        $ticket = $request->query->get('ticket');
+        $ticket = $request->query->getString('ticket');
         $state['cas:ticket'] = $ticket;
 
         // Find authentication source

src/Auth/Source/CAS.php

@@ -19,7 +19,7 @@
  *
  * @package SimpleSAML\Test
  */
-class CASTest extends TestCase
+final class CASTest extends TestCase
 {
     /** @var \SimpleSAML\Configuration */
     protected Configuration $config;
@@ -112,6 +112,7 @@ public function testNoTicket(): void
 
         $c = new Controller\CAS($this->config);
         $c->setAuthState(new class () extends Auth\State {
+            /** @return array<mixed>|null */
             public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
             {
                 return [];
@@ -141,6 +142,7 @@ public function testUnknownAuthSource(): void
 
         $c = new Controller\CAS($this->config);
         $c->setAuthState(new class () extends Auth\State {
+            /** @return array<mixed>|null */
             public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
             {
                 return [CAS::AUTHID => 'somethingElse'];
@@ -169,6 +171,7 @@ public function testNormalOperation(): void
 
         $c = new Controller\CAS($this->config);
         $c->setAuthState(new class () extends Auth\State {
+            /** @return array<mixed>|null */
             public static function loadState(string $id, string $stage, bool $allowMissing = false): ?array
             {
                 return [CAS::AUTHID => 'something'];
@@ -180,7 +183,10 @@ public function __construct()
                 //dummy
             }
 
-            public function authenticate(Request $request, array &$state): Response
+            /**
+             * @param array<mixed> $state
+             */
+            public function authenticate(array &$state): void
             {
                 //dummy
             }
@@ -192,6 +198,8 @@ public function __construct()
                     {
                         //dummy
                     }
+
+                    /** @param array<mixed> $state */
                     public function finalStep(array &$state): void
                     {
                         //dummy