Some phpstan issues #2393

belgattitude · 2019-03-01T22:07:14Z

Hey Grav Team

Would you be interested in adding phpstan ?

Here's a little example of setup and few fixes.

If you're not used to phpstan, have a look to the composer.json script section or run

$ composer phpstan

For now, I've just setup level 0 (it goes to 7, but there will be a lot of alerts then). I would be happy to help to fix some alerts... and help increase the level.

Let me know.

…them

…ect case

…:$uploadObjects.

belgattitude · 2019-03-01T22:55:59Z

An example:

 ------ -------------------------------------------------------------------------------------------------- 
  Line   Grav/Common/Assets/Traits/AssetUtilsTrait.php (in context of class Grav\Common\Assets\BaseAsset)  
 ------ -------------------------------------------------------------------------------------------------- 
  90     Call to an undefined method Grav\Common\Assets\BaseAsset::cssRewrite().                           
 ------ -------------------------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------- 
  Line   Grav/Common/GPM/Remote/GravCore.php                                  
 ------ --------------------------------------------------------------------- 
  29     Grav\Common\GPM\Remote\GravCore::__construct() does not call parent  
         constructor from Grav\Common\GPM\Remote\AbstractPackageCollection.   
 ------ --------------------------------------------------------------------- 

 ------ ------------------------------------------------------- 
  Line   Grav/Common/Scheduler/Job.php                          
 ------ ------------------------------------------------------- 
  480    Call to static method sendEmail() on an unknown class  
         Grav\Plugin\Email\Utils.                               
 ------ ------------------------------------------------------- 

 ------ ---------------------------------------------------------------- 
  Line   Grav/Common/Service/AccountsServiceProvider.php                 
 ------ ---------------------------------------------------------------- 
  15     LogicException (User class was called too early!) thrown while  
         autoloading class Grav\Common\User\User.                        
  48     LogicException (User class was called too early!) thrown while  
         autoloading class Grav\Common\User\User.                        
  61     LogicException (User class was called too early!) thrown while  
         autoloading class Grav\Common\User\User.                        
 ------ ---------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------ 
  Line   Grav/Common/User/Group.php                                              
 ------ ------------------------------------------------------------------------ 
  115    Method Grav\Common\Data\Data::blueprints() invoked with 1 parameter, 0  
         required.                                                               
 ------ ------------------------------------------------------------------------ 

 ------ ---------------------------------------------------------------- 
  Line   Grav/Common/User/User.php                                       
 ------ ---------------------------------------------------------------- 
         LogicException (User class was called too early!) thrown while  
         autoloading class Grav\Common\User\User.                        
 ------ ---------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------- 
  Line   Grav/Console/ConsoleTrait.php (in context of class Grav\Console\ConsoleCommand)  
 ------ --------------------------------------------------------------------------------- 
  132    Access to an undefined property                                                  
         Grav\Console\ConsoleCommand::$local_config.                                      
 ------ --------------------------------------------------------------------------------- 

 ------ ------------------------------------------------ 
  Line   Grav/Framework/File/Formatter/CsvFormatter.php  
 ------ ------------------------------------------------ 
  47     Undefined variable: $lines                      
 ------ ------------------------------------------------ 

 ------ ----------------------------------------------------------------------------------------------------------- 
  Line   Grav/Framework/Flex/Traits/FlexAuthorizeTrait.php (in context of class Grav\Framework\Flex\FlexDirectory)  
 ------ ----------------------------------------------------------------------------------------------------------- 
  43     Call to an undefined method                                                                                
         Grav\Framework\Flex\FlexDirectory::exists().                                                               
 ------ ----------------------------------------------------------------------------------------------------------- 

 ------ --------------------------------------- 
  Line   Grav/Framework/Form/FormFlashFile.php  
 ------ --------------------------------------- 
  48     Call to an undefined static method     
         Grav\Framework\Psr7\Stream::create().  
 ------ ---------------------------------------

Some of those problems are a little more complex and I could not find how to solve them easily:

Stream::create() family are solved PSR-17 (stream factories). Nyholm/psr7 implements them here.
FlexAuthorizeTrait.php as an example of an issue... Maybe create an abstract method exists(), so you're sure all classes using it, will have to implement it.
In general I don't really understand the need for PSR7 decorator and traits... You can inject the interface in the constructor (if null sets Nyholm/psr7 as default for example).
User.php and LogicException.... Can possibly break autoloading. This case can probably be solved by a factory and a Userinterface ?
....

