Rector: added dependency and basic rules (#4213)

* Added rector and basic rules * Fixes --------- Co-authored-by: Colin Mollenhour <colin@mollenhour.com>
OpenMage · Sep 22, 2024 · 75cd467 · 75cd467
1 parent e7b47ab
commit 75cd467
Show file tree

Hide file tree

Showing 24 changed files with 174 additions and 52 deletions.
diff --git a/.ddev/commands/web/rector b/.ddev/commands/web/rector
@@ -4,5 +4,4 @@
 ## Usage: rector
 ## Example: ddev rector <path-to-files>
 
-cp -n vendor/sreichel/openmage-rector/rector.php rector.php
 php vendor/bin/rector process "$@"
diff --git a/.gitattributes b/.gitattributes
@@ -3,6 +3,7 @@
 /.github export-ignore
 /dev export-ignore
 /docs export-ignore
+/tests export-ignore
 
 /.all-contributorsrc export-ignore
 /.gitattributes export-ignore
@@ -16,6 +17,7 @@
 /.phpmd.dist.xml export-ignore
 /.phpstan.dist.baseline.neon export-ignore
 /.phpstan.dist.neon export-ignore
+/rector.php export-ignore
 
 /README.md export-ignore
 

diff --git a/.github/labeler.yml b/.github/labeler.yml
@@ -895,11 +895,9 @@
 'phpunit':
   - changed-files:
     - any-glob-to-any-file: [
-      dev/test/*,
-      dev/phpunit*,
-      dev/sonar*,
-      .github/workflows/phpunit.yml,
-      .github/workflows/sonar.yml
+      phpunit*,
+      tests/*,
+      .github/workflows/phpunit.yml
     ]
 
 'ddev':
@@ -908,3 +906,10 @@
       .ddev/*,
       .ddev/**/*
     ]
+
+'rector':
+  - changed-files:
+      - any-glob-to-any-file: [
+        rector.php,
+        .github/workflows/rector.yml
+      ]
diff --git a/.github/workflows/check-files.yml b/.github/workflows/check-files.yml
@@ -83,6 +83,7 @@ jobs:
             **phpcs**
             **php-cs-fixer**
             **phpstan**
+            rector.php
             tests/
             phpunit*
 

diff --git a/.github/workflows/rector.yml b/.github/workflows/rector.yml
@@ -0,0 +1,37 @@
+name: Rector
+
+on:
+  workflow_call:
+  # Allow manually triggering the workflow.
+  workflow_dispatch:
+
+jobs:
+  rector:
+    name: Validation
+    runs-on: [ubuntu-latest]
+
+    steps:
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: 7.4
+
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Get composer cache directory
+        id: composer-cache
+        run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
+
+      - name: Cache dependencies
+        uses: actions/cache@v4
+        with:
+          path: ${{ steps.composer-cache.outputs.dir }}
+          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
+          restore-keys: ${{ runner.os }}-composer-
+
+      - name: Install dependencies
+        run: composer install --prefer-dist --no-progress --ignore-platform-req=ext-*
+
+      - name: Rector
+        run: php vendor/bin/rector process --dry-run
diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml
@@ -106,6 +106,14 @@ jobs:
     if: needs.check.outputs.xml > 0
     uses: ./.github/workflows/syntax-xml.yml
 
+  rector:
+    name: Rector
+    needs: [check, phpcs, php-cs-fixer]
+    if: |
+      needs.check.outputs.php > 0 ||
+      needs.check.outputs.workflow > 0
+    uses: ./.github/workflows/rector.yml
+
   # DOES NOT run by default
   # runs on schedule or when workflow or unit tests changed
   unit_tests:

diff --git a/app/code/core/Mage/Catalog/Model/Product.php b/app/code/core/Mage/Catalog/Model/Product.php
@@ -699,7 +699,7 @@ public function getStockItem()
      */
     public function hasStockItem()
     {
-        return !!$this->_stockItem;
+        return (bool) $this->_stockItem;
     }
 
     /**

diff --git a/app/code/core/Mage/Catalog/Model/Product/Type/Abstract.php b/app/code/core/Mage/Catalog/Model/Product/Type/Abstract.php
@@ -561,7 +561,7 @@ public function checkProductBuyState($product = null)
                 if ($option->getIsRequire()) {
                     $customOption = $this->getProduct($product)
                         ->getCustomOption(self::OPTION_PREFIX . $option->getId());
-                    if (!$customOption || $customOption->getValue() === null || strlen($customOption->getValue()) === 0) {
+                    if (!$customOption || $customOption->getValue() === null || (string) $customOption->getValue() === '') {
                         $this->getProduct($product)->setSkipCheckRequiredOption(true);
                         Mage::throwException(
                             Mage::helper('catalog')->__('The product has required options')

diff --git a/app/code/core/Mage/Cms/Model/Page.php b/app/code/core/Mage/Cms/Model/Page.php
@@ -126,9 +126,6 @@ public function checkIdentifier($identifier, $storeId)
 
     /**
      * Retrieves cms page title from DB by passed identifier.
-     *
-     * @param string $identifier
-     * @return string
      */
     public function getCmsPageTitleByIdentifier(string $identifier): string
     {
@@ -139,7 +136,6 @@ public function getCmsPageTitleByIdentifier(string $identifier): string
      * Retrieves cms page title from DB by passed id.
      *
      * @param string|int $id
-     * @return string
      */
     public function getCmsPageTitleById($id): string
     {
@@ -150,7 +146,6 @@ public function getCmsPageTitleById($id): string
      * Retrieves cms page identifier from DB by passed id.
      *
      * @param string|int $id
-     * @return string
      */
     public function getCmsPageIdentifierById($id): string
     {

diff --git a/app/code/core/Mage/GoogleAnalytics/Block/Ga.php b/app/code/core/Mage/GoogleAnalytics/Block/Ga.php
@@ -204,7 +204,7 @@ protected function _getEnhancedEcommerceDataForAnalytics4()
             if ($productViewed->getAttributeText('manufacturer')) {
                 $_item['item_brand'] = $productViewed->getAttributeText('manufacturer');
             }
-            array_push($eventData['items'], $_item);
+            $eventData['items'][] = $_item;
             $result[] = ['view_item', $eventData];
         } elseif ($moduleName == 'catalog' && $controllerName == 'category') {
             // Log this event when the user has been presented with a list of items of a certain category.
@@ -241,7 +241,7 @@ protected function _getEnhancedEcommerceDataForAnalytics4()
                 if ($productViewed->getCategory()->getName()) {
                     $_item['item_category'] = $productViewed->getCategory()->getName();
                 }
-                array_push($eventData['items'], $_item);
+                $eventData['items'][] = $_item;
                 $index++;
                 $eventData['value'] += $productViewed->getFinalPrice();
             }
@@ -274,7 +274,7 @@ protected function _getEnhancedEcommerceDataForAnalytics4()
                 if ($itemCategory) {
                     $_item['item_category'] = $itemCategory;
                 }
-                array_push($eventData['items'], $_item);
+                $eventData['items'][] = $_item;
                 $eventData['value'] += $_product->getFinalPrice() * $productInCart->getQty();
             }
             $eventData['value'] = $helper->formatPrice($eventData['value']);
@@ -306,7 +306,7 @@ protected function _getEnhancedEcommerceDataForAnalytics4()
                     if ($itemCategory) {
                         $_item['item_category'] = $itemCategory;
                     }
-                    array_push($eventData['items'], $_item);
+                    $eventData['items'][] = $_item;
                     $eventData['value'] += $_product->getFinalPrice();
                 }
                 $eventData['value'] = $helper->formatPrice($eventData['value']);
@@ -352,7 +352,7 @@ protected function _getEnhancedEcommerceDataForAnalytics4()
                     if ($itemCategory) {
                         $_item['item_category'] = $itemCategory;
                     }
-                    array_push($orderData['items'], $_item);
+                    $orderData['items'][] = $_item;
                 }
                 $result[] = ['purchase', $orderData];
             }

