neos-development-collection icon indicating copy to clipboard operation
neos-development-collection copied to clipboard

TASK: Replace PHPCS with PHP-CS-Fixer to enforce coding guidelines

Open bwaidelich opened this issue 1 year ago • 6 comments

Adds PHP-CS-Fixer as dev dependency as replacement for PHPCS and configures the following rules:

  • PSR12 PSR-12 Coding Guideline – that was previously covered by PHPCS
  • no_unused_imports disallows unused PHP namespace imports
  • ordered_imports requires use statements to be ordered alphabetically

It makes those checks part of our composer:lint script and adds:

  • composer lint:php-cs-fixer to check them and
  • composer lint:php-cs-fixer:fix to fix violations
  • composer lint:fix to fix any linter violations (currently the same)

Resolves: #4580

bwaidelich avatar Mar 17 '24 12:03 bwaidelich

This build will fail. Locally I get:

Found 237 of 739 files that can be fixed in 4.083 seconds, 22.000 MB memory used

When I run composer lint:php-cs-fixer:fix it fixes those namespace imports and afterwards I get

Found 0 of 739 files that can be fixed in 0.030 seconds, 18.000 MB memory used

bwaidelich avatar Mar 17 '24 12:03 bwaidelich

Its a bit sad that we really need another dev dependency for this :/

That makes stuff just needless complex 😅

i currently only use phpstan locally to be honest and even ignore phpcs as it slows me down :P

mhsdesign avatar Mar 17 '24 13:03 mhsdesign

Its a bit sad that we really need another dev dependency for this :/ That makes stuff just needless complex

It's just a dev dependency, it doesn't affect complexity of our code and in the mid-term can even help us to keep complexity lower. But of course there is a cost attached to any new dependency..

i currently only use phpstan locally to be honest and even ignore phpcs as it slows me down :P

You are free to ignore it, but our CI won't and IMO that's a good thing that helps us to establish a common coding style. PHPStorm has built-in support for all three quality tools that highlight errors right away. And if configured correctly, it will create code in the same style and fix it.

Long story short: I think it's valuable to have the namespace imports cleaned up by a tool (and it was you that brought this up *g). PHPCS won't be able to do this, and from reading the comments on https://github.com/squizlabs/PHP_CodeSniffer/pull/1106 it will most likely never support it.

Having said this, maybe PHP-CS-Fixer supports the other checks that we currently do with PHPCS, so we could at least replace it!?

bwaidelich avatar Mar 18 '24 17:03 bwaidelich

Having said this, maybe PHP-CS-Fixer supports the other checks that we currently do with PHPCS, so we could at least replace it!?

That's seems to be the case.. @mhsdesign What do you think of b418924dea957076da84fd893c022a55ec59ae9b ?

Note: This is just an experiment. We should all agree if whether want to switch the quality tools

bwaidelich avatar Mar 18 '24 17:03 bwaidelich

Thanks a lot @bwaidelich !

I must say this looks good in my eyes, but i have only used phpstan so far and let php cs be handled by the ci (rather i forget about its existence all the time).

Getting rid of php cs might be a good idea as they have a super slow release cycle and no clear maintainer, which lead to weird hotfixes and lost time in the past.

So overall im not opinionated in which tool to learn / use but do plead for simplicity so 1 tool should be enough 😅

mhsdesign avatar Mar 18 '24 18:03 mhsdesign

A super great goal would be imo to then get also rid of our styleci, as i havent figured out how to run it locally and this is currently always a real bummer when developing as i always forget how many spaces before an anonymous function :D.

So if we can reduce the complexity from two linters to only php cs fixer that would great and im happy to assist.

mhsdesign avatar Mar 18 '24 18:03 mhsdesign