Don't want to be sound negative ;) just some thoughts

rhukster · 2019-03-02T00:17:39Z

I really like the idea of utilizing phpstan and resolving some of these issues reported. However, it might be something where we properly sort issues as part of Grav 1.7 (as 1.6 is so close to release). no real harm in adding into now though, and fixing simple things.

belgattitude · 2019-03-02T09:59:54Z

Looks good to me.

So for the config:

Moved the phpstan.neon out of public/base directory. No need to worry of updating .htaccess and apache/nginx configs.
Added a phpstan bootstrap to define GRAV_USER_INSTANCE prior to analysis
Added a phpstan composer script for level 0.
Added initial exclusions.

For the bugs:

Major: Invalid private level in FileCache::init().
Medium: Fixed parent::__construct() calling where applicable.
Medium: Fixed some invalid cases (notably Twig_SimpleFunction)
Minor: Some invalid typehints (phpdoc) or missing properties.

To go forward, I would like to know if it's possible to run coverage. I saw that c3 is not installed. I can enable but I'd like to be sure it does not end in public directory. Let me know if you already have already something (codecov...) ? Will feel more confident about what I can work on ;)

From there, I can help with:

 ------ ------------------------------------------------ 
  Line   Grav/Framework/File/Formatter/CsvFormatter.php  
 ------ ------------------------------------------------ 
  47     Undefined variable: $lines                      
 ------ ------------------------------------------------

and with PHP7.3

------ ------------------------------------------------------------------------------------------------------------------- 
  Line   Grav/Common/GPM/Remote/GravCore.php                                                                                
 ------ ------------------------------------------------------------------------------------------------------------------- 
  76     Regex pattern is invalid: Compilation failed: invalid range in character class at offset 3 in pattern: /[\w-\.]+/  
 ------ -------------------------------------------------------------------------------------------------------------------

The next issues will need to be tackled in separate P/R.

For that I suggest to merge this one in 1.6 as soon as you feel good with it. (to not be trapped in rebase nightmares)

belgattitude · 2019-03-02T10:10:29Z

FYI, current report with PHP7.3

 ------ -------------------------------------------------------------------------------------------------- 
  Line   Grav/Common/Assets/Traits/AssetUtilsTrait.php (in context of class Grav\Common\Assets\BaseAsset)  
 ------ -------------------------------------------------------------------------------------------------- 
  90     Call to an undefined method Grav\Common\Assets\BaseAsset::cssRewrite().                           
 ------ -------------------------------------------------------------------------------------------------- 

 ------ ------------------------------------------------------------------------------------------------------------------- 
  Line   Grav/Common/GPM/Remote/GravCore.php                                                                                
 ------ ------------------------------------------------------------------------------------------------------------------- 
  76     Regex pattern is invalid: Compilation failed: invalid range in character class at offset 3 in pattern: /[\w-\.]+/  
 ------ ------------------------------------------------------------------------------------------------------------------- 

 ------ ---------------------------------------------------------------------------------- 
  Line   Grav/Common/User/Group.php                                                        
 ------ ---------------------------------------------------------------------------------- 
  115    Method Grav\Common\Data\Data::blueprints() invoked with 1 parameter, 0 required.  
 ------ ---------------------------------------------------------------------------------- 

 ------ --------------------------------------------------------------------------------- 
  Line   Grav/Console/ConsoleTrait.php (in context of class Grav\Console\ConsoleCommand)  
 ------ --------------------------------------------------------------------------------- 
  132    Access to an undefined property Grav\Console\ConsoleCommand::$local_config.      
 ------ --------------------------------------------------------------------------------- 

 ------ ------------------------------------------------ 
  Line   Grav/Framework/File/Formatter/CsvFormatter.php  
 ------ ------------------------------------------------ 
  47     Undefined variable: $lines                      
 ------ ------------------------------------------------ 

 ------ ----------------------------------------------------------------------------------------------------------- 
  Line   Grav/Framework/Flex/Traits/FlexAuthorizeTrait.php (in context of class Grav\Framework\Flex\FlexDirectory)  
 ------ ----------------------------------------------------------------------------------------------------------- 
  43     Call to an undefined method Grav\Framework\Flex\FlexDirectory::exists().                                   
 ------ -----------------------------------------------------------------------------------------------------------

belgattitude · 2019-03-02T10:11:50Z

