Modest refactoring of Node classes

Signed-off-by: Kamil Tekiela <tekiela246@gmail.com>
phpmyadmin · Oct 15, 2022 · 84ea3c8 · 84ea3c8
1 parent 0ca6b45
commit 84ea3c8
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 77 deletions.
diff --git a/libraries/classes/Navigation/NavigationTree.php b/libraries/classes/Navigation/NavigationTree.php
@@ -215,10 +215,8 @@ public function __construct($template, DatabaseInterface $dbi)
      */
     private function getNavigationDbPos(): int
     {
-        $retval = 0;
-
         if (strlen($GLOBALS['db'] ?? '') === 0) {
-            return $retval;
+            return 0;
         }
 
         /*
@@ -379,14 +377,12 @@ private function buildPathPart(array $path, string $type2, int $pos2, string $ty
             return false;
         }
 
-        $retval = $db;
-
         $containers = $this->addDbContainers($db, $type2, $pos2);
 
         array_shift($path); // remove db
 
-        if ((count($path) <= 0 || ! array_key_exists($path[0], $containers)) && count($containers) != 1) {
-            return $retval;
+        if (($path === [] || ! array_key_exists($path[0], $containers)) && count($containers) != 1) {
+            return $db;
         }
 
         if (count($containers) === 1) {
@@ -398,8 +394,6 @@ private function buildPathPart(array $path, string $type2, int $pos2, string $ty
             }
         }
 
-        $retval = $container;
-
         if (count($container->children) <= 1) {
             $dbData = $db->getData($container->realName, $pos2, $this->searchClause2);
             foreach ($dbData as $item) {
@@ -441,7 +435,7 @@ private function buildPathPart(array $path, string $type2, int $pos2, string $ty
 
         array_shift($path); // remove container
         if (count($path) <= 0) {
-            return $retval;
+            return $container;
         }
 
         /** @var NodeTable|null $table */
@@ -458,17 +452,22 @@ private function buildPathPart(array $path, string $type2, int $pos2, string $ty
 
             $container->addChild($node);
             $table = $container->getChild($path[0], true);
+            if ($table === null) {
+                return false;
+            }
         }
 
-        $retval = $table ?? false;
         $containers = $this->addTableContainers($table, $pos2, $type3, $pos3);
         array_shift($path); // remove table
-        if (count($path) <= 0 || ! array_key_exists($path[0], $containers)) {
-            return $retval;
+        if ($path === [] || ! array_key_exists($path[0], $containers)) {
+            return $table;
         }
 
         $container = $table->getChild($path[0], true);
-        $retval = $container ?? false;
+        if ($container === null) {
+            return false;
+        }
+
         $tableData = $table->getData($container->realName, $pos3);
         foreach ($tableData as $item) {
             switch ($container->realName) {
@@ -497,7 +496,7 @@ private function buildPathPart(array $path, string $type2, int $pos2, string $ty
             $container->addChild($node);
         }
 
-        return $retval;
+        return $container;
     }
 
     /**
@@ -515,7 +514,7 @@ private function buildPathPart(array $path, string $type2, int $pos2, string $ty
      * @param int       $pos3  The position for the pagination of
      *                         the branch at the third level of the tree
      *
-     * @return array An array of new nodes
+     * @return Node[] An array of new nodes
      */
     private function addTableContainers(NodeTable $table, int $pos2, string $type3, int $pos3): array
     {
@@ -568,7 +567,7 @@ private function addTableContainers(NodeTable $table, int $pos2, string $type3,
      * @param int          $pos2 The position for the pagination of
      *                           the branch at the second level of the tree
      *
-     * @return array An array of new nodes
+     * @return Node[] An array of new nodes
      */
     private function addDbContainers(NodeDatabase $db, string $type, int $pos2): array
     {

diff --git a/libraries/classes/Navigation/Nodes/Node.php b/libraries/classes/Navigation/Nodes/Node.php
@@ -369,7 +369,7 @@ public function getPaths(): array
      *
      * @return array
      */
-    public function getData($type, $pos, $searchClause = '')
+    public function getData(string $type, int $pos, string $searchClause = ''): array
     {
         if (isset($GLOBALS['cfg']['Server']['DisableIS']) && ! $GLOBALS['cfg']['Server']['DisableIS']) {
             return $this->getDataFromInfoSchema($pos, $searchClause);

diff --git a/libraries/classes/Navigation/Nodes/NodeDatabase.php b/libraries/classes/Navigation/Nodes/NodeDatabase.php
@@ -321,7 +321,7 @@ private function getWhereClauseForSearch(
      *
      * @return array
      */
-    public function getData($type, $pos, $searchClause = '')
+    public function getData(string $type, int $pos, string $searchClause = ''): array
     {
         $pos = (int) $pos;
         $retval = [];

diff --git a/libraries/classes/Navigation/Nodes/NodeTable.php b/libraries/classes/Navigation/Nodes/NodeTable.php
@@ -148,7 +148,7 @@ public function getPresence($type = '', $searchClause = '')
      *
      * @return array
      */
-    public function getData($type, $pos, $searchClause = '')
+    public function getData(string $type, int $pos, string $searchClause = ''): array
     {
         $maxItems = $GLOBALS['cfg']['MaxNavigationItems'];
         $retval = [];

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -5030,16 +5030,6 @@ parameters:
 			count: 1
 			path: libraries/classes/Navigation/NavigationTree.php
 
-		-
-			message: "#^Method PhpMyAdmin\\\\Navigation\\\\NavigationTree\\:\\:addDbContainers\\(\\) return type has no value type specified in iterable type array\\.$#"
-			count: 1
-			path: libraries/classes/Navigation/NavigationTree.php
-
-		-
-			message: "#^Method PhpMyAdmin\\\\Navigation\\\\NavigationTree\\:\\:addTableContainers\\(\\) return type has no value type specified in iterable type array\\.$#"
-			count: 1
-			path: libraries/classes/Navigation/NavigationTree.php
-
 		-
 			message: "#^Method PhpMyAdmin\\\\Navigation\\\\NavigationTree\\:\\:buildPathPart\\(\\) has parameter \\$path with no value type specified in iterable type array\\.$#"
 			count: 1
@@ -5085,6 +5075,11 @@ parameters:
 			count: 1
 			path: libraries/classes/Navigation/NavigationTree.php
 
+		-
+			message: "#^Parameter \\#1 \\$table of method PhpMyAdmin\\\\Navigation\\\\NavigationTree\\:\\:addTableContainers\\(\\) expects PhpMyAdmin\\\\Navigation\\\\Nodes\\\\NodeTable, PhpMyAdmin\\\\Navigation\\\\Nodes\\\\Node given\\.$#"
+			count: 1
+			path: libraries/classes/Navigation/NavigationTree.php
+
 		-
 			message: "#^Property PhpMyAdmin\\\\Navigation\\\\NavigationTree\\:\\:\\$aPath type has no value type specified in iterable type array\\.$#"
 			count: 1
@@ -5165,16 +5160,6 @@ parameters:
 			count: 1
 			path: libraries/classes/Navigation/Nodes/Node.php
 
-		-
-			message: "#^Strict comparison using \\!\\=\\= between PhpMyAdmin\\\\Navigation\\\\Nodes\\\\Node and null will always evaluate to true\\.$#"
-			count: 1
-			path: libraries/classes/Navigation/Nodes/Node.php
-
-		-
-			message: "#^Unreachable statement \\- code above always terminates\\.$#"
-			count: 1
-			path: libraries/classes/Navigation/Nodes/Node.php
-
 		-
 			message: "#^Method PhpMyAdmin\\\\Navigation\\\\Nodes\\\\NodeColumn\\:\\:__construct\\(\\) has parameter \\$item with no value type specified in iterable type array\\.$#"
 			count: 1

diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -8519,6 +8519,9 @@
     </PossiblyUndefinedArrayOffset>
   </file>
   <file src="libraries/classes/Navigation/NavigationTree.php">
+    <ArgumentTypeCoercion occurrences="1">
+      <code>$table</code>
+    </ArgumentTypeCoercion>
     <DocblockTypeContradiction occurrences="2">
       <code>''</code>
       <code>isset($this-&gt;pos)</code>
@@ -8529,9 +8532,7 @@
       <code>$this-&gt;pos3Name</code>
       <code>$this-&gt;pos3Name</code>
     </InvalidPropertyAssignmentValue>
-    <MixedArgument occurrences="30">
-      <code>$container-&gt;children</code>
-      <code>$container-&gt;realName</code>
+    <MixedArgument occurrences="27">
       <code>$db</code>
       <code>$hiddenCounts[$db]</code>
       <code>$item</code>
@@ -8558,7 +8559,6 @@
       <code>$paths['vPath_clean']</code>
       <code>$separator</code>
       <code>$separator</code>
-      <code>$table</code>
       <code>$value</code>
     </MixedArgument>
     <MixedArrayAccess occurrences="11">
@@ -8577,32 +8577,19 @@
     <MixedArrayOffset occurrences="1">
       <code>$hiddenCounts[$db]</code>
     </MixedArrayOffset>
-    <MixedAssignment occurrences="15">
-      <code>$container</code>
+    <MixedAssignment occurrences="11">
       <code>$db</code>
       <code>$item</code>
       <code>$item</code>
       <code>$part</code>
       <code>$path</code>
       <code>$path</code>
       <code>$path</code>
-      <code>$retval</code>
-      <code>$retval</code>
       <code>$separator</code>
       <code>$separator</code>
-      <code>$table</code>
       <code>$urlParams['pos3_name']</code>
       <code>$value</code>
     </MixedAssignment>
-    <MixedInferredReturnType occurrences="1">
-      <code>Node|bool</code>
-    </MixedInferredReturnType>
-    <MixedMethodCall occurrences="4">
-      <code>addChild</code>
-      <code>addChild</code>
-      <code>getChild</code>
-      <code>getChild</code>
-    </MixedMethodCall>
     <MixedOperand occurrences="2">
       <code>$separators[0]</code>
       <code>$separators[0]</code>
@@ -8612,15 +8599,6 @@
       <code>$node</code>
       <code>$node</code>
     </MixedPropertyAssignment>
-    <MixedPropertyFetch occurrences="3">
-      <code>$container-&gt;children</code>
-      <code>$container-&gt;realName</code>
-      <code>$container-&gt;realName</code>
-    </MixedPropertyFetch>
-    <MixedReturnStatement occurrences="2">
-      <code>$retval</code>
-      <code>$retval</code>
-    </MixedReturnStatement>
     <PossiblyFalseOperand occurrences="2">
       <code>strrpos($class, '\\')</code>
       <code>strrpos($class, '\\')</code>
@@ -8644,18 +8622,9 @@
       <code>$_POST['searchClause']</code>
       <code>$_POST['searchClause2']</code>
     </PossiblyInvalidPropertyAssignmentValue>
-    <PossiblyNullArgument occurrences="3">
-      <code>$container-&gt;realName</code>
+    <PossiblyNullArgument occurrences="1">
       <code>$database</code>
-      <code>$table</code>
     </PossiblyNullArgument>
-    <PossiblyNullPropertyFetch occurrences="2">
-      <code>$container-&gt;parent</code>
-      <code>$container-&gt;realName</code>
-    </PossiblyNullPropertyFetch>
-    <PossiblyNullReference occurrences="1">
-      <code>addChild</code>
-    </PossiblyNullReference>
     <RedundantCast occurrences="5">
       <code>(int) $GLOBALS['cfg']['FirstLevelNavigationItems']</code>
       <code>(int) $GLOBALS['cfg']['FirstLevelNavigationItems']</code>
@@ -8682,6 +8651,9 @@
       <code>$_POST['pos2_value'] ?? 0</code>
       <code>$_POST['pos3_value']</code>
     </RiskyCast>
+    <TypeDoesNotContainType occurrences="1">
+      <code>$path === []</code>
+    </TypeDoesNotContainType>
   </file>
   <file src="libraries/classes/Navigation/NodeFactory.php">
     <MixedMethodCall occurrences="1">