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

PHPStan 1.6.0 with bleedingEdge will remove previous/next/parent node attributes #7088

Closed
ondrejmirtes opened this issue Apr 4, 2022 · 7 comments
Assignees
Labels

Comments

@ondrejmirtes
Copy link
Contributor

Feature Request

Just to let you know - I'm currently fighting with memory leaks in PHPStan and I have some impressive results. I don't know yet when PHPStan 1.6.0 will be released as there's lot more work to be done, but one thing I know for sure now is that I no longer want to be using PhpParser\NodeVisitor\NodeConnectingVisitor.

By default nothing will break in PHPStan 1.6.0, but users can opt-in for new and potentially breaking features: https://phpstan.org/blog/what-is-bleeding-edge

So users with bleedingEdge included in their config would probably have broken Rector.

What you can do: the easy fix is to add these lines to Rector's PHPStan config:

services:
	-
		class: PhpParser\NodeVisitor\NodeConnectingVisitor
		tags:
			- phpstan.parser.richParserNodeVisitor

But obviously this leaks memory and is very inefficient. What I do instead in PHPStan core is to adapt rules not to use 'parent' node attribute, but instead write different visitors for different scenarios.

Check this PR to see what I mean: phpstan/phpstan-src#1175

There's a bunch of new visitors there: ArrayFilterArgVisitor, ArrayMapArgVisitor, NewAssignedToPropertyVisitor, ParentStmtTypesVisitor, TryCatchTypeVisitor

And the assigned attributes are read in places where 'parent' was used instead.

@TomasVotruba
Copy link
Member

TomasVotruba commented Apr 4, 2022

Hi Ondra, thanks for heads up 👍

What I do instead in PHPStan core is to adapt rules not to use 'parent' node attribute, but instead write different visitors for different scenarios.

That could be way to go here too. We'll have to investigate each rule if we can modify it that way.

Do you have some PRs of such "moving away from parent" in PHPStan rules? We could inspire there.

ParentStmtTypesVisitor sounds good 👍

@ondrejmirtes
Copy link
Contributor Author

I did all the work in PHPStan itself in this PR: phpstan/phpstan-src#1175

@samsonasik
Copy link
Member

@ondrejmirtes $node->getAttribute('parentStmtTypes') returns array of types instead of Nodes ? How to get `parent' Node?

@ondrejmirtes
Copy link
Contributor Author

You don't. You need to write a visitor that gathers the specific data and sets it to attributes. Check my PR.

@samsonasik
Copy link
Member

I am not sure about specific data, as it can be anything in single upper to be checked after found, or up until very first Node or collections.

@ondrejmirtes
Copy link
Contributor Author

Exactly - you need to write a different visitor for each use case and gather specific data.

@samsonasik
Copy link
Member

resolved at rectorphp/rector-src#2014 with compatibility config, we may can update to use specific visitor when possible in the future.

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

No branches or pull requests

3 participants