Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APPLY_FIXES and for PHP_PHPCSFIXER linter #3963

Open
hirnschmalz opened this issue Sep 4, 2024 · 10 comments · May be fixed by #3974
Open

APPLY_FIXES and for PHP_PHPCSFIXER linter #3963

hirnschmalz opened this issue Sep 4, 2024 · 10 comments · May be fixed by #3974
Assignees
Labels
bug Something isn't working

Comments

@hirnschmalz
Copy link

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
  1. Run MegaLinter locally with npx mega-linter-runner --release v8.0.0 (I also tried npx mega-linter-runner --release v8.0.0 --fix)
  2. 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

@hirnschmalz hirnschmalz added the bug Something isn't working label Sep 4, 2024
@nvuillam
Copy link
Member

nvuillam commented Sep 4, 2024

@llaville any idea ? :)

@llaville
Copy link
Collaborator

llaville commented Sep 4, 2024

@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
Copy link
Collaborator

echoix commented Sep 4, 2024

@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
Copy link
Collaborator

llaville commented Sep 4, 2024

@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
Copy link
Collaborator

llaville commented Sep 4, 2024

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 llaville self-assigned this Sep 6, 2024
@llaville
Copy link
Collaborator

llaville commented Sep 8, 2024

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
Copy link
Collaborator

llaville commented Sep 8, 2024

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 !

php-cs-fixer

php-cs-fixer_2

@llaville
Copy link
Collaborator

llaville commented Sep 8, 2024

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 added a commit that referenced this issue Sep 8, 2024
@llaville llaville linked a pull request Sep 8, 2024 that will close this issue
4 tasks
@llaville
Copy link
Collaborator

llaville commented Sep 8, 2024

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
Copy link
Collaborator

llaville commented Sep 8, 2024

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

llaville added a commit that referenced this issue Sep 10, 2024
llaville added a commit that referenced this issue Sep 11, 2024
llaville added a commit that referenced this issue Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants