From 16c9f1f9a54c49d243d3beda2949570f9c5c2dfb Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Wed, 24 Nov 2021 22:21:56 +0100
Subject: [PATCH] Fix several coding style issues; make phpcs/psalter mandatory
to pass
---
.github/workflows/php.yml | 4 +-
lib/SimpleSAML/Locale/Localization.php | 2 +-
phpcs.xml | 5 +
.../XHTML/TemplateTranslationTest.php | 94 +++++++++++++------
tests/modules/saml/lib/IdP/SAML2Test.php | 66 +++++++++----
5 files changed, 121 insertions(+), 50 deletions(-)
diff --git a/.github/workflows/php.yml b/.github/workflows/php.yml
index 3a855ad06..dcf6388c8 100644
--- a/.github/workflows/php.yml
+++ b/.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
diff --git a/lib/SimpleSAML/Locale/Localization.php b/lib/SimpleSAML/Locale/Localization.php
index 06b4ccff4..d88127b04 100644
--- a/lib/SimpleSAML/Locale/Localization.php
+++ b/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');
}
}
diff --git a/phpcs.xml b/phpcs.xml
index 328ad8127..5f39d24af 100644
--- a/phpcs.xml
+++ b/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"/>
diff --git a/tests/lib/SimpleSAML/XHTML/TemplateTranslationTest.php b/tests/lib/SimpleSAML/XHTML/TemplateTranslationTest.php
index d4c2db9e2..d94ef5626 100644
--- a/tests/lib/SimpleSAML/XHTML/TemplateTranslationTest.php
+++ b/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());
}
diff --git a/tests/modules/saml/lib/IdP/SAML2Test.php b/tests/modules/saml/lib/IdP/SAML2Test.php
index 1b017fe58..bc4aa2979 100644
--- a/tests/modules/saml/lib/IdP/SAML2Test.php
+++ b/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']);
--
GitLab