Phpstan level7 (#2)

* Simplify array structure * Remove array merges in favor of explicit options * Split allowAll options * Tweak array type * Revert "Tweak array type" This reverts commit 4738baf. * Add explicit cast * Skip check on lowest symfony 4 * Skip check on lowest symfony 4 * Skip check on lowest symfony 4 * SKip * SKip
fruitcake · Feb 19, 2022 · 277ac8d · 277ac8d
1 parent 84d0d02
commit 277ac8d
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 82 deletions.
diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml
@@ -46,7 +46,7 @@ jobs:
 
     - name: Analyse with PHPStan
       run: composer analyse
-      if: matrix.os == 'ubuntu-latest'
+      if: matrix.os == 'ubuntu-latest' && matrix.symfony != '4.x'
 
     - name: Check PSR-12 Codestyle
       run: composer test

diff --git a/composer.json b/composer.json
@@ -32,7 +32,7 @@
     },
     "scripts": {
         "test": "phpunit",
-        "analyse": "phpstan analyse src tests --level=6",
+        "analyse": "phpstan analyse src tests --level=7",
         "check-style": "phpcs -p --standard=PSR12 --exclude=Generic.Files.LineLength --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 src tests",
         "fix-style": "phpcbf -p --standard=PSR12 --exclude=Generic.Files.LineLength --runtime-set ignore_errors_on_exit 1 --runtime-set ignore_warnings_on_exit 1 src tests"
     },

diff --git a/src/CorsService.php b/src/CorsService.php
@@ -18,30 +18,33 @@
 
 /**
  * @phpstan-type CorsInputOptions array{
- *  'allowedOrigins'?: array{string}|array{},
- *  'allowedOriginsPatterns'?: array{string}|array{},
+ *  'allowedOrigins'?: string[],
+ *  'allowedOriginsPatterns'?: string[],
  *  'supportsCredentials'?: bool,
- *  'allowedHeaders'?: array{string}|array{},
- *  'allowedMethods'?: array{string}|array{},
- *  'exposedHeaders'?: array{string}|array{},
+ *  'allowedHeaders'?: string[],
+ *  'allowedMethods'?: string[],
+ *  'exposedHeaders'?: string[]|false,
  *  'maxAge'?: int|bool|null,
- *  'allowed_origins'?: array{string}|array{},
- *  'allowed_origins_patterns'?: array{string}|array{},
+ *  'allowed_origins'?: string[],
+ *  'allowed_origins_patterns'?: string[],
  *  'supports_credentials'?: bool,
- *  'allowed_headers'?: array{string}|array{},
- *  'allowed_methods'?: array{string}|array{},
- *  'exposed_headers'?: array{string}|array{},
+ *  'allowed_headers'?: string[],
+ *  'allowed_methods'?: string[],
+ *  'exposed_headers'?: string[]|false,
  *  'max_age'?: int|bool|null
  * }
  *
  * @phpstan-type CorsNormalizedOptions array{
- *  'allowedOrigins': array{string}|array{}|true,
- *  'allowedOriginsPatterns': array{string}|array{},
+ *  'allowedOrigins': string[],
+ *  'allowedOriginsPatterns': string[],
  *  'supportsCredentials': bool,
- *  'allowedHeaders': array{string}|array{}|bool,
- *  'allowedMethods': array{string}|array{}|bool,
- *  'exposedHeaders': array{string}|array{},
- *  'maxAge': int|bool|null
+ *  'allowedHeaders': string[],
+ *  'allowedMethods': string[],
+ *  'exposedHeaders': string[],
+ *  'maxAge': int|bool|null,
+ *  'allowAllOrigins': bool,
+ *  'allowAllHeaders': bool,
+ *  'allowAllMethods': bool,
  * }
  */
 class CorsService
