refactor: add types, misc minor refactoring/cleanup

Attribute/RateLimit.php

@@ -2,24 +2,25 @@
 
 namespace Noxlogic\RateLimitBundle\Attribute;
 
+use Symfony\Component\HttpFoundation\Request;
+
 #[\Attribute(\Attribute::IS_REPEATABLE |\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD)]
 final class RateLimit
 {
-    /**
-     * @var array HTTP Methods protected by this attribute. Defaults to all method
-     */
-    public array $methods = [];
-
     public function __construct(
-        $methods = [],
+        /**
+         * @var array<Request::METHOD_*> $methods HTTP Methods protected by this attribute. Defaults to all methods
+         *                                        Passing strings is allowed for backward-compatibility but deprecated. Pass an array instead
+         */
+        public array|string $methods = [],
 
         /**
-         * @var int Number of calls per period
+         * @var int<-1, max> Number of calls per period
          */
         public int $limit = -1,
 
         /**
-         * @var int Number of seconds of the time period in which the calls can be made
+         * @var positive-int Number of seconds of the time period in which the calls can be made
          */
         public int $period = 3600,
 
@@ -31,36 +32,52 @@ public function __construct(
         // @RateLimit annotation used to support single method passed as string, keep that for retrocompatibility
         if (!is_array($methods)) {
             $this->methods = [$methods];
-        } else {
-            $this->methods = $methods;
         }
     }
 
+    /**
+     * @return int<-1, max>
+     */
     public function getLimit(): int
     {
         return $this->limit;
     }
 
+    /**
+     * @param int<-1, max> $limit
+     */
     public function setLimit(int $limit): void
     {
         $this->limit = $limit;
     }
 
+    /**
+     * @return array<Request::METHOD_*>
+     */
     public function getMethods(): array
     {
         return $this->methods;
     }
 
-    public function setMethods($methods): void
+    /**
+     * @param array<Request::METHOD_*> $methods Passing strings is allowed for backward-compatibility but deprecated. Pass an array instead
+     */
+    public function setMethods(array|string $methods): void
     {
         $this->methods = (array) $methods;
     }
 
+    /**
+     * @return positive-int
+     */
     public function getPeriod(): int
     {
         return $this->period;
     }
 
+    /**
+     * @param positive-int $period
+     */
     public function setPeriod(int $period): void
     {
         $this->period = $period;

DependencyInjection/Configuration.php

@@ -2,13 +2,13 @@
 
 namespace Noxlogic\RateLimitBundle\DependencyInjection;
 
+use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 
 /**
  * This is the class that validates and merges configuration from your app/config files
- *
  */
 class Configuration implements ConfigurationInterface
 {
@@ -22,7 +22,11 @@ public function getConfigTreeBuilder(): TreeBuilder
         $treeBuilder = new TreeBuilder('noxlogic_rate_limit');
         $rootNode = $treeBuilder->getRootNode();
 
-        $rootNode // @phpstan-ignore method.notFound
+        if (!$rootNode instanceof ArrayNodeDefinition) {
+            throw new \InvalidArgumentException('The "noxlogic_rate_limit" config root node must be an array');
+        }
+
+        $rootNode // @phpstan-ignore-line
             ->canBeDisabled()
             ->children()
                 ->enumNode('storage_engine')

EventListener/BaseListener.php

@@ -10,22 +10,13 @@ abstract class BaseListener
      */
     protected $parameters;
 
-    /**
-     * @param $name
-     * @param $value
-     */
-    public function setParameter($name, $value)
+    public function setParameter(string $name, mixed $value): void
     {
         $this->parameters[$name] = $value;
     }
 
-    /**
-     * @param  $name
-     * @param  mixed $default
-     * @return mixed
-     */
-    public function getParameter($name, $default = null)
+    public function getParameter(string $name, mixed $default = null): mixed
     {
-        return isset($this->parameters[$name]) ? $this->parameters[$name] : $default;
+        return $this->parameters[$name] ?? $default;
     }
 }

EventListener/HeaderModificationListener.php

@@ -7,19 +7,12 @@
 
 class HeaderModificationListener extends BaseListener
 {
-
-    /**
-     * @param array $defaultParameters
-     */
-    public function __construct($defaultParameters = array())
+    public function __construct(array $defaultParameters = [])
     {
         $this->parameters = $defaultParameters;
     }
 
-    /**
-     * @param ResponseEvent $event
-     */
-    public function onKernelResponse($event)
+    public function onKernelResponse(ResponseEvent $event): void
     {
         $request = $event->getRequest();
 

EventListener/RateLimitAnnotationListener.php

@@ -17,26 +17,17 @@
 
 class RateLimitAnnotationListener extends BaseListener
 {
-    protected EventDispatcherInterface $eventDispatcher;
-
-    protected RateLimitService $rateLimitService;
-
-    protected PathLimitProcessor $pathLimitProcessor;
-
     public function __construct(
-        EventDispatcherInterface $eventDispatcher,
-        RateLimitService $rateLimitService,
-        PathLimitProcessor $pathLimitProcessor
+        protected EventDispatcherInterface $eventDispatcher,
+        protected RateLimitService $rateLimitService,
+        protected PathLimitProcessor $pathLimitProcessor
     ) {
-        $this->eventDispatcher = $eventDispatcher;
-        $this->rateLimitService = $rateLimitService;
-        $this->pathLimitProcessor = $pathLimitProcessor;
     }
 
     public function onKernelController(ControllerEvent $event): void
     {
         // Skip if the bundle isn't enabled (for instance in test environment)
-        if( ! $this->getParameter('enabled', true)) {
+        if(! $this->getParameter('enabled', true)) {
             return;
         }
 
@@ -120,10 +111,8 @@ public function onKernelController(ControllerEvent $event): void
             });
             $event->stopPropagation();
         }
-
     }
 
-
     /**
      * @param RateLimit[] $rateLimits
      */
@@ -134,19 +123,19 @@ protected function findBestMethodMatch(Request $request, array $rateLimits): ?Ra
             return $this->pathLimitProcessor->getRateLimit($request);
         }
 
-        $best_match = null;
+        $bestMatch = null;
         foreach ($rateLimits as $rateLimit) {
             if (in_array($request->getMethod(), $rateLimit->getMethods(), true)) {
-                $best_match = $rateLimit;
+                $bestMatch = $rateLimit;
             }
 
             // Only match "default" annotation when we don't have a best match
-            if ($best_match === null && count($rateLimit->methods) === 0) {
-                $best_match = $rateLimit;
+            if ($bestMatch === null && count($rateLimit->methods) === 0) {
+                $bestMatch = $rateLimit;
             }
         }
 
-        return $best_match;
+        return $bestMatch;
     }
 
     /** @param RateLimit[] $rateLimits */

Service/Storage/DoctrineCache.php

@@ -62,7 +62,7 @@ public function resetRate($key)
         return true;
     }
 
-    private function createRateInfo(array $info)
+    private function createRateInfo(array $info): RateLimitInfo
     {
         $rateLimitInfo = new RateLimitInfo();
         $rateLimitInfo->setLimit($info['limit']);

Service/Storage/Memcache.php

@@ -64,7 +64,7 @@ public function resetRate($key)
         return true;
     }
 
-    private function createRateInfo(array $info)
+    private function createRateInfo(array $info): RateLimitInfo
     {
         $rateLimitInfo = new RateLimitInfo();
         $rateLimitInfo->setLimit($info['limit']);

Service/Storage/PhpRedisCluster.php

@@ -1,9 +1,7 @@
 <?php
 
-
 namespace Noxlogic\RateLimitBundle\Service\Storage;
 
-
 class PhpRedisCluster extends PhpRedis
 {
 

Service/Storage/PsrCache.php

@@ -69,7 +69,7 @@ public function resetRate($key)
         return true;
     }
 
-    private function createRateInfo(array $info)
+    private function createRateInfo(array $info): RateLimitInfo
     {
         $rateLimitInfo = new RateLimitInfo();
         $rateLimitInfo->setLimit($info['limit']);

Service/Storage/SimpleCache.php

@@ -61,7 +61,7 @@ public function resetRate($key)
         return true;
     }
 
-    private function createRateInfo(array $info)
+    private function createRateInfo(array $info): RateLimitInfo
     {
         $rateLimitInfo = new RateLimitInfo();
         $rateLimitInfo->setLimit($info['limit']);

Tests/Attribute/RateLimitTest.php

@@ -43,7 +43,7 @@ public function testConstructionWithMethods(): void
             1234,
             1000
         );
-        $this->assertCount(2, $attribute->methods);
+        self::assertCount(2, $attribute->methods);
     }
 
     public function testConstructWithStringAsMethods(): void
@@ -53,6 +53,6 @@ public function testConstructWithStringAsMethods(): void
             1234,
             1000
         );
-        $this->assertEquals(['POST'], $attribute->methods);
+        self::assertSame(['POST'], $attribute->methods);
     }
 }

