megalinter icon indicating copy to clipboard operation
megalinter copied to clipboard

Fix for issue #3963

Open llaville opened this issue 1 year ago • 11 comments

Fixes #3963

Readiness Checklist

Author/Contributor

  • [ ] Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • [ ] If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • [ ] Label as breaking if this is a large fundamental change
  • [ ] Label as either automation, bug, documentation, enhancement, infrastructure, or performance

llaville avatar Sep 08 '24 17:09 llaville

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
✅ API spectral 1 0 1.77s
⚠️ BASH bash-exec 6 1 0.04s
✅ BASH shellcheck 6 0 0.23s
✅ BASH shfmt 6 0 0 0.85s
✅ COPYPASTE jscpd yes no 4.43s
✅ DOCKERFILE hadolint 128 0 24.34s
✅ JSON jsonlint 20 0 0.2s
✅ JSON v8r 22 0 32.43s
⚠️ MARKDOWN markdownlint 266 0 297 43.98s
✅ MARKDOWN markdown-table-formatter 266 0 0 137.42s
⚠️ PYTHON bandit 212 66 3.89s
✅ PYTHON black 212 0 0 6.42s
✅ PYTHON flake8 212 0 2.45s
✅ PYTHON isort 212 0 0 1.4s
✅ PYTHON mypy 212 0 16.74s
✅ PYTHON pylint 212 0 33.72s
✅ PYTHON ruff 212 0 0 0.51s
✅ REPOSITORY checkov yes no 37.91s
✅ REPOSITORY git_diff yes no 0.76s
⚠️ REPOSITORY grype yes 24 15.35s
✅ REPOSITORY secretlint yes no 15.23s
✅ REPOSITORY trivy yes no 31.91s
✅ REPOSITORY trivy-sbom yes no 0.36s
⚠️ REPOSITORY trufflehog yes 1 8.18s
✅ SPELL cspell 713 0 14.72s
⚠️ SPELL lychee 348 5 7.53s
✅ XML xmllint 3 0 0 1.02s
✅ YAML prettier 160 0 0 5.15s
✅ YAML v8r 102 0 193.02s
✅ YAML yamllint 161 0 3.38s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

github-actions[bot] avatar Sep 08 '24 17:09 github-actions[bot]

I'll do a rerun of the failed ones. They aren't always 100% non-flaky..

echoix avatar Sep 08 '24 17:09 echoix

I'll do a rerun of the failed ones. They aren't always 100% non-flaky..

I've found origin of regression (introduced in my first commit, and fixed by second commit). Wait and see if CI is agree with my analysis, but locally it seems to be good !

Cross fingers ;-)

llaville avatar Sep 10 '24 15:09 llaville

All seems ok except for saleforces_* linters (trivy scanner found vulnerabilities)

llaville avatar Sep 10 '24 15:09 llaville

All seems ok except for saleforces_* linters (trivy scanner found vulnerabilities)

They also appear in the auto update PR since yesterday

echoix avatar Sep 10 '24 16:09 echoix

All seems ok except for saleforces_* linters (trivy scanner found vulnerabilities)

Do you want it merged now ? If so, when ready, change the title to remove the Work in progress, and I'll do a final read and merge

echoix avatar Sep 10 '24 16:09 echoix

All seems ok except for saleforces_* linters (trivy scanner found vulnerabilities)

Do you want it merged now ? If so, when ready, change the title to remove the Work in progress, and I'll do a final read and merge

I need to write some doc and changelog. I will do in next hours.

llaville avatar Sep 10 '24 16:09 llaville

Finally found another problem. Need to investigate more. I'll be busy tomorrow, so I'll do it only thursday.

llaville avatar Sep 10 '24 18:09 llaville

Finally, found it ! Now it's time for explains.

On my first approach and commit, I've introduced a test https://github.com/oxsecurity/megalinter/pull/3974/commits/fb1510a841bc1ac1a65dc9a70b92411e0c307625#diff-6edf526a9f6b62ef500e048620fd0d43bbc65957ed84e9033d2ff9f8f03206b4R259

that was usefull for PHPCSFIXER but add a regression for other linters.

So, I go back with second commit https://github.com/oxsecurity/megalinter/pull/3974/commits/0661847cae293c2db45aa08953828f04f7a6b075

It was an error, because even if it restored previous good behaviour for other linters, it won't work anymore for PHPCSFIXER.

My final version that works for all linters is :

  • extracted code that activate apply_fixes feature (more pretty and easy to debug) on new function def manage_apply_fixes(self, params)

That gave an extra debug log for each linter; for example :

[Activation] + PHP_PHPCSFIXER (PHP) was activated by ENABLE strategies
[Apply Fixes] is enabled for + PHP_PHPCSFIXER (PHP)
  • define default apply_fixes value to False and remove first if test condition that was source or problem

if self.cli_lint_fix_arg_name is None:

  • restore fix test file status to original value (before apply fixes), that was in wrong state https://github.com/oxsecurity/megalinter/pull/3974/commits/fb1510a841bc1ac1a65dc9a70b92411e0c307625#diff-6a17d64fcb7bf9898f3d9d0f22cd88bcd6bd083ce6dab6abc74ffaabcb106f4e

Last but not least, I've also :

  • added note on PHPCSFIXER about apply_fixes
  • added entry into CHANGELOG

That's all !

llaville avatar Sep 11 '24 06:09 llaville

@llaville amazing, thanks for your hard work :) I'll chech your PR asap :)

nvuillam avatar Sep 11 '24 06:09 nvuillam

This pull request 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 pull request should stay open, please remove the O: stale 🤖 label or comment on the pull request.

github-actions[bot] avatar Oct 12 '24 01:10 github-actions[bot]

@llaville I just merged, please can you make another PR to fix the doc in linter_text descriptor property ? :)

nvuillam avatar Nov 11 '24 13:11 nvuillam

@nvuillam new PR #4249 to fix the doc

llaville avatar Nov 11 '24 15:11 llaville