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

Use static code analysis #1943

Open
deguif opened this issue Mar 23, 2021 · 6 comments
Open

Use static code analysis #1943

deguif opened this issue Mar 23, 2021 · 6 comments

Comments

@deguif
Copy link
Collaborator

deguif commented Mar 23, 2021

I started working on using PHPStan to detect potential wrong code.
Many PRs I opened recently were related to this analysis.
This can be tracked here: https://github.com/deguif/Elastica/actions?query=branch%3Aphpstan+

Here's the current 35 remaining errors:

------ ------------------------------------------------------------------- 
  Line   Aggregation/GeohashGrid.php                                        
 ------ ------------------------------------------------------------------- 
  48     Result of && is always false.                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this  
         check by setting treatPhpDocTypesAsCertain: false in your          
         phpstan.neon.dist.                                                 
  49     Else branch is unreachable because ternary operator condition is   
         always true.                                                       
 ------ ------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------- 
  Line   Bulk/Action/AbstractDocument.php                                   
 ------ ------------------------------------------------------------------- 
  124    Result of && is always false.                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this  
         check by setting treatPhpDocTypesAsCertain: false in your          
         phpstan.neon.dist.                                                 
 ------ ------------------------------------------------------------------- 

 ------ ---------------------------------------------------------------------- 
  Line   Client.php                                                            
 ------ ---------------------------------------------------------------------- 
  363    Result of method Elastica\Client::_initConnections() (void) is used.  
 ------ ---------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------- 
  Line   ClientConfiguration.php                                            
 ------ ------------------------------------------------------------------- 
  85     Parameter #1 $dsn of static method                                 
         Elastica\ClientConfiguration::parseDsn() expects                   
         Nyholm\Dsn\Configuration\Url,                                      
         Nyholm\Dsn\Configuration\Dsn|Nyholm\Dsn\Configuration\DsnFunction  
         given.                                                             
 ------ ------------------------------------------------------------------- 

 ------ ------------------------------------------------------- 
  Line   Connection.php                                         
 ------ ------------------------------------------------------- 
  340    Unreachable statement - code above always terminates.  
 ------ ------------------------------------------------------- 

 ------ ------------------------------------------------------- 
  Line   Document.php                                           
 ------ ------------------------------------------------------- 
  273    Unreachable statement - code above always terminates.  
 ------ ------------------------------------------------------- 

 ------ ------------------------------------------------------------------- 
  Line   Index.php                                                          
 ------ ------------------------------------------------------------------- 
  402    Else branch is unreachable because ternary operator condition is   
         always true.                                                       
  702    Result of && is always false.                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this  
         check by setting treatPhpDocTypesAsCertain: false in your          
         phpstan.neon.dist.                                                 
 ------ ------------------------------------------------------------------- 

 ------ ------------------------------------------------------- 
  Line   Mapping.php                                            
 ------ ------------------------------------------------------- 
  197    Unreachable statement - code above always terminates.  
 ------ ------------------------------------------------------- 

 ------ --------------------------------------------------------------------- 
  Line   Pipeline.php                                                         
 ------ --------------------------------------------------------------------- 
  116    Array (array<Elastica\Processor\AbstractProcessor>) does not accept  
         array.                                                               
  119    Array (array<Elastica\Processor\AbstractProcessor>) does not accept  
         array<mixed, mixed>.                                                 
  119    Parameter #1 $arr1 of function array_merge expects array,            
         Elastica\Processor\AbstractProcessor given.                          
 ------ --------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------ 
  Line   Processor/ForeachProcessor.php                                    
 ------ ------------------------------------------------------------------ 
  29     Else branch is unreachable because ternary operator condition is  
         always true.                                                      
 ------ ------------------------------------------------------------------ 

 ------ ----------------------------------------------------------------------- 
  Line   Query.php                                                              
 ------ ----------------------------------------------------------------------- 
  41     Instanceof between null and Elastica\Suggest will always evaluate to   
         false.                                                                 
         💡 Because the type is coming from a PHPDoc, you can turn off this      
         check by setting treatPhpDocTypesAsCertain: false in your              
         phpstan.neon.dist.                                                     
  43     Instanceof between null and Elastica\Collapse will always evaluate to  
         false.                                                                 
         💡 Because the type is coming from a PHPDoc, you can turn off this      
         check by setting treatPhpDocTypesAsCertain: false in your              
         phpstan.neon.dist.                                                     
  390    Result of && is always false.                                          
         💡 Because the type is coming from a PHPDoc, you can turn off this      
         check by setting treatPhpDocTypesAsCertain: false in your              
         phpstan.neon.dist.                                                     
 ------ ----------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------- 
  Line   Query/BoolQuery.php                                                
 ------ ------------------------------------------------------------------- 
  110    Result of && is always false.                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this  
         check by setting treatPhpDocTypesAsCertain: false in your          
         phpstan.neon.dist.                                                 
 ------ ------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------- 
  Line   Query/DisMax.php                                                   
 ------ ------------------------------------------------------------------- 
  27     Result of && is always false.                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this  
         check by setting treatPhpDocTypesAsCertain: false in your          
         phpstan.neon.dist.                                                 
 ------ ------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------- 
  Line   Query/SpanMulti.php                                                
 ------ ------------------------------------------------------------------- 
  51     Result of && is always false.                                      
         💡 Because the type is coming from a PHPDoc, you can turn off this  
         check by setting treatPhpDocTypesAsCertain: false in your          
         phpstan.neon.dist.                                                 
 ------ ------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------ 
  Line   Query/Terms.php                                                   
 ------ ------------------------------------------------------------------ 
  65     Else branch is unreachable because ternary operator condition is  
         always true.                                                      
 ------ ------------------------------------------------------------------ 

 ------ ----------------------------------------------------------- 
  Line   QueryBuilder.php                                           
 ------ ----------------------------------------------------------- 
  73     Method Elastica\QueryBuilder::query() should return        
         Elastica\QueryBuilder\DSL\Query but returns                
         Elastica\QueryBuilder\Facade.                              
  83     Method Elastica\QueryBuilder::aggregation() should return  
         Elastica\QueryBuilder\DSL\Aggregation but returns          
         Elastica\QueryBuilder\Facade.                              
  93     Method Elastica\QueryBuilder::suggest() should return      
         Elastica\QueryBuilder\DSL\Suggest but returns              
         Elastica\QueryBuilder\Facade.                              
  103    Method Elastica\QueryBuilder::collapse() should return     
         Elastica\QueryBuilder\DSL\Collapse but returns             
         Elastica\QueryBuilder\Facade.                              
 ------ ----------------------------------------------------------- 

 ------ ------------------------------ 
  Line   Request.php                   
 ------ ------------------------------ 
  190    If condition is always true.  
 ------ ------------------------------ 

 ------ --------------------------------------------------------------- 
  Line   Script/AbstractScript.php                                      
 ------ --------------------------------------------------------------- 
  61     Method Elastica\Script\AbstractScript::create() should return  
         Elastica\Script\Script|Elastica\Script\ScriptId but returns    
         Elastica\Script\AbstractScript.                                
  71     Method Elastica\Script\AbstractScript::create() should return  
         Elastica\Script\Script|Elastica\Script\ScriptId but returns    
         static(Elastica\Script\AbstractScript).                        
  74     Unreachable statement - code above always terminates.          
 ------ --------------------------------------------------------------- 

 ------ --------------------------------------------------------------------- 
  Line   Search.php                                                           
 ------ --------------------------------------------------------------------- 
  340    Parameter #2 $query of method Elastica\Search::setOptionsAndQuery()  
         expects array|Elastica\Query|string, Elastica\Suggest given.         
 ------ --------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------ 
  Line   Transport/Guzzle.php                                              
 ------ ------------------------------------------------------------------ 
  120    Strict comparison using === between '0' and array() will always   
         evaluate to false.                                                
  134    Else branch is unreachable because ternary operator condition is  
         always true.                                                      
 ------ ------------------------------------------------------------------ 

 ------ ----------------------------------------------------------------------- 
  Line   Transport/Http.php                                                     
 ------ ----------------------------------------------------------------------- 
  102    Result of && is always true.                                           
  123    Strict comparison using === between '0' and array() will always        
         evaluate to false.                                                     
  130    Else branch is unreachable because previous condition is always true.  
 ------ -----------------------------------------------------------------------
@ruflin
Copy link
Owner

ruflin commented Mar 24, 2021

Do you plan to have it enabled as part of CI in the future? Are there false positives sometimes?

@deguif
Copy link
Collaborator Author

deguif commented Mar 24, 2021

Do you plan to have it enabled as part of CI in the future?

Yes absolutely, having it enabled in the CI would be benefit IMO.

Are there false positives sometimes?

Yes, sometimes. But they can be skipped by ignoredErrors config or doc comments like (@phpstan-ignore-line, @phpstan-ignore-next-line). See https://phpstan.org/user-guide/ignoring-errors

@thePanz
Copy link
Collaborator

thePanz commented Oct 27, 2021

Thank you @deguif for this effort! 👍
IMHO there are some cases where the type-hints are so broad that even phpstan can get confused.
One case is the setQuery: see #1739

Should we plan for a major review of the codebase, being more strict on types? wdyt?

@ruflin
Copy link
Owner

ruflin commented Oct 27, 2021

In general I'm on board about making the code base more strict especially for the scenarios where the type is clear. Lets see if it will force us to do some breaking changes but often these are actually bugs we fix ;-)

@deguif
Copy link
Collaborator Author

deguif commented Oct 28, 2021

I'm 👍 on being more strict on types.
This probably means bumping minimum PHP version to a more recent to handle complex type like union type.

@ruflin
Copy link
Owner

ruflin commented Oct 28, 2021

Maybe 8 is a good option for this. Time to branch off 7.x?

@deguif deguif mentioned this issue May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants