Feature suggestion: AbstractDocblockSniff

[jrfnl]

Currently, the PEAR and the Squiz standard both contain a number of docblock related sniffs which are not configurable and are opinionated on what tags should be present in the various docblocks. Additionally, the various sniffs contain duplicate code.

External standards can, of course, create their own sniffs for these type of checks, however, that would involve yet more code duplication.

@gsherwood Would there be interest in an AbstractDocblockSniff ?

I imagine that sniff could roughly look like:

abstract class AbstractDocblockSniff implements Sniff
{
    /**
     * Format similar to the format used in the PEAR.Commenting.FileComment sniff.
     *
     * I.e.:
     * `tagname` => array(
     *     'tag_required'     => true/false,
     *     'allow_multiple'   => true/false,
     *     'content_required' => true/false,
     * )
     */
    public $tags = array();

    /**
     * Whether the tag order is important and should be checked.
     *
     * @var bool
     */
    public $checkTagOrder = false;

    abstract public function register();

    public function processToken()
    {
        $docblockOpener = $this->findTargetDocblock();
        if ($docblockOpener === false) {
            $this->MissingDocBlock();
            return;
        }

        $this->examineDocBlockFormat();
        $this->examineTags();
    }

    /**
     * Find the target docblock based on whatever token(s) was registered in the register() method.
     */
    abstract public function findTargetDocblock();

    public function missingDocBlock()
    {
        // Throw error for missing docblock.
    }

    public function examineDocblockFormat()
    {
        // Check for docblock vs comment format.
        // Superfluous empty comment lines etc.
    }

    public function examineTags()
    {
        // Check based on tag_required status
        // Check based on allow_multiple status
        // Check based on content_required status

        // Allow for additional tag specific checks in child sniffs.
        if (method_exists($this, 'examine'.$tagName)) {
            call_user_func(array($this, 'examine'.$tagName), $args);
        }

        // Maybe check tag order.
    }

    // Possibly some examineTagName() methods for common tags.
    // Those can, of course, be overloaded by child sniffs with either a completely different
    // check of with a call to the parent in combination with additional checks.

    // Can be used by child sniffs from the tag specific check methods.
    public function checkPunctuation( $string )
    {
        // Check for capital at start of phrase.
        // Check for valid end of sentence character.
    }
}

This would allow some simplification of the code used in the various current PHPCS sniffs dealing with docblocks and allow for far greater flexibility for external standards which want to check docblocks against their own rules for required/multiple etc.

[jrfnl]

@gsherwood Just checking: had a chance to think about this yet ?

[gsherwood]

I like the concept, but I haven't had time to take a look at the details or think about how it might work. It's probably going to take me a while to get to.

[jrfnl]

@gsherwood If you're open to it, I'd be happy to have a first go at this and adjust the existing comment sniffs (where relevant) to use the abstract. That should help figure out any kinks in my proposal above and if I'd run into situations which need discussion I could post those here ?

[gsherwood]

Go for it :)

[jrfnl]

Question: should the abstract be allowed to throw (generic) errors or should all error throwing be done in the concrete classes ?

[jrfnl]

FYI: https://github.com/squizlabs/PHP_CodeSniffer/issues/2189#issuecomment-448011305

Just FYI: I've started work on this as, as soon as I started work on the AbstractDocblockSniff - #2222 -, I realized some things would be easier if some of the new utility functions would already be available.

I hope to slowly but surely get a complete WIP branch ready by the time 3.4.0/3.4.1 is released, so I can start pulling the individual bits from it as soon as 3.5.0 opens for commits.

Once I've cleaned up what I've done so far, I'll put a WIP branch online somewhere so anyone interested can have a look at the direction this is going in.

[jrfnl]

Closing as replaced by PHPCSStandards/PHPCSUtils#517