Fix several coding style issues; make phpcs/psalter mandatory to pass

.github/workflows/php.yml

@@ -194,7 +194,7 @@ jobs:
         uses: codecov/codecov-action@v1
 
       - name: PHP Code Sniffer
-        continue-on-error: true
+        continue-on-error: false
         run: php vendor/bin/phpcs
 
       - name: Psalm
@@ -202,5 +202,5 @@ jobs:
         run: php vendor/bin/psalm --show-info=true
 
       - name: Psalter
-        continue-on-error: true
+        continue-on-error: false
         run: php vendor/bin/psalter --issues=UnnecessaryVarAnnotation --dry-run

lib/SimpleSAML/Locale/Localization.php

@@ -269,7 +269,7 @@ class Localization
         $this->addDomain($this->localeDir, 'attributes');
 
         list($theme,) = explode(':', $this->configuration->getString('theme.use', 'default'));
-        if($theme !== 'default') {
+        if ($theme !== 'default') {
             $this->addModuleDomain($theme, null, 'attributes');
         }
     }

phpcs.xml

@@ -14,7 +14,12 @@
     <file>src</file>
     <file>tests</file>
     <file>www</file>
+
     <exclude-pattern>modules/adfs/*</exclude-pattern>
+    <exclude-pattern>www/assets/css/stylesheet.css</exclude-pattern>
+    <exclude-pattern>www/assets/js/bundle.js</exclude-pattern>
+    <exclude-pattern>www/assets/js/logout.js</exclude-pattern>
+    <exclude-pattern>www/assets/js/stylesheet.js</exclude-pattern>
 
     <!-- This is the rule we inherit from. If you want to exlude some specific rules, see the docs on how to do that -->
     <rule ref="PSR12"/>

tests/lib/SimpleSAML/XHTML/TemplateTranslationTest.php

@@ -13,6 +13,9 @@ use SimpleSAML\XHTML\Template;
 use Symfony\Bridge\Twig\Extension\TranslationExtension;
 use Symfony\Component\Finder\Finder;
 use Symfony\Component\Finder\SplFileInfo;
+use Twig\Environment;
+use Twig\Extra\Intl\IntlExtension;
+use Twig\Loader\FilesystemLoader;
 use Twig\TwigFilter;
 use Twig\TwigFunction;
 
@@ -21,7 +24,8 @@ use Twig\TwigFunction;
  */
 class TemplateTranslationTest extends TestCase
 {
-    public function testCoreCardinalityErrorTemplate(): void {
+    public function testCoreCardinalityErrorTemplate(): void
+    {
         $c = Configuration::loadFromArray([], '', 'simplesaml');
         $t = new Template($c, 'core:cardinality_error.twig');
 
@@ -30,18 +34,18 @@ class TemplateTranslationTest extends TestCase
             'test 2' => [1, 2],
         ];
 
-        $getContent = function() {
-            /** @var Template $this */
+        $getContent = function () {
+            /** @var \SimpleSAML\XHTML\Template $this */
             return $this->getContents();
         };
         $html = $getContent->call($t);
 
         $this->assertStringContainsString('got 0 values, want 1', $html);
         $this->assertStringContainsString('got 1 values, want 2', $html);
-
     }
 
-    public function testCoreLoginUserPassTemplate(): void {
+    public function testCoreLoginUserPassTemplate(): void
+    {
         $c = Configuration::loadFromArray([], '', 'simplesaml');
         $t = new Template($c, 'core:loginuserpass.twig');
 
@@ -53,8 +57,8 @@ class TemplateTranslationTest extends TestCase
         $t->data['rememberMeEnabled'] = false;
         $t->data['stateparams'] = [];
 
-        $getContent = function() {
-            /** @var Template $this */
+        $getContent = function () {
+            /** @var \SimpleSAML\XHTML\Template $this */
             return $this->getContents();
         };
         $html = $getContent->call($t);
@@ -62,7 +66,8 @@ class TemplateTranslationTest extends TestCase
         $this->assertStringContainsString('value="h.c oersted"', $html);
     }
 
-    public function testCoreLogoutIframeTemplate(): void {
+    public function testCoreLogoutIframeTemplate(): void
+    {
         $c = Configuration::loadFromArray([], '', 'simplesaml');
         $t = new Template($c, 'core:logout-iframe.twig');
 
@@ -86,8 +91,8 @@ class TemplateTranslationTest extends TestCase
             ],
         ];
 
-        $getContent = function() {
-            /** @var Template $this */
+        $getContent = function () {
+            /** @var \SimpleSAML\XHTML\Template $this */
             return $this->getContents();
         };
         $html = $getContent->call($t);
@@ -96,7 +101,8 @@ class TemplateTranslationTest extends TestCase
         $this->assertStringContainsString('ze missing service', $html);
     }
 
-    public function testAuthStatusTemplate(): void {
+    public function testAuthStatusTemplate(): void
+    {
         $c = Configuration::loadFromArray([], '', 'simplesaml');
         $t = new Template($c, 'auth_status.twig');
 
@@ -106,44 +112,78 @@ class TemplateTranslationTest extends TestCase
         $t->data['trackid'] = '';
         $t->data['authData'] = false;
 
-        $getContent = function() {
-            /** @var Template $this */
+        $getContent = function () {
+            /** @var \SimpleSAML\XHTML\Template $this */
             return $this->getContents();
         };
         $html = $getContent->call($t);
 
-        $this->assertStringContainsString('Your session is valid for ' . $t->data['remaining'] . ' seconds from now.', $html);
+        $this->assertStringContainsString(
+            'Your session is valid for ' . $t->data['remaining'] . ' seconds from now.',
+            $html
+        );
     }
 
-    public function testValidateTwigFiles()
+    public function testValidateTwigFiles(): void
     {
         $root = dirname(dirname((dirname(dirname(__DIR__)))));
 
         // Setup basic twig environment
-        $loader = new \Twig\Loader\FilesystemLoader(['templates', 'modules'], $root);
-        $twig = new \Twig\Environment($loader, ['cache' => false]);
+        $loader = new FilesystemLoader(['templates', 'modules'], $root);
+        $twig = new Environment($loader, ['cache' => false]);
 
         $twigTranslator = new TwigTranslator([Translate::class, 'translateSingularGettext']);
         $twig->addExtension(new TranslationExtension($twigTranslator));
-        $twig->addExtension(new \Twig\Extra\Intl\IntlExtension());
+        $twig->addExtension(new IntlExtension());
 
         // Fake functions
-        $twig->addFunction(new TwigFunction('asset', function() { return ''; }));
-        $twig->addFunction(new TwigFunction('moduleURL', function() { return ''; }));
+        $twig->addFunction(
+            new TwigFunction(
+                'asset',
+                function () {
+                    return '';
+                }
+            )
+        );
+        $twig->addFunction(
+            new TwigFunction(
+                'moduleURL',
+                function () {
+                    return '';
+                }
+            )
+        );
 
         // Fake filters
-        $twig->addFilter(new TwigFilter('translateFromArray', function() { return ''; }, ['needs_context' => true]));
-        $twig->addFilter(new TwigFilter('entityDisplayName', function() { return ''; }));
+        $twig->addFilter(
+            new TwigFilter(
+                'translateFromArray',
+                function () {
+                    return '';
+                },
+                ['needs_context' => true]
+            )
+        );
+        $twig->addFilter(
+            new TwigFilter(
+                'entityDisplayName',
+                function () {
+                    return '';
+                }
+            )
+        );
 
         $files = Finder::create()
             ->name('*.twig')
-            ->in([
-                $root . '/templates',
-                $root . '/modules'
-            ]);
+            ->in(
+                [
+                    $root . '/templates',
+                    $root . '/modules'
+                ]
+            );
 
         foreach ($files as $file) {
-            /** @var SplFileInfo $file */
+            /** @var \Symfony\Component\Finder\SplFileInfo $file */
             $twig->load($file->getRelativePathname());
         }
 

tests/modules/saml/lib/IdP/SAML2Test.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace SimpleSAML\Test\Module\saml\IdP;
 
 use InvalidArgumentException;
+use SAML2\XML\Chunk;
 use SimpleSAML\Configuration;
 use SimpleSAML\Error\Exception;
 use SimpleSAML\IdP;
@@ -288,8 +289,13 @@ EOT;
         $this->assertArrayHasKey('SingleLogoutService', $hostedMd);
         $this->assertIsArray($hostedMd['SingleLogoutService']);
         $this->assertCount(1, $hostedMd['SingleLogoutService']);
-        $this->assertEquals(['Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect', 
-            'Location' => 'http://localhost/simplesaml/saml2/idp/SingleLogoutService.php'], $hostedMd['SingleLogoutService'][0]);
+        $this->assertEquals(
+            [
+                'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
+                'Location' => 'http://localhost/simplesaml/saml2/idp/SingleLogoutService.php'
+            ],
+            $hostedMd['SingleLogoutService'][0]
+        );
 
         $this->assertArrayHasKey('keys', $hostedMd);
         $this->assertIsArray($hostedMd['keys']);
@@ -349,8 +355,14 @@ EOT;
         $this->assertArrayHasKey('ArtifactResolutionService', $hostedMd);
         $this->assertIsArray($hostedMd['ArtifactResolutionService']);
         $this->assertCount(1, $hostedMd['ArtifactResolutionService']);
-        $this->assertEquals(['Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:SOAP', 'index' => 0,
-           'Location' => 'http://localhost/simplesaml/saml2/idp/ArtifactResolutionService.php'], $hostedMd['ArtifactResolutionService'][0]);
+        $this->assertEquals(
+            [
+                'Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:SOAP',
+                'index' => 0,
+                'Location' => 'http://localhost/simplesaml/saml2/idp/ArtifactResolutionService.php'
+            ],
+            $hostedMd['ArtifactResolutionService'][0]
+        );
     }
 
     public function testIdPGetHostedMetadataHolderOfKey(): void
@@ -361,9 +373,14 @@ EOT;
         $this->assertArrayHasKey('SingleSignOnService', $hostedMd);
         $this->assertIsArray($hostedMd['SingleSignOnService']);
         $this->assertCount(2, $hostedMd['SingleSignOnService']);
-        $this->assertEquals(['Binding' => 'urn:oasis:names:tc:SAML:2.0:profiles:holder-of-key:SSO:browser',
-            'Location' => 'http://localhost/simplesaml/saml2/idp/SSOService.php',
-            'hoksso:ProtocolBinding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect'], $hostedMd['SingleSignOnService'][0]);
+        $this->assertEquals(
+            [
+                'Binding' => 'urn:oasis:names:tc:SAML:2.0:profiles:holder-of-key:SSO:browser',
+                'Location' => 'http://localhost/simplesaml/saml2/idp/SSOService.php',
+                'hoksso:ProtocolBinding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect'
+            ],
+            $hostedMd['SingleSignOnService'][0]
+        );
         $this->assertEquals(['Binding' => 'urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect',
            'Location' => 'http://localhost/simplesaml/saml2/idp/SSOService.php'], $hostedMd['SingleSignOnService'][1]);
     }
@@ -527,19 +544,26 @@ EOT;
     {
         $dom = \SAML2\DOMDocumentFactory::create();
         $republishRequest = $dom->createElementNS('http://eduid.cz/schema/metadata/1.0', 'eduidmd:RepublishRequest');
-        $republishTarget = $dom->createElementNS('http://eduid.cz/schema/metadata/1.0', 'eduidmd:RepublishTarget', 'http://edugain.org/');
+        $republishTarget = $dom->createElementNS(
+            'http://eduid.cz/schema/metadata/1.0',
+            'eduidmd:RepublishTarget',
+            'http://edugain.org/'
+        );
         $republishRequest->appendChild($republishTarget);
-        $ext = [new \SAML2\XML\Chunk($republishRequest)];
+        $ext = [new Chunk($republishRequest)];
 
         $config = [
             'saml:Extensions' => $ext,
-            ];
+        ];
         $md = $this->idpMetadataHandlerHelper($config);
 
         $this->assertArrayHasKey('saml:Extensions', $md);
         $this->assertCount(1, $md['saml:Extensions']);
-        $this->assertInstanceOf(\SAML2\XML\Chunk::class, $md['saml:Extensions'][0]);
-        $this->assertEquals('http://edugain.org/', $md['saml:Extensions'][0]->getXML()->firstChild->firstChild->textContent);
+        $this->assertInstanceOf(Chunk::class, $md['saml:Extensions'][0]);
+        $this->assertEquals(
+            'http://edugain.org/',
+            $md['saml:Extensions'][0]->getXML()->firstChild->firstChild->textContent
+        );
     }
 
     /**
@@ -723,15 +747,17 @@ EOT;
         $globalConfig = [
             'technicalcontact_email' => 'na@example.org',
             'technicalcontact_name' => 'Someone von Somewhere',
-	];
+        ];
 
-        $config = ['contacts' => [
-            [
-               'contactType'       => 'technical',
-               'emailAddress'      => 'j.doe@example.edu',
-               'surName'           => 'Doe',
-            ],
-        ]];
+        $config = [
+            'contacts' => [
+                [
+                    'contactType'       => 'technical',
+                    'emailAddress'      => 'j.doe@example.edu',
+                    'surName'           => 'Doe',
+                ],
+            ]
+        ];
         $md = $this->idpMetadataHandlerHelper($config, $globalConfig);
 
         $this->assertCount(1, $md['contacts']);