From 2833ed98a653b54079459cf05de2f0844d2b03a2 Mon Sep 17 00:00:00 2001
From: Henning <leutz@pcsg.de>
Date: Sat, 22 Feb 2025 16:45:37 +0100
Subject: [PATCH] fix(phpstan): code improvements and resolve warnings
Reviewed code and made several improvements which provide better error handling, code quality, and
resolved warnings. The changes include:
- Syntax modifications for better readability,
- Ensured variable type checks before using methods,
- Resolved warning instances in phpstan,
- Adapted code to comply with modern PHP nullable type indicator.
Alongside, made version update in phive/phars.xml. Also, unnecessary comments or instances are
removed for a cleaner code.
Related: quiqqer/coupons#16
---
.phive/phars.xml | 2 +-
ajax/backend/getCouponPrice.php | 2 +-
ajax/delete.php | 6 +-
phpstan-baseline.neon | 71 -------------------
src/QUI/ERP/Coupons/CouponCode.php | 14 ++--
src/QUI/ERP/Coupons/Events.php | 2 +-
src/QUI/ERP/Coupons/Handler.php | 10 +--
.../Products/DigitalCouponProductType.php | 7 +-
src/QUI/ERP/Coupons/Products/Handler.php | 1 -
.../Products/PhysicalCouponProductType.php | 4 +-
10 files changed, 26 insertions(+), 93 deletions(-)
diff --git a/.phive/phars.xml b/.phive/phars.xml
index 5bfa092..cccdab5 100644
--- a/.phive/phars.xml
+++ b/.phive/phars.xml
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<phive xmlns="https://phar.io/phive">
- <phar name="phpstan" version="1.11.8" installed="1.11.8" location="./tools/phpstan" copy="false"/>
+ <phar name="phpstan" version="1.*" installed="1.12.13" location="./tools/phpstan" copy="false"/>
<phar name="phpunit" version="^10.5.20" installed="10.5.20" location="./tools/phpunit" copy="false"/>
<phar name="phpcs" version="^3.10.1" installed="3.10.1" location="./tools/phpcs" copy="false"/>
<phar name="phpcbf" version="^3.10.1" installed="3.10.1" location="./tools/phpcbf" copy="false"/>
diff --git a/ajax/backend/getCouponPrice.php b/ajax/backend/getCouponPrice.php
index 7f6cd3a..2d17227 100644
--- a/ajax/backend/getCouponPrice.php
+++ b/ajax/backend/getCouponPrice.php
@@ -28,7 +28,7 @@ function ($couponId, $vat) {
continue;
}
- if ($vat !== false) {
+ if ($vat !== false && method_exists($PriceFactor, 'setVat')) {
$PriceFactor->setVat($vat);
}
diff --git a/ajax/delete.php b/ajax/delete.php
index ca71f5c..2d5ab80 100644
--- a/ajax/delete.php
+++ b/ajax/delete.php
@@ -36,9 +36,11 @@ function ($ids) {
);
return false;
- } catch (QUI\Permissions\Exception $Exception) {
- throw $Exception;
} catch (Exception $Exception) {
+ if ($Exception instanceof QUI\Permissions\Exception) {
+ throw $Exception;
+ }
+
QUI\System\Log::writeException($Exception);
QUI::getMessagesHandler()->addError(
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
index 66cad0d..e69de29 100644
--- a/phpstan-baseline.neon
+++ b/phpstan-baseline.neon
@@ -1,71 +0,0 @@
-parameters:
- ignoreErrors:
- -
- message: "#^Call to an undefined method QUI\\\\ERP\\\\Products\\\\Interfaces\\\\PriceFactorInterface\\:\\:setVat\\(\\)\\.$#"
- count: 1
- path: ajax/backend/getCouponPrice.php
-
- -
- message: "#^Dead catch \\- Exception is never thrown in the try block\\.$#"
- count: 1
- path: ajax/delete.php
-
- -
- message: "#^Call to an undefined method QUI\\\\ERP\\\\Products\\\\Interfaces\\\\PriceFactorInterface\\:\\:setVat\\(\\)\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/CouponCode.php
-
- -
- message: "#^Method QUI\\\\ERP\\\\Coupons\\\\CouponCode\\:\\:getDiscounts\\(\\) should return array\\<QUI\\\\ERP\\\\Discount\\\\Discount\\> but returns array\\<int\\<0, max\\>, QUI\\\\CRUD\\\\Child\\>\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/CouponCode.php
-
- -
- message: "#^Offset 'standard' on array\\{title\\: array\\{de\\: 'Gutschein \\- Versand', en\\: 'Coupon delivery'\\}, type\\: 'ProductAttributeList', public\\: true, standard\\: false, requiredField\\: true, options\\: array\\{entries\\: array\\{array\\{title\\: array\\{de\\: 'per E \\- Mail', en\\: 'via email'\\}, sum\\: 0, type\\: 2, selected\\: true, userinput\\: false\\}, array\\{title\\: array\\{de\\: 'per Post', en\\: 'via mail'\\}, sum\\: 0, type\\: 2, selected\\: false, userinput\\: false\\}\\}\\}\\}\\|array\\{title\\: array\\{de\\: 'Gutschein Wert', en\\: 'Coupon amount'\\}, type\\: 'FloatType', public\\: false, standard\\: false, requiredField\\: true\\}\\|array\\{title\\: array\\{de\\: 'Gutschein\\-Code als…', en\\: 'Provide coupon code…'\\}, description\\: array\\{de\\: 'Der Gutschein wird…', en\\: 'The coupon is also…'\\}, type\\: 'BoolType', public\\: false, standard\\: false, requiredField\\: false\\}\\|array\\{title\\: array\\{de\\: 'Gutschein\\-Code ist…', en\\: 'Coupon code is…'\\}, description\\: array\\{de\\: 'Übertragbare…', en\\: 'Transferable…'\\}, type\\: 'BoolType', public\\: false, standard\\: false, requiredField\\: false\\}\\|array\\{title\\: array\\{de\\: 'Gutschein\\-Code per…', en\\: 'Send coupon code…'\\}, description\\: array\\{de\\: 'Der Gutschein\\-Code…', en\\: 'The coupon code is…'\\}, type\\: 'BoolType', public\\: false, standard\\: false, requiredField\\: false\\}\\|array\\{title\\: array\\{de\\: 'Gutschein\\-Code…', en\\: 'Coupon code…'\\}, type\\: 'IntType', public\\: false, standard\\: false, requiredField\\: true\\}\\|array\\{title\\: array\\{de\\: 'Gutschein…', en\\: 'Coupon description'\\}, description\\: array\\{de\\: 'Diese Beschreibung…', en\\: 'This description…'\\}, type\\: 'InputMultiLang', public\\: false, standard\\: false, requiredField\\: false\\}\\|array\\{title\\: array\\{de\\: 'Ist Einzweck…', en\\: 'Is single purpose…'\\}, description\\: array\\{de\\: 'Einzweck\\-Gutscheine…', en\\: 'Single\\-purpose…'\\}, type\\: 'BoolType', public\\: false, standard\\: false, requiredField\\: false\\}\\|array\\{title\\: array\\{de\\: 'Kunde darf…', en\\: 'Customer can choose…'\\}, description\\: array\\{de\\: 'Ist diese Funktion…', en\\: 'if this option is…'\\}, type\\: 'BoolType', public\\: false, standard\\: false, requiredField\\: false\\} in empty\\(\\) always exists and is always falsy\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Events.php
-
- -
- message: "#^Parameter \\#1 \\$id of static method QUI\\\\ERP\\\\Coupons\\\\Handler\\:\\:getCouponCode\\(\\) expects int, string\\|false given\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Handler.php
-
- -
- message: "#^Call to method createPDF\\(\\) on an unknown class QUI\\\\HtmlToPdf\\\\Document\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Call to method setAttribute\\(\\) on an unknown class QUI\\\\HtmlToPdf\\\\Document\\.$#"
- count: 7
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Call to method setContentHTML\\(\\) on an unknown class QUI\\\\HtmlToPdf\\\\Document\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Call to method setFooterHTML\\(\\) on an unknown class QUI\\\\HtmlToPdf\\\\Document\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Call to method setHeaderHTML\\(\\) on an unknown class QUI\\\\HtmlToPdf\\\\Document\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Instantiated class QUI\\\\HtmlToPdf\\\\Document not found\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Negated boolean expression is always false\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
-
- -
- message: "#^Parameter \\#1 \\$key of method QUI\\\\ERP\\\\Accounting\\\\Article\\:\\:getCustomField\\(\\) expects string, int given\\.$#"
- count: 1
- path: src/QUI/ERP/Coupons/Products/Handler.php
diff --git a/src/QUI/ERP/Coupons/CouponCode.php b/src/QUI/ERP/Coupons/CouponCode.php
index d8af822..9ff795b 100644
--- a/src/QUI/ERP/Coupons/CouponCode.php
+++ b/src/QUI/ERP/Coupons/CouponCode.php
@@ -276,6 +276,7 @@ public function getDiscounts(): array
}
}
+ // @phpstan-ignore-next-line
return $discounts;
}
@@ -307,7 +308,7 @@ public function getGroupIds(): array
* @throws QUI\Database\Exception
* @throws ExceptionStack
*/
- public function redeem(QUI\Interfaces\Users\User $User = null, AbstractOrder $Order = null): void
+ public function redeem(null | QUI\Interfaces\Users\User $User = null, null | AbstractOrder $Order = null): void
{
if (is_null($User)) {
$User = QUI::getUserBySession();
@@ -487,14 +488,13 @@ public function checkOrderRedemption(?OrderInterface $Order): void
foreach ($this->discountIds as $discountId) {
try {
- /** @var QUI\ERP\Discount\Discount $Discount */
$Discount = $DiscountHandler->getChild($discountId);
} catch (Exception $Exception) {
$discountError = $Exception->getMessage();
continue;
}
- if ($Discount->canUsedInOrder($Order)) {
+ if (method_exists($Discount, 'canUsedInOrder') && $Discount->canUsedInOrder($Order)) {
$discountsValid = true;
break;
}
@@ -525,8 +525,10 @@ public function checkOrderRedemption(?OrderInterface $Order): void
* @param OrderInterface|null $Order
* @return bool
*/
- public function isRedeemable(QUI\Interfaces\Users\User $User = null, OrderInterface $Order = null): bool
- {
+ public function isRedeemable(
+ null | QUI\Interfaces\Users\User $User = null,
+ null | OrderInterface $Order = null
+ ): bool {
try {
$this->checkRedemption($User);
} catch (CouponCodeException) {
@@ -731,7 +733,7 @@ public function addToOrder(QUI\ERP\Order\OrderInProcess $Order): void
foreach ($discounts as $Discount) {
$PriceFactor = $Discount->toPriceFactor(null, $Order->getCustomer());
- if ($vat !== false) {
+ if ($vat !== false && method_exists($PriceFactor, 'setVat')) {
$PriceFactor->setVat($vat);
}
diff --git a/src/QUI/ERP/Coupons/Events.php b/src/QUI/ERP/Coupons/Events.php
index d528a83..a14c40e 100644
--- a/src/QUI/ERP/Coupons/Events.php
+++ b/src/QUI/ERP/Coupons/Events.php
@@ -670,7 +670,7 @@ protected static function createProductFields(): void
'workingtitles' => $field['title'],
'description' => !empty($field['description']) ? $field['description'] : null,
'systemField' => 0,
- 'standardField' => !empty($field['standard']) ? 1 : 0,
+ 'standardField' => !empty($field['standard']) ? 1 : 0, // @phpstan-ignore-line
'publicField' => !empty($field['public']) ? 1 : 0,
'options' => !empty($field['options']) ? $field['options'] : null,
'requiredField' => !empty($field['requiredField']) ? 1 : 0
diff --git a/src/QUI/ERP/Coupons/Handler.php b/src/QUI/ERP/Coupons/Handler.php
index ae0a2b0..98594f9 100644
--- a/src/QUI/ERP/Coupons/Handler.php
+++ b/src/QUI/ERP/Coupons/Handler.php
@@ -193,7 +193,7 @@ public static function createCouponCode(array $discountIds, array $settings = []
]);
}
- return self::getCouponCode(QUI::getPDO()->lastInsertId());
+ return self::getCouponCode((int)QUI::getPDO()->lastInsertId());
}
/**
@@ -310,7 +310,7 @@ public static function editCouponCode(int $id, array $discountIds, array $settin
* @return CouponCode[]|int
* @throws CouponCodeException|QUI\Exception
*/
- public static function search(array $searchParams, bool $countOnly = false): array|int
+ public static function search(array $searchParams, bool $countOnly = false): array | int
{
$couponCodes = [];
$Grid = new Grid($searchParams);
@@ -434,7 +434,7 @@ public static function existsCode(string $code): bool
*
* @return QUI\Projects\Site|false
*/
- public static function getRegistrationSite(): bool|QUI\Projects\Site
+ public static function getRegistrationSite(): bool | QUI\Projects\Site
{
try {
$Conf = QUI::getPackage('quiqqer/coupons')->getConfig();
@@ -464,7 +464,7 @@ public static function getRegistrationSite(): bool|QUI\Projects\Site
*
* @throws Exception
*/
- public static function deleteExpiredCouponCodes(int $days = null): void
+ public static function deleteExpiredCouponCodes(null | int $days = null): void
{
$Now = new DateTime();
$where = [
@@ -498,7 +498,7 @@ public static function deleteExpiredCouponCodes(int $days = null): void
*
* @throws Exception
*/
- public static function deleteRedeemedCouponCodes(int $days = null): void
+ public static function deleteRedeemedCouponCodes(null | int $days = null): void
{
$where = [
'useDate' => [
diff --git a/src/QUI/ERP/Coupons/Products/DigitalCouponProductType.php b/src/QUI/ERP/Coupons/Products/DigitalCouponProductType.php
index 6dbd81c..eafd968 100644
--- a/src/QUI/ERP/Coupons/Products/DigitalCouponProductType.php
+++ b/src/QUI/ERP/Coupons/Products/DigitalCouponProductType.php
@@ -4,6 +4,7 @@
use QUI;
use QUI\ERP\Products\Product\Types\DigitalProduct;
+use QUI\Locale;
/**
* Class DigitalProduct
@@ -52,7 +53,7 @@ public function __construct(int $pid, array $product = [])
* @param QUI\Locale|null $Locale
* @return string
*/
- public static function getTypeTitle(QUI\Locale $Locale = null): string
+ public static function getTypeTitle(null | QUI\Locale $Locale = null): string
{
if ($Locale === null) {
$Locale = QUI::getLocale();
@@ -62,10 +63,10 @@ public static function getTypeTitle(QUI\Locale $Locale = null): string
}
/**
- * @param QUI\Locale $Locale
+ * @param Locale|null $Locale
* @return string
*/
- public static function getTypeDescription($Locale = null): string
+ public static function getTypeDescription(null | QUI\Locale $Locale = null): string
{
if ($Locale === null) {
$Locale = QUI::getLocale();
diff --git a/src/QUI/ERP/Coupons/Products/Handler.php b/src/QUI/ERP/Coupons/Products/Handler.php
index 6ffeafe..8e5a715 100644
--- a/src/QUI/ERP/Coupons/Products/Handler.php
+++ b/src/QUI/ERP/Coupons/Products/Handler.php
@@ -57,7 +57,6 @@ public static function createCouponCodesFromOrder(QUI\ERP\Order\AbstractOrder $O
$Customer = $Order->getCustomer();
$Currency = $Order->getCurrency();
- /** @var QUI\ERP\Accounting\Article $Article */
foreach ($Order->getArticles() as $Article) {
try {
// Do not parse coupon codes / discounts
diff --git a/src/QUI/ERP/Coupons/Products/PhysicalCouponProductType.php b/src/QUI/ERP/Coupons/Products/PhysicalCouponProductType.php
index 9b8298b..69635fd 100644
--- a/src/QUI/ERP/Coupons/Products/PhysicalCouponProductType.php
+++ b/src/QUI/ERP/Coupons/Products/PhysicalCouponProductType.php
@@ -51,7 +51,7 @@ public function __construct(int $pid, array $product = [])
* @param QUI\Locale|null $Locale
* @return string
*/
- public static function getTypeTitle(QUI\Locale $Locale = null): string
+ public static function getTypeTitle(null | QUI\Locale $Locale = null): string
{
if ($Locale === null) {
$Locale = QUI::getLocale();
@@ -64,7 +64,7 @@ public static function getTypeTitle(QUI\Locale $Locale = null): string
* @param QUI\Locale $Locale
* @return string
*/
- public static function getTypeDescription($Locale = null): string
+ public static function getTypeDescription(null | QUI\Locale $Locale = null): string
{
if ($Locale === null) {
$Locale = QUI::getLocale();
--
GitLab