PHP 8: Code Review UIComponent

ILIAS-eLearning · Mar 16, 2022 · 30bb612 · 30bb612
1 parent d4ada43
commit 30bb612
Show file tree

Hide file tree

Showing 15 changed files with 199 additions and 216 deletions.
diff --git a/Services/UIComponent/AdvancedSelectionList/classes/class.ilAdvancedSelectionListGUI.php b/Services/UIComponent/AdvancedSelectionList/classes/class.ilAdvancedSelectionListGUI.php
diff --git a/Services/UIComponent/Button/classes/class.ilButtonBase.php b/Services/UIComponent/Button/classes/class.ilButtonBase.php
@@ -180,7 +180,7 @@ protected function renderAttributesHelper(array $a_attr) : string
             }
         }
 
-        if (sizeof($res)) {
+        if (count($res)) {
             return " " . implode(" ", $res);
         }
         return "";
@@ -200,7 +200,7 @@ protected function renderAttributes(array $a_additional_attr = null) : string
             $attr["disabled"] = "disabled";
         }
 
-        if (sizeof($a_additional_attr)) {
+        if (count($a_additional_attr)) {
             $attr = array_merge($attr, $a_additional_attr);
         }
 

diff --git a/Services/UIComponent/Button/classes/class.ilImageLinkButton.php b/Services/UIComponent/Button/classes/class.ilImageLinkButton.php
@@ -34,7 +34,7 @@ public static function getInstance() : self
     // properties
     //
 
-    public function setImage(string $a_value, bool $a_is_internal = true)
+    public function setImage(string $a_value, bool $a_is_internal = true) : void
     {
         if ($a_is_internal) {
             $a_value = ilUtil::getImagePath($a_value);

diff --git a/Services/UIComponent/Button/classes/class.ilLinkButton.php b/Services/UIComponent/Button/classes/class.ilLinkButton.php
@@ -44,7 +44,7 @@ public function getUrl() : string
         return $this->url;
     }
 
-    public function setTarget(string $a_value)
+    public function setTarget(string $a_value) : void
     {
         $this->target = trim($a_value);
     }

diff --git a/Services/UIComponent/CharSelector/classes/class.ilCharSelectorGUI.php b/Services/UIComponent/CharSelector/classes/class.ilCharSelectorGUI.php
@@ -237,7 +237,11 @@ public function addToPage() : void
         $tpl->addCss(ilUtil::getStyleSheetLocation('', 'char_selector_style.css', 'Services/UIComponent/CharSelector'));
         $tpl->addJavaScript('./Services/UIComponent/CharSelector/js/ilCharSelector.js');
         $tpl->addLightbox($this->getSelectorHTML(), 2);
-        $tpl->addOnLoadCode('il.CharSelector.init(' . json_encode($this->jsconfig) . ',' . json_encode($this->jstexts) . ')');
+        $tpl->addOnLoadCode(
+            'il.CharSelector.init(' .
+            json_encode($this->jsconfig, JSON_THROW_ON_ERROR) . ',' .
+            json_encode($this->jstexts, JSON_THROW_ON_ERROR) . ')'
+        );
         $this->added_to_page = true;
     }
 
@@ -295,7 +299,7 @@ public function saveState() : void
             'open' => ilSession::get('char_selector_open'),
             'current_page' => ilSession::get('char_selector_current_page'),
             'current_subpage' => ilSession::get('char_selector_current_subpage'),
-        ));
+        ), JSON_THROW_ON_ERROR);
         exit;
     }
 

diff --git a/Services/UIComponent/CharSelector/classes/ilCharSelectorConfig.php b/Services/UIComponent/CharSelector/classes/ilCharSelectorConfig.php
@@ -442,7 +442,7 @@ public function getDefinition() : string
     {
         $definition = implode(' ', $this->custom_items);
         foreach ($this->added_blocks as $block_name) {
-            $definition = $definition . ' [' . $block_name . ']';
+            $definition .= ' [' . $block_name . ']';
         }
         return trim($definition);
     }

diff --git a/Services/UIComponent/CheckboxListOverlay/classes/class.ilCheckboxListOverlayGUI.php b/Services/UIComponent/CheckboxListOverlay/classes/class.ilCheckboxListOverlayGUI.php
@@ -39,12 +39,12 @@ public function __construct(
         $this->setId($a_id);
     }
 
