Skip to content

Commit

Permalink
Merge pull request moodlehq#165 from stronk7/setup_teardown_parent
Browse files Browse the repository at this point in the history 
New sniff to detect correct setUp/tearDown parent calls
  • Loading branch information
@andrewnicols
andrewnicols authored May 31, 2024
2 parents 4431d70 + 7d14837 commit bc14e30
Show file tree
Hide file tree
Showing 6 changed files with 588 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ 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.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.

### 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).
Expand Down
237 changes: 237 additions & 0 deletions moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,237 @@
<?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\Sniffs\PHPUnit;

use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;

/**
* Checks that a test file setUp and tearDown methods are always calling to parent.
*
* @copyright 2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class ParentSetUpTearDownSniff implements Sniff
{
/**
* @var string[] Methods to verify that they are calling to parent (setup like).
*/
private static array $setUpMethods = [
'setUp',
'setUpBeforeClass',
];

/**
* @var string[] Methods to verify that they are calling to parent (teardown like).
*/
private static array $tearDownMethods = [
'tearDown',
'tearDownAfterClass',
];

/**
* Register for open tag (only process once per file).
*/
public function register(): array {
return [T_OPEN_TAG];
}

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

// Before starting any check, let's look for various things.

// If we aren't checking Moodle 4.5dev (405) and up, nothing to check.
// Make and exception for codechecker phpunit tests, so they are run always.
if (!MoodleUtil::meetsMinimumMoodleVersion($phpcsFile, 405) && !MoodleUtil::isUnitTestRunning()) {
return; // @codeCoverageIgnore
}

// If the file is not a unit test file, nothing to check.
if (!MoodleUtil::isUnitTest($phpcsFile) && !MoodleUtil::isUnitTestRunning()) {
return; // @codeCoverageIgnore
}

// We only want to do this once per file.
$prevopentag = $phpcsFile->findPrevious(T_OPEN_TAG, $stackPtr - 1);
if ($prevopentag !== false) {
return; // @codeCoverageIgnore
}

// Get the file tokens, for ease of use.
$tokens = $phpcsFile->getTokens();

// These are the methods we are going to check.
$allMethods = array_merge(self::$setUpMethods, self::$tearDownMethods);

// Iterate over all the classes (hopefully only one, but that's not this sniff problem).
$cStart = $stackPtr;
while ($cStart = $phpcsFile->findNext(T_CLASS, $cStart + 1)) {
// Only interested in classes that are unit test classes.
if (MoodleUtil::isUnitTestCaseClass($phpcsFile, $cStart) === false) {
continue;
}

// Iterate over all the methods in the class.
$mStart = $cStart;
while ($mStart = $phpcsFile->findNext(T_FUNCTION, $mStart + 1, $tokens[$cStart]['scope_closer'])) {
$method = $phpcsFile->getDeclarationName($mStart);
// Only interested in setUp and tearDown methods.
if (!in_array($method, $allMethods)) {
continue;
}

// Iterate over all the parent:: calls in the method.
$pStart = $mStart;
$correctParentCalls = [];
while ($pStart = $phpcsFile->findNext(T_PARENT, $pStart + 1, $tokens[$mStart]['scope_closer'])) {
// The next-next token should be the parent method being named.
$methodCall = $phpcsFile->findNext(T_STRING, $pStart + 1, $pStart + 3);
// If we are calling to an incorrect parent method, report it. No fixable.
if (
$methodCall !== false &&
$tokens[$methodCall]['content'] !== $method &&
in_array($tokens[$methodCall]['content'], $allMethods) // Other parent calls may be correct.
) {
$wrongMethod = $tokens[$methodCall]['content'];
// We are calling to incorrect parent method.
$phpcsFile->addError(
'The %s() method in unit tests cannot call to parent::%s().',
$pStart,
'Incorrect' . ucfirst($method),
[$method, $wrongMethod]
);
}

// If we are calling to the correct parent method, annotate it.
if (
$methodCall !== false &&
$tokens[$methodCall]['content'] === $method
) {
$correctParentCalls[] = $pStart;
}
}

// If there are multiple calls to correct parent, report it. Not fixable.
if (count($correctParentCalls) > 1) {
$phpcsFile->addError(
'The %s() method in unit tests must call to parent::%s() only once.',
end($correctParentCalls),
'Multiple' . ucfirst($method),
[$method, $method]
);
}

// If there are no calls to correct parent, report it.
if (count($correctParentCalls) === 0) {
// Unlikely case of empty method, report it and continue. Not fixable.
// Find the next thing that is not an empty token.
$ignore = \PHP_CodeSniffer\Util\Tokens::$emptyTokens;

$nextValidStatement = $phpcsFile->findNext(
$ignore,
$tokens[$mStart]['scope_opener'] + 1,
$tokens[$mStart]['scope_closer'],
true
);
if ($nextValidStatement === false) {
$phpcsFile->addError(
'The %s() method in unit tests must not be empty',
$mStart,
'Empty' . ucfirst($method),
[$method]
);

continue;
}

// If the method is not empty, let's report the missing call. Fixable.
$fix = $phpcsFile->addFixableError(
'The %s() method in unit tests must always call to parent::%s().',
$mStart,
'Missing' . ucfirst($method),
[$method, $method]
);

if ($fix) {
// If the current method is a setUp one, let's add the call at the beginning.
if (in_array($method, self::$setUpMethods)) {
$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContent(
$this->findSetUpInsertionPoint($phpcsFile, $mStart),
"\n" . ' parent::' . $method . '();'
);
$phpcsFile->fixer->endChangeset();
}

// If the current method is a tearDown one, let's add the call at the end.
if (in_array($method, self::$tearDownMethods)) {
$phpcsFile->fixer->beginChangeset();
$phpcsFile->fixer->addContentBefore(
$tokens[$mStart]['scope_closer'] - 1,
' parent::' . $method . '();' . "\n"
);
$phpcsFile->fixer->endChangeset();
}
}
}
}
}
}

