NEW Enable fixing linting issues automatically

bin/mdphpcs

@@ -1,7 +1,6 @@
 #!/usr/bin/env php
 <?php
 
-use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Exceptions\DeepExitException;
 use SilverStripe\MD_PHP_CodeSniffer\Sniffer;
 
@@ -33,6 +32,7 @@ $numArgs = count($args);
 $removeArgs = [];
 
 $usingExplicitStandard = false;
+$fixing = false;
 
 // Check for any args we need from the CLI input
 for ($i = 0; $i < $numArgs; $i++) {
@@ -46,6 +46,11 @@ for ($i = 0; $i < $numArgs; $i++) {
         $removeArgs[] = $i;
     }
 
+    if ($arg === '--fix') {
+        $fixing = true;
+        $removeArgs[] = $i;
+    }
+
     if ($arg === '--standard' || str_starts_with($arg, '--standard=')) {
         $usingExplicitStandard = true;
     }
@@ -60,15 +65,9 @@ $_SERVER['argv'] = array_values($_SERVER['argv']);
 // Assume PHP if no explicit language was passed in.
 $lintLanguage ??= 'PHP';
 
-// PHPCS will just exist silently, so it's up to use to tell the user that their stuff won't
-// be properly linted
-if ($lintLanguage === 'JS' && !Config::getExecutablePath('eslint')) {
-    throw new RuntimeException('No eslint executable found');
-}
-
 try {
     $sniffer = new Sniffer();
-    $exitCode = $sniffer->run($lintLanguage, $usingExplicitStandard);
+    $exitCode = $sniffer->run($lintLanguage, $fixing, $usingExplicitStandard);
     exit($exitCode);
 } catch (DeepExitException $e) {
     echo $e->getMessage();

src/CodeBlock.php

@@ -0,0 +1,29 @@
+<?php
+
+namespace SilverStripe\MD_PHP_CodeSniffer;
+
+use PHP_CodeSniffer\Files\DummyFile;
+
+class CodeBlock extends DummyFile
+{
+    public int $num;
+
+    public string $realPath;
+
+    private string $finalContent = '';
+
+    public function cleanUp()
+    {
+        $this->finalContent = $this->content ?? '';
+        parent::cleanUp();
+    }
+
+    public function getContent(): ?string
+    {
+        if ($this->content) {
+            return $this->content;
+        }
+
+        return $this->finalContent;
+    }
+}

src/FixerReport.php

@@ -0,0 +1,89 @@
+<?php
+
+namespace SilverStripe\MD_PHP_CodeSniffer;
+
+use PHP_CodeSniffer\Exceptions\DeepExitException;
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Reports\Cbf;
+
+class FixerReport extends Cbf
+{
+    /**
+     * Generate a partial report for a single processed code block and store the result in the dummy files.
+     *
+     * Should return TRUE if it printed or stored data about the code block
+     * and FALSE if it ignored the code block. Returning TRUE indicates that the code block and
+     * its data should be counted in the grand totals.
+     *
+     * @param array $report Prepared report data.
+     * @param \PHP_CodeSniffer\File $phpcsFile The file being reported on.
+     * @param bool $showSources NOT USED
+     * @param int $width NOT USED
+     */
+    public function generateFileReport($report, File $phpcsFile, $showSources = false, $width = 80): bool
+    {
+        $errors = $phpcsFile->getFixableCount();
+        if ($errors !== 0) {
+            if (PHP_CODESNIFFER_VERBOSITY > 0) {
+                ob_end_clean();
+                $startTime = microtime(true);
+                echo "\t=> Fixing file: $errors/$errors violations remaining";
+                if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                    echo PHP_EOL;
+                }
+            }
+
+            $fixed = $phpcsFile->fixer->fixFile();
+        }
+
+        if ($phpcsFile->config->stdin === true) {
+            // Replacing STDIN, so output current file to STDOUT
+            // even if nothing was fixed. Exit here because we
+            // can't process any more than 1 file in this setup.
+            $fixedContent = $phpcsFile->fixer->getContents();
+            throw new DeepExitException($fixedContent, 1);
+        }
+
+        if ($errors === 0) {
+            return false;
+        }
+
+        if (PHP_CODESNIFFER_VERBOSITY > 0) {
+            if ($fixed === false) {
+                echo 'ERROR';
+            } else {
+                echo 'DONE';
+            }
+
+            $timeTaken = ((microtime(true) - $startTime) * 1000);
+            if ($timeTaken < 1000) {
+                $timeTaken = round($timeTaken);
+                echo " in {$timeTaken}ms".PHP_EOL;
+            } else {
+                $timeTaken = round(($timeTaken / 1000), 2);
+                echo " in $timeTaken secs".PHP_EOL;
+            }
+        }
+
+        // NOTE: This is the only change from the parent method!
+        // We've ripped out all of the code here which would have written changes to the file.
+        // Instead, we need to find the old content for a given block and override that with
+        // the new content. This is done back in the Sniffer class.
+
+        if (PHP_CODESNIFFER_VERBOSITY > 0) {
+            if ($fixed === true) {
+                echo "\t=> Fixed content stored in memory".PHP_EOL;
+            }
+            ob_start();
+        }
+
+        $errorCount = $phpcsFile->getErrorCount();
+        $warningCount = $phpcsFile->getWarningCount();
+        $fixableCount = $phpcsFile->getFixableCount();
+        $fixedCount = ($errors - $fixableCount);
+        echo $report['filename'] . ">>$errorCount>>$warningCount>>$fixableCount>>$fixedCount" . PHP_EOL;
+
+        return $fixed;
+
+    }
+}

src/Sniffer.php

@@ -7,9 +7,9 @@
 use League\CommonMark\Extension\CommonMark\Node\Block\FencedCode;
 use League\CommonMark\Node\Query;
 use League\CommonMark\Parser\MarkdownParser;
+use LogicException;
 use PHP_CodeSniffer\Config;
 use PHP_CodeSniffer\Exceptions\DeepExitException;
-use PHP_CodeSniffer\Files\DummyFile;
 use PHP_CodeSniffer\Files\FileList;
 use PHP_CodeSniffer\Reporter;
 use PHP_CodeSniffer\Runner;
@@ -30,16 +30,20 @@ public function __construct()
         $this->parser = new MarkdownParser($environment);
     }
 
