feature #887 Added More Phpdoc Fixers (GrahamCampbell, keradus)

This PR was merged into the 1.5 branch.

Discussion
----------

Added More Phpdoc Fixers

TODO:
* [x] Added phpdoc_seperation
* [x] Fix the behaviour with some annotations like the phpunit ones - introduced a tag class
* [x] Refactor the docblock classes to model the situation more clearly
* [x] Add tests for the `Tag` class
* [x] Add tests for the `DocBlock` class
* [x] Added phpdoc_order
* [x] Perform regression testing on league/flysystem
* [x] Based on results from testing, make improvements
* [x] Add phpdoc_var fixer
* [x] Perform regression testing on symfony/symfony
* [x] Based on results from testing, make improvements, add more test cases
* [x] Move the phpdoc_order fixer to contrib level
* [x] Add a phpdoc_trim fixer at symfony level, partly taken from phpdoc_separation
* [x] Added phpdoc_no_return_void
* [x] Review fixer orders
* [x] Perform regression testing on guzzle/guzzle
* [x] `@see`, `@link`, `@deprecated`, `@since`, `@author`, `@copyright`, and `@license` annotations should be kept together rather than forced apart by the separation fixer
* [x] Support annotations with descriptions including bank lines
* [x] Perform regression testing on doctrine/common
* [x] Perform regression testing on guzzle/guzzle and symfony/symfony again
* [x] Fix more bugs, and add more test cases
* [x] Perform regression testing on flysystem guzzle, doctrine, symfony again
* [x] Analyse current code coverage and cleanup code
* [x] Add tests for the `TagComparator`
* [x] Add missing `DocBlock` class tests
* [x] Add tests for the `Line` class
* [x] Add tests for the `Annotation` class
* [x] Test on dingo/api, factory-muffin, aws sdk v2 and v3
* [x] Discuss fixer names
* [x] Add fixers to change var to type and type to var
* [x] Add a fixer to ensure short descriptions end in a `.`, `?`, or `!`
* [x] Review by @keradus
* [x] Cleanup the docblock classes
* [x] Add a fixer to remove package and subpackage annotations
* [x] Complete reviews and testing

I THINK WE'RE DONE! :)

Commits
-------

0006059 Enhance tests of order of phpdoc fixers
9f3aab8 Added more phpdoc fixers

README.rst

@@ -327,15 +327,50 @@ Choose from the list of available fixers:
                 Docblocks should have the same
                 indentation as the documented subject.
 
+* **phpdoc_no_empty_return** [symfony]
+                @return void and @return null
+                annotations should be omitted from
+                phpdocs.
+
+* **phpdoc_no_package** [symfony]
+                @package and @subpackage annotations
+                should be omitted from phpdocs.
+
 * **phpdoc_params** [symfony]
                 All items of the @param, @throws,
                 @return, @var, and @type phpdoc tags
                 must be aligned vertically.
 
+* **phpdoc_separation** [symfony]
+                Annotations in phpdocs should be
+                grouped together so that annotations
+                of the same type immediately follow
+                each other, and annotations of a
+                different type are separated by a
+                single blank line.
+
+* **phpdoc_short_description** [symfony]
+                Phpdocs short descriptions should end
+                in either a full stop, exclamation
+                mark, or question mark.
+
 * **phpdoc_to_comment** [symfony]
                 Docblocks should only be used on
                 structural elements.
 
+* **phpdoc_trim** [symfony]
+                Phpdocs should start and end with
+                content, excluding the very fist and
+                last line of the docblocks.
+
+* **phpdoc_type_to_var** [symfony]
+                @type should always be written as
+                @var.
+
+* **phpdoc_var_without_name** [symfony]
+                @var and @type annotations should not
+                contain the variable name.
+
 * **remove_leading_slash_use** [symfony]
                 Remove leading slashes in use clauses.
 
@@ -414,6 +449,16 @@ Choose from the list of available fixers:
                 __construct. Warning! This could
                 change code behavior.
 
+* **phpdoc_order** [contrib]
+                Annotations in phpdocs should be
+                ordered so that param annotations come
+                first, then throws annotations, then
+                return annotations.
+
+* **phpdoc_var_to_type** [contrib]
+                @var should always be written as
+                @type.
+
 * **short_array_syntax** [contrib]
                 PHP array's should use the PHP 5.4
                 short-syntax.

Symfony/CS/DocBlock/Annotation.php

