Add ImplodeCallFixer

PHP-CS-Fixer · Aug 22, 2018 · f063eae · f063eae
1 parent 791a415
commit f063eae
Show file tree

Hide file tree

Showing 18 changed files with 365 additions and 18 deletions.
diff --git a/README.rst b/README.rst
@@ -733,6 +733,13 @@ Choose from the list of available rules:
 
   Convert ``heredoc`` to ``nowdoc`` where possible.
 
+* **implode_call** [@Symfony:risky]
+
+  Function ``implode`` must be called with 2 arguments in the documented
+  order.
+
+  *Risky rule: risky when the function ``implode`` is overridden.*
+
 * **include** [@Symfony]
 
   Include/Require and file path should be divided with a single space.

diff --git a/src/DocBlock/Annotation.php b/src/DocBlock/Annotation.php
@@ -261,7 +261,7 @@ public function remove()
      */
     public function getContent()
     {
-        return implode($this->lines);
+        return implode('', $this->lines);
     }
 
     public function supportTypes()

diff --git a/src/DocBlock/DocBlock.php b/src/DocBlock/DocBlock.php
@@ -159,7 +159,7 @@ public function getAnnotationsOfType($types)
      */
     public function getContent()
     {
-        return implode($this->lines);
+        return implode('', $this->lines);
     }
 
     private function findAnnotationLength($start)