-    public function run(string $lintLanguage, bool $usingExplicitStandard = false): int
+    public function run(string $lintLanguage, bool $fixing, bool $usingExplicitStandard = false): int
     {
-        // Prevents errors when unexpected args are passed in, and forces fixing OFF
+        // MUST be false when not fixing, and MUST be true when fixing.
+        // This affects how codesniffer treats various CLI args, changes the output, and defines how some rules are actioned.
         if (!defined('PHP_CODESNIFFER_CBF')) {
-            define('PHP_CODESNIFFER_CBF', false);
+            define('PHP_CODESNIFFER_CBF', $fixing);
+        }
+        if (PHP_CODESNIFFER_CBF !== $fixing) {
+            throw new LogicException('PHP_CODESNIFFER_CBF was defined with an incorrect value');
         }
 
         $sniffer = new Runner();
         $sniffer->checkRequirements();
-        $sniffer->config = $this->prepareConfig($usingExplicitStandard, $lintLanguage);
+        $sniffer->config = $this->prepareConfig($usingExplicitStandard, $lintLanguage, $fixing);
         $sniffer->init();
         $sniffer->reporter = new Reporter($sniffer->config);
 
@@ -61,31 +65,57 @@ public function run(string $lintLanguage, bool $usingExplicitStandard = false):
         // Add code blocks to the file list for linting
         $todo = [];
         foreach ($codeBlocks as $block) {
-            $dummy = new DummyFile($block['content'], $sniffer->ruleset, $sniffer->config);
-            $dummy->path = $block['path']; 
-            $todo[$dummy->path] = $dummy;
+            $dummy = new CodeBlock($block['content'], $sniffer->ruleset, $sniffer->config);
+            $dummy->num = $block['num'];
+            $dummy->path = $block['path'];
+            $dummy->realPath = $block['realpath'];
+            $todo[] = $dummy;
         }
 
         // Do the actual linting
         $numErrors = $this->sniff($sniffer, $todo);
+
+        // The dummy files have the fixed content stored - but we still need to write that to the original files.
+        // There's no good AST to markdown renderer for league/commonmark so we're just doing a bit of an ugly
+        // search and replace.
+        if ($fixing) {
+            /** @var CodeBlock $dummy */
+            foreach ($todo as $dummy) {
+                if ($dummy->getFixedCount() < 1) {
+                    continue;
+                }
+
+                if (!is_file($dummy->realPath)) {
+                    // 3 is the exit code phpcs uses for errors like this
+                    throw new DeepExitException("Can't find file {$dummy->realPath} to set new content", 3);
+                }
+
+                /** @var FencedCode $mdBlock */
+                $mdBlock = $codeBlocks[$dummy->path]['md'];
+                $indent = str_repeat(' ', $mdBlock->getOffset());
+                $origBlockContent = preg_replace('/^/m', $indent, $mdBlock->getLiteral());
+                $newBlockContent = preg_replace('/^/m', $indent, preg_replace('/\s*<\?php\n?/', '', $dummy->getContent()));
+                $newFileContent = str_replace($origBlockContent, $newBlockContent, file_get_contents($dummy->realPath));
+
+                file_put_contents($dummy->realPath, $newFileContent);
+            }
+        }
+
         $sniffer->reporter->printReports();
 
         if ($numErrors === 0) {
             // No errors found.
             return 0;
-            /* we can't fix errors directly yet.
         } else if ($sniffer->reporter->totalFixable === 0) {
             // Errors found, but none of them can be fixed by PHPCBF.
             return 1;
         } else {
             // Errors found, and some can be fixed by PHPCBF.
             return 2;
-        */
         }
-        return 1;
     }
 