@@ -0,0 +1,115 @@
+<?php
+
+/*
+ * This file is part of the PHP CS utility.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\CS\DocBlock;
+
+/**
+ * This represents an entire annotation from a docblock.
+ *
+ * @author Graham Campbell <graham@mineuk.com>
+ */
+class Annotation
+{
+    /**
+     * The lines that make up the annotation.
+     *
+     * Note that the array indexes represent the position in the docblock.
+     *
+     * @var Lines[]
+     */
+    private $lines;
+
+    /**
+     * The associated tag.
+     *
+     * @var Tag|null
+     */
+    private $tag;
+
+    /**
+     * Create a new line instance.
+     *
+     * @param Lines[] $lines
+     */
+    public function __construct(array $lines)
+    {
+        $this->lines = $lines;
+    }
+
+    /**
+     * Get the start position of this annotation.
+     *
+     * @return int
+     */
+    public function getStart()
+    {
+        $keys = array_keys($this->lines);
+
+        return $keys[0];
+    }
+
+    /**
+     * Get the end position of this annotation.
+     *
+     * @return int
+     */
+    public function getEnd()
+    {
+        $keys = array_keys($this->lines);
+
+        return end($keys);
+    }
+
+    /**
+     * Get the associated tag.
+     *
+     * @return Tag
+     */
+    public function getTag()
+    {
+        if (null === $this->tag) {
+            $values = array_values($this->lines);
+            $this->tag = new Tag($values[0]->getContent());
+        }
+
+        return $this->tag;
+    }
+
+    /**
+     * Remove this annotation by removing all its lines.
+     */
+    public function remove()
+    {
+        foreach ($this->lines as $line) {
+            $line->remove();
+        }
+    }
+
+    /**
+     * Get the annotation content.
+     *
+     * @return string
+     */
+    public function getContent()
+    {
+        return implode($this->lines);
+    }
+
+    /**
+     * Get the string representation of object.
+     *
+     * @return string
+     */
+    public function __toString()
+    {
+        return $this->getContent();
+    }
+}

Symfony/CS/DocBlock/DocBlock.php