diff --git a/src/Fixer/FunctionNotation/ImplodeCallFixer.php b/src/Fixer/FunctionNotation/ImplodeCallFixer.php
@@ -0,0 +1,150 @@
+<?php
+
+/*
+ * This file is part of PHP CS Fixer.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *     Dariusz Rumiński <dariusz.ruminski@gmail.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace PhpCsFixer\Fixer\FunctionNotation;
+
+use PhpCsFixer\AbstractFixer;
+use PhpCsFixer\FixerDefinition\CodeSample;
+use PhpCsFixer\FixerDefinition\FixerDefinition;
+use PhpCsFixer\Tokenizer\Analyzer\ArgumentsAnalyzer;
+use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer;
+use PhpCsFixer\Tokenizer\Token;
+use PhpCsFixer\Tokenizer\Tokens;
+
+/**
+ * @author Kuba Werłos <werlos@gmail.com>
+ */
+final class ImplodeCallFixer extends AbstractFixer
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function getDefinition()
+    {
+        return new FixerDefinition(
+            'Function `implode` must be called with 2 arguments in the documented order.',
+            [
+                new CodeSample("<?php\nimplode(\$pieces, '');\n"),
+                new CodeSample("<?php\nimplode(\$pieces);\n"),
+            ],
+            null,
+            'Risky when the function `implode` is overridden.'
+        );
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isRisky()
+    {
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isCandidate(Tokens $tokens)
+    {
+        return $tokens->isTokenKindFound(T_STRING);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getPriority()
+    {
+        // must be run after NoAliasFunctionsFixer
+        // must be run before MethodArgumentSpaceFixer
+        return -1;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function applyFix(\SplFileInfo $file, Tokens $tokens)
+    {
+        $functionsAnalyzer = new FunctionsAnalyzer();
+
+        foreach ($tokens as $index => $token) {
+            if (!$tokens[$index]->equals([T_STRING, 'implode'], false)) {
+                continue;
+            }
+
+            if (!$functionsAnalyzer->isGlobalFunctionCall($tokens, $index)) {
+                continue;
+            }
+
+            $argumentsIndices = $this->getArgumentIndices($tokens, $index);
+
+            if (1 === \count($argumentsIndices)) {
+                $firstArgumentIndex = \key($argumentsIndices);
+                $tokens->insertAt($firstArgumentIndex, [
+                    new Token([T_CONSTANT_ENCAPSED_STRING, "''"]),
+                    new Token(','),
+                    new Token([T_WHITESPACE, ' ']),
+                ]);
+
+                continue;
+            }
+
+            if (2 === \count($argumentsIndices)) {
+                list($firstArgumentIndex, $secondArgumentIndex) = \array_keys($argumentsIndices);
+
+                // If the first argument is string we have nothing to do
+                if ($tokens[$firstArgumentIndex]->isGivenKind(T_CONSTANT_ENCAPSED_STRING)) {
+                    continue;
+                }
+                // If the second argument is not string we cannot make a swap
+                if (!$tokens[$secondArgumentIndex]->isGivenKind(T_CONSTANT_ENCAPSED_STRING)) {
+                    continue;
+                }
+
+                // collect tokens from first argument
+                $firstArgumenteEndIndex = $argumentsIndices[\key($argumentsIndices)];
+                $newSecondArgumentTokens = [];
+                for ($i = \key($argumentsIndices); $i <= $firstArgumenteEndIndex; ++$i) {
+                    $newSecondArgumentTokens[] = clone $tokens[$i];
+                    $tokens->clearAt($i);
+                }
+
+                $tokens->insertAt($firstArgumentIndex, clone $tokens[$secondArgumentIndex]);
+
+                // insert above increased the second argument index
+                ++$secondArgumentIndex;
+                $tokens->clearAt($secondArgumentIndex);
+                $tokens->insertAt($secondArgumentIndex, $newSecondArgumentTokens);
+            }
+        }
+    }
+
+    /**
+     * @param Tokens $tokens
+     * @param int    $functionNameIndex
+     *
+     * @return array<int, int> In the format: startIndex => endIndex
+     */
+    private function getArgumentIndices(Tokens $tokens, $functionNameIndex)
+    {
+        $argumentsAnalyzer = new ArgumentsAnalyzer();
+
+        $openParenthesis = $tokens->getNextTokenOfKind($functionNameIndex, ['(']);
+        $closeParenthesis = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParenthesis);
+
+        $indices = [];
+
+        foreach ($argumentsAnalyzer->getArguments($tokens, $openParenthesis, $closeParenthesis) as $startIndexCandidate => $endIndex) {
+            $indices[$tokens->getNextMeaningfulToken($startIndexCandidate - 1)] = $tokens->getPrevMeaningfulToken($endIndex + 1);
+        }
+
+        return $indices;
+    }
+}
diff --git a/src/Fixer/FunctionNotation/MethodArgumentSpaceFixer.php b/src/Fixer/FunctionNotation/MethodArgumentSpaceFixer.php
@@ -106,6 +106,15 @@ public function configure(array $configuration = null)
         }
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function getPriority()
+    {
+        // must be run after ImplodeCallFixer
+        return -2;
+    }
+
     /**
      * {@inheritdoc}
      */

diff --git a/src/Fixer/PhpUnit/PhpUnitInternalClassFixer.php b/src/Fixer/PhpUnit/PhpUnitInternalClassFixer.php
@@ -147,7 +147,7 @@ private function updateDocBlockIfNeeded(Tokens $tokens, $docBlockIndex)
         }
         $doc = $this->makeDocBlockMultiLineIfNeeded($doc, $tokens, $docBlockIndex);
         $lines = $this->addInternalAnnotation($doc, $tokens, $docBlockIndex);
-        $lines = implode($lines);
+        $lines = implode('', $lines);
 
         $tokens[$docBlockIndex] = new Token([T_DOC_COMMENT, $lines]);
     }
@@ -227,7 +227,7 @@ private function makeDocBlockMultiLineIfNeeded(DocBlock $doc, Tokens $tokens, $d
         if (1 === \count($lines) && empty($doc->getAnnotationsOfType('internal'))) {
             $lines = $this->splitUpDocBlock($lines, $tokens, $docBlockIndex);
 
-            return new DocBlock(implode($lines));
+            return new DocBlock(implode('', $lines));
         }
 
         return $doc;
@@ -275,6 +275,6 @@ private function getSingleLineDocBlockEntry($line)
         }
         $line = \array_slice($line, $i);
 
