From 0c3c9308decfa54c27a851a513a449d77dea139e Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 14 Jun 2019 22:25:45 +0200 Subject: [PATCH 1/6] :sparkles: Add Fixer --- TwigCS/Command/TwigCSCommand.php | 2 +- TwigCS/Ruleset/Generic/BlankEOFSniff.php | 22 +- .../Ruleset/Generic/DelimiterSpacingSniff.php | 19 +- .../Generic/DisallowCommentedCodeSniff.php | 5 +- TwigCS/Runner/Fixer.php | 479 ++++++++++++++++++ TwigCS/{ => Runner}/Linter.php | 29 +- TwigCS/Sniff/AbstractSniff.php | 79 ++- TwigCS/Sniff/SniffInterface.php | 24 +- TwigCS/Tests/AbstractSniffTest.php | 2 +- 9 files changed, 600 insertions(+), 61 deletions(-) create mode 100644 TwigCS/Runner/Fixer.php rename TwigCS/{ => Runner}/Linter.php (85%) diff --git a/TwigCS/Command/TwigCSCommand.php b/TwigCS/Command/TwigCSCommand.php index fe4b044..0d54d53 100644 --- a/TwigCS/Command/TwigCSCommand.php +++ b/TwigCS/Command/TwigCSCommand.php @@ -10,9 +10,9 @@ use Symfony\Component\Console\Output\OutputInterface; use TwigCS\Config\Config; use TwigCS\Environment\StubbedEnvironment; -use TwigCS\Linter; use TwigCS\Report\TextFormatter; use TwigCS\Ruleset\Ruleset; +use TwigCS\Runner\Linter; use TwigCS\Token\Tokenizer; /** diff --git a/TwigCS/Ruleset/Generic/BlankEOFSniff.php b/TwigCS/Ruleset/Generic/BlankEOFSniff.php index 876ce32..c8ceef1 100644 --- a/TwigCS/Ruleset/Generic/BlankEOFSniff.php +++ b/TwigCS/Ruleset/Generic/BlankEOFSniff.php @@ -12,7 +12,6 @@ class BlankEOFSniff extends AbstractSniff { /** - * @param Token $token * @param int $tokenPosition * @param Token[] $tokens * @@ -20,23 +19,38 @@ class BlankEOFSniff extends AbstractSniff * * @throws Exception */ - public function process(Token $token, int $tokenPosition, array $tokens) + public function process(int $tokenPosition, array $tokens) { + $token = $tokens[$tokenPosition]; + if ($this->isTokenMatching($token, Token::EOF_TYPE)) { $i = 0; while (isset($tokens[$tokenPosition - ($i + 1)]) && $this->isTokenMatching($tokens[$tokenPosition - ($i + 1)], Token::EOL_TYPE) ) { - ++$i; + $i++; } if (1 !== $i) { // Either 0 or 2+ blank lines. - $this->addMessage( + $fix = $this->addFixableMessage( $this::MESSAGE_TYPE_ERROR, sprintf('A file must end with 1 blank line; found %d', $i), $token ); + + if ($fix) { + if (0 === $i) { + $this->fixer->addNewlineBefore($tokenPosition); + } else { + $this->fixer->beginChangeset(); + while ($i > 1) { + $this->fixer->replaceToken($tokenPosition - $i, ''); + $i--; + } + $this->fixer->endChangeset(); + } + } } } diff --git a/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php b/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php index 82671c1..53a7ed4 100644 --- a/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php +++ b/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php @@ -12,7 +12,6 @@ class DelimiterSpacingSniff extends AbstractSniff { /** - * @param Token $token * @param int $tokenPosition * @param Token[] $tokens * @@ -20,34 +19,37 @@ class DelimiterSpacingSniff extends AbstractSniff * * @throws Exception */ - public function process(Token $token, int $tokenPosition, array $tokens) + public function process(int $tokenPosition, array $tokens) { + $token = $tokens[$tokenPosition]; + if ($this->isTokenMatching($token, Token::VAR_START_TYPE) || $this->isTokenMatching($token, Token::BLOCK_START_TYPE) || $this->isTokenMatching($token, Token::COMMENT_START_TYPE) ) { - $this->processStart($token, $tokenPosition, $tokens); + $this->processStart($tokenPosition, $tokens); } if ($this->isTokenMatching($token, Token::VAR_END_TYPE) || $this->isTokenMatching($token, Token::BLOCK_END_TYPE) || $this->isTokenMatching($token, Token::COMMENT_END_TYPE) ) { - $this->processEnd($token, $tokenPosition, $tokens); + $this->processEnd($tokenPosition, $tokens); } return $token; } /** - * @param Token $token * @param int $tokenPosition * @param Token[] $tokens * * @throws Exception */ - public function processStart(Token $token, $tokenPosition, $tokens) + public function processStart(int $tokenPosition, array $tokens) { + $token = $tokens[$tokenPosition]; + // Ignore new line if ($this->isTokenMatching($tokens[$tokenPosition + 1], Token::EOL_TYPE)) { return; @@ -69,14 +71,15 @@ public function processStart(Token $token, $tokenPosition, $tokens) } /** - * @param Token $token * @param int $tokenPosition * @param Token[] $tokens * * @throws Exception */ - public function processEnd(Token $token, $tokenPosition, $tokens) + public function processEnd(int $tokenPosition, array $tokens) { + $token = $tokens[$tokenPosition]; + // Ignore new line if ($this->isTokenMatching($tokens[$tokenPosition - 1], Token::EOL_TYPE)) { return; diff --git a/TwigCS/Ruleset/Generic/DisallowCommentedCodeSniff.php b/TwigCS/Ruleset/Generic/DisallowCommentedCodeSniff.php index f9523d8..90f9a66 100644 --- a/TwigCS/Ruleset/Generic/DisallowCommentedCodeSniff.php +++ b/TwigCS/Ruleset/Generic/DisallowCommentedCodeSniff.php @@ -14,7 +14,6 @@ class DisallowCommentedCodeSniff extends AbstractSniff { /** - * @param Token $token * @param int $tokenPosition * @param Token[] $tokens * @@ -22,8 +21,10 @@ class DisallowCommentedCodeSniff extends AbstractSniff * * @throws Exception */ - public function process(Token $token, int $tokenPosition, array $tokens) + public function process(int $tokenPosition, array $tokens) { + $token = $tokens[$tokenPosition]; + if ($this->isTokenMatching($token, Token::COMMENT_START_TYPE)) { $i = $tokenPosition; $found = false; diff --git a/TwigCS/Runner/Fixer.php b/TwigCS/Runner/Fixer.php new file mode 100644 index 0000000..a67ddb4 --- /dev/null +++ b/TwigCS/Runner/Fixer.php @@ -0,0 +1,479 @@ + + */ + protected $tokens = []; + + /** + * A list of tokens that have already been fixed. + * + * We don't allow the same token to be fixed more than once each time + * through a file as this can easily cause conflicts between sniffs. + * + * @var int[] + */ + protected $fixedTokens = []; + + /** + * The last value of each fixed token. + * + * If a token is being "fixed" back to its last value, the fix is + * probably conflicting with another. + * + * @var array + */ + protected $oldTokenValues = []; + + /** + * A list of tokens that have been fixed during a changeset. + * + * All changes in changeset must be able to be applied, or else + * the entire changeset is rejected. + * + * @var array + */ + protected $changeset = []; + + /** + * Is there an open changeset. + * + * @var bool + */ + protected $inChangeset = false; + + /** + * Is the current fixing loop in conflict? + * + * @var bool + */ + protected $inConflict = false; + + /** + * The number of fixes that have been performed. + * + * @var int + */ + protected $numFixes = 0; + + + /** + * Starts fixing a new file. + * + * @param Ruleset $ruleset + * @param Tokenizer $tokenizer + * + * @return void + */ + public function __construct(Ruleset $ruleset, Tokenizer $tokenizer) + { + $this->ruleset = $ruleset; + $this->tokenizer = $tokenizer; + } + + /** + * @param array $tokens + */ + public function startFile(array $tokens) + { + $this->numFixes = 0; + $this->fixedTokens = []; + + $this->tokens = array_map(function (Token $token) { + return $token->getValue(); + }, $tokens); + + if (preg_match("/\r\n?|\n/", $this->getContents(), $matches) !== 1) { + // Assume there are no newlines. + $this->eolChar = "\n"; + } else { + $this->eolChar = $matches[0]; + } + } + + /** + * Attempt to fix the file by processing it until no fixes are made. + * + * @param string $file + * + * @return bool + */ + public function fixFile(string $file) + { + $contents = file_get_contents($file); + + $this->loops = 0; + while ($this->loops < 50) { + $this->inConflict = false; + + try { + $twigSource = new Source($contents, 'TwigCS'); + $stream = $this->tokenizer->tokenize($twigSource); + } catch (Exception $e) { + return false; + } + + $this->startFile($stream); + + $sniffs = $this->ruleset->getSniffs(); + foreach ($sniffs as $sniff) { + $sniff->processFile($stream); + } + + $this->loops++; + + if (0 === $this->numFixes && !$this->inConflict) { + // Nothing left to do. + break; + } + + // Only needed once file content has changed. + $contents = $this->getContents(); + } + + if ($this->numFixes > 0) { + return false; + } + + return true; + } + + /** + * Generates a text diff of the original file and the new content. + * + * @param string $filePath File path to diff the file against. + * + * @return string + */ + public function generateDiff($filePath) + { + $cwd = getcwd().DIRECTORY_SEPARATOR; + if (strpos($filePath, $cwd) === 0) { + $filename = substr($filePath, strlen($cwd)); + } else { + $filename = $filePath; + } + + $contents = $this->getContents(); + + $tempName = tempnam(sys_get_temp_dir(), 'phpcs-fixer'); + $fixedFile = fopen($tempName, 'w'); + fwrite($fixedFile, $contents); + + // We must use something like shell_exec() because whitespace at the end + // of lines is critical to diff files. + $filename = escapeshellarg($filename); + $cmd = "diff -u -L$filename -LPHP_CodeSniffer $filename \"$tempName\""; + + $diff = shell_exec($cmd); + + fclose($fixedFile); + if (is_file($tempName) === true) { + unlink($tempName); + } + + $diffLines = explode(PHP_EOL, $diff); + if (count($diffLines) === 1) { + // Seems to be required for cygwin. + $diffLines = explode("\n", $diff); + } + + $diff = []; + foreach ($diffLines as $line) { + if (isset($line[0]) === true) { + switch ($line[0]) { + case '-': + $diff[] = "\033[31m$line\033[0m"; + break; + case '+': + $diff[] = "\033[32m$line\033[0m"; + break; + default: + $diff[] = $line; + } + } + } + + $diff = implode(PHP_EOL, $diff); + + return $diff; + } + + /** + * Get a count of fixes that have been performed on the file. + * + * This value is reset every time a new file is started, or an existing file is restarted. + * + * @return int + */ + public function getFixCount() + { + return $this->numFixes; + } + + /** + * Get the current content of the file, as a string. + * + * @return string + */ + public function getContents() + { + $contents = implode($this->tokens); + + return $contents; + } + + /** + * Get the current fixed content of a token. + * + * This function takes changesets into account so should be used + * instead of directly accessing the token array. + * + * @param int $tokenPosition The position of the token in the token stack. + * + * @return string + */ + public function getTokenContent($tokenPosition) + { + if ($this->inChangeset && isset($this->changeset[$tokenPosition])) { + return $this->changeset[$tokenPosition]; + } + + return $this->tokens[$tokenPosition]; + } + + /** + * Start recording actions for a changeset. + */ + public function beginChangeset() + { + if ($this->inConflict) { + return; + } + + $this->changeset = []; + $this->inChangeset = true; + } + + /** + * Stop recording actions for a changeset, and apply logged changes. + */ + public function endChangeset() + { + if ($this->inConflict) { + return; + } + + $this->inChangeset = false; + + $success = true; + $applied = []; + foreach ($this->changeset as $tokenPosition => $content) { + $success = $this->replaceToken($tokenPosition, $content); + if (!$success) { + break; + } else { + $applied[] = $tokenPosition; + } + } + + if (!$success) { + // Rolling back all changes. + foreach ($applied as $tokenPosition) { + $this->revertToken($tokenPosition); + } + } + + $this->changeset = []; + } + + /** + * Stop recording actions for a changeset, and discard logged changes. + * + * @return void + */ + public function rollbackChangeset() + { + $this->inChangeset = false; + $this->inConflict = false; + + if (empty($this->changeset) === false) { + $this->changeset = []; + } + } + + + /** + * Replace the entire contents of a token. + * + * @param int $tokenPosition The position of the token in the token stack. + * @param string $content The new content of the token. + * + * @return bool If the change was accepted. + */ + public function replaceToken($tokenPosition, $content) + { + if ($this->inConflict) { + return false; + } + + if (!$this->inChangeset && isset($this->fixedTokens[$tokenPosition])) { + return false; + } + + if ($this->inChangeset) { + $this->changeset[$tokenPosition] = $content; + + return true; + } + + if (!isset($this->oldTokenValues[$tokenPosition])) { + $this->oldTokenValues[$tokenPosition] = [ + 'curr' => $content, + 'prev' => $this->tokens[$tokenPosition], + 'loop' => $this->loops, + ]; + } else { + if ($content === $this->oldTokenValues[$tokenPosition]['prev'] + && ($this->loops - 1) === $this->oldTokenValues[$tokenPosition]['loop'] + ) { + if ($this->oldTokenValues[$tokenPosition]['loop'] >= ($this->loops - 1)) { + $this->inConflict = true; + } + + return false; + } + + $this->oldTokenValues[$tokenPosition]['prev'] = $this->oldTokenValues[$tokenPosition]['curr']; + $this->oldTokenValues[$tokenPosition]['curr'] = $content; + $this->oldTokenValues[$tokenPosition]['loop'] = $this->loops; + } + + $this->fixedTokens[$tokenPosition] = $this->tokens[$tokenPosition]; + $this->tokens[$tokenPosition] = $content; + $this->numFixes++; + + return true; + } + + /** + * Reverts the previous fix made to a token. + * + * @param int $tokenPosition The position of the token in the token stack. + * + * @return bool If a change was reverted. + */ + public function revertToken($tokenPosition) + { + if (!isset($this->fixedTokens[$tokenPosition])) { + return false; + } + + $this->tokens[$tokenPosition] = $this->fixedTokens[$tokenPosition]; + unset($this->fixedTokens[$tokenPosition]); + $this->numFixes--; + + return true; + } + + /** + * Adds a newline to end of a token's content. + * + * @param int $tokenPosition The position of the token in the token stack. + * + * @return bool If the change was accepted. + */ + public function addNewline($tokenPosition) + { + $current = $this->getTokenContent($tokenPosition); + + return $this->replaceToken($tokenPosition, $current.$this->eolChar); + } + + /** + * Adds a newline to the start of a token's content. + * + * @param int $tokenPosition The position of the token in the token stack. + * + * @return bool If the change was accepted. + */ + public function addNewlineBefore($tokenPosition) + { + $current = $this->getTokenContent($tokenPosition); + + return $this->replaceToken($tokenPosition, $this->eolChar.$current); + } + + /** + * Adds content to the end of a token's current content. + * + * @param int $tokenPosition The position of the token in the token stack. + * @param string $content The content to add. + * + * @return bool If the change was accepted. + */ + public function addContent($tokenPosition, $content) + { + $current = $this->getTokenContent($tokenPosition); + + return $this->replaceToken($tokenPosition, $current.$content); + } + + /** + * Adds content to the start of a token's current content. + * + * @param int $tokenPosition The position of the token in the token stack. + * @param string $content The content to add. + * + * @return bool If the change was accepted. + */ + public function addContentBefore($tokenPosition, $content) + { + $current = $this->getTokenContent($tokenPosition); + + return $this->replaceToken($tokenPosition, $content.$current); + } +} diff --git a/TwigCS/Linter.php b/TwigCS/Runner/Linter.php similarity index 85% rename from TwigCS/Linter.php rename to TwigCS/Runner/Linter.php index 63f2c8d..90713e1 100644 --- a/TwigCS/Linter.php +++ b/TwigCS/Runner/Linter.php @@ -1,6 +1,6 @@ env = $env; - $this->tokenizer = $tokenizer; } @@ -43,25 +42,41 @@ public function __construct(Environment $env, Tokenizer $tokenizer) * * @param array $files List of files to process. * @param Ruleset $ruleset Set of rules to check. + * @param bool $fix If true, the linter will fix the file * * @return Report an object with all violations and stats. * * @throws Exception */ - public function run(array $files, Ruleset $ruleset) + public function run(array $files, Ruleset $ruleset, bool $fix = false) { if (empty($files)) { throw new Exception('No files to process, provide at least one file to be linted'); } $report = new Report(); + + if ($fix) { + $fixer = new Fixer($ruleset, $this->tokenizer); + + foreach ($ruleset->getSniffs() as $sniff) { + $sniff->enableFixer($fixer); + } + + foreach ($files as $file) { + $fixer->fixFile($file); + file_put_contents($file, $fixer->getContents()); + } + } + foreach ($ruleset->getSniffs() as $sniff) { - $sniff->enable($report); + $sniff->enableReport($report); } // Process foreach ($files as $file) { $this->setErrorHandler($report, $file); + $this->processTemplate($file, $ruleset, $report); // Add this file to the report. @@ -88,7 +103,7 @@ public function run(array $files, Ruleset $ruleset) */ public function processTemplate(string $file, Ruleset $ruleset, Report $report) { - $twigSource = new Source(file_get_contents($file), $file, $file); + $twigSource = new Source(file_get_contents($file), $file); // Tokenize + Parse. try { @@ -124,9 +139,7 @@ public function processTemplate(string $file, Ruleset $ruleset, Report $report) /** @var SniffInterface[] $sniffs */ $sniffs = $ruleset->getSniffs(); foreach ($sniffs as $sniff) { - foreach ($stream as $index => $token) { - $sniff->process($token, $index, $stream); - } + $sniff->processFile($stream); } return true; diff --git a/TwigCS/Sniff/AbstractSniff.php b/TwigCS/Sniff/AbstractSniff.php index beccd90..5ae4df5 100644 --- a/TwigCS/Sniff/AbstractSniff.php +++ b/TwigCS/Sniff/AbstractSniff.php @@ -5,6 +5,7 @@ use \Exception; use TwigCS\Report\Report; use TwigCS\Report\SniffViolation; +use TwigCS\Runner\Fixer; use TwigCS\Token\Token; /** @@ -17,25 +18,19 @@ abstract class AbstractSniff implements SniffInterface * * @var Report */ - protected $report; + protected $report = null; /** - * @var array + * @var Fixer */ - protected $messages; - - public function __construct() - { - $this->messages = []; - $this->report = null; - } + protected $fixer = null; /** * @param Report $report * * @return self */ - public function enable(Report $report) + public function enableReport(Report $report) { $this->report = $report; @@ -43,27 +38,26 @@ public function enable(Report $report) } /** + * @param Fixer $fixer + * * @return self */ - public function disable() + public function enableFixer(Fixer $fixer) { - $this->report = null; + $this->fixer = $fixer; return $this; } /** - * @return Report - * - * @throws Exception + * @return self */ - public function getReport() + public function disable() { - if (null === $this->report) { - throw new Exception('Sniff is disabled!'); - } + $this->report = null; + $this->fixer = null; - return $this->report; + return $this; } /** @@ -87,12 +81,19 @@ public function isTokenMatching(Token $token, int $type, string $value = null) * @param string $message * @param Token $token * - * @return self - * * @throws Exception */ public function addMessage(int $messageType, string $message, Token $token) { + if (null === $this->report) { + if (null !== $this->fixer) { + // We are fixing the file, ignore this + return; + } + + throw new Exception('Sniff is disabled!'); + } + $sniffViolation = new SniffViolation( $messageType, $message, @@ -101,9 +102,23 @@ public function addMessage(int $messageType, string $message, Token $token) ); $sniffViolation->setLinePosition($token->getPosition()); - $this->getReport()->addMessage($sniffViolation); + $this->report->addMessage($sniffViolation); + } - return $this; + /** + * @param int $messageType + * @param string $message + * @param Token $token + * + * @return bool + * + * @throws Exception + */ + public function addFixableMessage(int $messageType, string $message, Token $token) + { + $this->addMessage($messageType, $message, $token); + + return null !== $this->fixer; } /** @@ -119,4 +134,20 @@ public function stringifyValue(Token $token) return '\''.$token->getValue().'\''; } + + /** + * @param array $stream + */ + public function processFile(array $stream) + { + foreach ($stream as $index => $token) { + $this->process($index, $stream); + } + } + + /** + * @param int $tokenPosition + * @param Token[] $stream + */ + abstract protected function process(int $tokenPosition, array $stream); } diff --git a/TwigCS/Sniff/SniffInterface.php b/TwigCS/Sniff/SniffInterface.php index f25b0c4..4c08263 100644 --- a/TwigCS/Sniff/SniffInterface.php +++ b/TwigCS/Sniff/SniffInterface.php @@ -2,8 +2,8 @@ namespace TwigCS\Sniff; -use \Exception; use TwigCS\Report\Report; +use TwigCS\Runner\Fixer; use TwigCS\Token\Token; /** @@ -16,7 +16,7 @@ interface SniffInterface const MESSAGE_TYPE_ERROR = 2; /** - * Enable the sniff. + * Enable the sniff report. * * Once the sniff is enabled, it will be registered and executed when a template is tokenized or parsed. * Messages will be added to the given `$report` object. @@ -25,30 +25,28 @@ interface SniffInterface * * @return self */ - public function enable(Report $report); + public function enableReport(Report $report); /** - * Disable the sniff. + * Enable the sniff fixer. * - * It usually is disabled when the processing is over, it will reset the sniff internal values for next check. + * @param Fixer $fixer * * @return self */ - public function disable(); + public function enableFixer(Fixer $fixer); /** - * Get the current report. + * Disable the sniff. * - * @return Report + * It usually is disabled when the processing is over, it will reset the sniff internal values for next check. * - * @throws Exception A disabled sniff has no current report. + * @return self */ - public function getReport(); + public function disable(); /** - * @param Token $token - * @param int $tokenPosition * @param Token[] $stream */ - public function process(Token $token, int $tokenPosition, array $stream); + public function processFile(array $stream); } diff --git a/TwigCS/Tests/AbstractSniffTest.php b/TwigCS/Tests/AbstractSniffTest.php index e5d6c84..44e3fc3 100644 --- a/TwigCS/Tests/AbstractSniffTest.php +++ b/TwigCS/Tests/AbstractSniffTest.php @@ -6,9 +6,9 @@ use \ReflectionClass; use PHPUnit\Framework\TestCase; use TwigCS\Environment\StubbedEnvironment; -use TwigCS\Linter; use TwigCS\Report\SniffViolation; use TwigCS\Ruleset\Ruleset; +use TwigCS\Runner\Linter; use TwigCS\Sniff\SniffInterface; use TwigCS\Token\Tokenizer; From 9234aff4008acb8575f60a0ee82708ba14aaba88 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Fri, 14 Jun 2019 23:11:31 +0200 Subject: [PATCH 2/6] :sparkles: Add fix option for command --- TwigCS/Command/TwigCSCommand.php | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/TwigCS/Command/TwigCSCommand.php b/TwigCS/Command/TwigCSCommand.php index 0d54d53..112ee93 100644 --- a/TwigCS/Command/TwigCSCommand.php +++ b/TwigCS/Command/TwigCSCommand.php @@ -50,6 +50,12 @@ protected function configure() 'Run as if this was started in instead of the current working directory', getcwd() ), + new InputOption( + 'fix', + 'f', + InputOption::VALUE_NONE, + 'Automatically fix all the fixable violations' + ) ]) ->addArgument( 'paths', @@ -69,10 +75,11 @@ protected function configure() */ protected function execute(InputInterface $input, OutputInterface $output) { - $paths = $input->getArgument('paths'); - $exclude = $input->getOption('exclude'); - $level = $input->getOption('level'); + $paths = $input->getArgument('paths'); + $exclude = $input->getOption('exclude'); + $level = $input->getOption('level'); $currentDir = $input->getOption('working-dir'); + $fix = $input->getOption('fix'); $config = new Config([ 'paths' => $paths, @@ -87,7 +94,7 @@ protected function execute(InputInterface $input, OutputInterface $output) // Execute the linter. $twig = new StubbedEnvironment(); $linter = new Linter($twig, new Tokenizer($twig)); - $report = $linter->run($config->findFiles(), $ruleset); + $report = $linter->run($config->findFiles(), $ruleset, $fix); // Format the output. $reporter = new TextFormatter($input, $output); From 2477a3e61d03088fb4bb8c248cb6017def5492e2 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 15 Jun 2019 00:37:40 +0200 Subject: [PATCH 3/6] :rotating_light: Add tests for fixer --- TwigCS/Runner/Linter.php | 36 +++++++++++++------ TwigCS/Tests/AbstractSniffTest.php | 17 ++++++++- TwigCS/Tests/Fixtures/BlankEOFTest.fixed.twig | 2 ++ 3 files changed, 44 insertions(+), 11 deletions(-) create mode 100644 TwigCS/Tests/Fixtures/BlankEOFTest.fixed.twig diff --git a/TwigCS/Runner/Linter.php b/TwigCS/Runner/Linter.php index 90713e1..9ec1e26 100644 --- a/TwigCS/Runner/Linter.php +++ b/TwigCS/Runner/Linter.php @@ -57,16 +57,7 @@ public function run(array $files, Ruleset $ruleset, bool $fix = false) $report = new Report(); if ($fix) { - $fixer = new Fixer($ruleset, $this->tokenizer); - - foreach ($ruleset->getSniffs() as $sniff) { - $sniff->enableFixer($fixer); - } - - foreach ($files as $file) { - $fixer->fixFile($file); - file_put_contents($file, $fixer->getContents()); - } + $this->fix($files, $ruleset); } foreach ($ruleset->getSniffs() as $sniff) { @@ -92,6 +83,31 @@ public function run(array $files, Ruleset $ruleset, bool $fix = false) return $report; } + /** + * @param array $files + * @param Ruleset $ruleset + * + * @throws Exception + */ + public function fix(array $files, Ruleset $ruleset) + { + $fixer = new Fixer($ruleset, $this->tokenizer); + + foreach ($ruleset->getSniffs() as $sniff) { + $sniff->enableFixer($fixer); + } + + foreach ($files as $file) { + $success = $fixer->fixFile($file); + + if (!$success) { + throw new Exception("Cannot fix the file $file."); + } + + file_put_contents($file, $fixer->getContents()); + } + } + /** * Checks one template against the set of rules. * diff --git a/TwigCS/Tests/AbstractSniffTest.php b/TwigCS/Tests/AbstractSniffTest.php index 44e3fc3..d276ad0 100644 --- a/TwigCS/Tests/AbstractSniffTest.php +++ b/TwigCS/Tests/AbstractSniffTest.php @@ -8,6 +8,7 @@ use TwigCS\Environment\StubbedEnvironment; use TwigCS\Report\SniffViolation; use TwigCS\Ruleset\Ruleset; +use TwigCS\Runner\Fixer; use TwigCS\Runner\Linter; use TwigCS\Sniff\SniffInterface; use TwigCS\Token\Tokenizer; @@ -45,9 +46,12 @@ abstract public function testSniff(); protected function checkGenericSniff(SniffInterface $sniff, array $expects) { $ruleset = new Ruleset(); + $fixer = new Fixer($ruleset, new Tokenizer($this->env)); + try { $class = new ReflectionClass(get_called_class()); - $file = __DIR__.'/Fixtures/'.$class->getShortName().'.twig'; + $className = $class->getShortName(); + $file = __DIR__.'/Fixtures/'.$className.'.twig'; $ruleset->addSniff($sniff); $report = $this->lint->run([$file], $ruleset); @@ -68,5 +72,16 @@ protected function checkGenericSniff(SniffInterface $sniff, array $expects) $this->assertEquals($expects, $messagePositions); } + + $fixedFile = __DIR__.'/Fixtures/'.$className.'.fixed.twig'; + if (file_exists($fixedFile)) { + $sniff->enableFixer($fixer); + $fixer->fixFile($file); + + $diff = $fixer->generateDiff($fixedFile); + if ("" !== $diff) { + $this->fail($diff); + } + } } } diff --git a/TwigCS/Tests/Fixtures/BlankEOFTest.fixed.twig b/TwigCS/Tests/Fixtures/BlankEOFTest.fixed.twig new file mode 100644 index 0000000..1936aaf --- /dev/null +++ b/TwigCS/Tests/Fixtures/BlankEOFTest.fixed.twig @@ -0,0 +1,2 @@ +
+
From 2628a8bb65186a354c7c0cc6fec6016c7a47268a Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 15 Jun 2019 00:42:13 +0200 Subject: [PATCH 4/6] :lipstick: Fix lint --- TwigCS/Command/TwigCSCommand.php | 2 +- TwigCS/Runner/Fixer.php | 2 -- TwigCS/Tests/AbstractSniffTest.php | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/TwigCS/Command/TwigCSCommand.php b/TwigCS/Command/TwigCSCommand.php index 112ee93..78d6941 100644 --- a/TwigCS/Command/TwigCSCommand.php +++ b/TwigCS/Command/TwigCSCommand.php @@ -55,7 +55,7 @@ protected function configure() 'f', InputOption::VALUE_NONE, 'Automatically fix all the fixable violations' - ) + ), ]) ->addArgument( 'paths', diff --git a/TwigCS/Runner/Fixer.php b/TwigCS/Runner/Fixer.php index a67ddb4..7720d9a 100644 --- a/TwigCS/Runner/Fixer.php +++ b/TwigCS/Runner/Fixer.php @@ -99,7 +99,6 @@ class Fixer */ protected $numFixes = 0; - /** * Starts fixing a new file. * @@ -345,7 +344,6 @@ public function rollbackChangeset() } } - /** * Replace the entire contents of a token. * diff --git a/TwigCS/Tests/AbstractSniffTest.php b/TwigCS/Tests/AbstractSniffTest.php index d276ad0..85ae9f2 100644 --- a/TwigCS/Tests/AbstractSniffTest.php +++ b/TwigCS/Tests/AbstractSniffTest.php @@ -79,7 +79,7 @@ protected function checkGenericSniff(SniffInterface $sniff, array $expects) $fixer->fixFile($file); $diff = $fixer->generateDiff($fixedFile); - if ("" !== $diff) { + if ('' !== $diff) { $this->fail($diff); } } From d40e96046bbb393e58aac37db323b1579cf10f8f Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 15 Jun 2019 02:30:32 +0200 Subject: [PATCH 5/6] :lipstick: Minor clean --- TwigCS/Environment/StubbedEnvironment.php | 10 +++------- TwigCS/Ruleset/Ruleset.php | 7 +------ TwigCS/Runner/Fixer.php | 12 ------------ TwigCS/Sniff/AbstractSniff.php | 13 ------------- TwigCS/Tests/AbstractSniffTest.php | 23 +++++------------------ 5 files changed, 9 insertions(+), 56 deletions(-) diff --git a/TwigCS/Environment/StubbedEnvironment.php b/TwigCS/Environment/StubbedEnvironment.php index 04cbde4..df1c469 100644 --- a/TwigCS/Environment/StubbedEnvironment.php +++ b/TwigCS/Environment/StubbedEnvironment.php @@ -22,17 +22,17 @@ class StubbedEnvironment extends Environment /** * @var TwigFilter[] */ - private $stubFilters; + protected $stubFilters = []; /** * @var TwigFunction[] */ - private $stubFunctions; + protected $stubFunctions = []; /** * @var TwigTest[] */ - private $stubTests; + protected $stubTests = []; public function __construct() { @@ -44,10 +44,6 @@ public function __construct() $this->addTokenParser(new TransChoiceTokenParser()); $this->addTokenParser(new TransDefaultDomainTokenParser()); $this->addTokenParser(new TransTokenParser()); - - $this->stubFilters = []; - $this->stubFunctions = []; - $this->stubTests = []; } /** diff --git a/TwigCS/Ruleset/Ruleset.php b/TwigCS/Ruleset/Ruleset.php index 9866dca..58ae9c2 100644 --- a/TwigCS/Ruleset/Ruleset.php +++ b/TwigCS/Ruleset/Ruleset.php @@ -15,12 +15,7 @@ class Ruleset /** * @var SniffInterface[] */ - protected $sniffs; - - public function __construct() - { - $this->sniffs = []; - } + protected $sniffs = []; /** * @return SniffInterface[] diff --git a/TwigCS/Runner/Fixer.php b/TwigCS/Runner/Fixer.php index 7720d9a..d0faaa2 100644 --- a/TwigCS/Runner/Fixer.php +++ b/TwigCS/Runner/Fixer.php @@ -241,18 +241,6 @@ public function generateDiff($filePath) return $diff; } - /** - * Get a count of fixes that have been performed on the file. - * - * This value is reset every time a new file is started, or an existing file is restarted. - * - * @return int - */ - public function getFixCount() - { - return $this->numFixes; - } - /** * Get the current content of the file, as a string. * diff --git a/TwigCS/Sniff/AbstractSniff.php b/TwigCS/Sniff/AbstractSniff.php index 5ae4df5..df65a60 100644 --- a/TwigCS/Sniff/AbstractSniff.php +++ b/TwigCS/Sniff/AbstractSniff.php @@ -27,37 +27,24 @@ abstract class AbstractSniff implements SniffInterface /** * @param Report $report - * - * @return self */ public function enableReport(Report $report) { $this->report = $report; - - return $this; } /** * @param Fixer $fixer - * - * @return self */ public function enableFixer(Fixer $fixer) { $this->fixer = $fixer; - - return $this; } - /** - * @return self - */ public function disable() { $this->report = null; $this->fixer = null; - - return $this; } /** diff --git a/TwigCS/Tests/AbstractSniffTest.php b/TwigCS/Tests/AbstractSniffTest.php index 85ae9f2..5a52b15 100644 --- a/TwigCS/Tests/AbstractSniffTest.php +++ b/TwigCS/Tests/AbstractSniffTest.php @@ -18,22 +18,6 @@ */ abstract class AbstractSniffTest extends TestCase { - /** - * @var StubbedEnvironment - */ - private $env; - - /** - * @var Linter - */ - private $lint; - - public function setUp() - { - $this->env = new StubbedEnvironment(); - $this->lint = new Linter($this->env, new Tokenizer($this->env)); - } - /** * Should call $this->checkGenericSniff(new Sniff(), [...]); */ @@ -45,8 +29,10 @@ abstract public function testSniff(); */ protected function checkGenericSniff(SniffInterface $sniff, array $expects) { + $env = new StubbedEnvironment(); + $tokenizer = new Tokenizer($env); + $linter = new Linter($env, $tokenizer); $ruleset = new Ruleset(); - $fixer = new Fixer($ruleset, new Tokenizer($this->env)); try { $class = new ReflectionClass(get_called_class()); @@ -54,7 +40,7 @@ protected function checkGenericSniff(SniffInterface $sniff, array $expects) $file = __DIR__.'/Fixtures/'.$className.'.twig'; $ruleset->addSniff($sniff); - $report = $this->lint->run([$file], $ruleset); + $report = $linter->run([$file], $ruleset); } catch (Exception $e) { $this->fail($e->getMessage()); @@ -75,6 +61,7 @@ protected function checkGenericSniff(SniffInterface $sniff, array $expects) $fixedFile = __DIR__.'/Fixtures/'.$className.'.fixed.twig'; if (file_exists($fixedFile)) { + $fixer = new Fixer($ruleset, $tokenizer); $sniff->enableFixer($fixer); $fixer->fixFile($file); From 59c91b5912eca52168067a289c7d32fefbf6c068 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Sat, 15 Jun 2019 02:36:18 +0200 Subject: [PATCH 6/6] :sparkles: Add fixer for Delimiter spacing --- .../Ruleset/Generic/DelimiterSpacingSniff.php | 20 +++++++++++++++++-- .../Fixtures/DelimiterSpacingTest.fixed.twig | 14 +++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 TwigCS/Tests/Fixtures/DelimiterSpacingTest.fixed.twig diff --git a/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php b/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php index 53a7ed4..605e297 100644 --- a/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php +++ b/TwigCS/Ruleset/Generic/DelimiterSpacingSniff.php @@ -62,11 +62,19 @@ public function processStart(int $tokenPosition, array $tokens) } if (1 !== $count) { - $this->addMessage( + $fix = $this->addFixableMessage( $this::MESSAGE_TYPE_ERROR, sprintf('Expecting 1 whitespace after "%s"; found %d', $token->getValue(), $count), $token ); + + if ($fix) { + if (0 === $count) { + $this->fixer->addContent($tokenPosition, ' '); + } else { + $this->fixer->replaceToken($tokenPosition + 1, ' '); + } + } } } @@ -92,11 +100,19 @@ public function processEnd(int $tokenPosition, array $tokens) } if (1 !== $count) { - $this->addMessage( + $fix = $this->addFixableMessage( $this::MESSAGE_TYPE_ERROR, sprintf('Expecting 1 whitespace before "%s"; found %d', $token->getValue(), $count), $token ); + + if ($fix) { + if (0 === $count) { + $this->fixer->addContentBefore($tokenPosition, ' '); + } else { + $this->fixer->replaceToken($tokenPosition - 1, ' '); + } + } } } } diff --git a/TwigCS/Tests/Fixtures/DelimiterSpacingTest.fixed.twig b/TwigCS/Tests/Fixtures/DelimiterSpacingTest.fixed.twig new file mode 100644 index 0000000..06d9394 --- /dev/null +++ b/TwigCS/Tests/Fixtures/DelimiterSpacingTest.fixed.twig @@ -0,0 +1,14 @@ +{{ foo }} +{# comment #} +{% if foo %}{% endif %} + +{{- foo -}} +{#- comment -#} +{%- if foo -%}{%- endif -%} + +{{ + shouldNotCareAboutNewLine +}} +{%- if foo -%}{%- endif -%} + +{{ foo({'bar': {'baz': 'shouldNotCareAboutDoubleHashes'}}) }}