PHPMD - decrease cyclomatic complexity report level to 9 (#168)

kubawerlos · Oct 24, 2019 · 1753f57 · 1753f57
1 parent b1856e3
commit 1753f57
Show file tree

Hide file tree

Showing 9 changed files with 237 additions and 139 deletions.
diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@
 
 [![Build status](https://img.shields.io/travis/kubawerlos/php-cs-fixer-custom-fixers/master.svg)](https://travis-ci.org/kubawerlos/php-cs-fixer-custom-fixers)
 [![Code coverage](https://img.shields.io/coveralls/github/kubawerlos/php-cs-fixer-custom-fixers/master.svg)](https://coveralls.io/github/kubawerlos/php-cs-fixer-custom-fixers?branch=master)
-![Tests](https://img.shields.io/badge/tests-1353-brightgreen.svg)
+![Tests](https://img.shields.io/badge/tests-1358-brightgreen.svg)
 [![Mutation testing badge](https://badge.stryker-mutator.io/github.com/kubawerlos/php-cs-fixer-custom-fixers/master)](https://stryker-mutator.github.io)
 [![Psalm type coverage](https://shepherd.dev/github/kubawerlos/php-cs-fixer-custom-fixers/coverage.svg)](https://shepherd.dev/github/kubawerlos/php-cs-fixer-custom-fixers)
 

diff --git a/phpmd.xml b/phpmd.xml
@@ -11,7 +11,7 @@
 
     <rule ref='rulesets/codesize.xml/CyclomaticComplexity'>
         <properties>
-            <property name='reportLevel' value='10' />
+            <property name='reportLevel' value='9' />
         </properties>
     </rule>
     <rule ref='rulesets/codesize.xml/ExcessiveMethodLength' />

diff --git a/src/Fixer/PhpdocParamOrderFixer.php b/src/Fixer/PhpdocParamOrderFixer.php
@@ -31,6 +31,11 @@ function foo($a, $b, $c) {}
         );
     }
 
+    public function getPriority(): int
+    {
+        return 5;
+    }
+
     public function isCandidate(Tokens $tokens): bool
     {
         return $tokens->isAllTokenKindsFound([T_DOC_COMMENT, T_FUNCTION]);
@@ -55,21 +60,27 @@ public function fix(\SplFileInfo $file, Tokens $tokens): void
 
             $paramNames = $this->getParamNames($tokens, $functionIndex);
 
-            $newContent = $this->getSortedDocComment($tokens[$index]->getContent(), $paramNames);
+            $docBlock = new DocBlock($tokens[$index]->getContent());
+            $sorted = $this->getSortedAnnotations($docBlock->getAnnotations(), $paramNames);
+
+            foreach ($sorted as $annotationIndex => $annotationContent) {
+                /** @var Annotation $annotation */
+                $annotation = $docBlock->getAnnotation($annotationIndex);
+                $annotation->remove();
 
-            if ($newContent === $tokens[$index]->getContent()) {
+                /** @var Line $line */
+                $line = $docBlock->getLine($annotation->getStart());
+                $line->setContent($annotationContent);
+            }
+
+            if ($docBlock->getContent() === $tokens[$index]->getContent()) {
                 continue;
             }
 
-            $tokens[$index] = new Token([T_DOC_COMMENT, $newContent]);
+            $tokens[$index] = new Token([T_DOC_COMMENT, $docBlock->getContent()]);
         }
     }
 
-    public function getPriority(): int
-    {
-        return 5;
-    }
-
     /**
      * @return string[]
      */
@@ -90,25 +101,17 @@ private function getParamNames(Tokens $tokens, int $functionIndex): array
         return $paramNames;
     }
 
-    private function getSortedDocComment(string $comment, array $paramNames): string
+    private function getSortedAnnotations(array $annotations, array $paramNames): array
     {
-        $docBlock = new DocBlock($comment);
-        $firstParamIndex = null;
+        $paramFound = false;
         $annotationsBeforeParams = [];
         $paramsByName = \array_combine($paramNames, \array_fill(0, \count($paramNames), null));
         $superfluousParams = [];
         $annotationsAfterParams = [];
 
-        foreach ($docBlock->getAnnotations() as $index => $annotation) {
-            if ($firstParamIndex === null) {
-                if ($annotation->getTag()->getName() !== 'param') {
-                    $annotationsBeforeParams[] = $annotation->getContent();
-                    continue;
-                }
-                $firstParamIndex = $index;
-            }
-
+        foreach ($annotations as $annotation) {
             if ($annotation->getTag()->getName() === 'param') {
+                $paramFound = true;
                 foreach ($paramNames as $paramName) {
                     if (Preg::match(\sprintf('/@param\s+(?:[^\$](?:[^<\s]|<[^>]*>)*\s+)?(?:&|\.\.\.)?\s*(\Q%s\E)\b/', $paramName), $annotation->getContent(), $matches) === 1 && !isset($paramsByName[$matches[1]])) {
                         $paramsByName[$matches[1]] = $annotation->getContent();
@@ -119,21 +122,14 @@ private function getSortedDocComment(string $comment, array $paramNames): string
                 continue;
             }
 
-            $annotationsAfterParams[] = $annotation->getContent();
-        }
-
-        $sorted = \array_merge($annotationsBeforeParams, \array_values(\array_filter($paramsByName)), $superfluousParams, $annotationsAfterParams);
-
-        foreach ($sorted as $index => $annotationContent) {
-            /** @var Annotation $annotation */
-            $annotation = $docBlock->getAnnotation($index);
-            $annotation->remove();
+            if ($paramFound) {
+                $annotationsAfterParams[] = $annotation->getContent();
+                continue;
+            }
 
-            /** @var Line $line */
-            $line = $docBlock->getLine($annotation->getStart());
-            $line->setContent($annotationContent);
+            $annotationsBeforeParams[] = $annotation->getContent();
         }
 
-        return $docBlock->getContent();
+        return \array_merge($annotationsBeforeParams, \array_values(\array_filter($paramsByName)), $superfluousParams, $annotationsAfterParams);
     }
 }
diff --git a/src/Fixer/SingleSpaceAfterStatementFixer.php b/src/Fixer/SingleSpaceAfterStatementFixer.php
@@ -86,8 +86,8 @@ public function getConfigurationDefinition(): FixerConfigurationResolver
     {
         return new FixerConfigurationResolver([
             (new FixerOptionBuilder('allow_linebreak', 'whether to allow statement followed by linebreak'))
-                ->setDefault($this->allowLinebreak)
                 ->setAllowedTypes(['bool'])
+                ->setDefault($this->allowLinebreak)
                 ->getOption(),
         ]);
     }
@@ -97,6 +97,11 @@ public function configure(?array $configuration = null): void
         $this->allowLinebreak = $configuration['allow_linebreak'] ?? $this->allowLinebreak;
     }
 
+    public function getPriority(): int
+    {
+        return 0;
+    }
+
     public function isCandidate(Tokens $tokens): bool
     {
         return $tokens->isAnyTokenKindsFound($this->tokens);
@@ -114,26 +119,29 @@ public function fix(\SplFileInfo $file, Tokens $tokens): void
                 continue;
             }
 
-            if (!$tokens[$index + 1]->isGivenKind(T_WHITESPACE)) {
-                if ($token->isGivenKind(T_CLASS) && $tokens[$index + 1]->getContent() === '(') {
-                    continue;
-                }
-                if (!\in_array($tokens[$index + 1]->getContent(), [';', ':'], true)) {
-                    $tokens->insertAt($index + 1, new Token([T_WHITESPACE, ' ']));
-                }
+            if (!$this->canAddSpaceAfter($tokens, $index)) {
                 continue;
             }
 
-            if ($this->allowLinebreak && Preg::match('/\R/', $tokens[$index + 1]->getContent()) === 1) {
+            if ($tokens[$index + 1]->isGivenKind(T_WHITESPACE)) {
+                $tokens[$index + 1] = new Token([T_WHITESPACE, ' ']);
                 continue;
             }
 
-            $tokens[$index + 1] = new Token([T_WHITESPACE, ' ']);
+            $tokens->insertAt($index + 1, new Token([T_WHITESPACE, ' ']));
         }
     }
 
-    public function getPriority(): int
+    private function canAddSpaceAfter(Tokens $tokens, int $index): bool
     {
-        return 0;
+        if ($tokens[$index + 1]->isGivenKind(T_WHITESPACE)) {
+            return !$this->allowLinebreak || Preg::match('/\R/', $tokens[$index + 1]->getContent()) !== 1;
+        }
+
+        if ($tokens[$index]->isGivenKind(T_CLASS) && $tokens[$index + 1]->getContent() === '(') {
+            return false;
+        }
+
+        return !\in_array($tokens[$index + 1]->getContent(), [';', ':'], true);
     }
 }
diff --git a/src/Fixer/SingleSpaceBeforeStatementFixer.php b/src/Fixer/SingleSpaceBeforeStatementFixer.php
@@ -76,6 +76,11 @@ public function getDefinition(): FixerDefinitionInterface
         );
     }
 
+    public function getPriority(): int
+    {
+        return 0;
+    }
+
     public function isCandidate(Tokens $tokens): bool
     {
         return $tokens->isAnyTokenKindsFound($this->tokens);
@@ -93,31 +98,38 @@ public function fix(\SplFileInfo $file, Tokens $tokens): void
                 continue;
             }
 
-            if (!$tokens[$index - 1]->isGivenKind(T_WHITESPACE)) {
-                if (!\in_array($tokens[$index - 1]->getContent(), ['!', '(', '@', '[', '{'], true) && !$tokens[$index - 1]->isGivenKind(T_OPEN_TAG)) {
-                    $tokens->insertAt($index, new Token([T_WHITESPACE, ' ']));
-                }
+            if ($tokens[$index - 1]->isGivenKind(T_OPEN_TAG)) {
                 continue;
             }
 
-            if (Preg::match('/\R/', $tokens[$index - 1]->getContent()) === 1) {
+            if ($tokens[$index - 2]->isGivenKind(T_OPEN_TAG)) {
+                $this->fixTwoTokensAfterOpenTag($tokens, $index);
                 continue;
             }
 
-            if ($tokens[$index - 2]->isGivenKind(T_OPEN_TAG)) {
-                if (Preg::match('/\R/', $tokens[$index - 2]->getContent()) !== 1) {
-                    $tokens->clearAt($index - 1);
-                }
-
-                return;
-            }
+            $this->fixMoreThanTwoTokensAfterOpenTag($tokens, $index);
+        }
+    }
 
-            $tokens[$index - 1] = new Token([T_WHITESPACE, ' ']);
+    private function fixTwoTokensAfterOpenTag(Tokens $tokens, int $index): void
+    {
+        if ($tokens[$index - 1]->isGivenKind(T_WHITESPACE) && Preg::match('/\R/', $tokens[$index - 2]->getContent()) !== 1) {
+            $tokens->clearAt($index - 1);
         }
     }
 
-    public function getPriority(): int
+    private function fixMoreThanTwoTokensAfterOpenTag(Tokens $tokens, int $index): void
     {
-        return 0;
+        if ($tokens[$index - 1]->isGivenKind(T_WHITESPACE)) {
+            if (Preg::match('/\R/', $tokens[$index - 1]->getContent()) !== 1) {
+                $tokens[$index - 1] = new Token([T_WHITESPACE, ' ']);
+            }
+
+            return;
+        }
+
+        if (!\in_array($tokens[$index - 1]->getContent(), ['!', '(', '@', '[', '{'], true)) {
+            $tokens->insertAt($index, new Token([T_WHITESPACE, ' ']));
+        }
     }
 }
diff --git a/src/TokenRemover.php b/src/TokenRemover.php
@@ -31,33 +31,43 @@ public static function removeWithLinesIfPossible(Tokens $tokens, int $index): vo
     }
 
     private static function isTokenOnlyMeaningfulInLine(Tokens $tokens, int $index): bool
+    {
+        return !self::hasMeaningTokenInLineBefore($tokens, $index) && !self::hasMeaningTokenInLineAfter($tokens, $index);
+    }
+
+    private static function hasMeaningTokenInLineBefore(Tokens $tokens, int $index): bool
     {
         /** @var int $prevIndex */
         $prevIndex = $tokens->getNonEmptySibling($index, -1);
         if (!$tokens[$prevIndex]->isGivenKind([T_OPEN_TAG, T_WHITESPACE])) {
-            return false;
+            return true;
         }
 
         if ($tokens[$prevIndex]->isGivenKind(T_OPEN_TAG) && Preg::match('/\R$/', $tokens[$prevIndex]->getContent()) !== 1) {
-            return false;
+            return true;
         }
 
         if (Preg::match('/\R/', $tokens[$prevIndex]->getContent()) !== 1) {
             $prevPrevIndex = $tokens->getNonEmptySibling($prevIndex, -1);
             if (!$tokens[$prevPrevIndex]->isGivenKind(T_OPEN_TAG) || Preg::match('/\R$/', $tokens[$prevPrevIndex]->getContent()) !== 1) {
-                return false;
+                return true;
             }
         }
 
+        return false;
+    }
+
+    private static function hasMeaningTokenInLineAfter(Tokens $tokens, int $index): bool
+    {
         $nextIndex = $tokens->getNonEmptySibling($index, 1);
         if ($nextIndex === null) {
-            return true;
+            return false;
         }
         if (!$tokens[$nextIndex]->isGivenKind(T_WHITESPACE)) {
-            return false;
+            return true;
         }
 
-        return Preg::match('/\R/', $tokens[$nextIndex]->getContent()) === 1;
+        return Preg::match('/\R/', $tokens[$nextIndex]->getContent()) !== 1;
     }
 
     private static function handleWhitespaceBefore(Tokens $tokens, int $index): bool

diff --git a/tests/Fixer/PhpdocParamOrderFixerTest.php b/tests/Fixer/PhpdocParamOrderFixerTest.php
@@ -199,5 +199,95 @@ function foo($a) {}
                 function bar($a, $b) {}
             ',
         ];
+
+        yield [
+            '<?php
+                /**
+                 * @author John Doe
+                 *
+                 * @param bool $a
+                 * @param bool $b
+                 */
+                function foo($a, $b) {}
+            ',
+            '<?php
+                /**
+                 * @author John Doe
+                 *
+                 * @param bool $b
+                 * @param bool $a
+                 */
+                function foo($a, $b) {}
+            ',
+        ];
+
+        yield [
+            '<?php
+                /**
+                 * @param bool $a
+                 * @param bool $b
+                 *
+                 * @see example.com
+                 */
+                function foo($a, $b) {}
+            ',
+            '<?php
+                /**
+                 * @param bool $b
+                 * @param bool $a
+                 *
+                 * @see example.com
+                 */
+                function foo($a, $b) {}
+            ',
+        ];
+
+        yield [
+            '<?php
+                /**
+                 * @author John Doe
+                 *
+                 * @param bool $a
+                 * @param bool $b
+                 *
+                 * @see example.com
+                 */
+                function foo($a, $b) {}
+            ',
+            '<?php
+                /**
+                 * @author John Doe
+                 *
+                 * @param bool $b
+                 * @param bool $a
+                 *
+                 * @see example.com
+                 */
+                function foo($a, $b) {}
+            ',
+        ];
+
+        yield [
+            '<?php
+                /**
+                 * @author John Doe
+                 * @param bool $a
+                 * @param bool $b
+                 * @version 2.0
+                 * @see example.com
+                 */
+                function foo($a, $b) {}
+            ',
+            '<?php
+                /**
+                 * @author John Doe
+                 * @param bool $b
+                 * @version 2.0
+                 * @param bool $a
+                 * @see example.com
+                 */
+                function foo($a, $b) {}
+            ',
+        ];
     }
 }