Conditional types regressions

[ondrejmirtes]

Bug report

Hi @rajyan, there's some more work to do 😅

I updated the Composer integration test and there are some new errors. The build is green on 1.9.2:

 ------ ---------------------------------------------------------------- 
  Line   src/Composer/Command/ShowCommand.php                            
 ------ ---------------------------------------------------------------- 
  837    Parameter #2 $array of function implode expects array<string>,  
         array<int<0, max>|string, array<string>|string> given.          
 ------ ---------------------------------------------------------------- 

 ------ ------------------------------------------------------- 
  Line   src/Composer/Installer/SuggestedPackagesReporter.php   
 ------ ------------------------------------------------------- 
  129    Foreach overwrites $suggestion with its key variable.  
 ------ ------------------------------------------------------- 

 ------ -------------------------------------------------------------- 
  Line   src/Composer/Package/Package.php                              
 ------ -------------------------------------------------------------- 
  683    Parameter #4 $type of static method                           
         Composer\Util\ComposerMirror::processHgUrl() expects string,  
         string|null given.                                            
 ------ -------------------------------------------------------------- 

 ------ ------------------------------------------------------------- 
  Line   src/Composer/Util/Git.php                                    
 ------ ------------------------------------------------------------- 
  87     Parameter #1 $directory of method                            
         Composer\Util\Filesystem::removeDirectory() expects string,  
         string|null given.                                           
  251    Parameter #1 $directory of method                            
         Composer\Util\Filesystem::removeDirectory() expects string,  
         string|null given.                                           
 ------ ------------------------------------------------------------- 

In ShowCommand it should know the type is narrowed because the getter has an array shape type alias. So it should know it's already list<string>.

SuggestedPackagesReporter might be a bit tricky. This isn't new behaviour but it's reported in more cases because the conditonal types are now more "smart". The problem is in polluteScopeWithAlwaysIterableForeach: false (the configuration in phpstan-strict-rules which also adds the rule "foreach overwrites existing variable"). Here it is: https://phpstan.org/r/a58b7580-7018-418b-bbe6-157c5ee91758

The problem is that PHPStan with polluteScopeWithAlwaysIterableForeach: false SHOULD NOT know that a variable always exists if some array is non-empty. But here it is aware that if $k is non-empty then $v always exists. But with polluteScopeWithAlwaysIterableForeach: false we don't want this behaviour. It might be a bit tricky to fix...

And the Mercurial and Git problems are the same - the value is already checked and cannot be null at that point.

Thank you very much for looking into this, I'd be really gratefuly if you were able to fix at least some of those.

[ondrejmirtes]

Adding links:

• https://github.com/composer/composer/blob/8ff237afb61b8766efa576b8ae1cc8560c8aed96/src/Composer/Command/ShowCommand.php#L837
• https://github.com/composer/composer/blob/8ff237afb61b8766efa576b8ae1cc8560c8aed96/src/Composer/Installer/SuggestedPackagesReporter.php#L129
• https://github.com/composer/composer/blob/8ff237afb61b8766efa576b8ae1cc8560c8aed96/src/Composer/Package/Package.php#L683
• https://github.com/composer/composer/blob/8ff237afb61b8766efa576b8ae1cc8560c8aed96/src/Composer/Util/Git.php#L251

[rajyan]

Thank you for investigating!
I think I have some ideas to fixed them, gonna try it tomorrow morning.

[phpstan-bot]

@ondrejmirtes After the latest push in 1.9.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Foreach overwrites $v with its value variable.
+No errors

[rajyan]

closed by
phpstan/phpstan-src#2056
phpstan/phpstan-src#2059
phpstan/phpstan-src#2061

[ondrejmirtes]

👍

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.