Added phpstan level 0 and fixed various import issues

isotope / core

Core repository of Isotope eCommerce, an eCommerce extension for Contao Open Source CMS

https://isotopeecommerce.org

136 stars 107 forks source link

Added phpstan level 0 and fixed various import issues #2510

Closed Shadow-Devil closed 7 months ago

Shadow-Devil commented 8 months ago

Part of #2509. Adds phpstan with level 0, but there were some issues:

Phpstan needs the swissbilling-api dependency to be installed, so I added it as a dev dependency
In swissbilling-api there exists one class which is not psr-4 conform (ApiFactory) and this library also does not support php 8. I created a PR which also fixes these things (https://github.com/terminal42/swissbilling-api/pull/1).
There was some issue with the FrontendTemplateTrait, which declared the getTemplate method as not static. I also created a PR for this (https://github.com/contao/contao/pull/6692)
Phpstan often reportet "Unsafe usage of new static()." but I think this is intended, so I ignored this error for now. See also: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static

aschempp commented 7 months ago

Thank you @Shadow-Devil !

Shadow-Devil commented 7 months ago

I am a bit confused, since when I now run: vendor/terminal42/contao-build-tools/tools/phpstan/vendor/bin/phpstan analyze system

There is no #Function file_exists is unsafe to use# matched, which you explicitly ignored. Can you please tell me why you added it?

aschempp commented 6 months ago

There are two file_exists calls in system/modules/isotope/library/Isotope/Template.php. If I remove the ignoreError, I get that reported.

------ ------------------------------------------------------------------ 
  Line   isotope/library/Isotope/Template.php                              
 ------ ------------------------------------------------------------------ 
  50     Function file_exists is unsafe to use, rely on Symfony component  
         Filesystem::exists instead.                                       
  54     Function file_exists is unsafe to use, rely on Symfony component  
         Filesystem::exists instead.                                       
 ------ ------------------------------------------------------------------

aschempp commented 6 months ago

btw, you can simply run composer run phpstan 😎

fritzmg commented 6 months ago

Filesystem::exists is not safer than file_exists. It only introduces a check for \PHP_MAXPATHLEN - which file_exists does itself anyway. It looks like Symfony introduced the check in Symfony 2.3.38 (and later updated it in Symfony 2.7.35 to use \PHP_MAXPATHLEN) due to this PHP bug, which has since been fixed in PHP 7.1. The only advantage is that it will throw a nice IOException for you to catch in such a case.

aschempp commented 6 months ago

Idk, I get the error when I run phpstan, I never asked for why it is there 😉