[PHPStan] Set compatible with upcoming PHPStan 1.6.x with set NodeConnectingVisitor tags

[samsonasik]

@ondrejmirtes @TomasVotruba I try to set compatible for upcoming PHPStan 1.6.x with bleeding edge feature. I tried to register NodeConnectingVisitor to config/phpstan/static-reflection.neon and it cause error Multiple services definition:

Caused by
_PHPStan_ccec86fc8\Nette\DI\ServiceCreationException: Multiple services of type PhpParser\NodeVisitor\NodeConnectingVisitor found: 04, 0258

so I add to existing Service->set() definition in the config/services.php with define its tag there:

    $services->set(NodeConnectingVisitor::class)
        ->tag('phpstan.parser.richParserNodeVisitor');

ref rectorphp/rector#7088

[samsonasik]

All checks have passed 🎉 @TomasVotruba @ondrejmirtes it is ready for review.

@ondrejmirtes until the compatible code for create new Visitor per specific data for parent, next, prev, do you think it is enough to make it compatible?

[ondrejmirtes]

Yes, I think so, feel free to try this out with phpstan/phpstan 1.6.x-dev dependency.

[ondrejmirtes]

On the other hand, this is for the Symfony DIC, right? This isn't for the DIC that's used by DIC. So I think this will still be broken for 1.6.x-dev.

I think a possible fix instead is to require phpstan/phpstan 1.6.x-dev and do this:

conditionalTags:
	PhpParser\NodeVisitor\NodeConnectingVisitor:
		phpstan.parser.richParserNodeVisitor: true

[samsonasik]

@ondrejmirtes on rector scoped, phpstan is required in composer.json as ^1.5, not part of the scoped vendor:

https://github.com/rectorphp/rector/blob/740c3031a0cdcda38459b30939f394234c01167a/composer.json#L10

How to make it compatible for both phpstan 1.5 and 1.6.x-dev ?

[ondrejmirtes]

What if I:

1. Add support for tagging visitors with phpstan.parser.richParserNodeVisitor in PHPStan 1.5.5
2. You'd declare Rector to support PHPStan ^1.5.5.
3. You'd add the recommended code to Rector's phpstan.neon:

conditionalTags:
	PhpParser\NodeVisitor\NodeConnectingVisitor:
		phpstan.parser.richParserNodeVisitor: true

Then nothing will be blocked and Rector would be compatible even with 1.6 out of the box like this. Would you agree?

But first, please make sure that the point number 3) fixes your test suite on PHPStan 1.6.x-dev.

[samsonasik]

@ondrejmirtes that's seems will work I think /cc @TomasVotruba

@ondrejmirtes I updated requirement to 1.6.x-dev and conditional tags config, and it seems phpunit got erorr now:

There were 129 errors:

1) Rector\PHPUnit\Tests\Rector\ClassMethod\AddDoesNotPerformAssertionToNonAssertingTestRector\AddDoesNotPerformAssertionToNonAssertingTestRectorTest::test with data set #0 (Symplify\SmartFileSystem\SmartFileInfo Object (...))
_PHPStan_ccec86fc8\Nette\DI\InvalidConfigurationException: Unexpected item 'conditionalTags › PhpParser\NodeVisitor\NodeConnectingVisitor › phpstan.parser.richParserNodeVisitor'.

[samsonasik]

@ondrejmirtes by update to phpstan 1.6.x-dev in packages tests workflow, and temporary set 1.6.x-dev in target-repository build, it seems make CI green 🎉

Do you think the bleeding edge need to be enabled?

includes:
	- vendor/phpstan/phpstan/conf/bleedingEdge.neon

[samsonasik]

@ondrejmirtes remove the ->tag() call in services.php seems still green 🎉

[ondrejmirtes]

After I release 1.5.5 I'm gonna send a PR here and make sure it works.

[ondrejmirtes]

I'm gonna release 1.5.5 today, but it DOES NOT contain what I promised here, you're gonna have to wait a few more days for 1.5.6 :) Thanks for understanding.

[ondrejmirtes]

Hi, PHPStan 1.5.6 is out with detailed instructions: https://phpstan.org/blog/preprocessing-ast-for-custom-rules

This PR is OK - you can require ^1.5.6 instead of 1.6.x-dev - your code will work for both ^1.5.6 and 1.6.0.

Please release new Rector version so that users are ready and Rector doesn't stop working for them once I release PHPStan 1.6.0.

This also probably applies to custom rules in the Symplify organization (https://github.com/symplify).

[samsonasik]

@ondrejmirtes I rebased and updated require to phpstan ^1.5.6

[samsonasik]

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

[samsonasik]

same with deprecated-packages/symplify#4029, I am merging it ;)

[samsonasik]

@ondrejmirtes thank you for review 👍

[TomasVotruba]

Thank you both for great co-operation 👍