Add Error for empty setUp/tearDown methods

andrewnicols · May 31, 2024 · cea6d59 · cea6d59
1 parent 6a458cb
commit cea6d59
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 16 deletions.
diff --git a/moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php b/moodle/Sniffs/PHPUnit/ParentSetUpTearDownSniff.php
@@ -17,9 +17,11 @@
 
 namespace MoodleHQ\MoodleCS\moodle\Sniffs\PHPUnit;
 
+use MoodleHQ\MoodleCS\moodle\Util\Docblocks;
 use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Sniffs\Sniff;
+use PHPCSUtils\Utils\FunctionDeclarations;
 
 /**
  * Checks that a test file setUp and tearDown methods are always calling to parent.
@@ -145,10 +147,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
@@ -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
@@ -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
@@ -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
@@ -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.
+}