From 4d1a7b4a68d9c01ea92fcbb570cfd96b668150c5 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Fri, 31 May 2024 15:30:51 +0800 Subject: [PATCH] Add Error for empty setUp/tearDown methods --- .../PHPUnit/ParentSetUpTearDownSniff.php | 21 +++++++++++++++---- .../PHPUnit/ParentSetUpTearDownSniffTest.php | 12 +++++++---- .../fixtures/ParentSetUpTearDownCorrect.php | 4 ---- .../fixtures/ParentSetUpTearDownProblems.php | 7 +++++++ .../ParentSetUpTearDownProblems.php.fixed | 11 ++++++---- 5 files changed, 39 insertions(+), 16 deletions(-) diff --git a/moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php b/moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php index d9b9e2d..a6c545a 100644 --- a/moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php +++ b/moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php @@ -145,10 +145,23 @@ public function process(File $phpcsFile, $stackPtr): void { // If there are no calls to correct parent, report it. Fixable. if (count($correctParentCalls) === 0) { - // Any weird case where the method is empty, we need at very least 1 line. Skip. - $startLine = $tokens[$tokens[$mStart]['scope_opener']]['line']; - $endLine = $tokens[$tokens[$mStart]['scope_closer']]['line']; - if ($startLine === $endLine) { + // 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; } diff --git a/moodle/Tests/Sniffs/PHPUnit/ParentSetUpTearDownSniffTest.php b/moodle/Tests/Sniffs/PHPUnit/ParentSetUpTearDownSniffTest.php index 7076df7..79ab667 100644 --- a/moodle/Tests/Sniffs/PHPUnit/ParentSetUpTearDownSniffTest.php +++ b/moodle/Tests/Sniffs/PHPUnit/ParentSetUpTearDownSniffTest.php @@ -42,10 +42,10 @@ public static function parentSetUpTearDownProvider(): array { 'Problems' => [ 'fixture' => 'ParentSetUpTearDownProblems', 'errors' => [ - 5 => 'must always call to parent::setUp()', - 8 => 'tearDown() method in unit tests must always call', - 11 => 'moodle.PHPUnit.ParentSetUpTearDown.MissingSetUpBeforeClass', - 14 => 'moodle.PHPUnit.ParentSetUpTearDown.MissingTearDownAfterClass', + 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', @@ -58,6 +58,10 @@ public static function parentSetUpTearDownProvider(): array { 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' => [], ], diff --git a/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownCorrect.php b/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownCorrect.php index 5ba7765..e1b1172 100644 --- a/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownCorrect.php +++ b/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownCorrect.php @@ -24,10 +24,6 @@ public static function tearDownAfterClass(): void { } class another_correct_setup_teardown_test extends Something { - public function setUp(): void {} - public function tearDown(): void {} - public static function setUpBeforeClass(): void {} - public static function tearDownAfterClass(): void { } // Same line. public function ignoredMethod(): void { parent::setUp(); parent::tearDown(); diff --git a/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php b/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php index c4a5dc6..88819c8 100644 --- a/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php +++ b/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php @@ -87,3 +87,10 @@ public static function tearDownAfterClass(): void { require('somefile.php'); } } + +class empty_setup_teardown_test extends Something { + public function setUp(): void {} + public function tearDown(): void {} + public static function setUpBeforeClass(): void {} + public static function tearDownAfterClass(): void { } // Same line. +} diff --git a/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php.fixed b/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php.fixed index 22726de..cf7f704 100644 --- a/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php.fixed +++ b/moodle/Tests/Sniffs/PHPUnit/fixtures/ParentSetUpTearDownProblems.php.fixed @@ -3,19 +3,15 @@ defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS class missing_setup_teardown_test extends Something { public function setUp(): void { - parent::setUp(); } public function tearDown(): void { - parent::tearDown(); } public static function setUpBeforeClass(): void { - parent::setUpBeforeClass(); } public static function tearDownAfterClass(): void { - parent::tearDownAfterClass(); } } @@ -95,3 +91,10 @@ class best_insertion_setup_teardown_test extends Something { parent::tearDownAfterClass(); } } + +class empty_setup_teardown_test extends Something { + public function setUp(): void {} + public function tearDown(): void {} + public static function setUpBeforeClass(): void {} + public static function tearDownAfterClass(): void { } // Same line. +}