From 78d9b1e2de9e47640396d237109e5c2ff0e013a5 Mon Sep 17 00:00:00 2001
From: Henning <leutz@pcsg.de>
Date: Tue, 4 Mar 2025 13:23:47 +0100
Subject: [PATCH] fix(phpstan): enhance code readability and adopt safer
 practices

Several changes have been made across multiple files to enhance code readability and follow safer
coding practices.

1. Breakdown of long lines of attributes/code into multiple lines with added comments for better
readability. This was done in Submenu.php

2. Replaced '==' operation with 'empty()' function to check for empty strings in Submenu.php

3. Safer checking of if method 'getCachePath' exists on 'getSite' to avoid potential errors &
Ensure the last insert ID is always an integer in the database PDO in DropDownMenu.php and
Factory.php

4. Refactor conditions for more clear type hinting in multiple files; including
Independent/Handler.php, Independent/Items/AbstractMenuItem.php, Independent/Items/Site.php,
Independent/Menu.php, MegaMenu.php, and NavTabs.php

The concept of safe concatenation or use of '|null|' has been introduced where needed across many
files.

Related: quiqqer/package-menu#38
---
 src/QUI/Menu/Bricks/Submenu.php               | 32 ++++++++++---------
 src/QUI/Menu/DropDownMenu.php                 | 10 +++---
 src/QUI/Menu/Independent/Factory.php          |  3 +-
 src/QUI/Menu/Independent/Handler.php          |  6 ++--
 .../Independent/Items/AbstractMenuItem.php    | 10 +++---
 src/QUI/Menu/Independent/Items/Site.php       |  6 ++--
 src/QUI/Menu/Independent/Menu.php             |  8 ++---
 src/QUI/Menu/MegaMenu.php                     | 15 +++++----
 src/QUI/Menu/NavTabs.php                      |  3 +-
 9 files changed, 49 insertions(+), 44 deletions(-)

diff --git a/src/QUI/Menu/Bricks/Submenu.php b/src/QUI/Menu/Bricks/Submenu.php
index 9e8b139..c8d9308 100644
--- a/src/QUI/Menu/Bricks/Submenu.php
+++ b/src/QUI/Menu/Bricks/Submenu.php
@@ -32,19 +32,27 @@ public function __construct(array $attributes = [])
         // defaults values
         $this->setAttributes([
             'class' => 'quiqqer-menu-bricks-submenu',
-            'startId' => false, // id or site link
-            'menuId' => false, // id of an independent menu
-            'template' => 'list-buttonStyle', // 'list-buttonStyle', 'list-simple', 'box-imageTop', 'box-imageOverlay'
+            'startId' => false,
+            // id or site link
+            'menuId' => false,
+            // id of an independent menu
+            'template' => 'list-buttonStyle',
+            // 'list-buttonStyle', 'list-simple', 'box-imageTop', 'box-imageOverlay'
             'controlBgColor' => '',
             'controlBgPadding' => '1rem',
             'linkColor' => '',
             'linkColorHover' => '',
-            'itemsAlignment' => 'center', // 'start', 'center', 'end', 'space-between', 'space-around'
-            'showImages' => true, // if true, icons or images will be displayed
-            'imageFitMode' => 'cover', // any valid css property for image-fit attribute , i.e. 'cover', 'contain', 'scale-down'
-            'imageContainerHeight' => '',// any valid css property (with unit!) for height attribute, i.e. '150px', '10vw' or even clamp() function (if no value passed the container will be a square)
+            'itemsAlignment' => 'center',
+            // 'start', 'center', 'end', 'space-between', 'space-around'
+            'showImages' => true,
+            // if true, icons or images will be displayed
+            'imageFitMode' => 'cover',
+            // any valid css property for image-fit attribute , i.e. 'cover', 'contain', 'scale-down'
+            'imageContainerHeight' => '',
+            // any valid css property (with unit!) for height attribute, i.e. '150px', '10vw' or even clamp() function (if no value passed the container will be a square)
             'boxBgColor' => '#f5f5f6',
-            'boxWidth' => '250px'// any valid css property (with unit!) for height attribute, i.e. '250px', '10vw' or even clamp() function
+            'boxWidth' => '250px'
+            // any valid css property (with unit!) for height attribute, i.e. '250px', '10vw' or even clamp() function
         ]);
 
         parent::__construct($attributes);
@@ -52,12 +60,6 @@ public function __construct(array $attributes = [])
         $this->setAttribute('cacheable', false);
     }
 
