Enable PHPMD on tests code

briannesbitt · Jan 15, 2023 · 06a34b5 · 06a34b5
1 parent b67202a
commit 06a34b5
Show file tree

Hide file tree

Showing 15 changed files with 92 additions and 15 deletions.
diff --git a/.github/workflows/phpmd.yml b/.github/workflows/phpmd.yml
@@ -35,5 +35,8 @@ jobs:
             composer remove --no-update phpstan/extension-installer phpstan/phpstan ondrejmirtes/better-reflection friendsofphp/php-cs-fixer kylekatarnls/multi-tester --no-interaction --dev;
             composer update --prefer-dist --no-progress --prefer-${{ matrix.setup || 'stable' }};
 
-      - name: Run phpmd
+      - name: Check source code
         run: composer phpmd
+
+      - name: Check tests code
+        run: composer phpmd-test
diff --git a/composer.json b/composer.json
@@ -105,6 +105,7 @@
         "phpcs": "php-cs-fixer fix -v --diff --dry-run",
         "phpdoc": "php phpdoc.php",
         "phpmd": "phpmd src text /phpmd.xml",
+        "phpmd-test": "phpmd tests text /tests/phpmd-test.xml",
         "phpstan": "phpstan analyse --configuration phpstan.neon",
         "phpunit": "phpunit --verbose",
         "style-check": [

diff --git a/phpmd.xml b/phpmd.xml
@@ -32,11 +32,32 @@
         <exclude name="EvalExpression" />
         <exclude name="CouplingBetweenObjects" />
         <exclude name="CountInLoopExpression" />
+        <exclude name="DevelopmentCodeFragment" />
+    </rule>
+    <rule ref="rulesets/design.xml/DevelopmentCodeFragment">
+        <properties>
+            <property name="ignore-namespaces" value="true" />
+        </properties>
+    </rule>
+    <rule ref="rulesets/naming.xml">
+        <exclude name="ShortVariable" />
+        <exclude name="LongVariable" />
+        <exclude name="ShortMethodName" />
     </rule>
     <rule ref="rulesets/naming.xml/ShortVariable">
         <properties>
             <property name="exceptions" value="ci,id,to,tz" />
         </properties>
     </rule>
+    <rule ref="rulesets/naming.xml/LongVariable">
+        <properties>
+            <property name="subtract-suffixes" value="Enabled,Function,Factory,Timezone,FloorEnd,DiffOptions" />
+        </properties>
+    </rule>
+    <rule ref="rulesets/naming.xml/ShortMethodName">
+        <properties>
+            <property name="exceptions" value="eq,gt,is,lt,ne,to,tz" />
+        </properties>
+    </rule>
     <rule ref="rulesets/unusedcode.xml" />
 </ruleset>
diff --git a/tests/AbstractTestCase.php b/tests/AbstractTestCase.php
@@ -24,6 +24,9 @@
 use ReflectionProperty;
 use Throwable;
 
+/**
+ * @SuppressWarnings(PHPMD.NumberOfChildren)
+ */
 abstract class AbstractTestCase extends TestCase
 {
     /**

diff --git a/tests/Carbon/CreateSafeTest.php b/tests/Carbon/CreateSafeTest.php
@@ -164,7 +164,6 @@ public function testCreateSafeThrowsExceptionForInvalidDayForFebruaryInNonLeapYe
     public function testCreateSafePassesForInvalidDSTTime()
     {
         $message = '';
-        $date = null;
 
         try {
             // 1h jumped to 2h because of the DST, so 1h30 is not a safe date in PHP 5.4+

diff --git a/tests/Carbon/Fixtures/DumpCarbon.php b/tests/Carbon/Fixtures/DumpCarbon.php
@@ -22,6 +22,9 @@ final class DumpCarbon extends Carbon
 
     private $formatBroken = false;
 
+    /**
+     * @SuppressWarnings(PHPMD.DevelopmentCodeFragment)
+     */
     public function __construct($time = null, $tz = null)
     {
         ob_start();

diff --git a/tests/Carbon/LocalizationTest.php b/tests/Carbon/LocalizationTest.php
@@ -24,6 +24,7 @@
 use Symfony\Component\Translation\Loader\ArrayLoader;
 use Symfony\Component\Translation\MessageCatalogue;
 use Symfony\Component\Translation\MessageSelector;
+use Symfony\Component\Translation\Translator as SymfonyTranslator;
 use Symfony\Component\Translation\TranslatorInterface;
 use Tests\AbstractTestCase;
 use Tests\Carbon\Fixtures\MyCarbon;
@@ -691,7 +692,7 @@ public function testGetAvailableLocales()
         $translator->setMessages('zz_ZZ', []);
         $this->assertContains('zz_ZZ', Carbon::getAvailableLocales());
 
-        Carbon::setTranslator(new \Symfony\Component\Translation\Translator('en'));
+        Carbon::setTranslator(new SymfonyTranslator('en'));
         $this->assertSame(['en'], Carbon::getAvailableLocales());
     }
 

diff --git a/tests/Carbon/SettersTest.php b/tests/Carbon/SettersTest.php
@@ -330,7 +330,7 @@ public function testTimeZoneOfUnserialized()
         /** @var array $array */
         $array = $date;
 
-        foreach ($array as $_) {
+        foreach ($array as $item) {
         }
 
         $date->cleanupDumpProperties()->timezone = 'UTC';
@@ -859,6 +859,9 @@ public function testSubUnitNoOverflow()
         $this->assertSame(static::SET_UNIT_NO_OVERFLOW_SAMPLE, $results['end'] + $results['start'] + $results['current']);
     }
 
+    /**
+     * @SuppressWarnings(PHPMD.TooManyFields)
+     */
     private function failOperation(
         Carbon $original,
         Carbon $date,

diff --git a/tests/Carbon/TestingAidsTest.php b/tests/Carbon/TestingAidsTest.php
@@ -15,6 +15,7 @@
 
 use Carbon\Carbon;
 use DateTimeZone;
+use Exception;
 use InvalidArgumentException;
 use stdClass;
 use Tests\AbstractTestCase;
@@ -342,9 +343,9 @@ public function testWithTestNowWithException()
 
         try {
             Carbon::withTestNow($testNow, static function () {
-                throw new \Exception();
+                throw new Exception();
             });
-        } catch (\Exception $e) {
+        } catch (Exception $e) {
             // ignore
         }
 

diff --git a/tests/CarbonImmutable/LocalizationTest.php b/tests/CarbonImmutable/LocalizationTest.php
@@ -23,6 +23,7 @@
 use Symfony\Component\Translation\Loader\ArrayLoader;
 use Symfony\Component\Translation\MessageCatalogue;
 use Symfony\Component\Translation\MessageSelector;
+use Symfony\Component\Translation\Translator as SymfonyTranslator;
 use Symfony\Component\Translation\TranslatorInterface;
 use Tests\AbstractTestCase;
 use Tests\CarbonImmutable\Fixtures\MyCarbon;
@@ -663,7 +664,7 @@ public function testGetAvailableLocales()
         $translator->setMessages('zz_ZZ', []);
         $this->assertContains('zz_ZZ', Carbon::getAvailableLocales());
 
-        Carbon::setTranslator(new \Symfony\Component\Translation\Translator('en'));
+        Carbon::setTranslator(new SymfonyTranslator('en'));
         $this->assertSame(['en'], Carbon::getAvailableLocales());
     }
 

diff --git a/tests/CarbonImmutable/TestingAidsTest.php b/tests/CarbonImmutable/TestingAidsTest.php
@@ -17,6 +17,7 @@
 use Carbon\CarbonInterface;
 use Closure;
 use DateTimeZone;
+use Exception;
 use stdClass;
 use Tests\AbstractTestCase;
 
@@ -295,9 +296,9 @@ public function testWithTestNowWithException()
 
         try {
             Carbon::withTestNow($testNow, static function () {
-                throw new \Exception();
+                throw new Exception();
             });
-        } catch (\Exception $e) {
+        } catch (Exception $e) {
             // ignore
         }
 

diff --git a/tests/CarbonInterval/ForHumansTest.php b/tests/CarbonInterval/ForHumansTest.php
@@ -16,6 +16,7 @@
 use Carbon\Carbon;
 use Carbon\CarbonInterface;
 use Carbon\CarbonInterval;
+use Carbon\Translator as CarbonTranslator;
 use Symfony\Component\Translation\Loader\ArrayLoader;
 use Symfony\Component\Translation\Translator;
 use Tests\AbstractTestCase;
@@ -31,7 +32,7 @@ protected function tearDown(): void
 
     public function testGetTranslator()
     {
-        /** @var \Carbon\Translator $t */
+        /** @var CarbonTranslator $t */
         $t = CarbonInterval::getTranslator();
         $this->assertNotNull($t);
         $this->assertSame('en', $t->getLocale());
@@ -40,7 +41,7 @@ public function testGetTranslator()
 
     public function testResetTranslator()
     {
-        /** @var \Carbon\Translator $t */
+        /** @var CarbonTranslator $t */
         $t = MyCarbonInterval::getTranslator();
         $this->assertNotNull($t);
         $this->assertSame('en', $t->getLocale());
@@ -49,13 +50,13 @@ public function testResetTranslator()
 
     public function testSetTranslator()
     {
-        /** @var \Carbon\Translator $ori */
+        /** @var CarbonTranslator $ori */
         $ori = CarbonInterval::getTranslator();
         $t = new Translator('fr');
         $t->addLoader('array', new ArrayLoader());
         CarbonInterval::setTranslator($t);
 
-        /** @var \Carbon\Translator $t */
+        /** @var CarbonTranslator $t */
         $t = CarbonInterval::getTranslator();
         $this->assertNotNull($t);
         $this->assertSame('fr', $t->getLocale());
@@ -64,7 +65,7 @@ public function testSetTranslator()
 
     public function testDumpTranslator()
     {
-        $t = new \Carbon\Translator('tr_CY');
+        $t = new CarbonTranslator('tr_CY');
         $this->assertSame([
             'locale' => 'tr_CY',
         ], $t->__debugInfo());

diff --git a/tests/CarbonPeriod/IteratorTest.php b/tests/CarbonPeriod/IteratorTest.php
@@ -112,7 +112,7 @@ public function testKeyAndCurrentAreNullAfterIteration()
     }
 
     /**
-     * @dataProvider \Tests\CarbonPeriod\IteratorTest::dataForIterateBackwardsArguments
+     * @dataProvider dataForIterateBackwardsArguments
      */
     public function testIterateBackwards($arguments, $expected)
     {

diff --git a/tests/Localization/LocalizationTestCase.php b/tests/Localization/LocalizationTestCase.php
@@ -19,6 +19,9 @@
 use InvalidArgumentException;
 use Tests\AbstractTestCase;
 
+/**
+ * @SuppressWarnings(PHPMD.NumberOfChildren)
+ */
 abstract class LocalizationTestCase extends AbstractTestCase
 {
     public const LOCALE = 'en';

diff --git a/tests/phpmd-test.xml b/tests/phpmd-test.xml
@@ -0,0 +1,36 @@
+<?xml version="1.0"?>
+<ruleset
+    name="Mess detection rules for Carbon"
+    xmlns="http://pmd.sf.net/ruleset/1.0.0"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0
+             http://pmd.sf.net/ruleset_xml_schema.xsd"
+    xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd"
+>
+    <description>
+        Mess detection rules for Carbon
+    </description>
+    <rule ref="rulesets/cleancode.xml">
+        <exclude name="BooleanArgumentFlag" />
+        <exclude name="StaticAccess" />
+        <exclude name="IfStatementAssignment" />
+        <exclude name="UndefinedVariable" />
+        <exclude name="ErrorControlOperator" />
+    </rule>
+    <rule ref="rulesets/controversial.xml" />
+    <rule ref="rulesets/design.xml">
+        <exclude name="EvalExpression" />
+        <exclude name="CouplingBetweenObjects" />
+        <exclude name="CountInLoopExpression" />
+        <exclude name="DevelopmentCodeFragment" />
+    </rule>
+    <rule ref="rulesets/design.xml/DevelopmentCodeFragment">
+        <properties>
+            <property name="ignore-namespaces" value="true" />
+        </properties>
+    </rule>
+    <rule ref="rulesets/unusedcode.xml">
+        <exclude name="UnusedLocalVariable" />
+        <exclude name="UnusedFormalParameter" />
+    </rule>
+</ruleset>
-Original file line number
+Diff line change
@@ Expand Up / @@ -112,7 +112,7 @@ public function testKeyAndCurrentAreNullAfterIteration() @@
         }
         /**
-         * @dataProvider \Tests\CarbonPeriod\IteratorTest::dataForIterateBackwardsArguments
+         * @dataProvider dataForIterateBackwardsArguments
          */
         public function testIterateBackwards($arguments, $expected)
         {
@@ Expand Down @@