diff --git a/composer.json b/composer.json
@@ -47,6 +47,7 @@
         "phpmd/phpmd": "^2.13",
         "phpstan/phpstan": "^1.12.1",
         "phpunit/phpunit": "^9.5",
+        "rector/rector": "^1.2",
         "squizlabs/php_codesniffer": "^3.7",
         "symplify/vendor-patches": "^11.1"
     },
@@ -134,14 +135,18 @@
         "phpstan": "vendor/bin/phpstan analyze",
         "phpunit:test": "XDEBUG_MODE=off php vendor/bin/phpunit --no-coverage --testdox",
         "phpunit:coverage": "XDEBUG_MODE=coverage php vendor/bin/phpunit --testdox",
-        "phpunit:coverage-local": "XDEBUG_MODE=coverage php vendor/bin/phpunit --coverage-html build/coverage --testdox"
+        "phpunit:coverage-local": "XDEBUG_MODE=coverage php vendor/bin/phpunit --coverage-html build/coverage --testdox",
+        "rector:test": "vendor/bin/rector process --dry-run",
+        "rector:fix": "vendor/bin/rector process"
     },
     "scripts-descriptions": {
         "php-cs-fixer:test": "Run php-cs-fixer",
         "php-cs-fixer:fix": "Run php-cs-fixer and fix issues",
         "phpstan": "Run phpstan",
         "phpunit:test": "Run PHPUnit",
         "phpunit:coverage": "Run PHPUnit with code coverage",
-        "phpunit:coverage-local": "Run PHPUnit with local HTML code coverage"
+        "phpunit:coverage-local": "Run PHPUnit with local HTML code coverage",
+        "rector:test": "Run rector",
+        "rector:fix": "Run rector and fix issues"
     }
 }
