Make static fixes configurable (WIP)

andrewnicols · Sep 15, 2023 · 7bf89c3 · 7bf89c3
1 parent b86c003
commit 7bf89c3
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 15 deletions.
diff --git a/moodle-extra/ruleset.xml b/moodle-extra/ruleset.xml
@@ -84,8 +84,6 @@
 
     -->
 
-    <rule ref="moodle.Classes.UnitTestFormatting"/>
-
     <!--
         Detect issues with Unit Test dataProviders:
         - private providers
@@ -95,6 +93,10 @@
         - dataProviders which do not return an array or Iterable
         - dataProviders which can be converted to a static method (PHPUnit 10 compatibility)
     -->
-    <rule ref="moodle.PHPUnit.TestCaseProvider"/>
+    <rule ref="moodle.PHPUnit.TestCaseProvider">
+        <properties>
+            <property name="autofixStaticProviders" value="true"/>
+        </properties>
+    </rule>
 
 </ruleset>
diff --git a/moodle/Sniffs/PHPUnit/TestCaseProviderSniff.php b/moodle/Sniffs/PHPUnit/TestCaseProviderSniff.php
@@ -32,6 +32,13 @@
  */
 class TestCaseProviderSniff implements Sniff {
 
+    /**
+     * Whether to autofix static providers.
+     *
+     * @var bool
+     */
+    public $autofixStaticProviders = false;
+
     /**
      * Register for open tag (only process once per file).
      */
@@ -274,23 +281,27 @@ protected function checkDataProvider(
 
         // In preparation for PHPUnit 10, we want to recommend that data providers are statically defined.
         if (!$methodProps['is_static']) {
-            // We can make this fixable if the method does not contain any `$this`.
-            // Search the body.
-            $hasThis = false;
-            $bodyStart = $tokens[$providerPointer]['scope_opener'] + 1;
-            $bodyEnd = $tokens[$providerPointer]['scope_closer'] - 1;
-            while ($token = $file->findNext(T_VARIABLE, $bodyStart, $bodyEnd)) {
-                if ($tokens[$token]['content'] === '$this') {
-                    $hasThis = true;
-                    break;
+            $supportAutomatedFix = true;
+            if (!$this->autofixStaticProviders) {
+                $supportAutomatedFix = false;
+            } else {
+                // We can make this fixable if the method does not contain any `$this`.
+                // Search the body.
+                $bodyStart = $tokens[$providerPointer]['scope_opener'] + 1;
+                $bodyEnd = $tokens[$providerPointer]['scope_closer'] - 1;
+                while ($token = $file->findNext(T_VARIABLE, $bodyStart, $bodyEnd)) {
+                    if ($tokens[$token]['content'] === '$this') {
+                        $supportAutomatedFix = false;
+                        break;
+                    }
                 }
             }
 
-            if ($hasThis) {
+            if (!$supportAutomatedFix) {
                 $file->addWarning(
                     'Data provider method "%s" will need to be converted to static in future.',
                     $pointer + 2,
-                    'dataProviderSyntaxMethodNotStatic',
+                    'dataProviderNotStatic',
                     [
                         $methodName,
                     ]
@@ -299,7 +310,7 @@ protected function checkDataProvider(
                 $fix = $file->addFixableWarning(
                     'Data provider method "%s" will need to be converted to static in future.',
                     $pointer + 2,
-                    'dataProviderSyntaxMethodNotStatic',
+                    'dataProviderNotStatic',
                     [
                         $methodName,
                     ]

diff --git a/moodle/ruleset.xml b/moodle/ruleset.xml
@@ -90,6 +90,16 @@
 
     <rule ref="Zend.Files.ClosingTag"/>
 
+    <!--
+        Detect issues with Unit Test dataProviders:
+        - private providers
+        - providers which do not exist
+        - providers whose name is prefixed with _test
+        - incorrect casing of dataProvider
+        - dataProviders which do not return an array or Iterable
+    -->
+    <rule ref="moodle.PHPUnit.TestCaseProvider"/>
+
     <!-- Disable this exact error unless it's approved -->
     <rule ref="moodle.Commenting.InlineComment.SpacingAfter">
         <severity>0</severity>