@@ -63,34 +66,18 @@ public function __construct(array $options = [])
      */
     private function normalizeOptions(array $options = []): array
     {
-        $aliases = [
-            'supports_credentials' => 'supportsCredentials',
-            'allowed_origins' => 'allowedOrigins',
-            'allowed_origins_patterns' => 'allowedOriginsPatterns',
-            'allowed_headers' => 'allowedHeaders',
-            'allowed_methods' => 'allowedMethods',
-            'exposed_headers' => 'exposedHeaders',
-            'max_age'  => 'maxAge',
-        ];
-
-        // Normalize underscores
-        foreach ($aliases as $alias => $option) {
-            if (isset($options[$alias])) {
-                $options[$option] = $options[$alias];
-                unset($options[$alias]);
-            }
+        $options['allowedOrigins'] = $options['allowedOrigins'] ?? $options['allowed_origins'] ?? [];
+        $options['allowedOriginsPatterns'] =
+            $options['allowedOriginsPatterns'] ?? $options['allowed_origins_patterns'] ?? [];
+        $options['allowedMethods'] = $options['allowedMethods'] ?? $options['allowed_methods'] ?? [];
+        $options['allowedHeaders'] = $options['allowedHeaders'] ?? $options['allowed_headers'] ?? [];
+        $options['exposedHeaders'] = $options['exposedHeaders'] ?? $options['exposed_headers'] ?? [];
+        $options['supportsCredentials'] = $options['supportsCredentials'] ?? $options['supports_credentials'] ?? false;
+
+        if (!array_key_exists('maxAge', $options)) {
+            $options['maxAge'] = array_key_exists('max_age', $options) ? $options['max_age'] : 0;
         }
 
-        $options += [
-            'allowedOrigins' => [],
-            'allowedOriginsPatterns' => [],
-            'supportsCredentials' => false,
-            'allowedHeaders' => [],
-            'exposedHeaders' => [],
-            'allowedMethods' => [],
-            'maxAge' => 0,
-        ];
-
         if ($options['exposedHeaders'] === false) {
             $options['exposedHeaders'] = [];
         }
@@ -115,21 +102,14 @@ private function normalizeOptions(array $options = []): array
             }
         }
 
-        // normalize array('*') to true
-        if (in_array('*', $options['allowedOrigins'])) {
-            $options['allowedOrigins'] = true;
-        }
-        if (in_array('*', $options['allowedHeaders'])) {
-            $options['allowedHeaders'] = true;
-        } else {
-            $options['allowedHeaders'] = array_map('strtolower', $options['allowedHeaders']);
-        }
+        // Normalize case
+        $options['allowedHeaders'] = array_map('strtolower', $options['allowedHeaders']);
+        $options['allowedMethods'] = array_map('strtoupper', $options['allowedMethods']);
 
-        if (in_array('*', $options['allowedMethods'])) {
-            $options['allowedMethods'] = true;
-        } else {
-            $options['allowedMethods'] = array_map('strtoupper', $options['allowedMethods']);
-        }
+        // Normalize ['*'] to true
+        $options['allowAllOrigins'] = in_array('*', $options['allowedOrigins']);
+        $options['allowAllHeaders'] = in_array('*', $options['allowedHeaders']);
+        $options['allowAllMethods'] = in_array('*', $options['allowedMethods']);
 
         return $options;
     }
