Static analyzers

[szepeviktor]

Hello @gmazzap !

Very nice code. Psalm helps you keep code quality high.

Some things to consider

• use phpstan-wordpress, you don't have to write core functions stubs yourself!
• @phpstan is a better tool
• near-zero configuration
• try avoiding casting values $baseUrl = (string)$context->baseUrl();
• convert closures to proper methods
• figure out null default values

All the best to you!

diff --git a/composer.json b/composer.json
index e8db817..a5f08e2 100644
--- a/composer.json
+++ b/composer.json
@@ -29,7 +29,8 @@
         "brain/monkey": "^2",
         "squizlabs/php_codesniffer": "3.2.*",
         "inpsyde/php-coding-standards": "0.*",
-        "vimeo/psalm": "^3"
+        "phpstan/phpstan-shim": "^0.11.8",
+        "szepeviktor/phpstan-wordpress": "^0.1.2"
     },
     "scripts": {
         "qa": [
diff --git a/src/Assets.php b/src/Assets.php
index d216df9..f8c8d88 100644
--- a/src/Assets.php
+++ b/src/Assets.php
@@ -810,7 +810,9 @@ class Assets
      */
     private function tryRelativeUrl(string $url, string $folder, ?string $baseUrl = null): string
     {
+        /** @var string[] */
         $urlData = parse_url($url);
+        /** @var string[] */
         $baseData = parse_url($baseUrl ?? $this->context->baseUrl());

         $urlHost = (string)($urlData['host'] ?? '');
diff --git a/src/UrlResolver/DirectUrlResolver.php b/src/UrlResolver/DirectUrlResolver.php
index 9275d2f..883d217 100644
--- a/src/UrlResolver/DirectUrlResolver.php
+++ b/src/UrlResolver/DirectUrlResolver.php
@@ -29,6 +29,7 @@ final class DirectUrlResolver implements UrlResolver
         $baseUrl = $this->context->baseUrl();
         $basePath = $this->context->basePath();

+        /** @var string[] */
         $urlData = parse_url($relative);
         $path = trim((string)($urlData['path'] ?? ''), '/');
         $query = (string)($urlData['query'] ?? '');
diff --git a/src/Version/LastModifiedVersion.php b/src/Version/LastModifiedVersion.php
index 564fe9b..b2e1e43 100644
--- a/src/Version/LastModifiedVersion.php
+++ b/src/Version/LastModifiedVersion.php
@@ -7,7 +7,7 @@ use Brain\Assets\Context\Context;
 final class LastModifiedVersion implements Version
 {
     /**
-     * @var array<string, array{0:string, 1:int}>
+     * @var array<string, array>
      */
     private $bases = [];

phpstan.neon.dist

# Start command: vendor/bin/phpstan analyze

includes:
    - phar://phpstan.phar/conf/bleedingEdge.neon
    - vendor/szepeviktor/phpstan-wordpress/extension.neon
parameters:
    level: max
    paths:
        - %currentWorkingDirectory%/src/
    ignoreErrors:
        # Uses func_get_args()
        - '#^Function add_query_arg invoked with [123] parameters?, 0 required\.$#'

[gmazzap]

Thanks a lot for your feedback.

@phpstan is a better tool

Tried it in the past, didn't like it. IMO too opinionated (trying to push code style instead of just analysing code) and it seems to me it aims to prove correctness rather than incorrectness. (I prefer the other way around). So I like psalm better. (I actually prefer the approach of phan, but unfortunately that is still lacking too many features).

try avoiding casting values $baseUrl = (string)$context->baseUrl();

Sometimes this is necessary to make the analizer happy, which is sad, but most of the times it is necessary because WP functions return values after filtering. So, when dealing with WP, you can never be sure of the type unless you cast.
Sure, you can document via doc bloc the type, make the analyzer happy, and pretend all is fine... until some plugin filters an int returning a string and your code broke.
So... I prefer casting, it's no big deal. And if the type is already what I cast... that in PHP is basically a noop with no impact on performance.

convert closures to proper methods

Closures I use are attached to hooks, and that has a reason, maybe two.
First of all, to attach methods to hooks it is needed to make them public. I don't want that.
Moreover, my codestyle can recognize closures attached to hooks automatically, relaxing type checks (because, again, you can't trust WordPress with types).
That said, I agree that too complex closures should be avoided, but I checked in the package and the most complex closure has a cyclomatic complexityof 2... which I can definetively live with.
Expecially considering that where closures are currently used I really don't want to use public methods.

figure out null default values
Honestly not sure what you mean with this.
Yes, there are quite a lot of null defaults, mostly for optional parameters where I can figure out "sensitive default" that in great majority of caes should be fine. Defaulting to null I can keep the public API easier to use (less params to provide), but still open to customization if someone really wants / needs to.

[szepeviktor]

@gmazzap Thank you for your openness!