@@ -0,0 +1,188 @@
+<?php
+
+/*
+ * This file is part of the PHP CS utility.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\CS\DocBlock;
+
+use Symfony\CS\Utils;
+
+/**
+ * This class represents a docblock.
+ *
+ * It internally splits it up into "lines" that we can manipulate.
+ *
+ * @author Graham Campbell <graham@mineuk.com>
+ */
+class DocBlock
+{
+    /**
+     * The array of lines.
+     *
+     * @var Line[]
+     */
+    private $lines = array();
+
+    /**
+     * The array of annotations.
+     *
+     * @var Annotation[]|null
+     */
+    private $annotations;
+
+    /**
+     * Create a new docblock instance.
+     *
+     * @param string $content
+     */
+    public function __construct($content)
+    {
+        foreach (Utils::splitLines($content) as $line) {
+            $this->lines[] = new Line($line);
+        }
+    }
+
+    /**
+     * Get this docblock's lines.
+     *
+     * @return Line[]
+     */
+    public function getLines()
+    {
+        return $this->lines;
+    }
+
+    /**
+     * Get a single line.
+     *
+     * @param int $pos
+     *
+     * @return Line|null
+     */
+    public function getLine($pos)
+    {
+        if (isset($this->lines[$pos])) {
+            return $this->lines[$pos];
+        }
+    }
+
+    /**
+     * Get this docblock's annotations.
+     *
+     * @return Annotation[]
+     */
+    public function getAnnotations()
+    {
+        if (null === $this->annotations) {
+            $this->annotations = array();
+            $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;
+    }
+
+    private function findAnnotationLength($start)
+    {
+        $index = $start;
+
+        while ($line = $this->getLine(++$index)) {
+            if ($line->containsATag()) {
+                // we've 100% reached the end of the description if we get here
+                break;
+            }
+
+            if (!$line->containsUsefulContent()) {
+                // if we next line is also non-useful, or contains a tag, then we're done here
+                $next = $this->getLine($index + 1);
+                if (null === $next || !$next->containsUsefulContent() || $next->containsATag()) {
+                    break;
+                }
+                // otherwise, continue, the annotation must have contained a blank line in its description
+            }
+        }
+
+        return $index - $start;
+    }
+
+    /**
+     * Get a single annotation.
+     *
+     * @param int $pos
+     *
+     * @return Annotation|null
+     */
+    public function getAnnotation($pos)
+    {
+        $annotations = $this->getAnnotations();
+
+        if (isset($annotations[$pos])) {
+            return $annotations[$pos];
+        }
+    }
+
+    /**
+     * Get specific types of annotations only.
+     *
+     * If none exist, we're returning an empty array.
+     *
+     * @param string|string[] $types
+     *
+     * @return Annotations[]
+     */
+    public function getAnnotationsOfType($types)
+    {
+        $annotations = array();
+        $types = (array) $types;
+
+        foreach ($this->getAnnotations() as $annotation) {
+            $tag = $annotation->getTag()->getName();
+            foreach ($types as $type) {
+                if ($type === $tag) {
+                    $annotations[] = $annotation;
+                }
+            }
+        }
+
+        return $annotations;
+    }
+
+    /**
+     * Get the actual content of this docblock.
+     *
+     * @return string
+     */
+    public function getContent()
+    {
+        return implode($this->lines);
+    }
+
+    /**
+     * Get the string representation of object.
+     *
+     * @return string
+     */
+    public function __toString()
+    {
+        return $this->getContent();
+    }
+}

Symfony/CS/DocBlock/Line.php

@@ -0,0 +1,137 @@
+<?php
+
+/*
+ * This file is part of the PHP CS utility.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\CS\DocBlock;
+
+/**
+ * This represents a line of a docblock.
+ *
+ * @author Graham Campbell <graham@mineuk.com>
+ */
+class Line
+{
+    /**
+     * The content of this line.
+     *
+     * @var string
+     */
+    private $content;
+
+    /**
+     * Create a new line instance.
+     *
+     * @param string $content
+     */
+    public function __construct($content)
+    {
+        $this->content = $content;
+    }
+
+    /**
+     * Get the content of this line.
+     *
+     * @return int
+     */
+    public function getContent()
+    {
+        return $this->content;
+    }
+
+    /**
+     * Does this line contain useful content?
+     *
+     * If the line contains text or tags, then this is true.
+     *
+     * @return bool
+     */
+    public function containsUsefulContent()
+    {
+        return 0 !== preg_match('/\\*\s*\S+/', $this->content) && !$this->isTheStart() && !$this->isTheEnd();
+    }
+
+    /**
+     * Does the line contain a tag?
+     *
+     * If this is true, then it must be the first line of an annotation.
+     *
+     * @return bool
+     */
+    public function containsATag()
+    {
+        return 0 !== preg_match('/\\*\s*@/', $this->content);
+    }
+
+    /**
+     * Is the line the start of a docblock?
+     *
+     * @return bool
+     */
+    public function isTheStart()
+    {
+        return false !== strpos($this->content, '/**');
+    }
+
+    /**
+     * Is the line the end of a docblock?
+     *
+     * @return bool
+     */
+    public function isTheEnd()
+    {
+        return false !== strpos($this->content, '*/');
+    }
+
+    /**
+     * Set the content of this line.
+     *
+     * @param string $content
+     */
+    public function setContent($content)
+    {
+        $this->content = $content;
+    }
+
+    /**
+     * Remove this line by clearing its contents.
+     *
+     * Note that this method technically brakes the internal state of the
+     * docblock, but is useful when we need to retain the indexes of lines
+     * during the execution of an algorithm.
+     */
+    public function remove()
+    {
+        $this->content = '';
+    }
+
+    /**
+     * Append a blank docblock line to this line's contents.
+     *
+     * Note that this method technically brakes the internal state of the
+     * docblock, but is useful when we need to retain the indexes of lines
+     * during the execution of an algorithm.
+     */
+    public function addBlank()
+    {
+        preg_match_all('/\ *\*/', $this->content, $matches);
+
+        $this->content .= $matches[0][0]."\n";
+    }
+
+    /**
+     * Get the string representation of object.
+     *
+     * @return string
+     */
+    public function __toString()
+    {
+        return $this->content;
+    }
+}

Symfony/CS/DocBlock/Tag.php

@@ -0,0 +1,79 @@
+<?php
+
+/*
+ * This file is part of the PHP CS utility.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\CS\DocBlock;
+
+/**
+ * This represents a tag, as defined by the proposed PSR PHPDoc standard.
+ *
+ * @author Graham Campbell <graham@mineuk.com>
+ */
+class Tag
+{
+    /**
+     * All the tags defined by the proposed PSR PHPDoc standard.
+     *
+     * @var string[]
+     */
+    private static $tags = array(
+        'api', 'author', 'category', 'copyright', 'deprecated', 'example',
+        'global', 'internal', 'license', 'link', 'method', 'package', 'param',
+        'property', 'return', 'see', 'since', 'struct', 'subpackage', 'throws',
+        'todo', 'typedef', 'uses', 'var', 'version',
+    );
+
+    /**
+     * The tag name.
+     *
+     * @var string
+     */
+    private $name;
+
+    /**
+     * Create a new tag instance.
+     *
+     * @param string $content
+     */
+    public function __construct($content)
+    {
+        preg_match_all('/@[a-zA-Z0-9_]+/', $content, $matches);
+
+        if (isset($matches[0][0])) {
+            $this->name = strtolower(ltrim($matches[0][0], '@'));
+        } else {
+            $this->name = 'other';
+        }
+    }
+
+    /**
+     * Get the tag name.
+     *
+     * This may be "param", or "return", etc.
+     *
+     * @return string
+     */
+    public function getName()
+    {
+        return $this->name;
+    }
+
+    /**
+     * Is the tag a known tag.
+     *
+     * This is defined by if it exists in the proposed PSR PHPDoc standard.
+     *
+     * @return bool
+     */
+    public function valid()
+    {
+        return in_array($this->name, self::$tags, true);
+    }
+}

Symfony/CS/DocBlock/TagComparator.php

@@ -0,0 +1,58 @@
+<?php
+
+/*
+ * This file is part of the PHP CS utility.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\CS\DocBlock;
+
+/**
+ * This class is responsible for comparing tags to see if they should be kept
+ * together, or kept apart.
+ *
+ * @author Graham Campbell <graham@mineuk.com>
+ */
+class TagComparator
+{
+    /**
+     * Groups of tags that should be allowed to immediately follow each other.
+     *
+     * @var array
+     */
+    private static $groups = array(
+        array('deprecated', 'link', 'see', 'since'),
+        array('author', 'copyright', 'license'),
+        array('package', 'subpackage'),
+    );
+
+    /**
+     * Should the given tags be kept together, or kept apart?
+     *
+     * @param Tag $first
+     * @param Tag $second
+     *
+     * @return bool
+     */
+    public static function shouldBeTogether(Tag $first, Tag $second)
+    {
+        $firstName = $first->getName();
+        $secondName = $second->getName();
+
+        if ($firstName === $secondName) {
+            return true;
+        }
+
+        foreach (self::$groups as $group) {
+            if (in_array($firstName, $group, true) && in_array($secondName, $group, true)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

Symfony/CS/Fixer/Contrib/EregToPregFixer.php

@@ -113,7 +113,7 @@ class EregToPregFixer extends AbstractFixer
     }
 
     /**
-     * Check the validity of a PCRE
+     * Check the validity of a PCRE.
      *
      * @param string $pattern the regular expression
      *

Symfony/CS/Fixer/Contrib/HeaderCommentFixer.php

@@ -24,7 +24,7 @@ class HeaderCommentFixer extends AbstractFixer
     private static $headerComment = '';
 
     /**
-     * Sets the desired header text
+     * Sets the desired header text.
      *
      * The given text will be trimmed and enclosed into a multiline comment.
      * If the text is empty, when a file get fixed, the header comment will be
@@ -79,9 +79,10 @@ class HeaderCommentFixer extends AbstractFixer
     }
 
     /**
-     * Encloses the given text in a comment block
+     * Encloses the given text in a comment block.
+     *
+     * @param string $header
      *
-     * @param  string $header
      * @return string
      */
     private static function encloseTextInComment($header)
@@ -97,7 +98,7 @@ class HeaderCommentFixer extends AbstractFixer
     }
 
     /**
-     * Removes the header comment, if any
+     * Removes the header comment, if any.
      *
      * @param Tokens $tokens
      */
@@ -110,9 +111,10 @@ class HeaderCommentFixer extends AbstractFixer
     }
 
     /**
-     * Finds the index where the header comment must be inserted
+     * Finds the index where the header comment must be inserted.
+     *
+     * @param Tokens $tokens
      *
-     * @param  Tokens $tokens
      * @return int
      */
     private function findHeaderCommentInsertionIndex(Tokens $tokens)
@@ -128,7 +130,7 @@ class HeaderCommentFixer extends AbstractFixer
     }
 
     /**
-     * Inserts the header comment at the given index
+     * Inserts the header comment at the given index.
      *
      * @param Tokens $tokens
      * @param int    $index

Symfony/CS/Fixer/Contrib/Php4ConstructorFixer.php

@@ -262,15 +262,14 @@ class Php4ConstructorFixer extends AbstractFixer
      * @param int    $startIndex the search start index
      * @param int    $endIndex   the search end index
      *
-     * @return array|null {
-     *                    The function/method data, if a match is found.
+     * @return array|null An associative array, if a match is found:
      *
-     *      @var int   $nameIndex  the index of the function/method name
-     *      @var int   $startIndex the index of the function/method start
-     *      @var int   $endIndex   the index of the function/method end
-     *      @var int   $bodyIndex  the index of the function/method body
-     *      @var array $modifiers  the modifiers as array keys and their index as value, e.g. array(T_PUBLIC => 10)
-     * }
+     *     - nameIndex (int): The index of the function/method name.
+     *     - startIndex (int): The index of the function/method start.
+     *     - endIndex (int): The index of the function/method end.
+     *     - bodyIndex (int): The index of the function/method body.
+     *     - modifiers (array): The modifiers as array keys and their index as
+     *       the values, e.g. array(T_PUBLIC => 10).
      */
     private function findFunction(Tokens $tokens, $name, $startIndex, $endIndex)
     {

Symfony/CS/Fixer/Contrib/PhpdocOrderFixer.php

@@ -0,0 +1,146 @@
+<?php
+
+/*
+ * This file is part of the PHP CS utility.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Symfony\CS\Fixer\Contrib;
+
+use Symfony\CS\AbstractFixer;
+use Symfony\CS\DocBlock\DocBlock;
+use Symfony\CS\Tokenizer\Tokens;
+
+/**
+ * @author Graham Campbell <graham@mineuk.com>
+ */
+class PhpdocOrderFixer extends AbstractFixer
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function fix(\SplFileInfo $file, $content)
+    {
+        $tokens = Tokens::fromCode($content);
+
+        foreach ($tokens->findGivenKind(T_DOC_COMMENT) as $token) {
+            $content = $token->getContent();
+            // move param to start, return to end, leave throws in the middle
+            $content = $this->moveParamAnnotations($content);
+            // we're parsing the content again to make sure the internal
+            // state of the dockblock is correct after the modifications
+            $content = $this->moveReturnAnnotations($content);
+            // persist the content at the end
+            $token->setContent($content);
+        }
+
+        return $tokens->generateCode();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getDescription()
+    {
+        return 'Annotations in phpdocs should be ordered so that param annotations come first, then throws annotations, then return annotations.';
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getPriority()
+    {
+        // must be run before the PhpdocSeperationFixer
+
+        /*
+         * Should be run before the php_doc_seperation fixer so that if we
+         * create incorrect annotation grouping while moving the annotations
+         * about, we're still ok.
+         */
+        return 5;
+    }
+
+    /**
+     * Move all param annotations in before throws and return annotations.
+     *
+     * @param string $content
+     *
+     * @return string
+     */
+    private function moveParamAnnotations($content)
+    {
+        $doc = new DocBlock($content);
+        $params = $doc->getAnnotationsOfType('param');
+
+        // nothing to do if there are no param annotations
+        if (empty($params)) {
+            return $content;
+        }
+
+        $others = $doc->getAnnotationsOfType(array('throws', 'return'));
+
+        if (empty($others)) {
+            return $content;
+        }
+
+        // get the index of the final line of the final param annotation
+        $end = end($params)->getEnd();
+
+        $line = $doc->getLine($end);
+
+        // move stuff about if required
+        foreach ($others as $other) {
+            if ($other->getStart() < $end) {
+                // we're doing this to maintain the original line indexes
+                $line->setContent($line->getContent().$other->getContent());
+                $other->remove();
+            }
+        }
+
+        return $doc->getContent();
+    }
+
+    /**
+     * Move all return annotations after param and throws annotations.
+     *
+     * @param string $content
+     *
+     * @return string
+     */
+    private function moveReturnAnnotations($content)
+    {
+        $doc = new DocBlock($content);
+        $returns = $doc->getAnnotationsOfType('return');
+
+        // nothing to do if there are no return annotations
+        if (empty($returns)) {
+            return $content;
+        }
+
+        $others = $doc->getAnnotationsOfType(array('param', 'throws'));
+
+        // nothing to do if there are no other annotations
+        if (empty($others)) {
+            return $content;
+        }
+
+        // get the index of the first line of the first return annotation
+        $start = $returns[0]->getStart();
+        $line = $doc->getLine($start);
+
+        // move stuff about if required
+        foreach (array_reverse($others) as $other) {
+            if ($other->getEnd() > $start) {
+                // we're doing this to maintain the original line indexes
+                $line->setContent($other->getContent().$line->getContent());
+                $other->remove();
+            }
+        }
+
+        return $doc->getContent();
+    }
+}