-    private function prepareConfig(bool $usingExplicitStandard, string $lintLanguage): Config
+    private function prepareConfig(bool $usingExplicitStandard, string $lintLanguage, bool $fixing): Config
     {
         // Creating the Config object populates it with all required settings based on the phpcs/phpcbf CLI arguments provided.
         $config = new Config();
@@ -109,6 +139,19 @@ private function prepareConfig(bool $usingExplicitStandard, string $lintLanguage
             $config->standards = [__DIR__ . '/../phpcs.default.xml'];
         }
 
+        if ($fixing) {
+            // Override some of the command line settings that might break the fixes.
+            $config->generator = null;
+            $config->explain = false;
+            $config->interactive = false;
+            $config->cache = false;
+            $config->showSources = false;
+            $config->recordErrors = false;
+            $config->reportFile = null;
+            $config->reports = [FixerReport::class => null];
+            $config->dieOnUnknownArg = false;
+        }
+
         return $config;
     }
 
@@ -145,10 +188,15 @@ private function findFencedCodeblocks(FileList $paths, string $lintLanguage): ar
                 $content .= '<?php' . PHP_EOL . $block->getLiteral();
 
                 // Report each block separately (by making the path unique) so it's treated as its own file
-                // This lets us lint for things like namespaces more easily
-                $blocks[] = [
+                // This lets us lint for things like namespaces more easily since the namespace in an earlier block
+                // won't be counted towards a later block in the same file
+                $key = dirname($path) . '/' . basename($path, '.md') . "_{$n}" . '.md';
+                $blocks[$key] = [
                     'content' => $content,
-                    'path' => dirname($path) . '/' . basename($path, '.md') . "_{$n}" . '.md',
+                    'path' => $key,
+                    'realpath' => $path,
+                    'num' => $n,
+                    'md' => $block,
                 ];
             }
         }
@@ -172,14 +220,15 @@ private function sniff(Runner $sniffer, array $todo): int
         set_error_handler([$sniffer, 'handleErrors']);
 
         $lastDir  = '';
-        $numFiles = count($todo);
+        $numBlocks = count($todo);
 
-        // Process each "file" sequentially - running sniff in parallel isn't supported
+        // Process each block sequentially - running sniff in parallel isn't supported
         // We're not actually running this across real files, but we should give the same output we'd get if we were.
         $numProcessed = 0;
-        foreach ($todo as $path => $file) {
-            if ($file->ignored === false) {
-                $currDir = dirname($path);
+        /** @var CodeBlock $block */
+        foreach ($todo as $block) {
+            if ($block->ignored === false) {
+                $currDir = dirname($block->realPath);
                 if ($lastDir !== $currDir) {
                     if (PHP_CODESNIFFER_VERBOSITY > 0) {
                         echo 'Changing into directory ' . Common::stripBasepath($currDir, $sniffer->config->basepath) . PHP_EOL;
@@ -188,13 +237,13 @@ private function sniff(Runner $sniffer, array $todo): int
                     $lastDir = $currDir;
                 }
 
-                $sniffer->processFile($file);
+                $sniffer->processFile($block);
             } else if (PHP_CODESNIFFER_VERBOSITY > 0) {
-                echo 'Skipping ' . basename($file->path) . PHP_EOL;
+                echo 'Skipping ' . basename($block->path) . PHP_EOL;
             }
 
             $numProcessed++;
-            $sniffer->printProgress($file, $numFiles, $numProcessed);
+            $sniffer->printProgress($block, $numBlocks, $numProcessed);
         }
 
         restore_error_handler();
@@ -226,4 +275,4 @@ private function sniff(Runner $sniffer, array $todo): int
 
         return $return;
     }
-}
+}