Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new sniff to detect and remove constructor @return docs #157

Merged
merged 1 commit into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
120 changes: 120 additions & 0 deletions moodle/Sniffs/Commenting/ConstructorReturnSniff.php
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>
* @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();
}
}
}
64 changes: 64 additions & 0 deletions moodle/Tests/Sniffs/Commenting/ConstructorReturnSniffTest.php
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>
* @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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}