[Scoper] Add CommunityNodeVisitorAbstract to be used for community that include getRuleDefinition() empty method

[samsonasik]

@TomasVotruba this is what I mean.

On rector-src code base, the AbstractRector still enforce to have getRuleDefinition() method, but not for community, but still have empty method for it.

This PR add CommunityNodeVisitorAbstract to be extended from AbstractRector on scoping process, to avoid issue like what @ruudk have at rectorphp/rector#8815 (comment)

[ruudk]

Do I understand correctly that I can just remove the whole getRuleDefinition and be done with it? I don't really want / need that anyway...

[ruudk]

Ah no, I just removed it and then it complains about it.

[samsonasik]

@ruudk this is for safe belt, the replace to use CommunityNodeVisitorAbstract happen on scoper process, so it have empty getRuleDefinition() for community that extends AbstractRector.

see https://3v4l.org/OAS8Q

[ruudk]

Got it, makes sense!

[samsonasik]

All checks have passed 🎉 @TomasVotruba I think it is ready.

[TomasVotruba]

This is intentional, getDefinition() is dropped in Rector 2.0, see https://github.com/rectorphp/rector-src/blob/main/UPGRADING.md#2-abstractrector-get-focused-on-code-the-getruledefinition-is-no-longer-required

[TomasVotruba]

This PR is wrong, as it would only silence the removal. The method getDefinitin() should be removed instead 👍

[ruudk]

If I remove getRuleDefinition I get an error too.

/Volumes/CS/www/cosmos/vendor/bin/rector process /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php
PHP Fatal error: Class Dev\Rector\StaticMoneyConstructorRector contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface::getRuleDefinition) in /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php on line 23
Fatal error: Class Dev\Rector\StaticMoneyConstructorRector contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface::getRuleDefinition) in /Volumes/CS/www/cosmos/src-dev/Rector/StaticMoneyConstructorRector.php on line 23

[samsonasik]

@ruudk that seems due to overlapped with real dependency of symplify/rule-doc-generator, I guess remove #[\Override] is the way, since it exists on parent based on autoload, but not exists based on rector.

Could you create simple reproducible repo for that that show the error?

[ruudk]

It's because I use symplify/coding-standard 12.2.3

How to deal with that now?

[samsonasik]

@ruudk remove #[\Override] attribute on getRuleDefiniton() is the way

[ruudk]

That is not a solution, as now PHPStan complains that I should add #[Override] because I have checkMissingOverrideMethodAttribute: true

Method Dev\Rector\StaticMoneyConstructorRector::getRuleDefinition() overrides method Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface::getRuleDefinition() but is missing the #[\Override] attribute.
🔖 method.missingOverride
↳ src-dev/Rector/StaticMoneyConstructorRector.php:25

[ruudk]

So the problem is, that PHPStan sees different code than what Rector sees. So if I fix it for Rector, it breaks for PHPStan. And visa versa.

[samsonasik]

I think register to phpstan ignoreErrors is the way for that

[ruudk]

Sorry, but this is wrong. Ignoring that error in PHPStan is not a solution. Why does PHPStan see it that way?

[ruudk]

Shouldn't we remove DocumentedRuleInterface from https://github.com/rectorphp/rector-src/blob/main/src/Contract/Rector/RectorInterface.php ?

[samsonasik]

because PHPStan sees autoload from vendor/autoload.php, rector load RectorInterface that implements DocumentedRuleInterface from its prefixed vendor.

I will create new PR to remove DocumentedRuleInterface from RectorInterface on scoped prefixed vendor.

-interface RectorInterface extends NodeVisitor, DocumentedRuleInterface
+interface RectorInterface extends NodeVisitor

so it won't error.

[ruudk]

That seems like it! 😁

[samsonasik]

new PR:

• [Scoper] Remove implements DocumentedRuleInterface on RectorInterface on scoper process #6477

[TomasVotruba]

Thank you for pushing this one for a better solution than my first one 👏