refactor: re-enable phpstan project-wide

composer.json

@@ -33,7 +33,7 @@
         "psr/http-factory": "^1.0",
         "psr/http-message": "^1.0|^2.0",
         "psr/log": "^3.0.0",
-        "rector/rector": "^2.3.2",
+        "rector/rector": "2.3.1",
         "symfony/cache": "^7.3",
         "symfony/filesystem": "^7.3",
         "symfony/mailer": "^7.2.6",
@@ -74,7 +74,7 @@
         "patrickbussmann/oauth2-apple": "^0.3",
         "phpat/phpat": "^0.11.0",
         "phpbench/phpbench": "84.x-dev",
-        "phpstan/phpstan": "2.1.38",
+        "phpstan/phpstan": "2.1.33",
         "phpunit/phpunit": "^12.5.8",
         "predis/predis": "^3.0.0",
         "riskio/oauth2-auth0": "^2.4",
@@ -253,7 +253,7 @@
         "test": "composer phpunit",
         "test:stop": "composer phpunit -- --stop-on-error --stop-on-failure",
         "lint": "vendor/bin/mago lint --potentially-unsafe --minimum-fail-level=note",
-        "phpstan": "vendor/bin/phpstan analyse src tests --memory-limit=1G",
+        "phpstan": "vendor/bin/phpstan analyse --memory-limit=1G",
         "rector": "vendor/bin/rector process --no-ansi",
         "merge": "php -d\"error_reporting = E_ALL & ~E_DEPRECATED\" vendor/bin/monorepo-builder merge",
         "intl:plural": "./packages/intl/bin/plural-rules.php",

packages/auth/src/AccessControl/AccessControl.php

@@ -2,21 +2,21 @@
 
 namespace Tempest\Auth\AccessControl;
 
+use Tempest\Auth\Exceptions\AccessWasDenied;
 use UnitEnum;
 
 /**
- * @template Subject of object
- * @template Resource of object
+ * @template TSubject of object
+ * @template TResource of object
  */
 interface AccessControl
 {
     /**
      * Checks if the action is granted for the given resource and subject. If not, an exception is thrown.
      *
-     * @template Resource of object
      * @param UnitEnum|string $action An arbitrary action to check access for, e.g. 'view', 'edit', etc.
-     * @param Resource|class-string<Resource> $resource A model instance or class string of a model to check access for.
-     * @param null|Subject $subject An optional subject to check access against, e.g. a user or service account.
+     * @param TResource|class-string<TResource> $resource A model instance or class string of a model to check access for.
+     * @param null|TSubject $subject An optional subject to check access against, e.g. a user or service account.
      *
      * @throws AccessWasDenied
      */
@@ -25,10 +25,9 @@ public function ensureGranted(UnitEnum|string $action, object|string $resource,
     /**
      * Checks if the action is granted for the given resource and subject.
      *
-     * @template Resource of object
      * @param UnitEnum|string $action An arbitrary action to check access for, e.g. 'view', 'edit', etc.
-     * @param Resource|class-string<Resource> $resource A model instance or class string of a model to check access for.
-     * @param null|Subject $subject An optional subject to check access against, e.g. a user or service account.
+     * @param TResource|class-string<TResource> $resource A model instance or class string of a model to check access for.
+     * @param null|TSubject $subject An optional subject to check access against, e.g. a user or service account.
      */
     public function isGranted(UnitEnum|string $action, object|string $resource, ?object $subject = null): AccessDecision;
 }

packages/auth/src/AccessControl/PolicyBasedAccessControl.php

@@ -16,9 +16,9 @@
 use UnitEnum;
 
 /**
- * @template Subject of object
- * @template Resource of object
- * @implements AccessControl<Subject, Resource>
+ * @template TSubject of object
+ * @template TResource of object
+ * @implements AccessControl<TSubject, TResource>
  */
 final readonly class PolicyBasedAccessControl implements AccessControl
 {
@@ -119,11 +119,9 @@ private function ensureParameterAcceptsInput(?ParameterReflector $reflector, mix
             return;
         }
 
-        if (! ($type = $reflector?->getType())) {
-            return;
-        }
+        $type = $reflector->getType();
 
-        if ($type?->accepts($input)) {
+        if ($type->accepts($input)) {
             return;
         }
 

packages/auth/src/Authentication/AuthenticatableResolverInitializer.php

@@ -5,15 +5,12 @@
 use Tempest\Container\Container;
 use Tempest\Container\Initializer;
 use Tempest\Container\Singleton;
-use Tempest\Database\Database;
 
 final class AuthenticatableResolverInitializer implements Initializer
 {
     #[Singleton]
     public function initialize(Container $container): AuthenticatableResolver
     {
-        return new DatabaseAuthenticatableResolver(
-            database: $container->get(Database::class),
-        );
+        return new DatabaseAuthenticatableResolver();
     }
 }

packages/auth/src/Authentication/DatabaseAuthenticatableResolver.php

@@ -4,22 +4,15 @@
 
 use Tempest\Auth\Exceptions\AuthenticatableModelWasInvalid;
 use Tempest\Auth\Exceptions\ModelWasNotAuthenticatable;
-use Tempest\Database\Database;
 
 use function Tempest\Database\inspect;
 use function Tempest\Database\query;
 
 final readonly class DatabaseAuthenticatableResolver implements AuthenticatableResolver
 {
-    public function __construct(
-        private Database $database,
-    ) {}
-
     public function resolve(int|string $id, string $class): ?Authenticatable
     {
-        if (! is_a($class, Authenticatable::class, allow_string: true)) {
-            throw new ModelWasNotAuthenticatable($class);
-        }
+        $this->ensureClassIsAuthenticatable($class);
 
         return query($class)->findById($id);
     }
@@ -40,4 +33,11 @@ public function resolveId(Authenticatable $authenticatable): int|string
 
         return $id;
     }
+
+    private function ensureClassIsAuthenticatable(string $class): void
+    {
+        if (! is_a($class, Authenticatable::class, allow_string: true)) {
+            throw new ModelWasNotAuthenticatable($class);
+        }
+    }
 }

packages/auth/src/Exceptions/ModelWasNotAuthenticatable.php

@@ -8,7 +8,7 @@
 final class ModelWasNotAuthenticatable extends Exception implements AuthenticationException
 {
     public function __construct(
-        private readonly string $class,
+        string $class,
     ) {
         parent::__construct(
             sprintf('`%s` must be an instance of `%s`', $class, Authenticatable::class),

packages/auth/src/OAuth/GenericOAuthClient.php

@@ -53,8 +53,12 @@ public function __construct(
 
     public function buildAuthorizationUrl(array $scopes = [], array $options = []): string
     {
+        if ($scopes === []) {
+            $scopes = $this->config->scopes;
+        }
+
         return $this->provider->getAuthorizationUrl([
-            'scope' => $scopes ?? $this->config->scopes,
+            'scope' => $scopes,
             'redirect_uri' => $this->uri->createUri($this->config->redirectTo),
             ...$options,
         ]);
@@ -71,16 +75,27 @@ public function createRedirect(array $scopes = [], array $options = []): Redirec
 
     public function getState(): ?string
     {
-        return $this->provider->getState();
+        return $this->provider->getState() ?: null;
     }
 
     public function requestAccessToken(string $code): AccessToken
     {
         try {
-            return $this->provider->getAccessToken('authorization_code', [
+            $token = $this->provider->getAccessToken('authorization_code', [
                 'code' => $code,
                 'redirect_uri' => $this->uri->createUri($this->config->redirectTo),
             ]);
+
+            if ($token instanceof AccessToken) {
+                return $token;
+            }
+
+            return new AccessToken([
+                ...$token->getValues(),
+                'access_token' => $token->getToken(),
+                'refresh_token' => $token->getRefreshToken(),
+                'expires' => $token->getExpires(),
+            ]);
         } catch (IdentityProviderException $exception) {
             throw OAuthTokenCouldNotBeRetrieved::fromProviderException($exception);
         }

packages/auth/src/OAuth/OAuthClient.php

@@ -43,8 +43,6 @@ public function fetchUser(AccessToken $token): OAuthUser;
     /**
      * Authenticates a user based on the given OAuth callback request.
      *
-     * @template T of Authenticatable
-     *
      * @param Closure(OAuthUser): T $map A callback that should return an authenticatable model from the given OAuthUser. Typically, the callback is also responsible for saving the user to the database.
      */
     public function authenticate(Request $request, Closure $map): Authenticatable;

packages/auth/tests/AuthenticationAndOAuthSafetyTest.php

@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tempest\Auth\Tests;
+
+use League\OAuth2\Client\Provider\GenericProvider;
+use PHPUnit\Framework\Attributes\Test;
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+use ReflectionMethod;
+use Tempest\Auth\Authentication\DatabaseAuthenticatableResolver;
+use Tempest\Auth\Exceptions\ModelWasNotAuthenticatable;
+use Tempest\Auth\OAuth\GenericOAuthClient;
+
+final class AuthenticationAndOAuthSafetyTest extends TestCase
+{
+    #[Test]
+    public function database_authenticatable_resolver_rejects_non_authenticatable_classes(): void
+    {
+        $resolver = new DatabaseAuthenticatableResolver();
+        $resolve = new ReflectionMethod($resolver, 'resolve');
+
+        $this->expectException(ModelWasNotAuthenticatable::class);
+
+        $resolve->invoke($resolver, 1, \stdClass::class);
+    }
+
+    #[Test]
+    public function generic_oauth_client_state_is_null_before_generating_authorization_url(): void
+    {
+        $provider = new GenericProvider([
+            'clientId' => 'client-id',
+            'clientSecret' => 'client-secret', // @mago-expect lint:no-literal-password
+            'redirectUri' => 'https://example.com/callback',
+            'urlAuthorize' => 'https://provider.test/authorize',
+            'urlAccessToken' => 'https://provider.test/token', // @mago-expect lint:no-literal-password
+            'urlResourceOwnerDetails' => 'https://provider.test/user',
+        ]);
+
+        $reflection = new ReflectionClass(GenericOAuthClient::class);
+
+        /** @var GenericOAuthClient $client */
+        $client = $reflection->newInstanceWithoutConstructor();
+        $reflection->getProperty('provider')->setValue($client, $provider);
+
+        $this->assertNull($client->getState());
+    }
+}

packages/cache/src/Cache.php

@@ -37,11 +37,11 @@ public function put(Stringable|string $key, mixed $value, null|Duration|DateTime
     /**
      * Sets the specified keys to the specified values in the cache. Optionally, specify an expiration.
      *
-     * @template TKey of array-key
+     * @template TKey of Stringable|string
      * @template TValue
      *
-     * @param iterable<TKey,TValue> $array
-     * @return array<TKey,CacheItemInterface>
+     * @param iterable<TKey,TValue> $values
+     * @return array<string,CacheItemInterface>
      */
     public function putMany(iterable $values, null|Duration|DateTimeInterface $expiration = null): array;
 
@@ -53,11 +53,10 @@ public function get(Stringable|string $key): mixed;
     /**
      * Gets the values associated with the specified keys from the cache. If a key does not exist, null is returned for that key.
      *
-     * @template TKey of array-key
-     * @template TValue
+     * @template TKey of Stringable|string
      *
-     * @param iterable<TKey,TValue> $array
-     * @return array<TValue,mixed>
+     * @param iterable<array-key, TKey> $key
+     * @return array<string,mixed>
      */
     public function getMany(iterable $key): array;
 
@@ -79,7 +78,7 @@ public function decrement(Stringable|string $key, int $by = 1): int;
     /**
      * If the specified key already exists in the cache, the value is returned and the `$callback` is not executed. Otherwise, the result of the callback is stored, then returned.
      *
-     * @var null|Duration $stale Allow the value to be stale for the specified amount of time in addition to the time-to-live specified by `$expiration`. When a value is stale, it will still be returned as-is, but it will be refreshed in the background.
+     * @param null|Duration $stale Allow the value to be stale for the specified amount of time in addition to the time-to-live specified by `$expiration`. When a value is stale, it will still be returned as-is, but it will be refreshed in the background.
      */
     public function resolve(Stringable|string $key, Closure $callback, null|Duration|DateTimeInterface $expiration = null, ?Duration $stale = null): mixed;
 

packages/cache/src/Commands/CacheClearCommand.php

@@ -30,7 +30,6 @@
         private const string DEFAULT_CACHE = 'default';
 
         public function __construct(
-            private Cache $cache,
             private Container $container,
         ) {}
 
@@ -59,7 +58,7 @@ private function clearInternalCaches(bool $all = false): void
         {
             $caches = [ConfigCache::class, ViewCache::class, IconCache::class, DiscoveryCache::class];
 
-            if ($all === false && count($caches) > 1) {
+            if (! $all) {
                 $caches = $this->ask(
                     question: 'Which caches do you want to clear?',
                     options: $caches,

packages/cache/src/Commands/CacheStatusCommand.php

@@ -81,7 +81,6 @@ public function __invoke(bool $internal = true): void
 
             $this->console->header('User caches');
 
-            /** @var Cache $cache */
             foreach ($caches as $cache) {
                 $this->console->keyValue(
                     key: $this->getCacheName($cache),

packages/cache/src/InternalCacheInsightsProvider.php

@@ -7,6 +7,7 @@
 use Tempest\Core\DiscoveryCacheStrategy;
 use Tempest\Core\Insight;
 use Tempest\Core\InsightsProvider;
+use Tempest\Core\InsightType;
 use Tempest\Icon\IconCache;
 use Tempest\View\ViewCache;
 
@@ -25,14 +26,14 @@ public function getInsights(): array
     {
         return [
             'Discovery' => match ($this->discoveryCache->valid) {
-                false => new Insight('Invalid', Insight::ERROR),
+                false => new Insight('Invalid', InsightType::ERROR),
                 true => match ($this->discoveryCache->enabled) {
                     true => match ($this->discoveryCache->strategy) {
-                        DiscoveryCacheStrategy::FULL => new Insight('Enabled', Insight::SUCCESS),
-                        DiscoveryCacheStrategy::PARTIAL => new Insight('Enabled (partial)', Insight::SUCCESS),
+                        DiscoveryCacheStrategy::FULL => new Insight('Enabled', InsightType::SUCCESS),
+                        DiscoveryCacheStrategy::PARTIAL => new Insight('Enabled (partial)', InsightType::SUCCESS),
                         default => null, // INVALID and NONE are handled
                     },
-                    false => new Insight('Disabled', Insight::WARNING),
+                    false => new Insight('Disabled', InsightType::WARNING),
                 },
             },
             'Configuration' => $this->getInsight($this->configCache->enabled),
@@ -44,9 +45,9 @@ public function getInsights(): array
     private function getInsight(bool $enabled): Insight
     {
         if ($enabled) {
-            return new Insight('ENABLED', Insight::SUCCESS);
+            return new Insight('ENABLED', InsightType::SUCCESS);
         }
 
-        return new Insight('DISABLED', Insight::WARNING);
+        return new Insight('DISABLED', InsightType::WARNING);
     }
 }

packages/cache/src/Testing/TestingCache.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Tempest\Cache\Testing;
 
 use Closure;
@@ -18,9 +20,11 @@
 
 final class TestingCache implements Cache
 {
-    public bool $enabled {
-        get => $this->cache->enabled;
-        set => $this->cache->enabled = $value;
+    public bool $enabled = true {
+        set {
+            $this->cache->enabled = $value;
+            $this->enabled = $value;
+        }
     }
 
     private Cache $cache;