@@ -191,7 +171,7 @@ public function addPreflightRequestHeaders(Response $response, Request $request)
 
     public function isOriginAllowed(Request $request): bool
     {
-        if ($this->options['allowedOrigins'] === true) {
+        if ($this->options['allowAllOrigins'] === true) {
             return true;
         }
 
@@ -205,6 +185,7 @@ public function isOriginAllowed(Request $request): bool
             return true;
         }
 
+        /** @var string $pattern */
         foreach ($this->options['allowedOriginsPatterns'] as $pattern) {
             if (preg_match($pattern, $origin)) {
                 return true;
@@ -229,7 +210,7 @@ public function addActualRequestHeaders(Response $response, Request $request): R
 
     private function configureAllowedOrigin(Response $response, Request $request): void
     {
-        if ($this->options['allowedOrigins'] === true && !$this->options['supportsCredentials']) {
+        if ($this->options['allowAllOrigins'] === true && !$this->options['supportsCredentials']) {
             // Safe+cacheable, allow everything
             $response->headers->set('Access-Control-Allow-Origin', '*');
         } elseif ($this->isSingleOriginAllowed()) {
@@ -247,7 +228,7 @@ private function configureAllowedOrigin(Response $response, Request $request): v
 
     private function isSingleOriginAllowed(): bool
     {
-        if ($this->options['allowedOrigins'] === true || count($this->options['allowedOriginsPatterns']) > 0) {
+        if ($this->options['allowAllOrigins'] === true || count($this->options['allowedOriginsPatterns']) > 0) {
             return false;
         }
 
@@ -256,8 +237,8 @@ private function isSingleOriginAllowed(): bool
 
     private function configureAllowedMethods(Response $response, Request $request): void
     {
-        if ($this->options['allowedMethods'] === true) {
-            $allowMethods = strtoupper($request->headers->get('Access-Control-Request-Method'));
+        if ($this->options['allowAllMethods'] === true) {
+            $allowMethods = strtoupper((string) $request->headers->get('Access-Control-Request-Method'));
             $this->varyHeader($response, 'Access-Control-Request-Method');
         } else {
             $allowMethods = implode(', ', $this->options['allowedMethods']);
@@ -268,7 +249,7 @@ private function configureAllowedMethods(Response $response, Request $request):
 
     private function configureAllowedHeaders(Response $response, Request $request): void
     {
-        if ($this->options['allowedHeaders'] === true) {
+        if ($this->options['allowAllHeaders'] === true) {
             $allowHeaders = $request->headers->get('Access-Control-Request-Headers');
             $this->varyHeader($response, 'Access-Control-Request-Headers');
         } else {
@@ -302,8 +283,8 @@ public function varyHeader(Response $response, string $header): Response
     {
         if (!$response->headers->has('Vary')) {
             $response->headers->set('Vary', $header);
-        } elseif (!in_array($header, explode(', ', $response->headers->get('Vary')))) {
-            $response->headers->set('Vary', $response->headers->get('Vary') . ', ' . $header);
+        } elseif (!in_array($header, explode(', ', (string) $response->headers->get('Vary')))) {
+            $response->headers->set('Vary', ((string) $response->headers->get('Vary')) . ', ' . $header);
         }
 
         return $response;

diff --git a/tests/CorsServiceTest.php b/tests/CorsServiceTest.php
@@ -83,7 +83,7 @@ public function itNormalizesWildcardOrigins(): void
         $service = new CorsService(['allowedOrigins' => $origins]);
         $this->assertInstanceOf(CorsService::class, $service);
 
-        $this->assertEquals(true, $this->getOptionsFromService($service)['allowedOrigins']);
+        $this->assertTrue($this->getOptionsFromService($service)['allowAllOrigins']);
     }
 
     /**

diff --git a/tests/CorsTest.php b/tests/CorsTest.php
@@ -555,23 +555,15 @@ private function createValidPreflightRequest(): Request
 
     /**
      * @param CorsInputOptions $options
-     * @param array{'Vary'?: string} $responseHeaders
+     * @param string[] $responseHeaders
      * @return MockApp
      */
     private function createStackedApp(array $options = array(), array $responseHeaders = array()): MockApp
     {
-        $passedOptions = array_merge(
-            array(
-                'allowedHeaders'      => array('x-allowed-header', 'x-other-allowed-header'),
-                'allowedMethods'      => array('delete', 'get', 'post', 'put'),
-                'allowedOrigins'      => array('http://localhost'),
-                'exposedHeaders'      => false,
-                'maxAge'              => false,
-                'supportsCredentials' => false,
-            ),
-            $options
-        );
+        $options['allowedHeaders'] = $options['allowedHeaders'] ?? ['x-allowed-header', 'x-other-allowed-header'];
+        $options['allowedMethods'] = $options['allowedMethods'] ?? ['delete', 'get', 'post', 'put'];
+        $options['allowedOrigins'] = $options['allowedOrigins'] ?? ['http://localhost'];
 
-        return new MockApp($responseHeaders, $passedOptions);
+        return new MockApp($responseHeaders, $options);
     }
 }
diff --git a/tests/MockApp.php b/tests/MockApp.php
@@ -21,7 +21,7 @@
  */
 class MockApp
 {
-    /** @var array{'Vary'?: string} */
+    /** @var string[] */
     private $responseHeaders;
 
     /**
@@ -30,7 +30,7 @@ class MockApp
     private $cors;
 
     /**
-     * @param array{'Vary'?: string} $responseHeaders
+     * @param string[] $responseHeaders
      * @param CorsInputOptions $options
      */
     public function __construct(array $responseHeaders, array $options = [])