Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for PHPStan 2.0 #333

Open
hostep opened this issue Nov 11, 2024 · 4 comments
Open

Add support for PHPStan 2.0 #333

hostep opened this issue Nov 11, 2024 · 4 comments

Comments

@hostep
Copy link
Contributor

hostep commented Nov 11, 2024

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
Copy link
Member

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
Copy link
Contributor Author

hostep commented Nov 12, 2024

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
Copy link
Member

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

@hostep
Copy link
Contributor Author

hostep commented Nov 12, 2024

Here you go, a first attempt: #334

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants