Fix for issue oxsecurity#3963 (oxsecurity#3974)

* fix for issue oxsecurity#3963 * regression with others linters than php_cs_fixer came from this chunk of code (removed) * extract code to enable or disable apply fixes feature * restore test file for risky rule in initial state * add note about PHP-PHPCSFIXER apply fixes * update CHANGELOG for oxsecurity#3963 --------- Co-authored-by: Nicolas Vuillamy <[email protected]>
lukelloyd1985 · Nov 11, 2024 · a01f80f · a01f80f
1 parent 5f8f82f
commit a01f80f
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 27 deletions.
diff --git a/.automation/test/php/.mega-linter.yml.sample b/.automation/test/php/.mega-linter.yml.sample
@@ -0,0 +1,15 @@
+ENABLE:
+  - PHP
+DISABLE_LINTERS:
+  - PHP_PSALM
+  - PHP_PHPLINT
+  - PHP_PHPCS
+  - PHP_PHPSTAN
+LOG_LEVEL: debug
+PARALLEL: false
+UPDATED_SOURCES_REPORTER: true
+APPLY_FIXES: PHP_PHPCSFIXER
+PHP_PHPCSFIXER_CONFIG_FILE: .php-cs-fixer.risky.php
+PHP_PHPCSFIXER_ARGUMENTS:
+  - "--allow-risky=yes"
+  - "--diff"
diff --git a/.automation/test/php/.php-cs-fixer.risky.php b/.automation/test/php/.php-cs-fixer.risky.php
@@ -0,0 +1,13 @@
+<?php
+
+$finder = (new PhpCsFixer\Finder())
+    ->in('.')
+;
+
+return (new PhpCsFixer\Config())
+    ->setRules([
+        '@PER-CS' => true,
+        '@PhpCsFixer:risky' => true,
+    ])
+    ->setFinder($finder)
+;
diff --git a/.automation/test/php/php_fix_1.php b/.automation/test/php/php_fix_1.php
@@ -0,0 +1,3 @@
+<?php
+
+array_push($x, $y);
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -166,6 +166,7 @@ Note: Can be used with `oxsecurity/megalinter@beta` in your GitHub Action mega-l
   - Bash/Perl: Support shell scripts with no extension and only support perl shebangs at the beginning of a file in <https://github.com/oxsecurity/megalinter/pull/4076>
 
 - Fixes
+  - APPLY_FIXES and for PHP_PHPCSFIXER linter, by @llaville in [#3963](https://github.com/oxsecurity/megalinter/issues/3963)
   - Add debug traces to investigate reporters activation
   - Add more traces for ApiReporter
   - Activate ApiReporter by default

diff --git a/docs/descriptors/php_php_cs_fixer.md b/docs/descriptors/php_php_cs_fixer.md
@@ -20,6 +20,13 @@ description: How to use php-cs-fixer (configure, ignore files, ignore errors, he
 - Enable php-cs-fixer by adding `PHP_PHPCSFIXER` in [ENABLE_LINTERS variable](https://megalinter.io/beta/configuration/#activation-and-deactivation)
 - Disable php-cs-fixer by adding `PHP_PHPCSFIXER` in [DISABLE_LINTERS variable](https://megalinter.io/beta/configuration/#activation-and-deactivation)
 
+- Enable **autofixes** by adding `PHP_PHPCSFIXER` in [APPLY_FIXES variable](https://megalinter.io/beta/configuration/#apply-fixes)
+
+> [!NOTE]
+>
+> If you want to apply fixes on risky rules, you should set `PHP_PHPCSFIXER_ARGUMENTS` for additional value(s).
+> Read this [post](https://github.com/oxsecurity/megalinter/discussions/3973) to learn how to do.
+
 | Variable                                   | Description                                                                                                                                                                                                                                                                           | Default value                                   |
 |--------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-------------------------------------------------|
 | PHP_PHPCSFIXER_ARGUMENTS                   | User custom arguments to add in linter CLI call<br/>Ex: `-s --foo "bar"`                                                                                                                                                                                                              |                                                 |

diff --git a/megalinter/Linter.py b/megalinter/Linter.py
@@ -251,32 +251,8 @@ def __init__(self, params=None, linter_config=None):
 
         if self.is_active is True:
             self.show_elapsed_time = params.get("show_elapsed_time", False)
-            # Manage apply fixes flag on linter
-            param_apply_fixes = params.get("apply_fixes", "none")
-            # No fixing config on linter descriptor
-            if self.cli_lint_fix_arg_name is None:
-                self.apply_fixes = False
-            # APPLY_FIXES is "all"
-            elif param_apply_fixes == "all" or (
-                isinstance(param_apply_fixes, bool) and param_apply_fixes is True
-            ):
-                self.apply_fixes = True
-            # APPLY_FIXES is a comma-separated list in a single string
-            elif (
-                param_apply_fixes != "none"
-                and isinstance(param_apply_fixes, str)
-                and self.name in param_apply_fixes.split(",")
-            ):
-                self.apply_fixes = True
-            # APPLY_FIXES is a list of strings
-            elif (
-                param_apply_fixes != "none"
-                and isinstance(param_apply_fixes, list)
-                and (self.name in param_apply_fixes or param_apply_fixes[0] == "all")
-            ):
-                self.apply_fixes = True
-            else:
-                self.apply_fixes = False
+
+            self.manage_apply_fixes(params)
 
             # Disable lint_all_other_linters_files=true if we are in a standalone linter docker image,
             # because there are no other linters
@@ -482,6 +458,43 @@ def manage_activation(self, params):
                 f"[Activation] - {self.name} ({self.descriptor_id}) was not activated by {strategiesUsed} strategies"
             )
 
+    # Manage apply fixes flag on linter
+    def manage_apply_fixes(self, params):
+        self.apply_fixes = False
+
+        param_apply_fixes = params.get("apply_fixes", "none")
+
+        # APPLY_FIXES is "all"
+        if param_apply_fixes == "all" or (
+            isinstance(param_apply_fixes, bool) and param_apply_fixes is True
+        ):
+            self.apply_fixes = True
+        # APPLY_FIXES is a comma-separated list in a single string
+        elif (
+            param_apply_fixes != "none"
+            and isinstance(param_apply_fixes, str)
+            and self.name in param_apply_fixes.split(",")
+        ):
+            self.apply_fixes = True
+        # APPLY_FIXES is a list of strings
+        elif (
+            param_apply_fixes != "none"
+            and isinstance(param_apply_fixes, list)
+            and (self.name in param_apply_fixes or param_apply_fixes[0] == "all")
+        ):
+            self.apply_fixes = True
+        else:
+            self.apply_fixes = False
+
+        if self.apply_fixes:
+            logging.debug(
+                f"[Apply Fixes] is enabled for + {self.name} ({self.descriptor_id})"
+            )
+        else:
+            logging.debug(
+                f"[Apply Fixes] is disabled for + {self.name} ({self.descriptor_id})"
+            )
+
     # Manage configuration variables
     def load_config_vars(self, params):
         # Configuration file name: try first NAME + _FILE_NAME, then LANGUAGE + _FILE_NAME
@@ -1260,12 +1273,14 @@ def build_lint_command(self, file=None) -> list:
         # Add fix argument if defined
         if self.apply_fixes is True and (
             self.cli_lint_fix_arg_name is not None
+            or len(self.cli_lint_fix_remove_args) > 0
             or str(self.cli_executable_fix) != str(self.cli_executable)
         ):
             args_pos = len(self.cli_executable)
             cmd = cmd[args_pos:]  # Remove executable elements
             cmd = self.cli_executable_fix + cmd
-            cmd += [self.cli_lint_fix_arg_name]
+            if self.cli_lint_fix_arg_name is not None:
+                cmd += [self.cli_lint_fix_arg_name]
             self.try_fix = True
 
         # Add user-defined extra arguments if defined