Merge pull request #157 from andrewnicols/constructorShouldNotReturn

Add new sniff to detect and remove constructor @return docs

CHANGELOG.md

@@ -8,6 +8,7 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
 ## [Unreleased]
 ### Added
 - Add new `moodle.PHPUnit.ParentSetUpTearDown` sniff to verify, among other things, that all the `setUp()`, `tearDown()`, `setUpBeforeClass()` and `tearDownAfterClass()` methods in unit tests are properly calling to their parent counterparts. Applies to Moodle 4.5 and up.
+- Add new `moodle.Commenting.ConstructorReturn` sniff to check that constructors do not document a return value.
 
 ### Changed
 - Update composer dependencies to current versions, notably `PHP_CodeSniffer` (3.10.1) and `PHPCompatibility` (96072c30).

moodle/Sniffs/Commenting/ConstructorReturnSniff.php

@@ -0,0 +1,120 @@
+<?php
+
+// This file is part of Moodle - https://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANdTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <https://www.gnu.org/licenses/>.
+
+namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting;
+
+use MoodleHQ\MoodleCS\moodle\Util\Docblocks;
+use MoodleHQ\MoodleCS\moodle\Util\TokenUtil;
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Sniffs\Sniff;
+
+/**
+ * Checks that all files an classes have appropriate docs.
+ *
+ * @copyright  2024 Andrew Lyons <andrew@nicols.co.uk>
+ * @license    https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class ConstructorReturnSniff implements Sniff
+{
+    /**
+     * Register for class tags.
+     */
+    public function register() {
+
+        return [
+            T_CLASS,
+        ];
+    }
+
+    /**
+     * Processes php files and perform various checks with file.
+     *
+     * @param File $phpcsFile The file being scanned.
+     * @param int $stackPtr The position in the stack.
+     */
+    public function process(File $phpcsFile, $stackPtr) {
+        $tokens = $phpcsFile->getTokens();
+        $endClassPtr = $tokens[$stackPtr]['scope_closer'];
+
+        while (
+            ($methodPtr = $phpcsFile->findNext(T_FUNCTION, $stackPtr + 1, $endClassPtr)) !== false
+        ) {
+            $this->processClassMethod($phpcsFile, $methodPtr);
+            $stackPtr = $methodPtr;
+        }
+    }
+
+    /**
+     * Processes the class method.
+     *
+     * @param File $phpcsFile The file being scanned.
+     * @param int $stackPtr The position in the stack.
+     */
+    protected function processClassMethod(File $phpcsFile, int $stackPtr): void {
+        $objectName = TokenUtil::getObjectName($phpcsFile, $stackPtr);
+        if ($objectName !== '__constructor') {
+            // We only care about constructors.
+            return;
+        }
+
+        // Get docblock.
+        $docblockPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr);
+        if ($docblockPtr === null) {
+            // No docblocks for this constructor.
+            return;
+        }
+
+        $returnTokens = Docblocks::getMatchingDocTags($phpcsFile, $docblockPtr, '@return');
+        if (count($returnTokens) === 0) {
+            // No @return tag in the docblock.
+            return;
+        }
+
+        $fix = $phpcsFile->addFixableError(
+            'Constructor should not have a return tag in the docblock',
+            $returnTokens[0],
+            'ConstructorReturn'
+        );
+        if ($fix) {
+            $tokens = $phpcsFile->getTokens();
+            $phpcsFile->fixer->beginChangeset();
+
+            // Find the tokens at the start and end of the line.
+            $lineStart = $phpcsFile->findFirstOnLine(T_DOC_COMMENT_STAR, $returnTokens[0]);
+            if ($lineStart === false) {
+                $lineStart = $returnTokens[0];
+            }
+
+            $ptr = $phpcsFile->findNext(T_DOC_COMMENT_WHITESPACE, $lineStart);
+            for ($lineEnd = $lineStart; $lineEnd < $tokens[$docblockPtr]['comment_closer']; $lineEnd++) {
+                if ($tokens[$lineEnd]['line'] !== $tokens[$lineStart]['line']) {
+                    break;
+                }
+            }
+
+            if ($tokens[$lineEnd]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
+                $lineEnd--;
+            }
+
+            for ($ptr = $lineStart; $ptr <= $lineEnd; $ptr++) {
+                $phpcsFile->fixer->replaceToken($ptr, '');
+            }
+
+            $phpcsFile->fixer->endChangeset();
+        }
+    }
+}

