Simplify *StatementWithoutImpurePointsRule

[staabm]

identified in #4372

 ------ ------------------------------------------------------------------------------------------------------------------------------- 
  Line   src/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRule.php                                                            
 ------ ------------------------------------------------------------------------------------------------------------------------------- 
  34     Call to function array_key_exists() with non-falsy-string and array<non-falsy-string, *NEVER*> will always evaluate to false.  
         🪪  function.impossibleType                                                                                                    
         at src/Rules/DeadCode/CallToMethodStatementWithoutImpurePointsRule.php:34                                                      

 ------ ------------------------------------------------------------------------------------------------------------------------------- 
  Line   src/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRule.php                                                      
 ------ ------------------------------------------------------------------------------------------------------------------------------- 
  34     Call to function array_key_exists() with non-falsy-string and array<non-falsy-string, *NEVER*> will always evaluate to false.  
         🪪  function.impossibleType                                                                                                    
         at src/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRule.php:34                                                

[ondrejmirtes]

Oh now we're getting somewhere! Please add these cases as regression tests to the PR that improves the analysis.

[staabm]

Please add these cases as regression tests to the PR that improves the analysis.

done in 5f3c308

[ondrejmirtes]

I haven't looked at the code when I posted the first comment. I think this is a false positive in #4372. I also think the type array<non-falsy-string, *NEVER*> is really weird. *NEVER* as value type basically means the array is empty?

When we look at the code:

		$methods = [];
		foreach ($node->get(MethodWithoutImpurePointsCollector::class) as $collected) {
			foreach ($collected as [$className, $methodName, $classDisplayName]) {
				$className = strtolower($className);

				if (!array_key_exists($className, $methods)) {
					$methods[$className] = [];
				}
				$methods[$className][strtolower($methodName)] = $classDisplayName . '::' . $methodName;
			}
		}

On the first iteration, the array is empty. But on 2nd iteration, we don't know what $className contains. It might be a value that's already in the array (which means that array_key_exists returns true), or it might be a new value that's not in the array.

So I think the message "will always evaluate to false" is wrong here.

[staabm]

So I think the message "will always evaluate to false" is wrong here.

I agree with this observation and we shouldn't have this error.
I have no idea yet how to prevent the error.

the PR as is, still makes sense though, as the array-key-exists before array-append does not serve a purpose IMO:

https://3v4l.org/XoEIE

[ondrejmirtes]

Yeah, sure, it can be removed. But please add this piece of code as a regression test to the other PR to make sure this isn't reported there in the end.

As for my tip what's causing it: please check that $methods[$lowerClassName] is cleared in MutatingScope::$expressionTypes when the next loop iteration starts. Here

phpstan-src/src/Analyser/NodeScopeResolver.php

Line 1213 in e3fd45f

$bodyScope = $this->enterForeach($originalScope, $originalScope, $stmt, $nodeCallback);

and here

phpstan-src/src/Analyser/NodeScopeResolver.php

Line 1237 in e3fd45f

$bodyScope = $this->enterForeach($bodyScope, $originalScope, $stmt, $nodeCallback);

.

[ondrejmirtes]

Thank you.

[staabm]

But please add this piece of code as a regression test to the other PR to make sure this isn't reported there in the end.

have it here cb8e036

[staabm]

As for my tip what's causing it: please check that $methods[$lowerClassName] is cleared in MutatingScope::$expressionTypes when the next loop iteration starts. Here

while I find more and more side-quest cleanups like #4380 I am not able to figure out a proper fix.

I tried invalidating the expressions but it did not solve the array<non-falsy-string, *NEVER*> problem

[ondrejmirtes]

Found the problem (84b61d4), pushed fix to your PR.

[staabm]

thank you!