Save native expression types by ExpressionHolder

[rajyan]

Another step forward for phpstan/phpstan#8191 suggested in #1919 (comment)

[ondrejmirtes]

Sorry, I'm just on my phone. I'd expect this to be reverted here or handled better, is it? 728336c

[rajyan]

@ondrejmirtes
Yes, in different commits to make it clear :)
b674b3e

[ondrejmirtes]

Thank you very much! 😊

[ondrejmirtes]

FYI There's some bad performance regression in this PR, it makes analysis in two real world projects run indefinitely. I'll try to figure out what's the cause.

[ondrejmirtes]

This commit seems so innocent a098065 but it makes this file being analysed for 27 seconds instead of 2 seconds: https://gist.github.com/ondrejmirtes/693136deb16f88a98c1973ecaad23079

It probably uncovers some underlying problem with enums and the fix is going to lie somewhere else instead of changing the lines that changed here...

Currently investigating with Blackfire.

[ondrejmirtes]

Alright, the performance problem was fixed with this: 1ec058d

[ondrejmirtes]

I figured it out because I added some debug statements here:

--- a/src/Analyser/NodeScopeResolver.php
+++ b/src/Analyser/NodeScopeResolver.php
@@ -2697,7 +2697,16 @@ class NodeScopeResolver
 					ExpressionContext::createTopLevel(),
 				);
 				$armScope = $armResult->getScope();
+				var_dump('scope first:');
+				var_dump($scope->debug());
+
+				var_dump('armScope first:');
+				var_dump($armScope->debug());
 				$scope = $scope->mergeWith($armScope);
+
+				var_dump('scope second:');
+				var_dump($scope->debug());
+				var_dump('------');
 				$hasYield = $hasYield || $armResult->hasYield();
 				$throwPoints = array_merge($throwPoints, $armResult->getThrowPoints());
 				$matchScope = $matchScope->filterByFalseyValue($filteringExpr);

And the log made it obvious:

/Users/ondrej/Development/slevomat/app/services/PointOfInterest/PointOfInterestType.php
string(12) "scope first:"
array(1) {
  ["$this (Yes)"]=>
  string(51) "$this(Slevomat\PointOfInterest\PointOfInterestType)"
}
string(15) "armScope first:"
array(2) {
  ["$this (Yes)"]=>
  string(111) "$this(Slevomat\PointOfInterest\PointOfInterestType)&Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER"
  ["native $this"]=>
  string(111) "$this(Slevomat\PointOfInterest\PointOfInterestType)&Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER"
}
string(13) "scope second:"
array(2) {
  ["$this (Yes)"]=>
  string(51) "$this(Slevomat\PointOfInterest\PointOfInterestType)"
  ["native $this"]=>
  string(111) "$this(Slevomat\PointOfInterest\PointOfInterestType)&Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER"
}
string(6) "------"
string(12) "scope first:"
array(2) {
  ["$this (Yes)"]=>
  string(51) "$this(Slevomat\PointOfInterest\PointOfInterestType)"
  ["native $this"]=>
  string(111) "$this(Slevomat\PointOfInterest\PointOfInterestType)&Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER"
}
string(15) "armScope first:"
array(2) {
  ["$this (Yes)"]=>
  string(164) "$this(Slevomat\PointOfInterest\PointOfInterestType~Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER)&Slevomat\PointOfInterest\PointOfInterestType::MUSEUM"
  ["native $this"]=>
  string(164) "$this(Slevomat\PointOfInterest\PointOfInterestType~Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER)&Slevomat\PointOfInterest\PointOfInterestType::MUSEUM"
}
string(4) "here"
string(13) "scope second:"
array(2) {
  ["$this (Yes)"]=>
  string(51) "$this(Slevomat\PointOfInterest\PointOfInterestType)"
  ["native $this"]=>
  string(280) "($this(Slevomat\PointOfInterest\PointOfInterestType)&Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER)|($this(Slevomat\PointOfInterest\PointOfInterestType~Slevomat\PointOfInterest\PointOfInterestType::LOOKOUT_TOWER)&Slevomat\PointOfInterest\PointOfInterestType::MUSEUM)"
}
string(6) "------"

The native $this got longer and longer because the starting point wasn't the same.

[rajyan]

Thank you for the fix!
1ec058d
I had these fixes in a different PR branch, couldn't notice that this breaks the performance that much.

and thank you for
69d7e81
too. I was a little worried about the performance when using exprStringToExpr.