diff --git a/composer.lock b/composer.lock
diff --git a/rector.php b/rector.php
@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\CodeQuality\Rector as CodeQuality;
+use Rector\DeadCode\Rector as DeadCode;
+use Rector\Config\RectorConfig;
+use Rector\TypeDeclaration\Rector as TypeDeclaration;
+
+return RectorConfig::configure()
+    ->withPaths([
+        __DIR__ . '/app',
+        __DIR__ . '/dev',
+        __DIR__ . '/errors',
+        __DIR__ . '/lib',
+        __DIR__ . '/pub',
+        __DIR__ . '/shell',
+        __DIR__ . '/tests',
+    ])
+    ->withSkipPath(__DIR__ . '/vendor')
+    ->withSkip([
+        __DIR__ . '/shell/translations.php',
+        __DIR__ . '/shell/update-copyright.php.php'
+    ])
+    ->withRules([
+//        CodeQuality\BooleanNot\SimplifyDeMorganBinaryRector::class, # wait for https://github.com/rectorphp/rector/issues/8781
+        CodeQuality\BooleanNot\ReplaceMultipleBooleanNotRector::class,
+        CodeQuality\Foreach_\UnusedForeachValueToArrayKeysRector::class,
+        CodeQuality\FuncCall\ChangeArrayPushToArrayAssignRector::class,
+        CodeQuality\FuncCall\CompactToVariablesRector::class,
+        CodeQuality\Identical\SimplifyArraySearchRector::class,
+        CodeQuality\Identical\SimplifyConditionsRector::class,
+        CodeQuality\Identical\StrlenZeroToIdenticalEmptyStringRector::class,
+//        CodeQuality\If_\SimplifyIfReturnBoolRector::class,
+        CodeQuality\NotEqual\CommonNotEqualRector::class,
+        CodeQuality\LogicalAnd\LogicalToBooleanRector::class,
+        CodeQuality\Ternary\SimplifyTautologyTernaryRector::class,
+        DeadCode\ClassMethod\RemoveUselessParamTagRector::class,
+        DeadCode\ClassMethod\RemoveUselessReturnTagRector::class,
+        DeadCode\Property\RemoveUselessVarTagRector::class,
+        TypeDeclaration\ClassMethod\ReturnNeverTypeRector::class,
+    ]);
diff --git a/tests/unit/Mage/AdminNotification/Model/FeedTest.php b/tests/unit/Mage/AdminNotification/Model/FeedTest.php
@@ -23,9 +23,6 @@
 
 class FeedTest extends TestCase
 {
-    /**
-     * @var Mage_AdminNotification_Model_Feed
-     */
     public Mage_AdminNotification_Model_Feed $subject;
 
     public function setUp(): void

diff --git a/tests/unit/Mage/Adminhtml/Block/Customer/Edit/Tab/NewsletterTest.php b/tests/unit/Mage/Adminhtml/Block/Customer/Edit/Tab/NewsletterTest.php
@@ -34,7 +34,6 @@ public function setUp(): void
     }
 
     /**
-     * @return void
      *
      * @group Mage_Adminhtml
      * @group Mage_Adminhtml_Block

diff --git a/tests/unit/Mage/Cms/Block/Widget/BlockTest.php b/tests/unit/Mage/Cms/Block/Widget/BlockTest.php
@@ -23,9 +23,6 @@
 
 class BlockTest extends TestCase
 {
-    /**
-     * @var Mage_Cms_Block_Widget_Block
-     */
     public Mage_Cms_Block_Widget_Block $subject;
 
     public function setUp(): void

diff --git a/tests/unit/Mage/Cms/Block/Widget/Page/LinkTest.php b/tests/unit/Mage/Cms/Block/Widget/Page/LinkTest.php
@@ -23,9 +23,6 @@
 
 class LinkTest extends TestCase
 {
-    /**
-     * @var Mage_Cms_Block_Widget_Page_Link
-     */
     public Mage_Cms_Block_Widget_Page_Link $subject;
 
     public function setUp(): void

diff --git a/tests/unit/Mage/Cms/Helper/Wysiwyg/ImagesTest.php b/tests/unit/Mage/Cms/Helper/Wysiwyg/ImagesTest.php
@@ -26,9 +26,6 @@ class ImagesTest extends TestCase
 {
     public const TEST_STRING = '0123456789';
 
-    /**
-     * @var Mage_Cms_Helper_Wysiwyg_Images
-     */
     public Mage_Cms_Helper_Wysiwyg_Images $subject;
 
     public function setUp(): void