Sylius / Sylius

Open Source eCommerce Framework on Symfony
https://sylius.com
MIT License
7.85k stars 2.08k forks source link

Increase PHPStan level #14019

Open lruozzi9 opened 2 years ago

lruozzi9 commented 2 years ago

PHPStan provides different rule levels: https://phpstan.org/user-guide/rule-levels. Each of these levels increases the strictness of code checks. Actually, Sylius uses the level 1. This is a good starting point but for sure it is not the more reliable. It can cause some errors if you customize some components. For example, we have totally replaced the ChannelProductPrice system calculation by adding a new price calculation strategy, but the promotion system fails because there are many points in the code where it is taken for granted that exists a ChannelProductPrice entity.

So I think the level should be increased, also for a stronger and more typed application. To do this there are mainly two ways:

What do you think? I started working on upgrading to level two of PHPStan some time ago. ASAP I will start publishing the work in a draft PR so that we can have it public.

lruozzi9 commented 2 years ago

First PR is quite ready.

NoResponseMate commented 2 years ago

Thanks for the issue @lruozzi9 The PR looks fine just need to fix the remaining errors. Would you be able to continue the work or can it be taken over?

lruozzi9 commented 2 years ago

Hi @NoResponseMate! Of course, I will complete ASAP this first PR, but to fix the remaining problems I need to have merged this PR: https://github.com/SyliusLabs/PolyfillSymfonyFrameworkBundle/pull/3 to avoid adding some rules to ignore this kind of problem 🥲

lruozzi9 commented 2 years ago

Still waiting for a new release of https://github.com/SyliusLabs/PolyfillSymfonyFrameworkBundle.

NoResponseMate commented 2 years ago

Hi @lruozzi9! A new tag has just been released 🍻

lruozzi9 commented 2 years ago

Seen it 😎

lruozzi9 commented 2 years ago

Level 2 is ready 😎🎆

lruozzi9 commented 1 year ago

For the next level (6) there are 1289 errors 😅. This is because a lot of methods do not specify the types of parameters or the return type (like getId() of resources). So it is acceptable to start a baseline, what do you think?