Also this one is a little tricky:

PHP Warning:  Declaration of Grav\Framework\Cache\Adapter\FileCache::init($namespace, $directory) should be compatible with Grav\Framework\Cache\AbstractCache::init($namespace = '', $defaultLifetime = NULL) in /web/www/grav/system/src/Grav/Framework/Cache/Adapter/FileCache.php

This one is a definitely a bug... better to make a separate P/R

…7.1 only)

…ired)

belgattitude · 2019-03-02T11:14:48Z

I think for now I'll wait for merge or comments ;)

Thanks a lot for your work.

tests/phpstan/phpstan-bootstrap.php

rhukster · 2019-03-12T20:16:11Z

This is looking really good so far! I'm going to test it a little today.

mahagr · 2019-03-13T07:52:07Z

Level 0 has only a single error left, @rhukster can take a look into it. :)

mahagr · 2019-03-13T08:17:13Z

Level 1 issues sorted!

belgattitude · 2019-03-13T10:14:59Z

I see you're super motivated ;) Nice !

belgattitude added 12 commits March 1, 2019 22:36

Fix bug, method init must be protected

cfdb2dc

Added phpstan level 0

cf8a062

Added exclusions

77c165e

Fix: incorrect case in Twig_SimpleFunction

5247427

Fix, no abstract properties, if properties used in methods let's set …

1ba1ed1

…them

Fixed Psr\SimpleCache\InvalidArgumentException referenced with incorr…

b11bd1a

…ect case

added exclusions

9294168

Fixed: Access to an undefined property Grav\Framework\Form\FormFlash:…

35c0fd9

…:$uploadObjects.

Fixed: does not call parent constructor from Grav\Common\Iterator.

bae64f6

Fixed: does not call parent constructor from Grav\Common\Iterator.

5b2759b

Fixed: does not call parent constructor

c1cd878

Fixed: does not call parent constructor

2eac057

rhukster added the enhancement label Mar 2, 2019

belgattitude added 4 commits March 2, 2019 09:30

Minor: correct type for inflector

08a923d

Moved phpstan.neon out of public directory

f0b6d3e

Added exclusion

8a17ff6

set GRAV_USER_INSTANCE to prevent LogiException in User

7849398

Exlude Stream:create

ae95fc0

belgattitude added 7 commits March 2, 2019 11:19

Minor: Missing storage property

2273e27

Minor: Fixed missing properties (phpstan level 1)

4c4330e

Added type for $data

b37467e

Minor: Ensure $langs is initialized

72e9bef

Fix possible bug in $http_response_header status code retrieval (PHP …

ac663ca

…7.1 only)

Added exclusion for $http_response_header (isset is actually not requ…

a530481

…ired)

Strict null check

f5fbcfa

mahagr requested changes Mar 11, 2019

View reviewed changes

tests/phpstan/phpstan-bootstrap.php Show resolved Hide resolved

mahagr approved these changes Mar 13, 2019

View reviewed changes

mahagr merged commit ecd3942 into getgrav:1.6 Mar 13, 2019

belgattitude deleted the 1.6 branch March 13, 2019 10:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Some phpstan issues #2393

Some phpstan issues #2393

belgattitude commented Mar 1, 2019

belgattitude commented Mar 1, 2019

rhukster commented Mar 2, 2019

belgattitude commented Mar 2, 2019 •

edited

Loading

belgattitude commented Mar 2, 2019

belgattitude commented Mar 2, 2019 •

edited

Loading

belgattitude commented Mar 2, 2019

rhukster commented Mar 12, 2019

mahagr commented Mar 13, 2019

mahagr commented Mar 13, 2019

belgattitude commented Mar 13, 2019

Some phpstan issues #2393

Some phpstan issues #2393

Conversation

belgattitude commented Mar 1, 2019

belgattitude commented Mar 1, 2019

rhukster commented Mar 2, 2019

belgattitude commented Mar 2, 2019 • edited Loading

belgattitude commented Mar 2, 2019

belgattitude commented Mar 2, 2019 • edited Loading

belgattitude commented Mar 2, 2019

rhukster commented Mar 12, 2019

mahagr commented Mar 13, 2019

mahagr commented Mar 13, 2019

belgattitude commented Mar 13, 2019

belgattitude commented Mar 2, 2019 •

edited

Loading

belgattitude commented Mar 2, 2019 •

edited

Loading