-    /**
-     * (non-PHPdoc)
-     *
-     * @throws Exception
-     * @see \QUI\Control::create()
-     */
     public function getBody(): string
     {
         $Engine = QUI::getTemplateManager()->getEngine();
@@ -70,7 +72,7 @@ public function getBody(): string
 
         $linkColorHover = $this->getAttribute('linkColorHover');
 
-        if ($linkColor === '') {
+        if (empty($linkColor)) {
             $linkColorHover = 'inherit';
         }
 
diff --git a/src/QUI/Menu/DropDownMenu.php b/src/QUI/Menu/DropDownMenu.php
index 9b65eec..61535a9 100644
--- a/src/QUI/Menu/DropDownMenu.php
+++ b/src/QUI/Menu/DropDownMenu.php
@@ -48,16 +48,18 @@ public function __construct(array $attributes = [])
     public function getBody(): string
     {
         $cache = EventHandler::menuCacheName() . '/dropDownMenu/';
+        $siteCachePath = '';
 
         $attributes = $this->getAttributes();
         $attributes = array_filter($attributes, function ($entry) {
             return is_object($entry) === false;
         });
 
-        $cache .= md5(
-            $this->getSite()->getCachePath() .
-            serialize($attributes)
-        );
+        if (method_exists($this->getSite(), 'getCachePath')) {
+            $siteCachePath = $this->getSite()->getCachePath();
+        }
+
+        $cache .= md5($siteCachePath . serialize($attributes));
 
         try {
             return QUI\Cache\Manager::get($cache);
diff --git a/src/QUI/Menu/Independent/Factory.php b/src/QUI/Menu/Independent/Factory.php
index 884af3b..2a456aa 100644
--- a/src/QUI/Menu/Independent/Factory.php
+++ b/src/QUI/Menu/Independent/Factory.php
@@ -24,8 +24,7 @@ public static function createMenu(): Menu
             'data' => ''
         ]);
 
-        $lastId = QUI::getPDO()->lastInsertId();
-
+        $lastId = (int)QUI::getPDO()->lastInsertId();
         $Menu = Handler::getMenu($lastId);
 
         try {
diff --git a/src/QUI/Menu/Independent/Handler.php b/src/QUI/Menu/Independent/Handler.php
index 8e76687..a341696 100644
--- a/src/QUI/Menu/Independent/Handler.php
+++ b/src/QUI/Menu/Independent/Handler.php
@@ -36,8 +36,10 @@ public static function getMenu(int $menuId): Menu
      * @param QUI\Projects\Project|null $Project
      * @return string
      */
-    public static function getMenuCacheName(bool|int $menuId = false, QUI\Projects\Project $Project = null): string
-    {
+    public static function getMenuCacheName(
+        bool | int $menuId = false,
+        null | QUI\Projects\Project $Project = null
+    ): string {
         if ($Project) {
             $project = $Project->getName();
             $lang = $Project->getLang();
diff --git a/src/QUI/Menu/Independent/Items/AbstractMenuItem.php b/src/QUI/Menu/Independent/Items/AbstractMenuItem.php
index c8317f6..96bbc9b 100644
--- a/src/QUI/Menu/Independent/Items/AbstractMenuItem.php
+++ b/src/QUI/Menu/Independent/Items/AbstractMenuItem.php
@@ -32,7 +32,7 @@ public function __construct(array $attributes = [])
      * @param ?Locale $Locale
      * @return string
      */
-    public function getTitle(Locale $Locale = null): string
+    public function getTitle(null | Locale $Locale = null): string
     {
         if ($Locale === null) {
             $Locale = QUI::getLocale();
@@ -56,7 +56,7 @@ public function getTitle(Locale $Locale = null): string
      * @param ?Locale $Locale
      * @return string
      */
-    public function getShort(Locale $Locale = null): string
+    public function getShort(null | Locale $Locale = null): string
     {
         if ($Locale === null) {
             $Locale = QUI::getLocale();
@@ -86,7 +86,7 @@ public function getShort(Locale $Locale = null): string
      * @param Locale|null $Locale
      * @return string
      */
-    public function getName(Locale $Locale = null): string
+    public function getName(null | Locale $Locale = null): string
     {
         if ($Locale === null) {
             $Locale = QUI::getLocale();
@@ -117,7 +117,7 @@ public function getName(Locale $Locale = null): string
      * @param Locale|null $Locale
      * @return string
      */
-    public function getTitleAttribute(Locale $Locale = null): string
+    public function getTitleAttribute(null | Locale $Locale = null): string
     {
         return $this->getName($Locale);
     }
@@ -222,7 +222,7 @@ public function getCustomData(): mixed
      * @param Locale|null $Locale
      * @return string
      */
-    public function getHTML(QUI\Locale $Locale = null): string
+    public function getHTML(null | Locale $Locale = null): string
     {
         if ($Locale === null) {
             $Locale = QUI::getLocale();
diff --git a/src/QUI/Menu/Independent/Items/Site.php b/src/QUI/Menu/Independent/Items/Site.php
index 3d49d12..502dab0 100644
--- a/src/QUI/Menu/Independent/Items/Site.php
+++ b/src/QUI/Menu/Independent/Items/Site.php
@@ -102,7 +102,7 @@ public function getIcon(): string
      * @param Locale|null $Locale
      * @return string
      */
-    public function getName(Locale $Locale = null): string
+    public function getName(null | Locale $Locale = null): string
     {
         $Site = $this->getSite();
 
@@ -114,10 +114,10 @@ public function getName(Locale $Locale = null): string
     }
 
     /**
-     * @param QUI\Locale|null $Locale
+     * @param Locale|null $Locale
      * @return string
      */
-    public function getTitle(QUI\Locale $Locale = null): string
+    public function getTitle(null | Locale $Locale = null): string
     {
         $Site = $this->getSite();
 
diff --git a/src/QUI/Menu/Independent/Menu.php b/src/QUI/Menu/Independent/Menu.php
index 9f9d1df..b901fa0 100644
--- a/src/QUI/Menu/Independent/Menu.php
+++ b/src/QUI/Menu/Independent/Menu.php
@@ -34,7 +34,7 @@ class Menu
      * @throws QUI\Exception
      * @throws QUI\Database\Exception
      */
-    public function __construct(int|array $menuId)
+    public function __construct(int | array $menuId)
     {
         if (is_numeric($menuId)) {
             $data = Handler::getMenuData($menuId);
@@ -93,7 +93,7 @@ public function __construct(int|array $menuId)
      * @param array $children
      * @return void
      */
-    protected function buildChildren(AbstractMenuItem|Menu $Parent, array $children): void
+    protected function buildChildren(AbstractMenuItem | Menu $Parent, array $children): void
     {
         foreach ($children as $item) {
             $type = $item['type'];
@@ -180,7 +180,7 @@ public function getId(): int
      * @param QUI\Locale|null $Locale
      * @return string
      */
-    public function getTitle(QUI\Locale $Locale = null): string
+    public function getTitle(null | QUI\Locale $Locale = null): string
     {
         if ($this->title === null) {
             return '';
@@ -203,7 +203,7 @@ public function getTitle(QUI\Locale $Locale = null): string
      * @param QUI\Locale|null $Locale
      * @return string
      */
-    public function getWorkingTitle(QUI\Locale $Locale = null): string
+    public function getWorkingTitle(null | QUI\Locale $Locale = null): string
     {
         if ($this->workingTitle === null) {
             return '';
diff --git a/src/QUI/Menu/MegaMenu.php b/src/QUI/Menu/MegaMenu.php
index 542a515..9b2ee22 100644
--- a/src/QUI/Menu/MegaMenu.php
+++ b/src/QUI/Menu/MegaMenu.php
@@ -26,7 +26,7 @@ class MegaMenu extends AbstractMenu
     /**
      * @var SlideOut|SlideOutAdvanced|null
      */
-    protected SlideOutAdvanced|null|SlideOut $Mobile = null;
+    protected SlideOutAdvanced | null | SlideOut $Mobile = null;
 
     /**
      * @var array
@@ -101,17 +101,18 @@ public function __construct(array $attributes = [])
     public function getBody(): string
     {
         $cache = EventHandler::menuCacheName() . '/megaMenu/';
+        $siteCachePath = '';
 
         $attributes = $this->getAttributes();
         $attributes = array_filter($attributes, function ($entry) {
             return is_object($entry) === false;
         });
 
-        $cache .= md5(
-            $this->getSite()->getCachePath() .
-            serialize($attributes)
-        );
+        if (method_exists($this->getSite(), 'getCachePath')) {
+            $siteCachePath = $this->getSite()->getCachePath();
+        }
 
+        $cache .= md5($siteCachePath . serialize($attributes));
         $childControl = $this->getMenuControl($this->getAttribute('display'));
 
         $showMenuDelay = 0;
@@ -245,7 +246,7 @@ public function getStart(): QUI\Interfaces\Projects\Site
      * @param $control
      * @return false|string
      */
-    public function getMenuControl($control): bool|string
+    public function getMenuControl($control): bool | string
     {
         switch ($control) {
             case 'Image':
@@ -333,7 +334,7 @@ protected function getSite(): QUI\Interfaces\Projects\Site
      * @throws QUI\Exception
      * @throws Exception
      */
-    protected function getMobileMenu($slideOutParam): SlideOut|SlideOutAdvanced
+    protected function getMobileMenu($slideOutParam): SlideOut | SlideOutAdvanced
     {
         if ($this->getProject()->getConfig('mobileMenu.settings.type') == 'slideoutAdvanced') {
             $Menu = new QUI\Menu\SlideOutAdvanced($slideOutParam);
diff --git a/src/QUI/Menu/NavTabs.php b/src/QUI/Menu/NavTabs.php
index 3f65ada..4cf3bb1 100644
--- a/src/QUI/Menu/NavTabs.php
+++ b/src/QUI/Menu/NavTabs.php
@@ -86,7 +86,7 @@ public function getBody(): string
      * @return array|string
      * @throws QUI\Exception
      */
-    private function getChildrenFromParent(): array|string
+    private function getChildrenFromParent(): array | string
     {
         $ParentSite = null;
 
@@ -152,7 +152,6 @@ public function setData(array $data)
 
         $entries = [];
 
-        /** @var QUI\Projects\Site $Site */
         foreach ($data as $dataSet) {
             if (count($dataSet) < 2) {
                 continue;
-- 
GitLab