Browse Source

PhpdocLineSpanFixer - Introduction

Gert de Pagter 6 years ago
parent
commit
649ea588ec

+ 14 - 0
README.rst

@@ -1527,6 +1527,20 @@ Choose from the list of available rules:
 
   Fix PHPDoc inline tags, make ``@inheritdoc`` always inline.
 
+* **phpdoc_line_span**
+
+  Changes doc blocks from single to multi line, or reversed. Works for
+  class constants, properties and methods only.
+
+  Configuration options:
+
+  - ``const`` (``'multi'``, ``'single'``): whether const blocks should be single or
+    multi line; defaults to ``'multi'``
+  - ``method`` (``'multi'``, ``'single'``): whether method doc blocks should be single
+    or multi line; defaults to ``'multi'``
+  - ``property`` (``'multi'``, ``'single'``): whether property doc blocks should be
+    single or multi line; defaults to ``'multi'``
+
 * **phpdoc_no_access** [@Symfony, @PhpCsFixer]
 
   ``@access`` annotations should be omitted from PHPDoc.

+ 105 - 16
src/DocBlock/DocBlock.php

@@ -90,28 +90,88 @@ class DocBlock
      */
     public function getAnnotations()
     {
-        if (null === $this->annotations) {
-            $this->annotations = [];
-            $total = \count($this->lines);
-
-            for ($index = 0; $index < $total; ++$index) {
-                if ($this->lines[$index]->containsATag()) {
-                    // get all the lines that make up the annotation
-                    $lines = \array_slice($this->lines, $index, $this->findAnnotationLength($index), true);
-                    $annotation = new Annotation($lines);
-                    // move the index to the end of the annotation to avoid
-                    // checking it again because we know the lines inside the
-                    // current annotation cannot be part of another annotation
-                    $index = $annotation->getEnd();
-                    // add the current annotation to the list of annotations
-                    $this->annotations[] = $annotation;
-                }
+        if (null !== $this->annotations) {
+            return $this->annotations;
+        }
+
+        $this->annotations = [];
+        $total = \count($this->lines);
+
+        for ($index = 0; $index < $total; ++$index) {
+            if ($this->lines[$index]->containsATag()) {
+                // get all the lines that make up the annotation
+                $lines = \array_slice($this->lines, $index, $this->findAnnotationLength($index), true);
+                $annotation = new Annotation($lines);
+                // move the index to the end of the annotation to avoid
+                // checking it again because we know the lines inside the
+                // current annotation cannot be part of another annotation
+                $index = $annotation->getEnd();
+                // add the current annotation to the list of annotations
+                $this->annotations[] = $annotation;
             }
         }
 
         return $this->annotations;
     }
 
+    public function isMultiLine()
+    {
+        return 1 !== \count($this->lines);
+    }
+
+    /**
+     * Take a one line doc block, and turn it into a multi line doc block.
+     *
+     * @param string $indent
+     * @param string $lineEnd
+     */
+    public function makeMultiLine($indent, $lineEnd)
+    {
+        if ($this->isMultiLine()) {
+            return;
+        }
+
+        $lineContent = $this->getSingleLineDocBlockEntry($this->lines[0]);
+
+        if ('*' === $lineContent) {
+            $this->lines = [
+                new Line('/**'.$lineEnd),
+                new Line($indent.' *'.$lineEnd),
+                new Line($indent.' */'),
+            ];
+
+            return;
+        }
+
+        $this->lines = [
+            new Line('/**'.$lineEnd),
+            new Line($indent.' * '.$lineContent.$lineEnd),
+            new Line($indent.' */'),
+        ];
+    }
+
+    public function makeSingleLine()
+    {
+        if (!$this->isMultiLine()) {
+            return;
+        }
+
+        $usefulLines = array_filter(
+            $this->lines,
+            static function (Line $line) {
+                return $line->containsUsefulContent();
+            }
+        );
+
+        if (1 < \count($usefulLines)) {
+            return;
+        }
+
+        $lineContent = $this->getSingleLineDocBlockEntry(array_shift($usefulLines));
+
+        $this->lines = [new Line('/** '.$lineContent.' */')];
+    }
+
     /**
      * @param int $pos
      *
@@ -184,4 +244,33 @@ class DocBlock
 
         return $index - $start;
     }
+
+    /**
+     * @param string $lineString
+     *
+     * @return string
+     */
+    private function getSingleLineDocBlockEntry($lineString)
+    {
+        if (0 === \strlen($lineString)) {
+            return $lineString;
+        }
+
+        $lineString = str_replace('*/', '', $lineString);
+        $lineString = trim($lineString);
+        $lineArray = str_split($lineString);
+        $i = \count($lineArray);
+
+        do {
+            --$i;
+        } while ('*' !== $lineString[$i] && '*' !== $lineString[$i - 1] && '/' !== $lineString[$i - 2]);
+
+        if (' ' === $lineString[$i]) {
+            ++$i;
+        }
+
+        $lineArray = \array_slice($lineArray, $i);
+
+        return implode('', $lineArray);
+    }
 }

+ 156 - 0
src/Fixer/Phpdoc/PhpdocLineSpanFixer.php

@@ -0,0 +1,156 @@
+<?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\Phpdoc;
+
+use PhpCsFixer\AbstractFixer;
+use PhpCsFixer\DocBlock\DocBlock;
+use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
+use PhpCsFixer\Fixer\WhitespacesAwareFixerInterface;
+use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
+use PhpCsFixer\FixerConfiguration\FixerOptionBuilder;
+use PhpCsFixer\FixerDefinition\CodeSample;
+use PhpCsFixer\FixerDefinition\FixerDefinition;
+use PhpCsFixer\Tokenizer\Token;
+use PhpCsFixer\Tokenizer\Tokens;
+use PhpCsFixer\Tokenizer\TokensAnalyzer;
+
+/**
+ * @author Gert de Pagter <BackEndTea@gmail.com>
+ */
+final class PhpdocLineSpanFixer extends AbstractFixer implements WhitespacesAwareFixerInterface, ConfigurationDefinitionFixerInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function getDefinition()
+    {
+        return new FixerDefinition(
+            'Changes doc blocks from single to multi line, or reversed. Works for class constants, properties and methods only.',
+            [
+                new CodeSample("<?php\n\nclass Foo{\n    /** @var bool */\n    public \$var;\n}\n"),
+                new CodeSample(
+                    "<?php\n\nclass Foo{\n    /**\n    * @var bool\n    */\n    public \$var;\n}\n",
+                    ['property' => 'single']
+                ),
+            ]
+        );
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isCandidate(Tokens $tokens)
+    {
+        return $tokens->isTokenKindFound(T_DOC_COMMENT);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function createConfigurationDefinition()
+    {
+        return new FixerConfigurationResolver([
+            (new FixerOptionBuilder('const', 'Whether const blocks should be single or multi line'))
+                ->setAllowedValues(['single', 'multi'])
+                ->setDefault('multi')
+                ->getOption(),
+            (new FixerOptionBuilder('property', 'Whether property doc blocks should be single or multi line'))
+                ->setAllowedValues(['single', 'multi'])
+                ->setDefault('multi')
+                ->getOption(),
+            (new FixerOptionBuilder('method', 'Whether method doc blocks should be single or multi line'))
+                ->setAllowedValues(['single', 'multi'])
+                ->setDefault('multi')
+                ->getOption(),
+        ]);
+    }
+
+    protected function applyFix(\SplFileInfo $file, Tokens $tokens)
+    {
+        $analyzer = new TokensAnalyzer($tokens);
+
+        $elements = $analyzer->getClassyElements();
+
+        foreach ($elements as $index => $element) {
+            if (!$this->hasDocBlock($tokens, $index)) {
+                continue;
+            }
+
+            $type = $element['type'];
+            $docIndex = $this->getDocBlockIndex($tokens, $index);
+            $doc = new DocBlock($tokens[$docIndex]->getContent());
+
+            if ('multi' === $this->configuration[$type]) {
+                $doc->makeMultiLine($originalIndent = $this->detectIndent($tokens, $docIndex), $this->whitespacesConfig->getLineEnding());
+            } else {
+                $doc->makeSingleLine();
+            }
+
+            $tokens->offsetSet($docIndex, new Token([T_DOC_COMMENT, $doc->getContent()]));
+        }
+    }
+
+    /**
+     * @param Tokens $tokens
+     * @param int    $index
+     *
+     * @return bool
+     */
+    private function hasDocBlock(Tokens $tokens, $index)
+    {
+        $docBlockIndex = $this->getDocBlockIndex($tokens, $index);
+
+        return $tokens[$docBlockIndex]->isGivenKind(T_DOC_COMMENT);
+    }
+
+    /**
+     * @param Tokens $tokens
+     * @param int    $index
+     *
+     * @return int
+     */
+    private function getDocBlockIndex(Tokens $tokens, $index)
+    {
+        do {
+            $index = $tokens->getPrevNonWhitespace($index);
+        } while ($tokens[$index]->isGivenKind([
+            T_PUBLIC,
+            T_PROTECTED,
+            T_PRIVATE,
+            T_FINAL,
+            T_ABSTRACT,
+            T_COMMENT,
+            T_VAR,
+            T_STATIC,
+        ]));
+
+        return $index;
+    }
+
+    /**
+     * @param Tokens $tokens
+     * @param int    $index
+     *
+     * @return string
+     */
+    private function detectIndent(Tokens $tokens, $index)
+    {
+        if (!$tokens[$index - 1]->isWhitespace()) {
+            return ''; // cannot detect indent
+        }
+
+        $explodedContent = explode($this->whitespacesConfig->getLineEnding(), $tokens[$index - 1]->getContent());
+
+        return end($explodedContent);
+    }
+}

+ 1 - 0
tests/AutoReview/FixerFactoryTest.php

@@ -100,6 +100,7 @@ final class FixerFactoryTest extends TestCase
             [$fixers['general_phpdoc_annotation_remove'], $fixers['phpdoc_separation']],
             [$fixers['general_phpdoc_annotation_remove'], $fixers['phpdoc_trim']],
             [$fixers['general_phpdoc_annotation_remove'], $fixers['no_empty_phpdoc']],
+            [$fixers['general_phpdoc_annotation_remove'], $fixers['phpdoc_line_span']],
             [$fixers['indentation_type'], $fixers['phpdoc_indent']],
             [$fixers['implode_call'], $fixers['method_argument_space']],
             [$fixers['is_null'], $fixers['yoda_style']],

+ 103 - 0
tests/DocBlock/DocBlockTest.php

@@ -153,4 +153,107 @@ final class DocBlockTest extends TestCase
         static::assertInternalType('array', $annotations);
         static::assertCount(0, $annotations);
     }
+
+    public function testIsMultiLIne()
+    {
+        $doc = new DocBlock(self::$sample);
+
+        static::assertTrue($doc->isMultiLine());
+    }
+
+    /**
+     * @dataProvider provideDocBlocksToConvertToMultiLineCases
+     *
+     * @param string $inputDocBlock
+     * @param string $outputDocBlock
+     * @param mixed  $indent
+     * @param mixed  $newLine
+     */
+    public function testMakeMultiLIne($inputDocBlock, $outputDocBlock = null, $indent = '', $newLine = "\n")
+    {
+        $doc = new DocBlock($inputDocBlock);
+        $doc->makeMultiLine($indent, $newLine);
+
+        if (null === $outputDocBlock) {
+            $outputDocBlock = $inputDocBlock;
+        }
+
+        static::assertSame($outputDocBlock, $doc->getContent());
+    }
+
+    public function provideDocBlocksToConvertToMultiLineCases()
+    {
+        return [
+            'It keeps a multi line doc block as is' => [
+                "/**\n * Hello\n */",
+            ],
+            'It keeps a multi line doc block as is with multiple annotations' => [
+                "/**\n * @foo\n *@bar\n */",
+            ],
+            'It keeps a multi line doc block with indentation' => [
+                "/**\n\t *@foo\n\t */",
+            ],
+            'It Converts a single line to multi line with no indentation' => [
+                '/** Hello */',
+                "/**\n * Hello\n */",
+            ],
+            'It Converts a single line to multi line with correct indentation' => [
+                '/** Hello */',
+                "/**\n     * Hello\n     */",
+                '    ',
+            ],
+            'It Converts a single line to multi line with correct indentation and Line ending' => [
+                '/** Hello */',
+                "/**\r\n     * Hello\r\n     */",
+                '    ',
+                "\r\n",
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideDocBlocksToConvertToSingleLineCases
+     *
+     * @param string $inputDocBlock
+     * @param string $outputDocBlock
+     */
+    public function testMakeSingleLine($inputDocBlock, $outputDocBlock = null)
+    {
+        $doc = new DocBlock($inputDocBlock);
+        $doc->makeSingleLine();
+
+        if (null === $outputDocBlock) {
+            $outputDocBlock = $inputDocBlock;
+        }
+
+        static::assertSame($outputDocBlock, $doc->getContent());
+    }
+
+    public function provideDocBlocksToConvertToSingleLineCases()
+    {
+        return [
+            'It keeps a single line doc block as is' => [
+                '/** Hello */',
+            ],
+            'It converts a multi line doc block to a single line' => [
+                "/**\n * Hello\n */",
+                '/** Hello */',
+            ],
+            'It converts a multi line doc block to a single line with annotation' => [
+                "/**\n * @foo\n */",
+                '/** @foo */',
+            ],
+            'It converts a multi line doc block to a single line multiple empty lines' => [
+                "/**\n * @foo\n *\n *\n *\n * */",
+                '/** @foo */',
+            ],
+            'It changes an empty doc block to single line' => [
+                "/**\n *\n */",
+                '/**  */',
+            ],
+            'It does not change a multi line doc block if it can\'t' => [
+                self::$sample,
+            ],
+        ];
+    }
 }

+ 441 - 0
tests/Fixer/Phpdoc/PhpdocLineSpanFixerTest.php

@@ -0,0 +1,441 @@
+<?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\Tests\Fixer\Phpdoc;
+
+use PhpCsFixer\Tests\Test\AbstractFixerTestCase;
+
+/**
+ * @author Gert de Pagter <BackEndTea@gmail.com>
+ *
+ * @internal
+ *
+ * @covers \PhpCsFixer\Fixer\Phpdoc\PhpdocLineSpanFixer
+ */
+final class PhpdocLineSpanFixerTest extends AbstractFixerTestCase
+{
+    /**
+     * @param string      $expected
+     * @param null|string $input
+     * @param array       $config
+     *
+     * @dataProvider provideFixCases
+     */
+    public function testFix($expected, $input = null, array $config = [])
+    {
+        $this->fixer->configure($config);
+        $this->doTest($expected, $input);
+    }
+
+    /**
+     * @return array
+     */
+    public function provideFixCases()
+    {
+        return [
+            'It does not change doc blocks if not needed' => [
+                '<?php
+
+class Foo
+{
+    /**
+     * Important
+     */
+    const FOO_BAR = "foobar";
+
+    /**
+     * @var bool
+     */
+    public $variable = true;
+
+    /**
+     * @var bool
+     */
+    private $var = false;
+
+
+    /**
+     * @return void
+     */
+    public function hello() {}
+}
+',
+            ],
+            'It does change doc blocks to multi by default' => [
+                '<?php
+
+class Foo
+{
+    /**
+     * Important
+     */
+    const FOO_BAR = "foobar";
+
+    /**
+     * @var bool
+     */
+    public $variable = true;
+
+    /**
+     * @var bool
+     */
+    private $var = false;
+
+
+    /**
+     * @return void
+     */
+    public function hello() {}
+}
+',
+                '<?php
+
+class Foo
+{
+    /** Important */
+    const FOO_BAR = "foobar";
+
+    /** @var bool */
+    public $variable = true;
+
+    /** @var bool */
+    private $var = false;
+
+
+    /** @return void */
+    public function hello() {}
+}
+',
+            ],
+            'It does change doc blocks to single if configured to do so' => [
+                '<?php
+
+class Foo
+{
+    /** Important */
+    const FOO_BAR = "foobar";
+
+    /** @var bool */
+    public $variable = true;
+
+    /** @var bool */
+    private $var = false;
+
+
+    /** @return void */
+    public function hello() {}
+}
+',
+                '<?php
+
+class Foo
+{
+    /**
+     * Important
+     */
+    const FOO_BAR = "foobar";
+
+    /**
+     * @var bool
+     */
+    public $variable = true;
+
+    /**
+     * @var bool
+     */
+    private $var = false;
+
+
+    /**
+     * @return void
+     */
+    public function hello() {}
+}
+',
+                [
+                    'property' => 'single',
+                    'const' => 'single',
+                    'method' => 'single',
+                ],
+            ],
+            'It does not changes doc blocks from single if configured to do so' => [
+                '<?php
+
+class Foo
+{
+    /** Important */
+    const FOO_BAR = "foobar";
+
+    /** @var bool */
+    public $variable = true;
+
+    /** @var bool */
+    private $var = false;
+
+
+    /** @return void */
+    public function hello() {}
+}
+',
+                null,
+                [
+                    'property' => 'single',
+                    'const' => 'single',
+                    'method' => 'single',
+                ],
+            ],
+            'It can be configured to change certain elements to single line' => [
+                '<?php
+
+class Foo
+{
+    /**
+     * Important
+     */
+    const FOO_BAR = "foobar";
+
+    /** @var bool */
+    public $variable = true;
+
+    /** @var bool */
+    private $var = false;
+
+
+    /**
+     * @return void
+     */
+    public function hello() {}
+}
+',
+                '<?php
+
+class Foo
+{
+    /**
+     * Important
+     */
+    const FOO_BAR = "foobar";
+
+    /**
+     * @var bool
+     */
+    public $variable = true;
+
+    /**
+     * @var bool
+     */
+    private $var = false;
+
+
+    /**
+     * @return void
+     */
+    public function hello() {}
+}
+',
+                [
+                    'property' => 'single',
+                ],
+            ],
+            'It wont change a doc block to single line if it has multiple useful lines' => [
+                '<?php
+
+class Foo
+{
+    /**
+     * Important
+     * Really important
+     */
+    const FOO_BAR = "foobar";
+}
+',
+                null,
+                [
+                    'const' => 'single',
+                ],
+            ],
+            'It updates doc blocks correctly, even with more indentation' => [
+                '<?php
+
+if (false) {
+    class Foo
+    {
+        /** @var bool */
+        public $var = true;
+
+        /**
+         * @return void
+         */
+        public function hello () {}
+    }
+}
+',
+                '<?php
+
+if (false) {
+    class Foo
+    {
+        /**
+         * @var bool
+         */
+        public $var = true;
+
+        /** @return void */
+        public function hello () {}
+    }
+}
+',
+                [
+                    'property' => 'single',
+                ],
+            ],
+            'It can convert empty doc blocks' => [
+                '<?php
+
+class Foo
+{
+    /**
+     *
+     */
+    const FOO = "foobar";
+
+    /**  */
+    private $foo;
+}',
+                '<?php
+
+class Foo
+{
+    /**  */
+    const FOO = "foobar";
+
+    /**
+     *
+     */
+    private $foo;
+}',
+                [
+                    'property' => 'single',
+                ],
+            ],
+            'It can update doc blocks of static properties' => [
+                '<?php
+
+class Bar
+{
+    /**
+     * Important
+     */
+    public static $variable = "acme";
+}
+',
+                '<?php
+
+class Bar
+{
+    /** Important */
+    public static $variable = "acme";
+}
+',
+            ],
+            'It can update doc blocks of properties that use the var keyword instead of public' => [
+                '<?php
+
+class Bar
+{
+    /**
+     * Important
+     */
+    var $variable = "acme";
+}
+',
+                '<?php
+
+class Bar
+{
+    /** Important */
+    var $variable = "acme";
+}
+',
+            ],
+            'It can update doc blocks of static that do not declare visibility' => [
+                '<?php
+
+class Bar
+{
+    /**
+     * Important
+     */
+    static $variable = "acme";
+}
+',
+                '<?php
+
+class Bar
+{
+    /** Important */
+    static $variable = "acme";
+}
+',
+            ],
+        ];
+    }
+
+    /**
+     * @requires PHP 7.1
+     * @dataProvider provideFix71Cases
+     *
+     * @param string $expected
+     * @param string $input
+     * @param array  $config
+     */
+    public function testFix71($expected, $input = null, array $config = [])
+    {
+        $this->fixer->configure($config);
+        $this->doTest($expected, $input);
+    }
+
+    public function provideFix71Cases()
+    {
+        return [
+            'It can handle constants with visibility' => [
+                '<?php
+
+class Foo
+{
+    /**
+     *
+     */
+    public const FOO = "foobar";
+
+    /**  */
+    private $foo;
+}',
+                '<?php
+
+class Foo
+{
+    /**  */
+    public const FOO = "foobar";
+
+    /**
+     *
+     */
+    private $foo;
+}',
+                [
+                    'property' => 'single',
+                ],
+            ],
+        ];
+    }
+}

+ 29 - 0
tests/Fixtures/Integration/priority/general_phpdoc_annotation_remove,phpdoc_line_span.test

@@ -0,0 +1,29 @@
+--TEST--
+Integration of fixers: general_phpdoc_annotation_remove,phpdoc_line_span.
+--RULESET--
+{"general_phpdoc_annotation_remove": {"annotations": ["test"]}, "phpdoc_line_span": {"method": "single"}}
+--EXPECT--
+<?php
+class Foo
+{
+    /** @param string $name */
+    function hello($name)
+    {
+        return 'hello'. $name;
+    }
+}
+
+--INPUT--
+<?php
+class Foo
+{
+    /**
+     * @test
+     *
+     * @param string $name
+     */
+    function hello($name)
+    {
+        return 'hello'. $name;
+    }
+}