-        return implode($line);
+        return implode('', $line);
     }
 }
diff --git a/src/Fixer/PhpUnit/PhpUnitMethodCasingFixer.php b/src/Fixer/PhpUnit/PhpUnitMethodCasingFixer.php
@@ -267,7 +267,7 @@ private function updateDocBlock(Tokens $tokens, $docBlockIndex)
         }
 
         if ($docBlockNeesUpdate) {
-            $lines = implode($lines);
+            $lines = implode('', $lines);
             $tokens[$docBlockIndex] = new Token([T_DOC_COMMENT, $lines]);
         }
     }

diff --git a/src/Fixer/PhpUnit/PhpUnitNoExpectationAnnotationFixer.php b/src/Fixer/PhpUnit/PhpUnitNoExpectationAnnotationFixer.php
@@ -233,7 +233,7 @@ private function fixPhpUnitClass(Tokens $tokens, $startIndex, $endIndex)
             $newMethodsCode = '<?php $this->'
                 .(isset($annotations['expectedExceptionMessageRegExp']) ? 'setExpectedExceptionRegExp' : 'setExpectedException')
                 .'('
-                .implode($paramList, ', ')
+                .implode(', ', $paramList)
                 .');';
             $newMethods = Tokens::fromCode($newMethodsCode);
             $newMethods[0] = new Token([

diff --git a/src/Fixer/PhpUnit/PhpUnitOrderedCoversFixer.php b/src/Fixer/PhpUnit/PhpUnitOrderedCoversFixer.php
@@ -100,7 +100,7 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
                 );
             }
 
-            $tokens[$index] = new Token([T_DOC_COMMENT, implode($lines)]);
+            $tokens[$index] = new Token([T_DOC_COMMENT, implode('', $lines)]);
         }
     }
 }
diff --git a/src/Fixer/PhpUnit/PhpUnitTestAnnotationFixer.php b/src/Fixer/PhpUnit/PhpUnitTestAnnotationFixer.php
@@ -145,7 +145,7 @@ private function applyTestAnnotation(Tokens $tokens, $startIndex, $endIndex)
 
             $lines = $this->addTestAnnotation($lines, $tokens, $docBlockIndex);
 
-            $lines = implode($lines);
+            $lines = implode('', $lines);
             $tokens[$docBlockIndex] = new Token([T_DOC_COMMENT, $lines]);
         }
     }
@@ -167,7 +167,7 @@ private function applyTestPrefix(Tokens $tokens, $startIndex, $endIndex)
 
             $lines = $this->updateDocBlock($tokens, $docBlockIndex);
 
-            $lines = implode($lines);
+            $lines = implode('', $lines);
             $tokens[$docBlockIndex] = new Token([T_DOC_COMMENT, $lines]);
 
             $functionNameIndex = $tokens->getNextMeaningfulToken($i);
@@ -451,7 +451,7 @@ private function getSingleLineDocBlockEntry($line)
         }
         $line = \array_slice($line, $i);
 
