bitExpert / phpstan-magento

Magento specific extension for PHPStan
MIT License
134 stars 24 forks source link

Add support for PHPStan 2.0 #333

Open hostep opened 1 week ago

hostep commented 1 week ago

PHPStan 2.0 got released earlier today: https://github.com/phpstan/phpstan/releases/tag/2.0.0 We should take a look at the upgrade guide for extension developers in order to make this module compatible with version 2.

I already gave it a quick try on my local with a small module with the current codebase, and ran into an error:

     Internal error: PHPStan\Analyser\RuleErrorTransformer::transform(): Argument #1 ($ruleError) must be of type PHPStan\Rules\RuleError, string given, called in
     phar:///vendor/phpstan/phpstan/phpstan.phar/src/Analyser/FileAnalyser.php on line 119 while analysing file
     /path/to/some/code-file.php

This can be fixed in the codebase with something like this:

diff --git a/src/bitExpert/PHPStan/Magento/Rules/AbstractModelUseServiceContractRule.php b/src/bitExpert/PHPStan/Magento/Rules/AbstractModelUseServiceContractRule.php
index 17c47f9..4e34b0d 100644
--- a/src/bitExpert/PHPStan/Magento/Rules/AbstractModelUseServiceContractRule.php
+++ b/src/bitExpert/PHPStan/Magento/Rules/AbstractModelUseServiceContractRule.php
@@ -16,6 +16,7 @@ use PhpParser\Node;
 use PhpParser\Node\Expr\MethodCall;
 use PHPStan\Analyser\Scope;
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\VerbosityLevel;
@@ -39,7 +40,7 @@ class AbstractModelUseServiceContractRule implements Rule
     /**
      * @param Node $node
      * @param Scope $scope
-     * @return (string|\PHPStan\Rules\RuleError)[] errors
+     * @return \PHPStan\Rules\RuleError[] errors
      * @throws ShouldNotHappenException
      */
     public function processNode(Node $node, Scope $scope): array
@@ -63,11 +64,15 @@ class AbstractModelUseServiceContractRule implements Rule
         }

         return [
-            sprintf(
-                'Use service contracts to persist entities in favour of %s::%s() method',
-                $type->describe(VerbosityLevel::typeOnly()),
-                $node->name->name
+            RuleErrorBuilder::message(
+                sprintf(
+                    'Use service contracts to persist entities in favour of %s::%s() method',
+                    $type->describe(VerbosityLevel::typeOnly()),
+                    $node->name->name
+                )
             )
+            ->identifier('bitexpert.use-service-contracts-or-something-like-that')
+            ->build(),
         ];
     }
 }

Or something similar. This transformation of returning string to a RuleError should happen in more places. See https://phpstan.org/blog/using-rule-error-builder These changes can probably also already be done for support with the PHPStan 1.12.0 compatibility (haven't tested it), maybe that's worth it to reduce the amount of code changes between supporting PHPStan 1.x and 2.x? In case you plan on keep supporting version 1.x for a while?

That's at least one thing that will need to be updated. So far haven't run into other things, but I only looked around for 15 minutes or so.

Thanks!

shochdoerfer commented 1 week ago

Was already waiting for someone to request PHPStan 2.0 compatibility :)

I totally want to support PHPStan 2.0 but I can't give you a timeframe for it. I broke my ankle the other week and that complicates a few more things.

Mind preparing a PR with your patch? That's probably a good start to work on other issues (if there are any).

Since the older versions of the extension still work fine, I would let master "just" support PHPStan 2.x. Since older versions of the extension are still around, you can easily install the older versions. For now, I would want to avoid backporting features.

hostep commented 1 week ago

I broke my ankle the other week and that complicates a few more things.

Please don't tell me you're coding with your toes instead of your fingers? Sorry for the really bad joke 😛. Get well soon! And no problem for the delay, phpstan 2.0 is still very fresh.

I'll see what I can do in terms of a pull request later this week.

Do you have some preference in those error identifier names for this module? Should we use magento.xxx or bitexpert.xxx or bitexpert-magento.xxx or something else? (example from the Laravel world, where larastan phpstan extension exists)

shochdoerfer commented 1 week ago

I think bitexpert-magento.xxx makes the most sense.

hostep commented 1 week ago

Here you go, a first attempt: https://github.com/bitExpert/phpstan-magento/pull/334