Set up PHP Mess Detector (#22)

parcel-pro-nl · Oct 24, 2023 · 0ce4639 · 0ce4639
1 parent f161d45
commit 0ce4639
Show file tree

Hide file tree

Showing 12 changed files with 62 additions and 54 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,3 +15,4 @@ jobs:
         with:
           args: --ignore-platform-reqs
       - run: composer cs:check
+      - run: composer md
diff --git a/Controller/Adminhtml/Order/CustomTab.php b/Controller/Adminhtml/Order/CustomTab.php
@@ -7,6 +7,9 @@
 use Magento\Sales\Api\OrderRepositoryInterface;
 use Psr\Log\LoggerInterface;
 
+/**
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class CustomTab extends \Magento\Sales\Controller\Adminhtml\Order
 {
     /**
@@ -27,9 +30,6 @@ class CustomTab extends \Magento\Sales\Controller\Adminhtml\Order
      * @param OrderRepositoryInterface $orderRepository
      * @param LoggerInterface $logger
      * @param \Magento\Framework\View\LayoutFactory $layoutFactory
-     *
-     * @SuppressWarnings(PHPMD.ExcessiveParameterList)
-     * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
      */
     public function __construct(
         Action\Context $context,

diff --git a/Controller/Adminhtml/Shipment/Index.php b/Controller/Adminhtml/Shipment/Index.php
@@ -2,6 +2,7 @@
 
 namespace Parcelpro\Shipment\Controller\Adminhtml\Shipment;
 
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Setup\Exception;
 use Parcelpro\Shipment\Model\ParcelproFactory;
 
@@ -59,7 +60,7 @@ public function execute()
         } else {
             if ($order_id) {
                 if (is_array($order_id)) {
-                    foreach ($order_id as $k => $v) {
+                    foreach ($order_id as $v) {
                         $message .= $this->postToParcelPro($v);
                         $message .= "<br>";
                     }
@@ -163,7 +164,7 @@ public function postToParcelPro($order_id)
             $response_body = json_decode($response_body, true);
 
             if ($response_code != 200) {
-                throw new \Magento\Framework\Exception\LocalizedException(__(sprintf("De zending is niet succesvol aangemeld bij ParcelPro foutcode: %s, melding: %s", $response_code, $response_body['omschrijving']), 10));
+                throw new LocalizedException(__(sprintf("De zending is niet succesvol aangemeld bij ParcelPro foutcode: %s, melding: %s", $response_code, $response_body['omschrijving']), 10));
             }
 
             if (isset($response_body['Barcode'])) {
@@ -200,7 +201,7 @@ public function postToParcelPro($order_id)
 
                 return sprintf("Order %s succesvol aangemaakt", $order_id);
             }
-        } catch (\Magento\Framework\Exception\LocalizedException $e) {
+        } catch (LocalizedException $e) {
             return sprintf("Order %s geeft een error", $order_id);
         }
     }

diff --git a/Controller/Adminhtml/Shipment/Printlabel.php b/Controller/Adminhtml/Shipment/Printlabel.php
@@ -61,7 +61,7 @@ public function execute()
             if ($order_id) {
                 if (is_array($order_id)) {
                     $url = null;
-                    foreach ($order_id as $k => $v) {
+                    foreach ($order_id as $v) {
                         $res = $this->printlabelAction($v, true, null);
                         if (strpos($res, 'error') !== false) {
                             $message .= $res . "<br>";

diff --git a/Model/Carrier/Parcelpro.php b/Model/Carrier/Parcelpro.php
@@ -112,7 +112,7 @@ public function collectRates(RateRequest $request)
 
         $result = $this->_rateResultFactory->create();
         $am = $this->getAllowedMethods();
-        foreach ($am as $key => $value) {
+        foreach ($am as $key => $_) {
             if ($this->getConfigData($key)) {
                 $objectManager = \Magento\Framework\App\ObjectManager::getInstance();
                 $state = $objectManager->get('\Magento\Framework\App\State');
@@ -121,8 +121,6 @@ public function collectRates(RateRequest $request)
                     $object = $objectManager->create('\Magento\Sales\Model\AdminOrder\Create');
                     $total = $object->getQuote()->getSubtotal();
                     $grandTotal = $object->getQuote()->getGrandTotal();
-
-                    $freeBoxes = $this->getFreeBoxesCount($request);
                 } else {
                     $objectManager = \Magento\Framework\App\ObjectManager::getInstance();
                     $total = $objectManager->create('\Magento\Checkout\Model\Session')
@@ -182,7 +180,6 @@ public function collectRates(RateRequest $request)
 
                     if ($key == "custom_pricerule") {
                         $counter = 0;
-                        $carrier = null;
                         foreach ($pricerules as $pricerule) {
                             if ($pricerule['country'] != $countryId) {
                                 continue;
@@ -217,42 +214,4 @@ public function collectRates(RateRequest $request)
         }
         return $result;
     }
-
-    /**
-     * @param RateRequest $request
-     * @return int
-     */
-    private function getFreeBoxesCount(RateRequest $request)
-    {
-        $freeBoxes = 0;
-        if ($request->getAllItems()) {
-            foreach ($request->getAllItems() as $item) {
-                if ($item->getProduct()->isVirtual() || $item->getParentItem()) {
-                    continue;
-                }
-
-                if ($item->getHasChildren() && $item->isShipSeparately()) {
-                    $freeBoxes += $this->getFreeBoxesCountFromChildren($item);
-                } elseif ($item->getFreeShipping()) {
-                    $freeBoxes += $item->getQty();
-                }
-            }
-        }
-        return $freeBoxes;
-    }
-
-    /**
-     * @param mixed $item
-     * @return mixed
-     */
-    private function getFreeBoxesCountFromChildren($item)
-    {
-        $freeBoxes = 0;
-        foreach ($item->getChildren() as $child) {
-            if ($child->getFreeShipping() && !$child->getProduct()->isVirtual()) {
-                $freeBoxes += $item->getQty() * $child->getQty();
-            }
-        }
-        return $freeBoxes;
-    }
 }
diff --git a/Model/CustomConfigProvider.php b/Model/CustomConfigProvider.php
@@ -26,7 +26,6 @@ public function getConfig()
         $config = $this->getScopeConfig()->getValue('carriers/parcelpro', \Magento\Store\Model\ScopeInterface::SCOPE_STORE);
 
         $gebruikerid = $config["gebruiker_id"];
-        $apikey = $config["api_key"];
 
         $config = [
             'config' => [

diff --git a/Observer/Shipment.php b/Observer/Shipment.php
@@ -3,6 +3,7 @@
 namespace Parcelpro\Shipment\Observer;
 
 use Magento\Framework\Event\ObserverInterface;
+use Magento\Framework\Exception\LocalizedException;
 use Parcelpro\Shipment\Model\ParcelproFactory;
 
 class Shipment implements ObserverInterface
@@ -123,7 +124,7 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                         $response_body = json_decode($response_body, true);
 
                         if ($response_code != 200) {
-                            throw new \Magento\Framework\Exception\LocalizedException(__(sprintf("De zending is niet succesvol aangemeld bij ParcelPro foutcode: %s, melding: %s", $response_code, $response_body['omschrijving']), 10));
+                            throw new LocalizedException(__(sprintf("De zending is niet succesvol aangemeld bij ParcelPro foutcode: %s, melding: %s", $response_code, $response_body['omschrijving']), 10));
                         }
 
                         if (isset($response_body['Barcode'])) {
@@ -152,7 +153,7 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                     }
                 }
             }
-        } catch (\Magento\Framework\Exception\LocalizedException $e) {
+        } catch (LocalizedException $e) {
             $this->logger->debug($e);
         }
     }

diff --git a/Plugin/PluginBefore.php b/Plugin/PluginBefore.php
@@ -8,6 +8,9 @@ class PluginBefore
 {
     protected $_url = 'https://login.parcelpro.nl';
 
+    /**
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     */
     public function beforePushButtons(
         \Magento\Backend\Block\Widget\Button\Toolbar\Interceptor $subject,
         \Magento\Framework\View\Element\AbstractBlock $context,

diff --git a/Setup/InstallSchema.php b/Setup/InstallSchema.php
@@ -8,6 +8,9 @@
 
 class InstallSchema implements InstallSchemaInterface
 {
+    /**
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     */
     public function install(SchemaSetupInterface $setup, ModuleContextInterface $context)
     {
         $setup->startSetup();

diff --git a/Setup/UpgradeSchema.php b/Setup/UpgradeSchema.php
@@ -8,6 +8,9 @@
 
 class UpgradeSchema implements UpgradeSchemaInterface
 {
+    /**
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     */
     public function upgrade(
         SchemaSetupInterface $setup,
         ModuleContextInterface $context

diff --git a/composer.json b/composer.json
@@ -34,7 +34,8 @@
   },
   "require-dev": {
     "squizlabs/php_codesniffer": "^3.7",
-    "magento/community-edition": "^2.4"
+    "magento/community-edition": "^2.4",
+    "phpmd/phpmd": "^2.14"
   },
   "config": {
     "allow-plugins": {
@@ -45,6 +46,8 @@
   },
   "scripts": {
     "cs:check": "phpcs",
-    "cs:fix": "phpcbf"
+    "cs:fix": "phpcbf",
+    "md": "phpmd . ansi phpmd.xml --exclude vendor",
+    "md:text": "phpmd . text phpmd.xml --exclude vendor"
   }
 }
diff --git a/phpmd.xml b/phpmd.xml
@@ -0,0 +1,35 @@
+<?xml version="1.0"?>
+<ruleset name="Parcel Pro PHPMD Ruleset"
+         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>The Parcel Pro mess detection ruleset.</description>
+
+    <!-- See: https://phpmd.org/rules/index.html -->
+
+    <rule ref="rulesets/cleancode.xml">
+        <exclude name="StaticAccess"/>
+        <exclude name="ElseExpression"/>
+        <exclude name="BooleanArgumentFlag"/>
+    </rule>
+
+    <rule ref="rulesets/design.xml"/>
+
+    <rule ref="rulesets/unusedcode.xml">
+        <exclude name="UnusedLocalVariable"/>
+    </rule>
+    <!--
+    We don't want to use the standard "UnusedLocalVariable" rule, since it gives a false-positive in this scenario:
+    `foreach ($arr as $key => $value) {`
+    where only `$key` is used.
+    Instead, we want to be able to use `$_` instead of `$value` to indicate an unused variable.
+    The above example would then become:
+    `foreach ($arr as $key => $_) {`
+    -->
+    <rule ref="rulesets/unusedcode.xml/UnusedLocalVariable">
+        <properties>
+            <property name="exceptions" value="_"/>
+        </properties>
+    </rule>
+</ruleset>