-        return implode($line);
+        return implode('', $line);
     }
 
     /**
@@ -480,14 +480,14 @@ private function removeTestPrefixFromDependsAnnotation(Line $line)
         $line = \str_split($line->getContent());
 
         $dependsIndex = $this->findWhereDependsFunctionNameStarts($line);
-        $dependsFunctionName = implode(\array_slice($line, $dependsIndex));
+        $dependsFunctionName = implode('', \array_slice($line, $dependsIndex));
 
         if ($this->startsWith('test', $dependsFunctionName)) {
             $dependsFunctionName = $this->removeTestPrefix($dependsFunctionName);
         }
         array_splice($line, $dependsIndex);
 
-        return new Line(implode($line).$dependsFunctionName);
+        return new Line(implode('', $line).$dependsFunctionName);
     }
 
     /**
@@ -499,15 +499,15 @@ private function addTestPrefixToDependsAnnotation(Line $line)
     {
         $line = \str_split($line->getContent());
         $dependsIndex = $this->findWhereDependsFunctionNameStarts($line);
-        $dependsFunctionName = implode(\array_slice($line, $dependsIndex));
+        $dependsFunctionName = implode('', \array_slice($line, $dependsIndex));
 
         if (!$this->startsWith('test', $dependsFunctionName)) {
             $dependsFunctionName = $this->addTestPrefix($dependsFunctionName);
         }
 
         array_splice($line, $dependsIndex);
 
-        return new Line(implode($line).$dependsFunctionName);
+        return new Line(implode('', $line).$dependsFunctionName);
     }
 
     /**

diff --git a/src/Fixer/Phpdoc/PhpdocAlignFixer.php b/src/Fixer/Phpdoc/PhpdocAlignFixer.php
@@ -328,7 +328,7 @@ private function fixDocBlock($content)
             }
         }
 
-        return implode($lines);
+        return implode('', $lines);
     }
 
     /**

diff --git a/src/Fixer/Whitespace/NoExtraBlankLinesFixer.php b/src/Fixer/Whitespace/NoExtraBlankLinesFixer.php
@@ -383,7 +383,7 @@ private function removeMultipleBlankLines($index)
         $count = \count($parts);
 
         if ($count > $expected) {
-            $this->tokens[$index] = new Token([T_WHITESPACE, implode(\array_slice($parts, 0, $expected)).rtrim($parts[$count - 1], "\r\n")]);
+            $this->tokens[$index] = new Token([T_WHITESPACE, implode('', \array_slice($parts, 0, $expected)).rtrim($parts[$count - 1], "\r\n")]);
         }
     }
 

diff --git a/src/Fixer/Whitespace/NoTrailingWhitespaceFixer.php b/src/Fixer/Whitespace/NoTrailingWhitespaceFixer.php
@@ -101,7 +101,7 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
                     }
                 }
 
-                $content = implode($lines);
+                $content = implode('', $lines);
                 if ('' !== $content) {
                     $tokens[$index] = new Token([$token->getId(), $content]);
                 } else {

diff --git a/src/RuleSet.php b/src/RuleSet.php
@@ -170,6 +170,7 @@ final class RuleSet implements RuleSetInterface
             'fopen_flag_order' => true,
             'fopen_flags' => true,
             'function_to_constant' => true,
+            'implode_call' => true,
             'is_null' => true,
             'modernize_types_casting' => true,
             'native_constant_invocation' => [

diff --git a/tests/AutoReview/FixerFactoryTest.php b/tests/AutoReview/FixerFactoryTest.php
@@ -98,11 +98,13 @@ public function provideFixersPriorityCases()
             [$fixers['general_phpdoc_annotation_remove'], $fixers['phpdoc_trim']],
             [$fixers['general_phpdoc_annotation_remove'], $fixers['no_empty_phpdoc']],
             [$fixers['indentation_type'], $fixers['phpdoc_indent']],
+            [$fixers['implode_call'], $fixers['method_argument_space']],
             [$fixers['is_null'], $fixers['yoda_style']],
             [$fixers['list_syntax'], $fixers['binary_operator_spaces']],
             [$fixers['list_syntax'], $fixers['ternary_operator_spaces']],
             [$fixers['method_separation'], $fixers['braces']],
             [$fixers['method_separation'], $fixers['indentation_type']],
+            [$fixers['no_alias_functions'], $fixers['implode_call']],
             [$fixers['no_alias_functions'], $fixers['php_unit_dedicate_assert']],
             [$fixers['no_blank_lines_after_phpdoc'], $fixers['single_blank_line_before_namespace']],
             [$fixers['no_empty_comment'], $fixers['no_extra_blank_lines']],