-    public function setId($a_val)
+    public function setId(string $a_val) : void
     {
         $this->id = $a_val;
     }
 
-    public function getId()
+    public function getId() : string
     {
         return $this->id;
     }

diff --git a/Services/UIComponent/Explorer/classes/class.ilExplorer.php b/Services/UIComponent/Explorer/classes/class.ilExplorer.php
@@ -178,17 +178,15 @@ public function getTitle() : string
         return $this->title;
     }
 
-    public function setRoot($a_root_id)
+    public function setRoot($a_root_id) : void
     {
         #$this->tree = new ilTree(ROOT_FOLDER_ID,$a_root_id);
         $this->root_id = $a_root_id;
     }
 
-    public function getRoot()
+    public function getRoot() : ?int
     {
-        return $this->root_id == null ?
-            $this->tree->getRootId() :
-            $this->root_id;
+        return $this->root_id ?? $this->tree->getRootId();
     }
 
     public function setOrderColumn(string $a_column) : void
@@ -416,65 +414,65 @@ public function setOutput(
                         if ($object["child"] != $this->getRoot()) {
                             $parent_index = $this->getIndex($object);
                         }
-                        $this->format_options["$this->counter"]["parent"] = $object["parent"];
-                        $this->format_options["$this->counter"]["child"] = $object["child"];
-                        $this->format_options["$this->counter"]["title"] = $object["title"];
-                        $this->format_options["$this->counter"]["type"] = $object["type"];
-                        $this->format_options["$this->counter"]["obj_id"] = $object["obj_id"];
-                        $this->format_options["$this->counter"]["desc"] = "obj_" . $object["type"];
-                        $this->format_options["$this->counter"]["depth"] = $tab;
-                        $this->format_options["$this->counter"]["container"] = false;
-                        $this->format_options["$this->counter"]["visible"] = true;
-                        $this->format_options["$this->counter"]["highlighted_subtree"] = $a_highlighted_subtree;
+                        $this->format_options[(string) $this->counter]["parent"] = $object["parent"];
+                        $this->format_options[(string) $this->counter]["child"] = $object["child"];
+                        $this->format_options[(string) $this->counter]["title"] = $object["title"];
+                        $this->format_options[(string) $this->counter]["type"] = $object["type"];
+                        $this->format_options[(string) $this->counter]["obj_id"] = $object["obj_id"];
+                        $this->format_options[(string) $this->counter]["desc"] = "obj_" . $object["type"];
+                        $this->format_options[(string) $this->counter]["depth"] = $tab;
+                        $this->format_options[(string) $this->counter]["container"] = false;
+                        $this->format_options[(string) $this->counter]["visible"] = true;
+                        $this->format_options[(string) $this->counter]["highlighted_subtree"] = $a_highlighted_subtree;
 
                         // Create prefix array
                         for ($i = 0; $i < $tab; ++$i) {
-                            $this->format_options["$this->counter"]["tab"][] = 'blank';
+                            $this->format_options[(string) $this->counter]["tab"][] = 'blank';
                         }
 
                         // fix explorer (sometimes explorer disappears)
                         if ($parent_index == 0) {
-                            if (!$this->expand_all and !in_array($object["parent"], $this->expanded)) {
+                            if (!$this->expand_all && !in_array($object["parent"], $this->expanded)) {
                                 $this->expanded[] = $object["parent"];
                             }
                         }
 
                         // only if parent is expanded and visible, object is visible
-                        if ($object["child"] != $this->getRoot() and ((!$this->expand_all and !in_array($object["parent"], $this->expanded))
-                           or !$this->format_options["$parent_index"]["visible"])) {
+                        if ($object["child"] != $this->getRoot() && ((!$this->expand_all && !in_array($object["parent"], $this->expanded))
+                           or !$this->format_options[(string) $parent_index]["visible"])) {
                             if (!$this->forceExpanded($object["child"])) {
                                 // if parent is not expanded, and one child is
                                 // visible we don't need more information and
                                 // can skip the rest of the childs
-                                if ($this->format_options["$this->counter"]["visible"]) {
+                                if ($this->format_options[(string) $this->counter]["visible"]) {
                                     //echo "-setSkipping";
                                     $skip_rest = true;
                                 }
-                                $this->format_options["$this->counter"]["visible"] = false;
+                                $this->format_options[(string) $this->counter]["visible"] = false;
                             }
                         }
 
                         // if object exists parent is container
                         if ($object["child"] != $this->getRoot()) {
-                            $this->format_options["$parent_index"]["container"] = true;
+                            $this->format_options[(string) $parent_index]["container"] = true;
 
-                            if ($this->expand_all or in_array($object["parent"], $this->expanded)) {
+                            if ($this->expand_all || in_array($object["parent"], $this->expanded)) {
                                 //echo "<br>-".$object["child"]."-".$this->forceExpanded($object["child"])."-";
                                 if ($this->forceExpanded($object["parent"])) {
-                                    $this->format_options["$parent_index"]["tab"][($tab - 2)] = 'forceexp';
+                                    $this->format_options[(string) $parent_index]["tab"][($tab - 2)] = 'forceexp';
                                 } else {
-                                    $this->format_options["$parent_index"]["tab"][($tab - 2)] = 'minus';
+                                    $this->format_options[(string) $parent_index]["tab"][($tab - 2)] = 'minus';
                                 }
                             } else {
-                                $this->format_options["$parent_index"]["tab"][($tab - 2)] = 'plus';
+                                $this->format_options[(string) $parent_index]["tab"][($tab - 2)] = 'plus';
                             }
                         }
                         //echo "-"."$parent_index"."-";
                         //var_dump($this->format_options["$parent_index"]);
                         ++$this->counter;
 
                         // stop recursion if 2. level beyond expanded nodes is reached
-                        if ($this->expand_all or in_array($object["parent"], $this->expanded) or ($object["parent"] == 0)
+                        if ($this->expand_all || in_array($object["parent"], $this->expanded) or ($object["parent"] == 0)
                             or $this->forceExpanded($object["child"])) {
                             $highlighted_subtree = $a_highlighted_subtree ||
                                 ($object["child"] == $this->highlighted);
@@ -679,7 +677,7 @@ public function formatObject(
         $a_node_id,
         array $a_option,
         $a_obj_id = 0
-    ) {
+    ) : void {
         $lng = $this->lng;
         $ilErr = $this->error;
 
@@ -949,20 +947,20 @@ public function createLines(int $a_depth) : void
             if ($this->format_options[$i]["depth"] == $a_depth + 1
                and !$this->format_options[$i]["container"]
                 and $this->format_options[$i]["depth"] != 1) {
-                $this->format_options[$i]["tab"]["$a_depth"] = "quer";
+                $this->format_options[$i]["tab"][(string) $a_depth] = "quer";
             }
 
             if ($this->format_options[$i]["depth"] == $a_depth + 2) {
                 if ($this->is_in_array($i + 1, $this->format_options[$i]["depth"])) {
-                    $this->format_options[$i]["tab"]["$a_depth"] = "winkel";
+                    $this->format_options[$i]["tab"][(string) $a_depth] = "winkel";
                 } else {
-                    $this->format_options[$i]["tab"]["$a_depth"] = "ecke";
+                    $this->format_options[$i]["tab"][(string) $a_depth] = "ecke";
                 }
             }
 
             if ($this->format_options[$i]["depth"] > $a_depth + 2) {
                 if ($this->is_in_array($i + 1, $a_depth + 2)) {
-                    $this->format_options[$i]["tab"]["$a_depth"] = "hoch";
+                    $this->format_options[$i]["tab"][(string) $a_depth] = "hoch";
                 }
             }
         }

diff --git a/Services/UIComponent/Explorer2/classes/class.ilExplorerBaseGUI.php b/Services/UIComponent/Explorer2/classes/class.ilExplorerBaseGUI.php
@@ -73,7 +73,7 @@ public function __construct(
         // get open nodes
         $this->store = new ilSessionIStorage("expl2");
         $open_nodes = $this->store->get("on_" . $this->id);
-        $this->open_nodes = unserialize($open_nodes) ?: [];
+        $this->open_nodes = unserialize($open_nodes, ['allowed_classes' => false]) ?: [];
         if (!is_array($this->open_nodes)) {
             $this->open_nodes = array();
         }

diff --git a/Services/UIComponent/NestedList/classes/class.ilNestedList.php b/Services/UIComponent/NestedList/classes/class.ilNestedList.php
@@ -58,7 +58,7 @@ public function addListNode(
         string $a_content,
         string $a_id,
         $a_parent = 0
-    ) {
+    ) : void {
         $this->nodes[$a_id] = $a_content;
         $this->childs[$a_parent][] = $a_id;
     }

diff --git a/Services/UIComponent/SplitButton/classes/class.ilSplitButtonGUI.php b/Services/UIComponent/SplitButton/classes/class.ilSplitButtonGUI.php
@@ -67,7 +67,7 @@ public function getMenuItems() : array
      */
     public function setMenuItems(array $menu_items) : void
     {
-        array_walk($menu_items, function ($item, $idx) {
+        array_walk($menu_items, static function ($item, $idx) : void {
             if (!($item instanceof ilSplitButtonMenuItem)) {
                 throw new ilSplitButtonException(sprintf(
                     "Cannot set menu items, element at index '%s' is not of type 'ilSplitButtonItem'",

diff --git a/Services/UIComponent/SyntaxHighlighter/classes/class.ilSyntaxHighlighter.php b/Services/UIComponent/SyntaxHighlighter/classes/class.ilSyntaxHighlighter.php
@@ -87,10 +87,7 @@ public static function isSupported(string $a_lang) : bool
      */
     public static function getNewLanguageId(string $a_old_lang_id) : string
     {
-        if (isset(self::$v51_map[$a_old_lang_id])) {
-            return self::$v51_map[$a_old_lang_id];
-        }
-        return $a_old_lang_id;
+        return self::$v51_map[$a_old_lang_id] ?? $a_old_lang_id;
     }
 
 

diff --git a/Services/UIComponent/Tabs/classes/class.ilTabsGUI.php b/Services/UIComponent/Tabs/classes/class.ilTabsGUI.php
@@ -195,11 +195,8 @@ public function clearTargets() : void
     {
         global $DIC;
 
-        $ilHelp = null;
-        if (isset($DIC["ilHelp"])) {
-            $ilHelp = $DIC["ilHelp"];
-        }
-
+        $ilHelp = $DIC["ilHelp"] ?? null;
+
         if (!$this->getSetupMode()) {
             $ilHelp->setScreenIdComponent("");
         }
@@ -331,10 +328,7 @@ private function __getHTML(
         $sr_pre = "";
         $hash = "";
 
-        $ilHelp = null;
-        if (isset($DIC["ilHelp"])) {
-            $ilHelp = $DIC["ilHelp"];
-        }
+        $ilHelp = $DIC["ilHelp"] ?? null;
 
         $ilCtrl = $this->ctrl;
         $lng = $this->lng;

diff --git a/Services/WebServices/ECS/classes/Mapping/class.ilECSNodeMappingCmsExplorer.php b/Services/WebServices/ECS/classes/Mapping/class.ilECSNodeMappingCmsExplorer.php
@@ -142,7 +142,7 @@ public function buildFormItem($a_node_id, $a_type)
         }
     }
 
-    public function formatObject($tpl, $a_node_id, $a_option, $a_obj_id = 0)
+    public function formatObject($tpl, $a_node_id, $a_option, $a_obj_id = 0) : void
     {
         if (!isset($a_node_id) or !is_array($a_option)) {
             $this->ilias->raiseError(get_class($this) . "::formatObject(): Missing parameter or wrong datatype! " .

diff --git a/Services/WebServices/ECS/classes/Mapping/class.ilECSNodeMappingLocalExplorer.php b/Services/WebServices/ECS/classes/Mapping/class.ilECSNodeMappingLocalExplorer.php
@@ -147,7 +147,7 @@ public function buildFormItem($a_node_id, $a_type)
         }
     }
 
-    public function formatObject($tpl, $a_node_id, $a_option, $a_obj_id = 0)
+    public function formatObject($tpl, $a_node_id, $a_option, $a_obj_id = 0) : void
     {
         if (!isset($a_node_id) or !is_array($a_option)) {
             $this->ilias->raiseError(get_class($this) . "::formatObject(): Missing parameter or wrong datatype! " .