Skip to content

Commit

Permalink
Add new sniff to detect and remove constructor @return docs (#157)
Browse files Browse the repository at this point in the history
  • Loading branch information
andrewnicols committed Apr 17, 2024
1 parent 1ec84f4 commit a917e01
Show file tree
Hide file tree
Showing 5 changed files with 345 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
The format of this change log follows the advice given at [Keep a CHANGELOG](https://keepachangelog.com).

## [Unreleased]
### Added
- Add new `moodle.Commenting.ConstructorReturn` sniff to check that constructors do not document a return value.

## [v3.4.6] - 2024-04-03
### Fixed
Expand Down
129 changes: 129 additions & 0 deletions moodle/Sniffs/Commenting/ConstructorReturnSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
<?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() {

Check warning on line 37 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L37

Added line #L37 was not covered by tests

return [
T_CLASS,
];

Check warning on line 41 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L39-L41

Added lines #L39 - L41 were not covered by tests
}

/**
* 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'];

Check warning on line 52 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L50-L52

Added lines #L50 - L52 were not covered by tests

while (
($methodPtr = $phpcsFile->findNext(T_FUNCTION, $stackPtr + 1, $endClassPtr)) !== false

Check warning on line 55 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L55

Added line #L55 was not covered by tests
) {
$this->processClassMethod($phpcsFile, $methodPtr);
$stackPtr = $methodPtr;

Check warning on line 58 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L57-L58

Added lines #L57 - L58 were not covered by tests
}
}

/**
* 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') {

Check warning on line 70 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L68-L70

Added lines #L68 - L70 were not covered by tests
// We only care about constructors.
return;

Check warning on line 72 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L72

Added line #L72 was not covered by tests
}

// Get docblock.
$docblockPtr = Docblocks::getDocBlockPointer($phpcsFile, $stackPtr);
if ($docblockPtr === null) {

Check warning on line 77 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L76-L77

Added lines #L76 - L77 were not covered by tests
// No docblocks for this constructor.
return;

Check warning on line 79 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L79

Added line #L79 was not covered by tests
}

$returnTokens = Docblocks::getMatchingDocTags($phpcsFile, $docblockPtr, '@return');
if (count($returnTokens) === 0) {

Check warning on line 83 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L82-L83

Added lines #L82 - L83 were not covered by tests
// No @return tag in the docblock.
return;

Check warning on line 85 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L85

Added line #L85 was not covered by tests
}

$fix = $phpcsFile->addFixableError(
'Constructor should not have a return tag in the docblock',
$returnTokens[0],
'ConstructorReturn'

Check warning on line 91 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L88-L91

Added lines #L88 - L91 were not covered by tests
);
if ($fix) {
$tokens = $phpcsFile->getTokens();
$phpcsFile->fixer->beginChangeset();

Check warning on line 95 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L93-L95

Added lines #L93 - L95 were not covered by tests

// 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];

Check warning on line 100 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L98-L100

Added lines #L98 - L100 were not covered by tests
}

$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;

Check warning on line 106 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L103-L106

Added lines #L103 - L106 were not covered by tests
}
}

if ($tokens[$lineEnd]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
$lineEnd--;

Check warning on line 111 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L110-L111

Added lines #L110 - L111 were not covered by tests
}

// if ($tokens[$lineStart]['code'] === T_DOC_COMMENT_OPEN_TAG) {
// if ($tokens[$lineEnd]['code'] !== T_DOC_COMMENT_CLOSE_TAG) {
// $lineStart = $phpcsFile->findNext(T_DOC_COMMENT_WHITESPACE, $returnTokens[0]);
// }
// } elseif ($tokens[$lineEnd]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
// $lineEnd--;
// }

for ($ptr = $lineStart; $ptr <= $lineEnd; $ptr++) {
$phpcsFile->fixer->replaceToken($ptr, '');

Check warning on line 123 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L122-L123

Added lines #L122 - L123 were not covered by tests
}

$phpcsFile->fixer->endChangeset();

Check warning on line 126 in moodle/Sniffs/Commenting/ConstructorReturnSniff.php

View check run for this annotation

Codecov / codecov/patch

moodle/Sniffs/Commenting/ConstructorReturnSniff.php#L126

Added line #L126 was not covered by tests
}
}
}
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\MissingDocblockSniff
*/
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;
}
}

0 comments on commit a917e01

Please sign in to comment.