Tests/DependencyInjection/ConfigurationTest.php

@@ -25,9 +25,9 @@ private function getConfigs(array $configArray): array
 
     public function testUnconfiguredConfiguration(): void
     {
-        $configuration = $this->getConfigs(array());
+        $configuration = $this->getConfigs([]);
 
-        $this->assertSame(array(
+        $this->assertSame([
             'enabled' => true,
             'storage_engine' => 'redis',
             'redis_client' => 'default_client',
@@ -43,14 +43,14 @@ public function testUnconfiguredConfiguration(): void
             'rate_response_exception' => null,
             'rate_response_message' => 'You exceeded the rate limit',
             'display_headers' => true,
-            'headers' => array(
+            'headers' => [
                 'limit' => 'X-RateLimit-Limit',
                 'remaining' => 'X-RateLimit-Remaining',
                 'reset' => 'X-RateLimit-Reset',
-            ),
-            'path_limits' => array(),
+            ],
+            'path_limits' => [],
             'fos_oauth_key_listener' => true
-        ), $configuration);
+        ], $configuration);
     }
 
     public function testDisabledConfiguration(): void
@@ -134,6 +134,7 @@ public function testDefaultPathLimitMethods(): void
     public function testMustBeBasedOnExceptionClass(): void
     {
         $this->expectException(InvalidConfigurationException::class);
+
         $this->getConfigs(array('rate_response_exception' => '\StdClass'));
     }