From 6db0595b225e7b4aa36cfa47903cd3919e0c15c9 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 17 Oct 2022 06:16:05 +0700 Subject: [PATCH 1/4] Apply PHP 8.0 Syntax and constructor promotion Signed-off-by: Abdul Malik Ikhsan --- psalm-baseline.xml | 3 +-- src/Acl.php | 2 +- src/Assertion/AssertionAggregate.php | 2 +- src/Assertion/AssertionManager.php | 3 +-- src/Assertion/ExpressionAssertion.php | 26 ++++++------------------ src/Resource/GenericResource.php | 8 ++++---- src/Role/GenericRole.php | 8 ++++---- test/AclTest.php | 8 ++++---- test/Assertion/CallbackAssertionTest.php | 14 +++++-------- test/TestAsset/MockAssertion.php | 5 +---- 10 files changed, 28 insertions(+), 51 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 597af25..235c3fb 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -474,8 +474,7 @@ $value - function ($aclArg, $roleArg, $resourceArg, $privilegeArg) use ($value) { - function ($value) { + static fn($value) function () { diff --git a/src/Acl.php b/src/Acl.php index 36d7d9f..0409297 100644 --- a/src/Acl.php +++ b/src/Acl.php @@ -748,7 +748,7 @@ public function isAllowed($role = null, $resource = null, $privilege = null) // query on all privileges do { // depth-first search on $role if it is not 'allRoles' pseudo-parent - if (null !== $role && null !== ($result = $this->roleDFSAllPrivileges($role, $resource, $privilege))) { + if (null !== $role && null !== ($result = $this->roleDFSAllPrivileges($role, $resource))) { return $result; } diff --git a/src/Assertion/AssertionAggregate.php b/src/Assertion/AssertionAggregate.php index 2f2c7a6..7e56ae9 100644 --- a/src/Assertion/AssertionAggregate.php +++ b/src/Assertion/AssertionAggregate.php @@ -143,7 +143,7 @@ public function assert( if ($manager = $this->getAssertionManager()) { try { $assertion = $manager->get($assertion); - } catch (Exception $e) { + } catch (Exception) { throw new InvalidAssertionException( 'assertion "' . $assertion diff --git a/src/Assertion/AssertionManager.php b/src/Assertion/AssertionManager.php index f7ae28f..91843c5 100644 --- a/src/Assertion/AssertionManager.php +++ b/src/Assertion/AssertionManager.php @@ -46,11 +46,10 @@ public function validate($instance) * * @deprecated Please use {@see AssertionManager::validate()} instead. * - * @param mixed $instance * @throws InvalidArgumentException * @psalm-assert AssertionInterface $instance */ - public function validatePlugin($instance) + public function validatePlugin(mixed $instance) { try { $this->validate($instance); diff --git a/src/Assertion/ExpressionAssertion.php b/src/Assertion/ExpressionAssertion.php index b0aa4d9..90a4882 100644 --- a/src/Assertion/ExpressionAssertion.php +++ b/src/Assertion/ExpressionAssertion.php @@ -22,8 +22,8 @@ use function preg_match; use function property_exists; use function sprintf; +use function str_contains; use function str_replace; -use function strpos; use function strtolower; use function ucwords; @@ -68,7 +68,7 @@ final class ExpressionAssertion implements AssertionInterface public const OPERATOR_NSAME = '!=='; /** @var list */ - private static $validOperators = [ + private static array $validOperators = [ self::OPERATOR_EQ, self::OPERATOR_NEQ, self::OPERATOR_LT, @@ -83,15 +83,6 @@ final class ExpressionAssertion implements AssertionInterface self::OPERATOR_NSAME, ]; - /** @var mixed */ - private $left; - - /** @var string */ - private $operator; - - /** @var mixed */ - private $right; - /** * Constructor * @@ -102,11 +93,8 @@ final class ExpressionAssertion implements AssertionInterface * @param string $operator One of the OPERATOR constants (or their values) * @param mixed|array $right See the class description for valid values. */ - private function __construct($left, $operator, $right) + private function __construct(private $left, private $operator, private $right) { - $this->left = $left; - $this->operator = $operator; - $this->right = $right; } /** @@ -247,7 +235,7 @@ private function resolveOperandValue($operand, array $context) $contextProperty = $operand[self::OPERAND_CONTEXT_PROPERTY]; - if (strpos($contextProperty, '.') !== false) { // property path? + if (str_contains($contextProperty, '.')) { // property path? [$objectName, $objectField] = explode('.', $contextProperty, 2); return $this->getObjectFieldValue($context, $objectName, $objectField); } @@ -282,7 +270,7 @@ private function getObjectFieldValue(array $context, $objectName, $field) $object = $context[$objectName]; $accessors = ['get', 'is']; - $fieldAccessor = false === strpos($field, '_') + $fieldAccessor = ! str_contains($field, '_') ? $field : str_replace(' ', '', ucwords(str_replace('_', ' ', $field))); @@ -306,13 +294,11 @@ private function getObjectFieldValue(array $context, $objectName, $field) } /** - * @param mixed $left * @param string $operator - * @param mixed $right * @return bool|void * @throws RuntimeException If operand is not supported. */ - private static function evaluateExpression($left, $operator, $right) + private static function evaluateExpression(mixed $left, $operator, mixed $right) { // phpcs:disable SlevomatCodingStandard.Operators.DisallowEqualOperators switch ($operator) { diff --git a/src/Resource/GenericResource.php b/src/Resource/GenericResource.php index 19f9a6d..19193bb 100644 --- a/src/Resource/GenericResource.php +++ b/src/Resource/GenericResource.php @@ -4,7 +4,9 @@ namespace Laminas\Permissions\Acl\Resource; -class GenericResource implements ResourceInterface +use Stringable; + +class GenericResource implements ResourceInterface, Stringable { /** * Unique id of Resource @@ -36,10 +38,8 @@ public function getResourceId() /** * Defined by ResourceInterface; returns the Resource identifier * Proxies to getResourceId() - * - * @return string */ - public function __toString() + public function __toString(): string { return $this->getResourceId(); } diff --git a/src/Role/GenericRole.php b/src/Role/GenericRole.php index 2309a5f..0bd8979 100644 --- a/src/Role/GenericRole.php +++ b/src/Role/GenericRole.php @@ -4,7 +4,9 @@ namespace Laminas\Permissions\Acl\Role; -class GenericRole implements RoleInterface +use Stringable; + +class GenericRole implements RoleInterface, Stringable { /** * Unique id of Role @@ -36,10 +38,8 @@ public function getRoleId() /** * Defined by RoleInterface; returns the Role identifier * Proxies to getRoleId() - * - * @return string */ - public function __toString() + public function __toString(): string { return $this->getRoleId(); } diff --git a/test/AclTest.php b/test/AclTest.php index d587c8a..ef1a6dd 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -76,7 +76,7 @@ public function testRoleRegistryRemoveOne(): void */ public function testRoleRegistryRemoveOneNonExistent(): void { - $this->expectException(InvalidArgumentException::class, 'not found'); + $this->expectException(InvalidArgumentException::class); $this->acl->removeRole('nonexistent'); } @@ -235,7 +235,7 @@ public function testRoleRegistryDuplicate(): void { $roleGuest = new GenericRole('guest'); $roleRegistry = new Role\Registry(); - $this->expectException(InvalidArgumentException::class, 'already exists'); + $this->expectException(InvalidArgumentException::class); $roleRegistry ->add($roleGuest) ->add($roleGuest); @@ -249,7 +249,7 @@ public function testRoleRegistryDuplicateId(): void $roleGuest1 = new GenericRole('guest'); $roleGuest2 = new GenericRole('guest'); $roleRegistry = new Role\Registry(); - $this->expectException(InvalidArgumentException::class, 'already exists'); + $this->expectException(InvalidArgumentException::class); $roleRegistry ->add($roleGuest1) ->add($roleGuest2); @@ -1208,7 +1208,7 @@ public function testAclPassesPrivilegeToAssertClass(): void $acl->addRole('role'); $acl->addResource('resource'); $acl->allow('role', null, null, $assertion); - $allowed = $acl->isAllowed('role', 'resource', 'privilege', $assertion); + $allowed = $acl->isAllowed('role', 'resource', 'privilege'); $this->assertTrue($allowed); } diff --git a/test/Assertion/CallbackAssertionTest.php b/test/Assertion/CallbackAssertionTest.php index 89b5c5c..3a9cb85 100644 --- a/test/Assertion/CallbackAssertionTest.php +++ b/test/Assertion/CallbackAssertionTest.php @@ -4,6 +4,7 @@ namespace LaminasTest\Permissions\Acl\Assertion; +use Closure; use Laminas\Permissions\Acl; use Laminas\Permissions\Acl\Exception\InvalidArgumentException; use Laminas\Permissions\Acl\Resource\ResourceInterface; @@ -19,8 +20,7 @@ class CallbackAssertionTest extends TestCase public function testConstructorThrowsExceptionIfNotCallable(): void { $this->expectException( - InvalidArgumentException::class, - 'Invalid callback provided; not callable' + InvalidArgumentException::class ); new CallbackAssertion('I am not callable!'); } @@ -30,7 +30,7 @@ public function testConstructorThrowsExceptionIfNotCallable(): void */ public function testCallbackIsSet(): void { - $callback = function () { + $callback = static function (): void { }; $assert = new CallbackAssertion($callback); $this->assertSame($callback, $assert->peakCallback()); @@ -44,7 +44,7 @@ public function testAssertMethodPassArgsToCallback(): void $acl = new Acl\Acl(); $that = $this; $assert = new CallbackAssertion( - function ($aclArg, $roleArg, $resourceArg, $privilegeArg) use ($that, $acl) { + static function ($aclArg, $roleArg, $resourceArg, $privilegeArg) use ($that, $acl): bool { $that->assertSame($acl, $aclArg); $that->assertInstanceOf(RoleInterface::class, $roleArg); $that->assertEquals('guest', $roleArg->getRoleId()); @@ -68,11 +68,7 @@ public function testAssertMethod(): void { $acl = new Acl\Acl(); $roleGuest = new Acl\Role\GenericRole('guest'); - $assertMock = function ($value) { - return function ($aclArg, $roleArg, $resourceArg, $privilegeArg) use ($value) { - return $value; - }; - }; + $assertMock = static fn($value) => static fn($aclArg, $roleArg, $resourceArg, $privilegeArg) => $value; $acl->addRole($roleGuest); $acl->allow($roleGuest, null, 'somePrivilege', new CallbackAssertion($assertMock(true))); $this->assertTrue($acl->isAllowed($roleGuest, null, 'somePrivilege')); diff --git a/test/TestAsset/MockAssertion.php b/test/TestAsset/MockAssertion.php index 45f82dd..d90f143 100644 --- a/test/TestAsset/MockAssertion.php +++ b/test/TestAsset/MockAssertion.php @@ -8,11 +8,8 @@ class MockAssertion implements Acl\Assertion\AssertionInterface { - protected bool $returnValue; - - public function __construct(bool $returnValue) + public function __construct(protected bool $returnValue) { - $this->returnValue = $returnValue; } /** @inheritDoc */ From 38ee8674b1eef4e5c38bb469d157d4cd2652e5d4 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 17 Oct 2022 06:18:40 +0700 Subject: [PATCH 2/4] extract exception and exceptionMessage on CallbackAssertionTest Signed-off-by: Abdul Malik Ikhsan --- test/Assertion/CallbackAssertionTest.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Assertion/CallbackAssertionTest.php b/test/Assertion/CallbackAssertionTest.php index 3a9cb85..9a6a61c 100644 --- a/test/Assertion/CallbackAssertionTest.php +++ b/test/Assertion/CallbackAssertionTest.php @@ -19,9 +19,8 @@ class CallbackAssertionTest extends TestCase */ public function testConstructorThrowsExceptionIfNotCallable(): void { - $this->expectException( - InvalidArgumentException::class - ); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid callback provided; not callable'); new CallbackAssertion('I am not callable!'); } From 7bc5c3aa18b7208697b1f958af8b655b24832021 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 17 Oct 2022 06:21:27 +0700 Subject: [PATCH 3/4] extract exception and exceptionMessage on AclTest Signed-off-by: Abdul Malik Ikhsan --- test/AclTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/AclTest.php b/test/AclTest.php index ef1a6dd..8d6029b 100644 --- a/test/AclTest.php +++ b/test/AclTest.php @@ -77,6 +77,7 @@ public function testRoleRegistryRemoveOne(): void public function testRoleRegistryRemoveOneNonExistent(): void { $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('not found'); $this->acl->removeRole('nonexistent'); } @@ -236,6 +237,7 @@ public function testRoleRegistryDuplicate(): void $roleGuest = new GenericRole('guest'); $roleRegistry = new Role\Registry(); $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('already exists'); $roleRegistry ->add($roleGuest) ->add($roleGuest); @@ -250,6 +252,7 @@ public function testRoleRegistryDuplicateId(): void $roleGuest2 = new GenericRole('guest'); $roleRegistry = new Role\Registry(); $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('already exists'); $roleRegistry ->add($roleGuest1) ->add($roleGuest2); From e63af47cfa0b7a0b49bf23930917bb5ed5d5a594 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Mon, 17 Oct 2022 06:23:28 +0700 Subject: [PATCH 4/4] cs fix Signed-off-by: Abdul Malik Ikhsan --- test/Assertion/CallbackAssertionTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/test/Assertion/CallbackAssertionTest.php b/test/Assertion/CallbackAssertionTest.php index 9a6a61c..666c462 100644 --- a/test/Assertion/CallbackAssertionTest.php +++ b/test/Assertion/CallbackAssertionTest.php @@ -4,7 +4,6 @@ namespace LaminasTest\Permissions\Acl\Assertion; -use Closure; use Laminas\Permissions\Acl; use Laminas\Permissions\Acl\Exception\InvalidArgumentException; use Laminas\Permissions\Acl\Resource\ResourceInterface;