/**
* Find the best insertion point for parent::setUp calls.
*
* While it's technically correct to insert the parent::setUp call at the beginning of the method,
* it's better to insert it after some statements, like global or require/include ones.
*
* @param File $phpcsFile The file being scanned.
* @param int $mStart The position of the method.
* @return int The position where the parent::setUp method should be inserted.
*/
private function findSetUpInsertionPoint(File $phpcsFile, int $mStart): int {
// By default, we are going to insert it at the beginning.
$insertionPoint = $phpcsFile->getTokens()[$mStart]['scope_opener'];

// Let's find the first token in the method that is not a global, require or include.
// Do it ignoring both whitespace and comments.
$tokens = $phpcsFile->getTokens();
$mEnd = $tokens[$mStart]['scope_closer'];

$skipTokens = [T_WHITESPACE, T_COMMENT];
$allowedTokens = [T_GLOBAL, T_REQUIRE, T_REQUIRE_ONCE, T_INCLUDE, T_INCLUDE_ONCE];

while ($findPtr = $phpcsFile->findNext($skipTokens, $insertionPoint + 1, $mEnd, true)) {
// If we find a token that is not allowed, stop looking, insertion point determined.
if (!in_array($tokens[$findPtr]['code'], $allowedTokens)) {
break;
}

// Arrived here, we can advance the insertion point until the end of the allowed statement.
$insertionPoint = $phpcsFile->findEndOfStatement($findPtr, [T_COMMA]);
}

return $insertionPoint;
}
}
87 changes: 87 additions & 0 deletions moodle/Tests/Sniffs/PHPUnit/ParentSetUpTearDownSniffTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
<?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\PHPUnit;

use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;

/**
* Test the ParentSetUpTearDownSniff sniff.
*
* @copyright 2024 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*
* @covers \MoodleHQ\MoodleCS\moodle\Sniffs\PHPUnit\ParentSetUpTearDownSniff
*/
class ParentSetUpTearDownSniffTest extends MoodleCSBaseTestCase
{
/**
* Data provider for self::testParentSetUpTearDown
*/
public static function parentSetUpTearDownProvider(): array {
return [
'Correct' => [
'fixture' => 'ParentSetUpTearDownCorrect',
'errors' => [],
'warnings' => [],
],
'Problems' => [
'fixture' => 'ParentSetUpTearDownProblems',
'errors' => [
5 => 'The setUp() method in unit tests must not be empty',
8 => 'The tearDown() method in unit tests must not be empty',
11 => 'moodle.PHPUnit.ParentSetUpTearDown.EmptySetUpBeforeClass',
14 => 'moodle.PHPUnit.ParentSetUpTearDown.EmptyTearDownAfterClass',
21 => 'must call to parent::setUp() only once',
22 => 'moodle.PHPUnit.ParentSetUpTearDown.IncorrectSetUp',
27 => 'moodle.PHPUnit.ParentSetUpTearDown.MultipleTearDown',
29 => 'moodle.PHPUnit.ParentSetUpTearDown.IncorrectTearDown',
32 => 'moodle.PHPUnit.ParentSetUpTearDown.IncorrectSetUpBeforeClass',
35 => 'call to parent::setUpBeforeClass() only once',
40 => 'moodle.PHPUnit.ParentSetUpTearDown.MultipleTearDownAfterClass',
41 => 'cannot call to parent::setUpBeforeClass()',
46 => 'moodle.PHPUnit.ParentSetUpTearDown.MissingSetUp',
62 => 'moodle.PHPUnit.ParentSetUpTearDown.MissingTearDown',
69 => 'must always call to parent::setUpBeforeClass()',
83 => 'must always call to parent::tearDownAfterClass()',
92 => 'moodle.PHPUnit.ParentSetUpTearDown.EmptySetUp',
93 => 'moodle.PHPUnit.ParentSetUpTearDown.EmptyTearDown',
94 => 'moodle.PHPUnit.ParentSetUpTearDown.EmptySetUpBeforeClass',
95 => 'moodle.PHPUnit.ParentSetUpTearDown.EmptyTearDownAfterClass',
],
'warnings' => [],
],
];
}

/**
* @dataProvider parentSetUpTearDownProvider
*/
public function testParentSetUpTearDown(
string $fixture,
array $errors,
array $warnings
): void {
$this->setStandard('moodle');
$this->setSniff('moodle.PHPUnit.ParentSetUpTearDown');
$this->setFixture(sprintf("%s/fixtures/%s.php", __DIR__, $fixture));
$this->setWarnings($warnings);
$this->setErrors($errors);

$this->verifyCsResults();
}
}
Loading

0 comments on commit bc14e30

Please sign in to comment.