Developer Guide: php-cs-fixer

[mmattel]

This is a reminder that this needs to be documented in the developer guide when the referenced PR gets merged: owncloud/core#31198 (php-cs-fixer fix . --rules=native_function_invocation)

• document php-cs-fixer owncloud-codestyle checker Document php-cs-fixer in the developer documentation #1963
• document squizlabs/php_codesniffer used on acceptance test code @phil-davis

[DeepDiver1975]

THX @mmattel for taking care!

[mmattel]

https://github.com/FriendsOfPHP/PHP-CS-Fixer

[mmattel]

@settermjd referenced PR got merged. Removing Wait for PR.

@DeepDiver1975 pls help creating the doc. You can just paste your text here and we will setup it properly.

[mmattel]

Copied link from talk: https://central.owncloud.org/t/introducing-php-cs-fixer/13163

[settermjd]

Thanks @mmattel. I'll have this finalised this week.

[mmattel]

Adding https://github.com/owncloud/core/blob/master/.php_cs as reference for the current parameters. Maybe useful to others when documented.

And https://veewee.github.io/blog/optimizing-php-performance-by-fq-function-calls/ as reference of the background and possible performance gains. (was in the core PR a link, just easier to doc it here too)

[patrickjahns]

Later on the code style references will be available here: https://github.com/owncloud/coding-standard

We then need to see how much information needs to stay in the documentation - or if a link to a repository for further information makes more sense

[phil-davis]

PR owncloud/core#31484 selects a (hopefully) better udiff format for the diff output. If any of that sort of detail is documented (diff sample...) then make sure to use udiff in the doc.

[mmattel]

I just stumbled while writing a PR over a complaint of drone/php-cs-fixer. The code itself was fine.
I got some time ago a tip from @phil-davis how to fix that on the command line which I now cant find anymore. Remembering that I filed this issue, I looked what progress has been made and saw no changes. It would be really good to have some documentation for developers to support code writing and ease debugging with respect of php-cs-fixer related topics.

[phil-davis]

When you do:

make test-php-style

It spits out output that includes the actual command line it is running:

lib/composer/bin/php-cs-fixer fix -v --diff --diff-format udiff --dry-run --allow-risky yes

To make it fix the things it finds, just take off the --dry-run

lib/composer/bin/php-cs-fixer fix -v --diff --diff-format udiff --allow-risky yes

Then when you make test-php-style again, it will be happy.

When someone documents this thing, this can be included as a handy hint.

[mmattel]

The path given above lib/composer/bin/ may be dependent of how the repro is setup and must be used according the actual paths used. In files_primary_s3 it is vendor/bin/.

[mmattel]

Additional note made by experience:
if you use a namespace and extend the class, the extending class name MUST NOT have a php reserved name. I case you have that, you may get a "linting" error of php-cs-fixer and no checks are made.

[settermjd]

@mmattel, is this addressed now?

[patrickjahns]

We are trying to establish two make-file targets for the time being:

• test-php-style -> check for errors and exit with failure ( used in CI or pre-commit hooks )
• test-php-style-fix -> fix detected errors

Example:
https://github.com/owncloud/password_policy/blob/master/Makefile#L114-L122

[patrickjahns]

Also note:

We are moving towards segregating cli tool dependencies into vendor-bin folders - example also in
https://github.com/owncloud/password_policy/blob/master/Makefile

This has the benefit, that tools can fetch their dependencies independently from the ownCloud app. As the CLI tool can have any dependency and not affect the app itself.

[mmattel]

@patrickjahns it would be great to start documenting these things, even not 100% ready or final. Currently there is nothing. I am lacking knowledge to write the text...

[settermjd]

@patrickjahns, when more concrete information's available, can you provide the core text for the docs, pls?

[phil-davis]

One question is what to document. make test-php-style-fix can "just fix it" for you. So you can "shut your eyes" and write "randomly-formatted" code. Then let the fixer fix it for you :) If you do that, then you do not need to know what the standard is!

I guess we should document some links to the standard that is chosen (e.g. PSR-0, PSR-42, whatever) so that people have some idea what the style fixer is going to require/do.
But if we try to write much detail, then we will have to keep it up-to-date when the required style settings are changed.

[patrickjahns]

The coding standard is open and has its own repository - https://github.com/owncloud/coding-standard/

Latest cs-fixer configuration is to be found in the repository
https://github.com/owncloud/coding-standard/blob/master/src/Config.php

And yes - we shall not write more details then the configuration provides by default. Running the make test-php-style will provide reasonable details of what is wrong

I do not see the point of updating a documentation for a changing part that will be clearly documented in code. 🤷‍♂️

[phil-davis]

Re-opening and assigning myself to write a few words about squizlabs/php_codesniffer

See PR #1976