moodle/Tests/Sniffs/Commenting/ConstructorReturnSniffTest.php

@@ -0,0 +1,64 @@
+<?php
+
+// This file is part of Moodle - https://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <https://www.gnu.org/licenses/>.
+
+namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting;
+
+use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;
+
+/**
+ * Test the ConstructorReturnSniff sniff.
+ *
+ * @copyright  2024 onwards Andrew Lyons <andrew@nicols.co.uk>
+ * @license    https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ *
+ * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\Commenting\ConstructorReturnSniff
+ */
+class ConstructorReturnSniffTest extends MoodleCSBaseTestCase
+{
+    /**
+     * @dataProvider docblockCorrectnessProvider
+     */
+    public function testMissingDocblockSniff(
+        string $fixture,
+        array $errors,
+        array $warnings
+    ): void {
+        $this->setStandard('moodle');
+        $this->setSniff('moodle.Commenting.ConstructorReturn');
+        $this->setFixture(sprintf("%s/fixtures/ConstructorReturn/%s.php", __DIR__, $fixture));
+        $this->setWarnings($warnings);
+        $this->setErrors($errors);
+
+        $this->verifyCsResults();
+    }
+
+    public static function docblockCorrectnessProvider(): array {
+        $cases = [
+            [
+                'fixture' => 'general',
+                'errors' => [
+                    43 => 'Constructor should not have a return tag in the docblock',
+                    52 => 'Constructor should not have a return tag in the docblock',
+                    60 => 'Constructor should not have a return tag in the docblock',
+                    71 => 'Constructor should not have a return tag in the docblock',
+                ],
+                'warnings' => [],
+            ],
+        ];
+        return $cases;
+    }
+}

moodle/Tests/Sniffs/Commenting/fixtures/ConstructorReturn/general.php

@@ -0,0 +1,76 @@
+<?php
+
+namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\PHPUnit;
+
+/**
+ * No docblocks on constructor.
+ */
+class NoDocblockOnConstructor {
+    public function __constructor() {
+        return;
+    }
+}
+
+class DocBlockOnNonConstructor {
+    public function __constructor() {
+        return;
+    }
+
+    /**
+     * @return void
+     */
+    public function non__constructor() {
+        return;
+    }
+}
+
+class DocBlockOnConstructorNoReturn {
+    /**
+     * Example constructor docblock
+     *
+     * @param string $example
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasReturn {
+    /**
+     * Example constructor docblock
+     *
+     * @param string $example Some value
+     * @return void
+     * @todo This is a todo
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasInlineReturn {
+    /** @return self */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasNearlyInlineReturn {
+    /**
+     * @return self */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasReturnNoExtraTag {
+    /**
+     * Example constructor docblock
+     *
+     * @param string $example Some value
+     * @return void
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}

moodle/Tests/Sniffs/Commenting/fixtures/ConstructorReturn/general.php.fixed

@@ -0,0 +1,74 @@
+<?php
+
+namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\PHPUnit;
+
+/**
+ * No docblocks on constructor.
+ */
+class NoDocblockOnConstructor {
+    public function __constructor() {
+        return;
+    }
+}
+
+class DocBlockOnNonConstructor {
+    public function __constructor() {
+        return;
+    }
+
+    /**
+     * @return void
+     */
+    public function non__constructor() {
+        return;
+    }
+}
+
+class DocBlockOnConstructorNoReturn {
+    /**
+     * Example constructor docblock
+     *
+     * @param string $example
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasReturn {
+    /**
+     * Example constructor docblock
+     *
+     * @param string $example Some value
+     * @todo This is a todo
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasInlineReturn {
+    /** */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasNearlyInlineReturn {
+    /**
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}
+
+class DocBlockOnConstructorHasReturnNoExtraTag {
+    /**
+     * Example constructor docblock
+     *
+     * @param string $example Some value
+     */
+    public function __constructor(string $example) {
+        return;
+    }
+}