From 90f460b8c71c171aa7d87095c5a1e9bf8abaddf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Mon, 27 Jun 2022 20:24:50 +0200 Subject: [PATCH 1/2] Provide atomic interface for cache helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This introduces an alternative method for the cache helper that allows reducing the public API and removes the need for coordinating method calls. It also reduces the amount of method calls needed to use the object, which is usually irrelevant for small projects but should show improvements when scanning several files. Signed-off-by: Luís Cobucci --- src/Helpers/Cache.php | 17 +++++++++++++++++ src/Rules/AbstractFriendRule.php | 24 ++++++++++-------------- src/Rules/AbstractPackageRule.php | 27 ++++++++++----------------- src/Rules/AbstractTestTagRule.php | 17 ++++++----------- src/Rules/InjectableVersionRule.php | 7 +------ tests/Unit/CacheTest.php | 17 +++++++++++++++++ 6 files changed, 61 insertions(+), 48 deletions(-) diff --git a/src/Helpers/Cache.php b/src/Helpers/Cache.php index 0678120..7b79ba3 100644 --- a/src/Helpers/Cache.php +++ b/src/Helpers/Cache.php @@ -4,6 +4,9 @@ namespace DaveLiddament\PhpstanPhpLanguageExtensions\Helpers; +use function array_key_exists; +use Closure; + /** * @template TValue */ @@ -32,4 +35,18 @@ public function addEntry(string $key, $entry): void { $this->cache[$key] = $entry; } + + /** + * @param Closure():TValue $initializer + * + * @return TValue + */ + public function get(string $key, Closure $initializer): mixed + { + if (!array_key_exists($key, $this->cache)) { + $this->cache[$key] = $initializer(); + } + + return $this->cache[$key]; + } } diff --git a/src/Rules/AbstractFriendRule.php b/src/Rules/AbstractFriendRule.php index f619a57..4f891b0 100644 --- a/src/Rules/AbstractFriendRule.php +++ b/src/Rules/AbstractFriendRule.php @@ -45,26 +45,22 @@ protected function getErrorOrNull( $className = $classReflection->getName(); $fullMethodName = "{$className}::{$methodName}"; - if ($this->cache->hasEntry($fullMethodName)) { - $allowedCallingClassesFromMethod = $this->cache->getEntry($fullMethodName); - } else { - $allowedCallingClassesFromMethod = AttributeValueReader::getAttributeValuesForMethod( + $allowedCallingClassesFromMethod = $this->cache->get( + $fullMethodName, + static fn (): array => AttributeValueReader::getAttributeValuesForMethod( $classReflection->getNativeReflection(), $methodName, Friend::class - ); - $this->cache->addEntry($fullMethodName, $allowedCallingClassesFromMethod); - } + ) + ); - if ($this->cache->hasEntry($className)) { - $allowedCallingClassesFromClass = $this->cache->getEntry($className); - } else { - $allowedCallingClassesFromClass = AttributeValueReader::getAttributeValuesForClass( + $allowedCallingClassesFromClass = $this->cache->get( + $className, + static fn (): array => AttributeValueReader::getAttributeValuesForClass( $classReflection->getNativeReflection(), Friend::class - ); - $this->cache->addEntry($className, $allowedCallingClassesFromClass); - } + ) + ); $allowedCallingClasses = array_merge($allowedCallingClassesFromClass, $allowedCallingClassesFromMethod); diff --git a/src/Rules/AbstractPackageRule.php b/src/Rules/AbstractPackageRule.php index bdda039..e457416 100644 --- a/src/Rules/AbstractPackageRule.php +++ b/src/Rules/AbstractPackageRule.php @@ -4,6 +4,7 @@ namespace DaveLiddament\PhpstanPhpLanguageExtensions\Rules; +use function count; use DaveLiddament\PhpLanguageExtensions\Package; use DaveLiddament\PhpstanPhpLanguageExtensions\Config\TestConfig; use DaveLiddament\PhpstanPhpLanguageExtensions\Helpers\Cache; @@ -45,24 +46,16 @@ protected function getErrorOrNull( $fullMethodName = "{$className}::{$methodName}"; - if ($this->cache->hasEntry($fullMethodName)) { - $isMethodPackageLevel = $this->cache->getEntry($fullMethodName); - } else { - if ($nativeReflection->hasMethod($methodName)) { - $methodReflection = $nativeReflection->getMethod($methodName); - $isMethodPackageLevel = count($methodReflection->getAttributes(Package::class)) > 0; - } else { - $isMethodPackageLevel = false; - } - $this->cache->addEntry($fullMethodName, $isMethodPackageLevel); - } + $isMethodPackageLevel = $this->cache->get( + $fullMethodName, + static fn (): bool => $nativeReflection->hasMethod($methodName) + && count($nativeReflection->getMethod($methodName)->getAttributes(Package::class)) > 0 + ); - if ($this->cache->hasEntry($className)) { - $isClassPackageLevel = $this->cache->getEntry($className); - } else { - $isClassPackageLevel = count($nativeReflection->getAttributes(Package::class)) > 0; - $this->cache->addEntry($className, $isClassPackageLevel); - } + $isClassPackageLevel = $this->cache->get( + $className, + static fn (): bool => count($nativeReflection->getAttributes(Package::class)) > 0 + ); $isPackageLevel = $isClassPackageLevel || $isMethodPackageLevel; diff --git a/src/Rules/AbstractTestTagRule.php b/src/Rules/AbstractTestTagRule.php index 85e5bee..76b86dc 100644 --- a/src/Rules/AbstractTestTagRule.php +++ b/src/Rules/AbstractTestTagRule.php @@ -4,6 +4,7 @@ namespace DaveLiddament\PhpstanPhpLanguageExtensions\Rules; +use function count; use DaveLiddament\PhpLanguageExtensions\TestTag; use DaveLiddament\PhpstanPhpLanguageExtensions\Config\TestConfig; use DaveLiddament\PhpstanPhpLanguageExtensions\Helpers\Cache; @@ -45,17 +46,11 @@ protected function getErrorOrNull( $fullMethodName = "{$className}::{$methodName}"; - if ($this->cache->hasEntry($fullMethodName)) { - $isTestTag = $this->cache->getEntry($fullMethodName); - } else { - if ($nativeReflection->hasMethod($methodName)) { - $methodReflection = $nativeReflection->getMethod($methodName); - $isTestTag = count($methodReflection->getAttributes(TestTag::class)) > 0; - } else { - $isTestTag = false; - } - $this->cache->addEntry($fullMethodName, $isTestTag); - } + $isTestTag = $this->cache->get( + $fullMethodName, + static fn (): bool => $nativeReflection->hasMethod($methodName) + && count($nativeReflection->getMethod($methodName)->getAttributes(TestTag::class)) > 0 + ); if (!$isTestTag) { return null; diff --git a/src/Rules/InjectableVersionRule.php b/src/Rules/InjectableVersionRule.php index e15b331..0805b1c 100644 --- a/src/Rules/InjectableVersionRule.php +++ b/src/Rules/InjectableVersionRule.php @@ -73,12 +73,7 @@ public function processNode(Node $node, Scope $scope): array : $type->getReferencedClasses(); foreach ($classesToCheck as $className) { - if ($this->cache->hasEntry($className)) { - $classToUse = $this->cache->getEntry($className); - } else { - $classToUse = $this->checkClass($className); - $this->cache->addEntry($className, $classToUse); - } + $classToUse = $this->cache->get($className, fn () => $this->checkClass($className)); if (null !== $classToUse) { $message = sprintf( diff --git a/tests/Unit/CacheTest.php b/tests/Unit/CacheTest.php index c40b37c..74a521f 100644 --- a/tests/Unit/CacheTest.php +++ b/tests/Unit/CacheTest.php @@ -39,4 +39,21 @@ public function testAccessMissingEntry(): void $this->expectException(\LogicException::class); $this->cache->getEntry(self::ENTRY_1); } + + public function testEntriesShouldOnlyBeInitializedOnce(): void + { + $initializationCount = 0; + + $initializer = static function () use (&$initializationCount): string { + ++$initializationCount; + + return self::VALUE_1; + }; + + self::assertSame(self::VALUE_1, $this->cache->get(self::ENTRY_1, $initializer)); + self::assertSame(1, $initializationCount); + + self::assertSame(self::VALUE_1, $this->cache->get(self::ENTRY_1, $initializer)); + self::assertSame(1, $initializationCount); + } } From 8d6e83c7d20aa3d4ba5948c485337fb5fc6a3e38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= Date: Mon, 27 Jun 2022 20:34:24 +0200 Subject: [PATCH 2/2] Streamline cache's public API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This removes unused methods and their tests. Signed-off-by: Luís Cobucci --- src/Helpers/Cache.php | 23 ----------------------- tests/Unit/CacheTest.php | 18 ------------------ 2 files changed, 41 deletions(-) diff --git a/src/Helpers/Cache.php b/src/Helpers/Cache.php index 7b79ba3..6d311ee 100644 --- a/src/Helpers/Cache.php +++ b/src/Helpers/Cache.php @@ -15,30 +15,7 @@ class Cache /** @var array */ private array $cache = []; - public function hasEntry(string $key): bool - { - return array_key_exists($key, $this->cache); - } - - /** @return TValue */ - public function getEntry(string $key) - { - if (!array_key_exists($key, $this->cache)) { - throw new \LogicException('Call hasEntry first'); - } - - return $this->cache[$key]; - } - - /** @param TValue $entry */ - public function addEntry(string $key, $entry): void - { - $this->cache[$key] = $entry; - } - /** - * @param Closure():TValue $initializer - * * @return TValue */ public function get(string $key, Closure $initializer): mixed diff --git a/tests/Unit/CacheTest.php b/tests/Unit/CacheTest.php index 74a521f..a92708c 100644 --- a/tests/Unit/CacheTest.php +++ b/tests/Unit/CacheTest.php @@ -22,24 +22,6 @@ protected function setup(): void $this->cache = new Cache(); } - public function testEmptyCache(): void - { - $this->assertFalse($this->cache->hasEntry(self::ENTRY_1)); - } - - public function testAddValueToCache(): void - { - $this->cache->addEntry(self::ENTRY_1, self::VALUE_1); - $this->assertTrue($this->cache->hasEntry(self::ENTRY_1)); - $this->assertSame(self::VALUE_1, $this->cache->getEntry(self::ENTRY_1)); - } - - public function testAccessMissingEntry(): void - { - $this->expectException(\LogicException::class); - $this->cache->getEntry(self::ENTRY_1); - } - public function testEntriesShouldOnlyBeInitializedOnce(): void { $initializationCount = 0;