APPLY_FIXES and for PHP_PHPCSFIXER linter

[hirnschmalz]

Describe the bug
Violations found by PHP CS Fixer are not fixed.

To Reproduce
Steps to reproduce the behavior:

1. Create a .mega-linter.yml with the following content

APPLY_FIXES: all
ENABLE_LINTERS:
  - PHP_PHPCSFIXER

2. Run MegaLinter locally with npx mega-linter-runner --release v8.0.0 (I also tried npx mega-linter-runner --release v8.0.0 --fix)
3. See the output (which is not fixed)

Using cache file ".php-cs-fixer.cache".
   1) htdocs/packages/my-notifications/Classes/Notification/Notification.php (single_space_around_construct)
   2) htdocs/packages/my-crm/Classes/MyApi/Model/Lead/Lead.php (single_space_around_construct)
   3) htdocs/packages/my-crm/Classes/MyApi/Model/Lead/ContactDetails.php (single_space_around_construct)
   4) htdocs/packages/my-crm/Classes/MyApi/Model/Lead/LeadFieldsForCase.php (single_space_around_construct)
   5) htdocs/packages/my-crm/Classes/MyApi/Model/Lead/LeadSourceDetails.php (single_space_around_construct)
   6) htdocs/packages/my-crm/Classes/Utility/RegistryUtility.php (list_syntax)

Found 6 of 172 files that can be fixed in 0.551 seconds, 18.00 MB memory used

Expected behavior
PHP CS Fixer is called with fix (php php-cs-fixer.phar fix ...) and fixes the violations

[nvuillam]

@llaville any idea ? :)

[llaville]

@llaville any idea ? :)

yes I've already a solution.
sorry the fix is pending on my low priority todo list.
I'll propose a fix next week.

[echoix]

@llaville any idea ? :)

yes I've already a solution.

sorry the fix is pending on my low priority todo list.

I'll propose a fix next week.

Before forgetting, can you write what the fix consists about (in a sentence)

[llaville]

@llaville any idea ? :)

yes I've already a solution.
sorry the fix is pending on my low priority todo list.
I'll propose a fix next week.

Before forgetting, can you write what the fix consists about (in a sentence)

of course, but as I am AFK (just on phone now), I'll give you an answer in 90 minutes.

[llaville]

Here is my analysis of the situation :

I think origin of issue is located there https://github.com/oxsecurity/megalinter/blob/v8.0.0/megalinter/Linter.py#L1258-L1267

In case of PHP_CS_FIXER linter it's the same executable (without --dry-run option) that should run to apply fixes.
See definition at https://github.com/oxsecurity/megalinter/blob/v8.0.0/megalinter/descriptors/php.megalinter-descriptor.yml#L192-L193

Origin of this implementation came from PHPCS where there are two executables : one to check violation (phpcs) and another one to apply fixes (phpcbf)

Hope my quick explains are enough. Sorry i'm a bit tired tonight ;-)

[llaville]

I've a good new : I've prepared a fix for MegaLinter v8.0 and it seems that all run fines at least with PHP flavor (and PHP-CS-Fixer)
I need to rebuild a full version to see if there are some regressions with other linters.

I'll keep you aware.
Tomorrow, I'll be a bit busy, but I think I should be able to propose a PR in next 24/48 hours.

[llaville]

Here is a preview of results with risky rule(s) (need a special configuration that is not easy if we don't know the concept):
I'll give an explain on discussion on how to do it !

[llaville]

For MegaLinter Team members but also all users of PHP-CS-Linter, if you need help to configure this linter (for risky rules), please read this #3973

[llaville]

For MegaLinter Team, the PR 3974 is WIP because I've not apply all requires changes (CHANGELOG, build.sh contents) and more than that do more regression tests ....

Next to come on 24/48 hours

[llaville]

Thanks to CI that demonstrate the power of non-regression tests.

Even if it's ok for PHP, it seems that my fix has introduced errors with others linters. I'll have a check on these later now ... no more free time

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

[hirnschmalz]

Since the fix is still lin progress I just add a comment to keep this issue open.

[llaville]

@hirnschmalz The fix (PR #3974) is just waiting for review, but it seems that @nvuillam is a bit busy !

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

[hirnschmalz]

a comment to try to keep this issue alive 😉

[nvuillam]

@llaville my slowliness is caused to the fact that i feel that you update more than php stuff, like core behavior or Linter.py, and i'm scared of regressions ^^

Would you mind explain such updates ? 😊

[llaville]

@nvuillam Of course, I'll try to give another version of my previous explains

PHP-CS-Fixer is a special case in MegaLinter PHP linters support ecosystem.

There is only one exec to detect and fix rules issues (compare to PHPCS that require two executables: one to detect and one to fix)

Arguments are declared into descriptor like that

• https://github.com/oxsecurity/megalinter/blob/v8.1.0/megalinter/descriptors/php.megalinter-descriptor.yml#L186-L190

NB: the fix and --dry-run option that are used only to detect rule violations

To fix violations, we have to remove at runtime the --dry-run option.

It's my vision to simplify usage of check and fix (both commands) in PHP-CS-Fixer

This is the reason why I've declared

• https://github.com/oxsecurity/megalinter/blob/v8.1.0/megalinter/descriptors/php.megalinter-descriptor.yml#L192-L193

And code available in PR Linter.py that is compatible (for me) with all others linters.
See history of my previous tests attempt, that lead to regressions.

Other contents in this PR is just to show a complex example with PHP-CS-Fixer to fix risky rules (usage of --allow-risky=yes option)

Hope my explains are enough. Tell me if not, I'll try to give another version

[nvuillam]

@llaville i think i get it :)

But u still need to move the explanations in linter_text ^^

Are you available to do it now ? ( I pass the last PRs then try to generate a minor version today ^^ )

Otherwise tell me and I can do it for u :)

[llaville]

@nvuillam Will try to do, but later this afternoon