fix small issues, improve code

phpstan.neon

@@ -1,8 +1,10 @@
 parameters:
     scanFiles:
-      - src/IntelliSenseHelper.php
+        - src/IntelliSenseHelper.php
-      - ../../oxid-esales/oxideshop-ce/source/oxfunctions.php
+        - ../../oxid-esales/oxideshop-ce/source/oxfunctions.php
     paths:
         - src
-    level: 0
+    level: 9
     phpVersion: 70100
+    ignoreErrors:
+        - '#Psr\\Container\\ContainerExceptionInterface is not subtype of Throwable#'

src/Application/Controller/Admin/AdminOrder.php

@@ -58,24 +58,25 @@ class AdminOrder extends AdminController
         try {
             return oxNew(OrderRecipients::class, $this->order)->getSmsRecipient();
         } catch (noRecipientFoundException $e) {
-            Registry::getUtilsView()->addErrorToDisplay(
+            /** @var string $message */
-                Registry::getLang()->translateString($e->getMessage())
+            $message = Registry::getLang()->translateString($e->getMessage());
-            );
+            Registry::getUtilsView()->addErrorToDisplay($message);
         }
         return false;
     }
 
     /**
+     * @return void
      * @throws Exception
      */
-    public function send()
+    public function send(): void
     {
         $messageBody = Registry::getRequest()->getRequestEscapedParameter('messagebody');
 
         if (false === is_string($messageBody) || strlen($messageBody) <= 1) {
-            Registry::getUtilsView()->addErrorToDisplay(
+            /** @var string $message */
-                Registry::getLang()->translateString('D3LM_EXC_MESSAGE_NO_LENGTH')
+            $message = Registry::getLang()->translateString('D3LM_EXC_MESSAGE_NO_LENGTH');
-            );
+            Registry::getUtilsView()->addErrorToDisplay($message);
             return;
         }
 
@@ -85,17 +86,15 @@ class AdminOrder extends AdminController
         try {
             $sms = oxNew(Sms::class, $messageBody);
             if ($sms->sendOrderMessage($order)) {
+                $smsCount = $sms->getResponse() ? $sms->getResponse()->getSmsCount() : 0;
                 Registry::getUtilsView()->addErrorToDisplay(
-                    oxNew(successfullySentException::class, $sms->getResponse()->getSmsCount())
+                    oxNew(successfullySentException::class, $smsCount)
                 );
             } else {
                 $errorMsg = $sms->getResponse() instanceof ResponseInterface ? $sms->getResponse()->getErrorMessage() : 'no response';
-                Registry::getUtilsView()->addErrorToDisplay(
+                /** @var string $format */
-                    sprintf(
+                $format = Registry::getLang()->translateString('D3LM_EXC_MESSAGE_UNEXPECTED_ERR_SEND');
-                        Registry::getLang()->translateString('D3LM_EXC_MESSAGE_UNEXPECTED_ERR_SEND'),
+                Registry::getUtilsView()->addErrorToDisplay(sprintf($format, $errorMsg));
-                        $errorMsg
-                    )
-                );
             }
         } catch (noRecipientFoundException $e) {
             Registry::getUtilsView()->addErrorToDisplay($e);

src/Application/Controller/Admin/AdminUser.php

@@ -57,24 +57,26 @@ class AdminUser extends AdminController
         try {
             return oxNew(UserRecipients::class, $this->user)->getSmsRecipient();
         } catch (noRecipientFoundException $e) {
-            Registry::getUtilsView()->addErrorToDisplay(
+            /** @var string $message */
-                Registry::getLang()->translateString($e->getMessage())
+            $message = Registry::getLang()->translateString($e->getMessage());
-            );
+            Registry::getUtilsView()->addErrorToDisplay($message);
         }
         return false;
     }
 
     /**
+     * @return void
      * @throws Exception
      */
-    public function send()
+    public function send(): void
     {
+        /** @var string $messageBody */
         $messageBody = Registry::getRequest()->getRequestEscapedParameter('messagebody');
 
         if (strlen($messageBody) <= 1) {
-            Registry::getUtilsView()->addErrorToDisplay(
+            /** @var string $message */
-                Registry::getLang()->translateString('D3LM_EXC_MESSAGE_NO_LENGTH')
+            $message = Registry::getLang()->translateString('D3LM_EXC_MESSAGE_NO_LENGTH');
-            );
+            Registry::getUtilsView()->addErrorToDisplay($message);
             return;
         }
 
@@ -83,20 +85,15 @@ class AdminUser extends AdminController
 
         $sms = oxNew(Sms::class, $messageBody);
         if ($sms->sendUserAccountMessage($user)) {
-            Registry::getUtilsView()->addErrorToDisplay(
+            /** @var string $format */
-                sprintf(
+            $format = Registry::getLang()->translateString('D3LM_EXC_SMS_SUCC_SENT');
-                    Registry::getLang()->translateString('D3LM_EXC_SMS_SUCC_SENT'),
+            $smsCount = $sms->getResponse() ? $sms->getResponse()->getSmsCount() : 0;
-                    $sms->getResponse()->getSmsCount()
+            Registry::getUtilsView()->addErrorToDisplay(sprintf($format, $smsCount));
-                )
-            );
         } else {
             $errorMsg = $sms->getResponse() instanceof ResponseInterface ? $sms->getResponse()->getErrorMessage() : 'no response';
-            Registry::getUtilsView()->addErrorToDisplay(
+            /** @var string $format */
-                sprintf(
+            $format = Registry::getLang()->translateString('D3LM_EXC_MESSAGE_UNEXPECTED_ERR_SEND');
-                    Registry::getLang()->translateString('D3LM_EXC_MESSAGE_UNEXPECTED_ERR_SEND'),
+            Registry::getUtilsView()->addErrorToDisplay(sprintf($format, $errorMsg));
-                    $errorMsg
-                )
-            );
         }
     }
 }

