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

Do not warn about missing docblocks when a method has the #[\Override] attribute #160

Merged
merged 1 commit into from
May 29, 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]
### Changed
- Update composer dependencies to current versions, notably `PHP_CodeSniffer` (3.10.1) and `PHPCompatibility` (96072c30).
- The `moodle.Commenting.MissingDocblock` sniff will now detect use of the Override attribute (Fixes #155).

## [v3.4.6] - 2024-04-03
### Fixed
Expand Down
14 changes: 12 additions & 2 deletions moodle/Sniffs/Commenting/MissingDocblockSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

namespace MoodleHQ\MoodleCS\moodle\Sniffs\Commenting;

use MoodleHQ\MoodleCS\moodle\Util\Attributes;
use MoodleHQ\MoodleCS\moodle\Util\Docblocks;
use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
use MoodleHQ\MoodleCS\moodle\Util\TokenUtil;
Expand Down Expand Up @@ -183,6 +184,17 @@ protected function processFunctions(File $phpcsFile, int $stackPtr): void {

foreach ($missingDocblocks as $typePtr => $extendsOrImplements) {
$token = $tokens[$typePtr];
if ($extendsOrImplements) {
$attributes = Attributes::getAttributePointers($phpcsFile, $typePtr);
foreach ($attributes as $attributePtr) {
$attribute = Attributes::getAttributeProperties($phpcsFile, $attributePtr);
if ($attribute['attribute_name'] === '\Override') {
// Skip methods that are marked as overrides.
continue 2;
}
}
}

$objectName = TokenUtil::getObjectName($phpcsFile, $typePtr);
$objectType = TokenUtil::getObjectType($phpcsFile, $typePtr);

Expand All @@ -195,8 +207,6 @@ protected function processFunctions(File $phpcsFile, int $stackPtr): void {
[$objectType, $objectName]
);
}
} elseif ($extendsOrImplements) {
$phpcsFile->addWarning('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]);
} else {
$phpcsFile->addError('Missing docblock for %s %s', $typePtr, 'Missing', [$objectType, $objectName]);
}
Expand Down
23 changes: 18 additions & 5 deletions moodle/Tests/Sniffs/Commenting/MissingDocblockSniffTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\Commenting;

use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Files\DummyFile;
use PHP_CodeSniffer\Ruleset;

/**
* Test the MissingDocblockSniff sniff.
Expand Down Expand Up @@ -68,11 +65,11 @@ public static function docblockCorrectnessProvider(): array {
159 => 'Missing docblock for function test_method',
166 => 'Missing docblock for function test_method',
170 => 'Missing docblock for class example_extends',
171 => 'Missing docblock for function test_method',
175 => 'Missing docblock for class example_implements',
176 => 'Missing docblock for function test_method',
],
'warnings' => [
171 => 'Missing docblock for function test_method',
176 => 'Missing docblock for function test_method',
],
],
'File level tag, no class' => [
Expand Down Expand Up @@ -166,6 +163,22 @@ public static function docblockCorrectnessProvider(): array {
18 => 'Missing docblock for function this_is_a_dataprovider in testcase',
],
],
'With and without Overrides attributes' => [
'fixture' => 'with_and_without_overrides',
'fixtureFilename' => null,
'errors' => [
1 => 'Missing docblock for file with_and_without_overrides.php',
11 => 'Missing docblock for function has_override',
13 => 'Missing docblock for function no_override',
21 => 'Missing docblock for function has_override',
23 => 'Missing docblock for function no_override',
33 => 'Missing docblock for function no_override',
43 => 'Missing docblock for function no_override',
53 => 'Missing docblock for function no_override',
],
'warnings' => [
],
],
];

if (version_compare(PHP_VERSION, '8.0.0') >= 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\PHPUnit;


/**
* Class level docblock.
*/
class base_class {
#[\Override]
public function has_override(): void {}

public function no_override(): void {}
}

/**
* Base interface.
*/
interface base_interface {
#[\Override]
public function has_override(): void {}

public function no_override(): void {}
}

/**
* Class which extends another class.
*/
class child_class extends base_class {
#[\Override]
public function has_override(): void {}

public function no_override(): void {}
}

/**
* Interface which extends another interface.
*/
interface child_interface extends base_interface {
#[\Override]
public function has_override(): void {}

public function no_override(): void {}
}

/**
* Class which implements an interface.
*/
class child_class_implements_interface implements base_interface {
#[\Override]
public function has_override(): void {}

public function no_override(): void {}
}
187 changes: 187 additions & 0 deletions moodle/Tests/Util/AttributesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
<?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\Util;

use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;
use MoodleHQ\MoodleCS\moodle\Util\Attributes;
use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Files\DummyFile;
use PHP_CodeSniffer\Ruleset;

/**
* Test the Attributes specific moodle utilities class
*
* @copyright 2024 onwards Andrew Lyons <[email protected]>
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*
* @covers \MoodleHQ\MoodleCS\moodle\Util\Attributes
*/
class AttributesTest extends MoodleCSBaseTestCase
{
/**
* @dataProvider validTagsProvider
*/
public function testGetAttributePointers(
string $content,
$stackPtrSearch,
?array $expectations
): void {
$config = new Config([]);
$ruleset = new Ruleset($config);

$phpcsFile = new DummyFile($content, $ruleset, $config);
$phpcsFile->process();

$searchPtr = $phpcsFile->findNext($stackPtrSearch, 0);

$pointers = Attributes::getAttributePointers($phpcsFile, $searchPtr);
if (count($expectations)) {
foreach ($expectations as $expectation) {
$this->assertCount(
$expectation['count'],
array_filter($pointers, function ($pointer) use ($expectation, $phpcsFile) {
$properties = Attributes::getAttributeProperties($phpcsFile, $pointer);

return $properties['attribute_name'] === $expectation['name'];
})
);
}
} else {
$this->assertEmpty($pointers);
}
}

public static function validTagsProvider(): array {
return [
'No attributes' => [
'<?php
/**
* @param string $param
*/
function exampleFunction(string $param): void {}',
T_FUNCTION,
[],
],
'Attribute matches' => [
'<?php
/**
* @param string $param
*/
#[\Override]
function exampleFunction(string $param): void {}',
T_FUNCTION,
[
['name' => '\\Override', 'count' => 1],
],
],
'Multiple Attribute matches (same)' => [
'<?php
/**
* @param string $param
*/
#[\Override]
#[\Override]
function exampleFunction(string $param): void {}',
T_FUNCTION,
[
['name' => '\\Override', 'count' => 2],
],
],
'Multiple Attribute matches (different)' => [
'<?php
/**
* @param string $param
*/
#[\Override]
#[\Other]
#[\Override]
function exampleFunction(string $param): void {}',
T_FUNCTION,
[
['name' => '\\Override', 'count' => 2],
['name' => '\\Other', 'count' => 1],
],
],
'Attribute on other funciton' => [
'<?php
function otherFunction(): void {}
#[\Override]
#[\Override]
function exampleFunction(string $param): void {}',
T_FUNCTION,
[],
],
'Attributes with arguments' => [
'<?php
#[\Route("Example")]
#[\Override]
#[\Route]
#[\core\attribute\deprecated("thing")]
function exampleFunction(string $param): void {}',
T_FUNCTION,
[
['name' => '\\Override', 'count' => 1],
['name' => '\\Route', 'count' => 2],
['name' => '\\core\\attribute\\deprecated', 'count' => 1],
],
],
'Attribute start will only get that attribute' => [
'<?php
#[\Route("Example")]
#[\Override]
#[\Route]
#[\core\attribute\deprecated("thing")]
function exampleFunction(string $param): void {}',
T_ATTRIBUTE,
[
['name' => '\\Route', 'count' => 1],
],
],
'Attribute End will only get that attribute' => [
'<?php
#[\Route("Example")]
#[\Override]
#[\Route]
#[\core\attribute\deprecated("thing")]
function exampleFunction(string $param): void {}',
T_ATTRIBUTE_END,
[
['name' => '\\Route', 'count' => 1],
],
],
];
}

public function testGetAttributePropertiesNotAnAttribute(): void {
$config = new Config([]);
$ruleset = new Ruleset($config);

$content = <<<EOF
<?php
#[\Example()]
function foo() {}
EOF;

$phpcsFile = new DummyFile($content, $ruleset, $config);
$phpcsFile->process();

$searchPtr = $phpcsFile->findNext(T_FUNCTION, 0);

$this->assertNull(Attributes::getAttributeProperties($phpcsFile, $searchPtr));
}
}
Loading