Merge pull request #103 from andrewnicols/phpunit_final_test_classes

Unit test classes should be declared as either final or abstract

moodle/Sniffs/PHPUnit/TestCasesAbstractSniff.php

@@ -0,0 +1,87 @@
+<?php
+// This file is part of Moodle - http://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 <http://www.gnu.org/licenses/>.
+
+namespace MoodleHQ\MoodleCS\moodle\Sniffs\PHPUnit;
+
+// phpcs:disable moodle.NamingConventions
+
+use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
+use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Files\File;
+use PHPCSUtils\Utils\ObjectDeclarations;
+
+/**
+ * Checks that testcase classes are declared as abstract.
+ *
+ * @copyright  2024 Andrew Lyons <adrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class TestCasesAbstractSniff implements Sniff {
+    public function register() {
+        return [
+            T_OPEN_TAG,
+        ];
+    }
+
+    public function process(File $file, $pointer) {
+        // If the file is not a unit test file, nothing to check.
+        if (!MoodleUtil::isUnitTest($file) && !MoodleUtil::isUnitTestRunning()) {
+            return; // @codeCoverageIgnore
+        }
+
+        // If we aren't checking Moodle 4.4dev (404) and up, nothing to check.
+        // Make and exception for codechecker phpunit tests, so they are run always.
+        if (!MoodleUtil::meetsMinimumMoodleVersion($file, 404) && !MoodleUtil::isUnitTestRunning()) {
+            return; // @codeCoverageIgnore
+        }
+
+        // We only want to do this once per file.
+        $prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1);
+        if ($prevopentag !== false) {
+            return; // @codeCoverageIgnore
+        }
+
+        $cStart = $pointer;
+        while ($cStart = $file->findNext(T_CLASS, $cStart + 1)) {
+            if (MoodleUtil::isUnitTestCaseClass($file, $cStart) === false) {
+                // This class does not relate to a unit test.
+                continue;
+            }
+
+            $className = ObjectDeclarations::getName($file, $cStart);
+            if (substr($className, -9) !== '_testcase') {
+                continue;
+            }
+
+            $classInfo = ObjectDeclarations::getClassProperties($file, $cStart);
+
+            if (!$classInfo['is_abstract']) {
+                $fix = $file->addFixableWarning(
+                    'Testcase %s should be declared as abstract.',
+                    $cStart,
+                    'UnitTestClassesAbstract',
+                    [$className],
+                );
+
+                if ($fix) {
+                    $file->fixer->beginChangeset();
+                    $file->fixer->addContentBefore($cStart, 'abstract ');
+                    $file->fixer->endChangeset();
+                }
+            }
+        }
+    }
+}

moodle/Sniffs/PHPUnit/TestClassesFinalSniff.php

@@ -0,0 +1,124 @@
+<?php
+// This file is part of Moodle - http://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 <http://www.gnu.org/licenses/>.
+
+namespace MoodleHQ\MoodleCS\moodle\Sniffs\PHPUnit;
+
+// phpcs:disable moodle.NamingConventions
+
+use MoodleHQ\MoodleCS\moodle\Util\MoodleUtil;
+use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Files\File;
+use PHPCSUtils\Utils\ObjectDeclarations;
+
+/**
+ * Checks that test classes are declared as final.
+ *
+ * @copyright  2024 Andrew Lyons <adrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class TestClassesFinalSniff implements Sniff {
+    public function register() {
+        return [
+            T_OPEN_TAG,
+        ];
+    }
+
+    public function process(File $file, $pointer) {
+        // If the file is not a unit test file, nothing to check.
+        if (!MoodleUtil::isUnitTest($file) && !MoodleUtil::isUnitTestRunning()) {
+            return; // @codeCoverageIgnore
+        }
+
+        // If we aren't checking Moodle 4.4dev (404) and up, nothing to check.
+        // Make and exception for codechecker phpunit tests, so they are run always.
+        if (!MoodleUtil::meetsMinimumMoodleVersion($file, 404) && !MoodleUtil::isUnitTestRunning()) {
+            return; // @codeCoverageIgnore
+        }
+
+        // Get the file tokens, for ease of use.
+        $tokens = $file->getTokens();
+
+        // We only want to do this once per file.
+        $prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1);
+        if ($prevopentag !== false) {
+            return; // @codeCoverageIgnore
+        }
+
+        $cStart = $pointer;
+        while ($cStart = $file->findNext(T_CLASS, $cStart + 1)) {
+            if (MoodleUtil::isUnitTestCaseClass($file, $cStart) === false) {
+                // This class does not relate to a unit test.
+                continue;
+            }
+            $className = ObjectDeclarations::getName($file, $cStart);
+            if (substr($className, -5) !== '_test') {
+                continue;
+            }
+
+            $classInfo = ObjectDeclarations::getClassProperties($file, $cStart);
+
+            if ($classInfo['is_final']) {
+                // Already final.
+                continue;
+            }
+
+            if ($classInfo['is_abstract']) {
+                // See if this class has any abstract methods.
+                $mStart = $cStart + 1;
+                $hasAbstractMethod = false;
+
+                while ($mStart = $file->findNext(T_ABSTRACT, $mStart, $tokens[$cStart]['scope_closer']) !== false) {
+                    $hasAbstractMethod = true;
+                    break;
+                }
+                if ($hasAbstractMethod) {
+                    $file->addWarning(
+                        'Unit test %s should be declared as final and not abstract.',
+                        $cStart,
+                        'UnitTestClassesFinal',
+                        [$className],
+                    );
+                } else {
+                    $fix = $file->addFixableWarning(
+                        'Unit test %s should be declared as final and not abstract.',
+                        $cStart,
+                        'UnitTestClassesFinal',
+                        [$className],
+                    );
+
+                    if ($fix) {
+                        $file->fixer->beginChangeset();
+                        $file->fixer->replaceToken($classInfo['abstract_token'], 'final');
+                        $file->fixer->endChangeset();
+                    }
+                }
+            } else if (!$classInfo['is_final']) {
+                $fix = $file->addFixableWarning(
+                    'Unit test %s should be declared as final.',
+                    $cStart,
+                    'UnitTestClassesFinal',
+                    [$className],
+                );
+
+                if ($fix) {
+                    $file->fixer->beginChangeset();
+                    $file->fixer->addContentBefore($cStart, 'final ');
+                    $file->fixer->endChangeset();
+                }
+            }
+        }
+    }
+}

moodle/Tests/Sniffs/PHPUnit/TestClassesFinalSniffTest.php

@@ -0,0 +1,67 @@
+<?php
+// This file is part of Moodle - http://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 <http://www.gnu.org/licenses/>.
+
+namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\PHPUnit;
+
+// phpcs:disable moodle.NamingConventions
+
+use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;
+
+/**
+ * Test the TestClassesFinalSniff sniff.
+ *
+ * @copyright  2024 Andrew Lyons <andrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ *
+ * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\PHPUnit\TestClassesFinalSniff
+ */
+class TestclassesFinalSniffTest extends MoodleCSBaseTestCase {
+
+    /**
+     * Data provider for self::provider_phpunit_data_returntypes
+     */
+    public static function phpunit_classes_final_provider(): array {
+        return [
+            'Standard fixes' => [
+                'fixture' => 'testclassesfinal',
+                'errors' => [
+                ],
+                'warnings' => [
+                    15 => 'Unit test example_abstract_test_with_abstract_children_test should be declared as final and not abstract.',
+                    19 => 'Unit test example_abstract_test should be declared as final and not abstract.',
+                    23 => 'Unit test example_standard_test should be declared as final.',
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider phpunit_classes_final_provider
+     */
+    public function test_phpunit_classes_final(
+        string $fixture,
+        array $errors,
+        array $warnings
+    ): void {
+        $this->set_standard('moodle');
+        $this->set_sniff('moodle.PHPUnit.TestClassesFinal');
+        $this->set_fixture(sprintf("%s/fixtures/%s.php", __DIR__, $fixture));
+        $this->set_warnings($warnings);
+        $this->set_errors($errors);
+
+        $this->verify_cs_results();
+    }
+}

moodle/Tests/Sniffs/PHPUnit/TestcaseAbstractSniffTest.php

@@ -0,0 +1,65 @@
+<?php
+// This file is part of Moodle - http://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 <http://www.gnu.org/licenses/>.
+
+namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\PHPUnit;
+
+// phpcs:disable moodle.NamingConventions
+
+use MoodleHQ\MoodleCS\moodle\Tests\MoodleCSBaseTestCase;
+
+/**
+ * Test the TestCasesAbstractSniff sniff.
+ *
+ * @copyright  2024 Andrew Lyons <andrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ *
+ * @covers \MoodleHQ\MoodleCS\moodle\Sniffs\PHPUnit\TestcaseAbstractSniff
+ */
+class TestcaseAbstractSniffTest extends MoodleCSBaseTestCase {
+
+    /**
+     * Data provider for self::provider_phpunit_data_returntypes
+     */
+    public static function phpunit_classes_final_provider(): array {
+        return [
+            'Standard fixes' => [
+                'fixture' => 'testcaseclassesabstract',
+                'errors' => [
+                ],
+                'warnings' => [
+                    8 => 'Testcase example_testcase should be declared as abstract.',
+                ],
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider phpunit_classes_final_provider
+     */
+    public function test_phpunit_classes_final(
+        string $fixture,
+        array $errors,
+        array $warnings
+    ): void {
+        $this->set_standard('moodle');
+        $this->set_sniff('moodle.PHPUnit.TestCasesAbstract');
+        $this->set_fixture(sprintf("%s/fixtures/%s.php", __DIR__, $fixture));
+        $this->set_warnings($warnings);
+        $this->set_errors($errors);
+
+        $this->verify_cs_results();
+    }
+}

moodle/Tests/Sniffs/PHPUnit/fixtures/testcaseclassesabstract.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace MoodleHQ\MoodleCS\moodle\Tests\Sniffs\PHPUnit;
+
+defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures.
+
+// Testcases should always be abstract.
+class example_testcase extends \advanced_testcase {
+}
+
+abstract class abstract_example_testcase extends \advanced_testcase {
+}
+
+// Test classes cannot be abstract.
+abstract class example_abstract_test_with_abstract_children extends \advanced_testcase {
+    abstract public function test_something();
+}
+
+abstract class example_abstract_test extends \advanced_testcase {
+}
+
+// A regular test should be final.
+class example_standard_test extends \advanced_testcase {
+}
+
+// A final test is already final.
+final class example_final_test extends \advanced_testcase {
+}
+
+class not_a_test_class {
+}