src/Application/Model/Configuration.php

@@ -43,7 +43,9 @@ class Configuration
      */
     public function getApiToken(): string
     {
-        $token = trim(Registry::getConfig()->getConfigParam(self::GENERAL_APITOKEN));
+        /** @var string $token */
+        $token = Registry::getConfig()->getConfigParam(self::GENERAL_APITOKEN);
+        $token = trim($token);
 
         Assert::that($token)->string()->notEmpty();
 
@@ -63,7 +65,9 @@ class Configuration
      */
     public function getSmsSenderNumber()
     {
-        $number = trim(Registry::getConfig()->getConfigParam(self::SMS_SENDERNR));
+        /** @var string $number */
+        $number = Registry::getConfig()->getConfigParam(self::SMS_SENDERNR);
+        $number = trim($number);
 
         return strlen($number) ? $number : null;
     }
@@ -71,9 +75,11 @@ class Configuration
     /**
      * @return string|null
      */
-    public function getSmsSenderCountry(): string
+    public function getSmsSenderCountry(): ?string
     {
-        $country = trim(Registry::getConfig()->getConfigParam(self::SMS_SENDERCOUNTRY));
+        /** @var string $country */
+        $country = Registry::getConfig()->getConfigParam(self::SMS_SENDERCOUNTRY);
+        $country = trim($country);
         $country = strlen($country) ? strtoupper($country) : null;
 
         Assert::that($country)->nullOr()->string()->length(2);
@@ -82,10 +88,11 @@ class Configuration
     }
 
     /**
-     * @return array
+     * @return string[]
      */
     public function getOrderRecipientFields(): array
     {
+        /** @var string[] $customFields */
         $customFields = Registry::getConfig()->getConfigParam(self::ORDER_RECFIELDS);
 
         array_walk(
@@ -101,10 +108,11 @@ class Configuration
     }
 
     /**
-     * @return array
+     * @return string[]
      */
     public function getUserRecipientFields(): array
     {
+        /** @var string[] $customFields */
         $customFields = Registry::getConfig()->getConfigParam(self::USER_RECFIELDS);
 
         array_walk(
@@ -120,12 +128,13 @@ class Configuration
     }
 
     /**
-     * @param $checkPhoneFieldName
+     * @template T
-     * @param $checkCountryFieldName
+     * @param string $checkPhoneFieldName
-     * @param $args
+     * @param string $checkCountryFieldName
+     * @param array{checkKeys: bool, checkClassName: class-string<T>} $args
      * @return void
      */
-    protected function checkFieldExists(&$checkPhoneFieldName, $checkCountryFieldName, $args)
+    protected function checkFieldExists(string &$checkPhoneFieldName, string $checkCountryFieldName, array $args): void
     {
         $checkCountryFieldName = $args[self::ARGS_CHECKKEYS] ? trim($checkCountryFieldName) : $checkCountryFieldName;
         $checkPhoneFieldName = trim($checkPhoneFieldName);

src/Application/Model/Exceptions/abortSendingExceptionInterface.php

@@ -15,6 +15,6 @@ declare(strict_types=1);
 
 namespace D3\Linkmobility4OXID\Application\Model\Exceptions;
 
-interface abortSendingExceptionInterface
+interface abortSendingExceptionInterface extends \Throwable
 {
 }

src/Application/Model/Exceptions/successfullySentException.php

@@ -16,19 +16,21 @@ declare(strict_types=1);
 namespace D3\Linkmobility4OXID\Application\Model\Exceptions;
 
 use Exception;
+use OxidEsales\Eshop\Core\Exception\StandardException;
 use OxidEsales\Eshop\Core\Registry;
-use Throwable;
 
-class successfullySentException extends Exception
+class successfullySentException extends StandardException
 {
     /**
      * @param int            $messageCount
      * @param int            $code
-     * @param Throwable|null $previous
+     * @param Exception|null $previous
      */
-    public function __construct($messageCount = 1, $code = 0, Throwable $previous = null)
+    public function __construct($messageCount = 1, $code = 0, Exception $previous = null)
     {
-        $message = sprintf(Registry::getLang()->translateString('D3LM_EXC_SMS_SUCC_SENT'), $messageCount);
+        /** @var string $format */
+        $format = Registry::getLang()->translateString('D3LM_EXC_SMS_SUCC_SENT');
+        $message = sprintf($format, $messageCount);
 
         parent::__construct($message, $code, $previous);
     }

src/Application/Model/MessageSender.php

@@ -24,10 +24,11 @@ class MessageSender
 {
     /**
      * @param Order $order
-     * @param       $messageBody
+     * @param string $messageBody
+     * @return void
      * @throws Exception
      */
-    public function sendOrderFinishedMessage(Order $order, $messageBody)
+    public function sendOrderFinishedMessage(Order $order, string $messageBody): void
     {
         if ((oxNew(Configuration::class))->sendOrderFinishedMessage()) {
             $this->sendMessageByOrder($order, $messageBody);
@@ -36,10 +37,11 @@ class MessageSender
 
     /**
      * @param Order $order
-     * @param       $messageBody
+     * @param string $messageBody
+     * @return void
      * @throws Exception
      */
-    public function sendSendedNowMessage(Order $order, $messageBody)
+    public function sendSendedNowMessage(Order $order, string $messageBody): void
     {
         if ((oxNew(Configuration::class))->sendOrderSendedNowMessage()) {
             $this->sendMessageByOrder($order, $messageBody);
@@ -48,10 +50,11 @@ class MessageSender
 
     /**
      * @param Order $order
-     * @param       $messageBody
+     * @param string $messageBody
+     * @return void
      * @throws Exception
      */
-    public function sendCancelOrderMessage(Order $order, $messageBody)
+    public function sendCancelOrderMessage(Order $order, string $messageBody): void
     {
         if ((oxNew(Configuration::class))->sendOrderCanceledMessage()) {
             $this->sendMessageByOrder($order, $messageBody);
@@ -60,10 +63,11 @@ class MessageSender
 
     /**
      * @param Order $order
-     * @param       $messageBody
+     * @param string $messageBody
+     * @return void
      * @throws Exception
      */
-    public function sendMessageByOrder(Order $order, $messageBody)
+    public function sendMessageByOrder(Order $order, string $messageBody): void
     {
         if ((bool) strlen(trim($messageBody)) === false) {
             return;

src/Application/Model/MessageTypes/AbstractMessage.php

@@ -24,11 +24,16 @@ abstract class AbstractMessage
 {
     public const REMARK_IDENT = 'LINKMOB';
 
+    /** @var string */
     protected $message;
+    /** @var bool */
     protected $removeLineBreaks = true;
+    /** @var bool */
     protected $removeMultipleSpaces = true;
 
+    /** @var ResponseInterface */
     protected $response;
+    /** @var Recipient[] */
     protected $recipients = [];
 
     /**
@@ -48,13 +53,14 @@ abstract class AbstractMessage
     }
 
     /**
-     * @param $userId
+     * @param string $userId
-     * @param $recipients
+     * @param string $recipients
-     * @param $message
+     * @param string $message
      *
+     * @return void
      * @throws Exception
      */
-    protected function setRemark($userId, $recipients, $message)
+    protected function setRemark(string $userId, string $recipients, string $message): void
     {
         $remark = oxNew(Remark::class);
         $remark->assign([
@@ -74,7 +80,7 @@ abstract class AbstractMessage
     }
 
     /**
-     * @param array $recipients
+     * @param array<Recipient> $recipients
      * @return void
      */
     protected function setRecipients(array $recipients)
@@ -97,16 +103,16 @@ abstract class AbstractMessage
     }
 
     /**
-     * @param $message
+     * @param string $message
      *
      * @return string
      */
-    protected function sanitizeMessage($message): string
+    protected function sanitizeMessage(string $message): string
     {
         $message = trim(strip_tags($message));
         $message = $this->removeLineBreaks ? str_replace(["\r", "\n"], ' ', $message) : $message;
         $regexp = '/\s{2,}/m';
-        return $this->removeMultipleSpaces ? preg_replace($regexp, ' ', $message) : $message;
+        return $this->removeMultipleSpaces ? (string) preg_replace($regexp, ' ', $message) : $message;
     }
 
     abstract public function getTypeName(): string;

src/Application/Model/MessageTypes/Sms.php

@@ -25,6 +25,7 @@ use D3\Linkmobility4OXID\Application\Model\UserRecipients;
 use D3\LinkmobilityClient\Exceptions\ApiException;
 use D3\LinkmobilityClient\Request\RequestInterface;
 use D3\LinkmobilityClient\SMS\SmsRequestInterface;
+use D3\LinkmobilityClient\ValueObject\Recipient;
 use D3\LinkmobilityClient\ValueObject\Sender;
 use Exception;
 use GuzzleHttp\Exception\GuzzleException;
@@ -87,7 +88,7 @@ class Sms extends AbstractMessage
     }
 
     /**
-     * @param array $recipientsArray
+     * @param array<Recipient> $recipientsArray
      *
      * @return bool
      */
@@ -126,16 +127,17 @@ class Sms extends AbstractMessage
             return $response->isSuccessful();
         } catch (abortSendingExceptionInterface $e) {
             Registry::getLogger()->warning($e->getMessage());
-            Registry::getUtilsView()->addErrorToDisplay($e);
+            // Oxid does not accept throwable interface only exceptions according by definition
+            Registry::getUtilsView()->addErrorToDisplay($e->getMessage());
         } catch (GuzzleException $e) {
             Registry::getLogger()->warning($e->getMessage());
-            Registry::getUtilsView()->addErrorToDisplay($e);
+            Registry::getUtilsView()->addErrorToDisplay($e->getMessage());
         } catch (ApiException $e) {
             Registry::getLogger()->warning($e->getMessage());
-            Registry::getUtilsView()->addErrorToDisplay($e);
+            Registry::getUtilsView()->addErrorToDisplay($e->getMessage());
         } catch (InvalidArgumentException $e) {
             Registry::getLogger()->warning($e->getMessage());
-            Registry::getUtilsView()->addErrorToDisplay($e);
+            Registry::getUtilsView()->addErrorToDisplay($e->getMessage());
         }
 
         return false;

src/Application/Model/OrderRecipients.php

@@ -42,12 +42,16 @@ class OrderRecipients
     public function getSmsRecipient(): Recipient
     {
         foreach ($this->getSmsRecipientFields() as $phoneFieldName => $countryIdFieldName) {
-            $content = trim((string) $this->order->getFieldData($phoneFieldName));
+            /** @var string $content */
+            $content = $this->order->getFieldData($phoneFieldName) ?: '';
+            $content = trim($content);
             $country = oxNew(Country::class);
 
             try {
                 if (strlen($content)) {
-                    $country->load($this->order->getFieldData($countryIdFieldName));
+                    /** @var string $countryId */
+                    $countryId = $this->order->getFieldData(trim($countryIdFieldName));
+                    $country->load($countryId);
                     return oxNew(Recipient::class, $content, $country->getFieldData('oxisoalpha2'));
                 }
             } catch (NumberParseException $e) {

src/Application/Model/RequestFactory.php

@@ -25,6 +25,7 @@ class RequestFactory extends \D3\LinkmobilityClient\SMS\RequestFactory
     {
         $configuration = oxNew(Configuration::class);
 
+        /** @var SmsRequestInterface $request */
         $request = parent::getSmsRequest();
         $request->setTestMode($configuration->getTestMode())
             ->setSenderAddress(

src/Application/Model/UserRecipients.php

@@ -42,12 +42,16 @@ class UserRecipients
     public function getSmsRecipient(): Recipient
     {
         foreach ($this->getSmsRecipientFields() as $fieldName) {
-            $content = trim($this->user->getFieldData($fieldName));
+            /** @var string $content */
+            $content = $this->user->getFieldData($fieldName) ?: '';
+            $content = trim($content);
             $country = oxNew(Country::class);
 
             try {
                 if (strlen($content)) {
-                    $country->load($this->user->getFieldData('oxcountryid'));
+                    /** @var string $countryId */
+                    $countryId = $this->user->getFieldData('oxcountryid');
+                    $country->load($countryId);
                     return oxNew(Recipient::class, $content, $country->getFieldData('oxisoalpha2'));
                 }
             } catch (NumberParseException $e) {

src/Modules/Application/Model/OrderModel.php

@@ -17,14 +17,21 @@ namespace D3\Linkmobility4OXID\Modules\Application\Model;
 
 use D3\Linkmobility4OXID\Modules\Core\EmailCore;
 use OxidEsales\Eshop\Core\Email;
+use Psr\Container\ContainerExceptionInterface;
+use Psr\Container\NotFoundExceptionInterface;
 
 class OrderModel extends OrderModel_parent
 {
-    public function cancelOrder()
+    /**
+     * @return void
+     * @throws ContainerExceptionInterface
+     * @throws NotFoundExceptionInterface
+     */
+    public function cancelOrder(): void
     {
         parent::cancelOrder();
 
-        if ($this->getFieldData('oxstorno') === 1) {
+        if ((bool) $this->getFieldData('oxstorno') === true) {
             /** @var EmailCore $Email */
             $Email = oxNew(Email::class);
             $Email->d3SendCancelMessage($this);

src/Modules/Core/EmailCore.php

@@ -26,13 +26,16 @@ use Psr\Container\NotFoundExceptionInterface;
 
 class EmailCore extends EmailCore_parent
 {
+    /** @var string */
     protected $d3OrderCustSmsTemplate = 'd3sms_ordercust.tpl';
+    /** @var string */
     protected $d3OrderSendedNowSmsTemplate = 'd3sms_sendednow.tpl';
+    /** @var string */
     protected $d3OrderCanceledSmsTemplate = 'd3sms_ordercanceled.tpl';
 
     /**
      * @param Order $order
-     * @param null  $subject
+     * @param string $subject
      *
      * @return bool
      * @throws ContainerExceptionInterface
@@ -49,7 +52,7 @@ class EmailCore extends EmailCore_parent
 
     /**
      * @param Order $order
-     * @param null  $subject
+     * @param string $subject
      *
      * @return bool
      * @throws ContainerExceptionInterface
@@ -67,11 +70,12 @@ class EmailCore extends EmailCore_parent
     /**
      * @param Order $order
      *
+     * @return void
      * @throws ContainerExceptionInterface
      * @throws NotFoundExceptionInterface
      * @throws Exception
      */
-    public function d3SendOrderFinishedMessageToUser(Order $order)
+    public function d3SendOrderFinishedMessageToUser(Order $order): void
     {
         $messageSender = oxNew(MessageSender::class);
         $messageSender->sendOrderFinishedMessage($order, $this->d3GetOrderFinishedSmsMessageBody($order));
@@ -95,11 +99,12 @@ class EmailCore extends EmailCore_parent
     /**
      * @param Order $order
      *
+     * @return void
      * @throws ContainerExceptionInterface
      * @throws NotFoundExceptionInterface
      * @throws Exception
      */
-    public function d3SendedNowMessage(Order $order)
+    public function d3SendedNowMessage(Order $order): void
     {
         $messageSender = oxNew(MessageSender::class);
         $messageSender->sendSendedNowMessage($order, $this->d3GetSendedNowSmsMessageBody($order));
@@ -121,13 +126,14 @@ class EmailCore extends EmailCore_parent
     }
 
     /**
-     * @param $order
+     * @param Order $order
      *
+     * @return void
      * @throws ContainerExceptionInterface
      * @throws NotFoundExceptionInterface
      * @throws Exception
      */
-    public function d3SendCancelMessage($order)
+    public function d3SendCancelMessage(Order $order): void
     {
         $messageSender = oxNew(MessageSender::class);
         $messageSender->sendCancelOrderMessage($order, $this->d3GetCancelOrderSmsMessageBody($order));
@@ -157,6 +163,7 @@ class EmailCore extends EmailCore_parent
      */
     protected function d3GetTplRenderer(): TemplateRendererInterface
     {
+        /** @var TemplateRendererBridgeInterface $bridge */
         $bridge = ContainerFactory::getInstance()->getContainer()
             ->get(TemplateRendererBridgeInterface::class);
         $bridge->setEngine($this->_getSmarty());