From 5b622d45540e1a90c037b2c3cfd290153f2677f3 Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Fri, 1 Mar 2024 15:12:38 +0100
Subject: [PATCH 1/7] Refactor Options_Parser to non-static

---
 phpdotnet/phd/Options/Parser.php | 20 +++++---------------
 render.php                       |  3 ++-
 2 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/phpdotnet/phd/Options/Parser.php b/phpdotnet/phd/Options/Parser.php
index 8bb483cc..e70a107c 100644
--- a/phpdotnet/phd/Options/Parser.php
+++ b/phpdotnet/phd/Options/Parser.php
@@ -7,19 +7,11 @@ class Options_Parser
     private $defaultHandler;
     private $packageHandlers = array();
 
-    private function __construct() {
+    public function __construct() {
         $this->defaultHandler = new Options_Handler();
         $this->packageHandlers = $this->loadPackageHandlers();
     }
 
-    public static function instance() {
-        static $instance = null;
-        if ($instance == null) {
-            $instance = new self();
-        }
-        return $instance;
-    }
-
     private function loadPackageHandlers() {
         $packageList = Config::getSupportedPackages();
         $list = array();
@@ -91,19 +83,17 @@ private function checkOptions() {
         }
     }
 
-    public static function getopt() {
-        $self = self::instance();
-
+    public function getopt() {
         //validate options
-        $self->checkOptions();
+        $this->checkOptions();
 
-        $args = getopt($self->getShortOptions(), $self->getLongOptions());
+        $args = getopt($this->getShortOptions(), $this->getLongOptions());
         if ($args === false) {
             trigger_error("Something happend with getopt(), please report a bug", E_USER_ERROR);
         }
 
         foreach ($args as $k => $v) {
-            $handler = $self->handlerForOption($k);
+            $handler = $this->handlerForOption($k);
             if (is_callable($handler)) {
                 call_user_func($handler, $k, $v);
             } else {
diff --git a/render.php b/render.php
index 4cf66961..9cd67cb0 100644
--- a/render.php
+++ b/render.php
@@ -22,7 +22,8 @@
     Config::init(array());
 }
 
-Options_Parser::getopt();
+$optionsParser = new Options_Parser;
+$optionsParser->getopt();
 
 /* If no docbook file was passed, die */
 if (!is_dir(Config::xml_root()) || !is_file(Config::xml_file())) {

From 708e4a657fc316784aa35b872384903bbeda961f Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Fri, 1 Mar 2024 15:49:46 +0100
Subject: [PATCH 2/7] Refactor Options_Parser to inject all dependencies

Refactor Options_Parser to require all dependencies as constructor parameters.
Inject dependencies in render.php.
Add type hints for injected dependencies in Options_Parser.
Add method to check whether every element of the injected array is implementing Options_Interface.
---
 phpdotnet/phd/Options/Parser.php | 31 +++++++++++++++++++------------
 render.php                       |  8 +++++++-
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/phpdotnet/phd/Options/Parser.php b/phpdotnet/phd/Options/Parser.php
index e70a107c..bf1fa985 100644
--- a/phpdotnet/phd/Options/Parser.php
+++ b/phpdotnet/phd/Options/Parser.php
@@ -4,23 +4,30 @@
 class Options_Parser
 {
 
-    private $defaultHandler;
-    private $packageHandlers = array();
+    private Options_Interface $defaultHandler;
+    private array $packageHandlers = [];
 
-    public function __construct() {
-        $this->defaultHandler = new Options_Handler();
-        $this->packageHandlers = $this->loadPackageHandlers();
+    /**
+     * @param array<?Options_Interface> $packageHandlers
+     */
+    public function __construct(Options_Interface $defaultHandler, ?array $packageHandlers = []) {
+        $this->defaultHandler = $defaultHandler;
+        $this->validatePackageHandlers($packageHandlers);
+        $this->packageHandlers = $packageHandlers;
     }
 
-    private function loadPackageHandlers() {
-        $packageList = Config::getSupportedPackages();
-        $list = array();
-        foreach ($packageList as $package) {
-            if ($handler = Format_Factory::createFactory($package)->getOptionsHandler()) {
-                $list[strtolower($package)] = $handler;
+    /**
+     * @param array<?Options_Interface> $packageHandlers
+     *
+     * @return array<?Options_Interface>
+     */
+    private function validatePackageHandlers(array $packageHandlers): void {
+        foreach ($packageHandlers as $handler) {
+            if (!($handler instanceof Options_Interface)) {
+                trigger_error("All package handlers must implement Options_Interface", E_USER_ERROR);
+                break;
             }
         }
-        return $list;
     }
 
     public function handlerForOption($option) {
diff --git a/render.php b/render.php
index 9cd67cb0..18a21e5a 100644
--- a/render.php
+++ b/render.php
@@ -22,7 +22,13 @@
     Config::init(array());
 }
 
-$optionsParser = new Options_Parser;
+$packageHandlers = array();
+foreach (Config::getSupportedPackages() as $package) {
+    if ($handler = Format_Factory::createFactory($package)->getOptionsHandler()) {
+        $packageHandlers[strtolower($package)] = $handler;
+    }
+}
+$optionsParser = new Options_Parser(new Options_Handler, $packageHandlers);
 $optionsParser->getopt();
 
 /* If no docbook file was passed, die */

From ed138cd2bd2bfd70ae9f91c5e329664779a3b79c Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Fri, 1 Mar 2024 16:03:22 +0100
Subject: [PATCH 3/7] Minor refactoring of Options_Parser

Add type hints for methods that need them.
Refactor array declaration from array() to [].
Rename one method to better show its function.
---
 phpdotnet/phd/Options/Parser.php | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/phpdotnet/phd/Options/Parser.php b/phpdotnet/phd/Options/Parser.php
index bf1fa985..14d54c46 100644
--- a/phpdotnet/phd/Options/Parser.php
+++ b/phpdotnet/phd/Options/Parser.php
@@ -30,9 +30,12 @@ private function validatePackageHandlers(array $packageHandlers): void {
         }
     }
 
-    public function handlerForOption($option) {
+    /**
+     * @return ?array<Options_Interface, string>
+     */
+    private function handlerForOption(string $option): ?array {
         if (method_exists($this->defaultHandler, "option_{$option}")) {
-            return array($this->defaultHandler, "option_{$option}");
+            return [$this->defaultHandler, "option_{$option}"];
         }
 
         $opt = explode('-', $option);
@@ -40,15 +43,18 @@ public function handlerForOption($option) {
 
         if (isset($this->packageHandlers[$package])) {
             if (method_exists($this->packageHandlers[$package], "option_{$opt[1]}")) {
-                return array($this->packageHandlers[$package], "option_{$opt[1]}");
+                return [$this->packageHandlers[$package], "option_{$opt[1]}"];
             }
         }
-        return NULL;
+        return null;
     }
 
-    public function getLongOptions() {
+    /**
+     * @return array<string>
+     */
+    private function getLongOptions(): array {
         $defaultOptions = array_keys($this->defaultHandler->optionList());
-        $packageOptions = array();
+        $packageOptions = [];
         foreach ($this->packageHandlers as $package => $handler) {
             foreach ($handler->optionList() as $opt) {
                 $packageOptions[] = $package . '-' . $opt;
@@ -57,7 +63,7 @@ public function getLongOptions() {
         return array_merge($defaultOptions, $packageOptions);
     }
 
-    public function getShortOptions() {
+    private function getShortOptions(): string {
         return implode('', array_values($this->defaultHandler->optionList()));
     }
 
@@ -66,12 +72,12 @@ public function getShortOptions() {
      *
      * Fix Bug #54217 - Warn about nonexisting parameters
      */
-    private function checkOptions() {
+    private function validateOptions(): void {
         $argv = $_SERVER['argv'];
         $argc = $_SERVER['argc'];
 
         $short = str_split(str_replace(':', '', $this->getShortOptions()));
-        $long = array();
+        $long = [];
         foreach ($this->getLongOptions() as $opt) {
             $long[] = str_replace(':', '', $opt);
         }
@@ -91,8 +97,7 @@ private function checkOptions() {
     }
 
     public function getopt() {
-        //validate options
-        $this->checkOptions();
+        $this->validateOptions();
 
         $args = getopt($this->getShortOptions(), $this->getLongOptions());
         if ($args === false) {
@@ -100,7 +105,7 @@ public function getopt() {
         }
 
         foreach ($args as $k => $v) {
-            $handler = $this->handlerForOption($k);
+            $handler = $this->handlerForOption((string) $k);
             if (is_callable($handler)) {
                 call_user_func($handler, $k, $v);
             } else {
@@ -109,7 +114,4 @@ public function getopt() {
             }
         }
     }
-
 }
-
-

From 31a544a90d8d09f50983b8f520f29f041db68fba Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Fri, 1 Mar 2024 16:07:02 +0100
Subject: [PATCH 4/7] Refactor optionList method visibility and return type

---
 phpdotnet/phd/Options/Handler.php   | 5 ++++-
 phpdotnet/phd/Options/Interface.php | 3 +--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/phpdotnet/phd/Options/Handler.php b/phpdotnet/phd/Options/Handler.php
index d65dde53..0cc2cfe3 100644
--- a/phpdotnet/phd/Options/Handler.php
+++ b/phpdotnet/phd/Options/Handler.php
@@ -3,7 +3,10 @@
 
 class Options_Handler implements Options_Interface
 {
-    public function optionList()
+    /**
+     * @return array<string, string>
+     */
+    public function optionList(): array
     {
         return array(
             'format:'      => 'f:',        // The format to render (xhtml, pdf...)
diff --git a/phpdotnet/phd/Options/Interface.php b/phpdotnet/phd/Options/Interface.php
index aac07701..423c3dff 100644
--- a/phpdotnet/phd/Options/Interface.php
+++ b/phpdotnet/phd/Options/Interface.php
@@ -3,7 +3,6 @@
 
 interface Options_Interface {
 
-    function optionList();
+    public function optionList(): array;
 
 }
-

From 05ec9876caf15ce17350e670a425e6fc04a8f160 Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Fri, 1 Mar 2024 17:42:06 +0100
Subject: [PATCH 5/7] Refactor Handler

Add parameter and return types to all methods.
Make void returns explicit in methods that call other methods.
Change array declaration from array() to [].
Make code style consistent.
---
 phpdotnet/phd/Options/Handler.php | 171 +++++++++++++++++-------------
 1 file changed, 97 insertions(+), 74 deletions(-)

diff --git a/phpdotnet/phd/Options/Handler.php b/phpdotnet/phd/Options/Handler.php
index 0cc2cfe3..5cc50532 100644
--- a/phpdotnet/phd/Options/Handler.php
+++ b/phpdotnet/phd/Options/Handler.php
@@ -8,7 +8,7 @@ class Options_Handler implements Options_Interface
      */
     public function optionList(): array
     {
-        return array(
+        return [
             'format:'      => 'f:',        // The format to render (xhtml, pdf...)
             'noindex'      => 'I',         // Do not re-index
             'forceindex'   => 'r',         // Force re-indexing under all circumstances
@@ -33,29 +33,31 @@ public function optionList(): array
             'quit'         => 'Q',         // Do not run the render. Use with -S to just save the config.
             'memoryindex'  => 'M',         // Use sqlite in memory rather then file
             'packagedir'   => 'k:',        // Include path for external packages
-        );
+        ];
     }
 
-    public function option_M($k, $v)
+    public function option_M(string $k, mixed $v): void
     {
         $this->option_memoryindex($k, $v);
     }
-    public function option_memoryindex($k, $v)
+
+    public function option_memoryindex(string $k, mixed $v): void
     {
         Config::set_memoryindex(true);
     }
 
-    public function option_f($k, $v)
+    public function option_f(string $k, mixed $v): void
     {
-        if ($k == "f") {
-            return $this->option_format($k, $v);
-        } else {
-            return $this->option_outputfilename($k, $v);
+        if ($k === "f") {
+            $this->option_format($k, $v);
+            return;
         }
+        $this->option_outputfilename($k, $v);
     }
-    public function option_format($k, $v)
+
+    public function option_format(string $k, mixed $v): void
     {
-        $formats = array();
+        $formats = [];
         foreach((array)$v as $i => $val) {
             if (!in_array($val, $formats)) {
                 $formats[] = $val;
@@ -64,11 +66,12 @@ public function option_format($k, $v)
         Config::set_output_format($formats);
     }
 
-    public function option_e($k, $v)
+    public function option_e(string $k, mixed $v): void
     {
         $this->option_ext($k, $v);
     }
-    public function option_ext($k, $v)
+
+    public function option_ext(string $k, mixed $v): void
     {
         $bool = self::boolval($v);
         if ($bool === false) {
@@ -82,47 +85,52 @@ public function option_ext($k, $v)
         }
     }
 
-    public function option_g($k, $v)
+    public function option_g(string $k, mixed $v): void
     {
         $this->option_highlighter($k, $v);
     }
-    public function option_highlighter($k, $v)
+
+    public function option_highlighter(string $k, mixed $v): void
     {
         Config::setHighlighter($v);
     }
 
-    public function option_i($k, $v)
+    public function option_i(string $k, mixed $v): void
     {
         $this->option_noindex($k, 'true');
     }
-    public function option_noindex($k, $v)
+
+    public function option_noindex(string $k, mixed $v): void
     {
         Config::set_no_index(true);
     }
 
-    public function option_r($k, $v)
+    public function option_r(string $k, mixed $v): void
     {
         $this->option_forceindex($k, 'true');
     }
-    public function option_forceindex($k, $v)
+
+    public function option_forceindex(string $k, mixed $v): void
     {
         Config::set_force_index(true);
     }
 
-    public function option_t($k, $v)
+    public function option_t(string $k, mixed $v): void
     {
         $this->option_notoc($k, 'true');
     }
-    public function option_notoc($k, $v)
+
+    public function option_notoc(string $k, mixed $v): void
     {
         Config::set_no_toc(true);
     }
 
-    public function option_d($k, $v)
+    public function option_d(string $k, mixed $v): void
     {
         $this->option_docbook($k, $v);
     }
-    public function option_docbook($k, $v)
+
+    public function option_docbook(string $k, mixed $v): void
     {
         if (is_array($v)) {
             trigger_error("Can only parse one file at a time", E_USER_ERROR);
@@ -134,11 +142,12 @@ public function option_docbook($k, $v)
         Config::set_xml_file($v);
     }
 
-    public function option_o($k, $v)
+    public function option_o(string $k, mixed $v): void
     {
         $this->option_output($k, $v);
     }
-    public function option_output($k, $v)
+
+    public function option_output(string $k, mixed $v): void
     {
         if (is_array($v)) {
             trigger_error("Only a single output location can be supplied", E_USER_ERROR);
@@ -149,11 +158,11 @@ public function option_output($k, $v)
         if (!is_dir($v) || !is_readable($v)) {
             trigger_error(sprintf("'%s' is not a valid directory", $v), E_USER_ERROR);
         }
-        $v = (substr($v, strlen($v) - strlen(DIRECTORY_SEPARATOR)) == DIRECTORY_SEPARATOR) ? $v : ($v . DIRECTORY_SEPARATOR);
+        $v = (substr($v, strlen($v) - strlen(DIRECTORY_SEPARATOR)) === DIRECTORY_SEPARATOR) ? $v : ($v . DIRECTORY_SEPARATOR);
         Config::set_output_dir($v);
     }
 
-    public function option_outputfilename($k, $v)
+    public function option_outputfilename(string $k, mixed $v): void
     {
         if (is_array($v)) {
             trigger_error("Only a single output location can be supplied", E_USER_ERROR);
@@ -163,14 +172,16 @@ public function option_outputfilename($k, $v)
         Config::set_output_filename($file);
     }
 
-    public function option_p($k, $v)
+    public function option_p(string $k, mixed $v): void
     {
-        if ($k == "P") {
-            return $this->option_package($k, $v);
+        if ($k === "P") {
+            $this->option_package($k, $v);
+            return;
         }
         $this->option_partial($k, $v);
     }
-    public function option_partial($k, $v)
+
+    public function option_partial(string $k, mixed $v): void
     {
         $render_ids = Config::render_ids();
         foreach((array)$v as $i => $val) {
@@ -188,8 +199,8 @@ public function option_partial($k, $v)
         Config::set_render_ids($render_ids);
     }
 
-    public function option_package($k, $v) {
-
+    public function option_package(string $k, mixed $v): void
+    {
         foreach((array)$v as $package) {
             if (!in_array($package, Config::getSupportedPackages())) {
                 $supported = implode(', ', Config::getSupportedPackages());
@@ -199,23 +210,26 @@ public function option_package($k, $v) {
         Config::set_package($v);
     }
 
-    public function option_q($k, $v)
+    public function option_q(string $k, mixed $v): void
     {
         $this->option_quit($k, $v);
     }
-    public function option_quit($k, $v)
+
+    public function option_quit(string $k, mixed $v): void
     {
         Config::set_quit(true);
     }
 
-    public function option_s($k, $v)
+    public function option_s(string $k, mixed $v): void
     {
-        if ($k == "S") {
-            return $this->option_saveconfig($k, $v);
+        if ($k === "S") {
+            $this->option_saveconfig($k, $v);
+            return;
         }
         $this->option_skip($k, $v);
     }
-    public function option_skip($k, $v)
+
+    public function option_skip(string $k, mixed $v): void
     {
         $skip_ids = Config::skip_ids();
         foreach((array)$v as $i => $val) {
@@ -232,7 +246,8 @@ public function option_skip($k, $v)
         }
         Config::set_skip_ids($skip_ids);
     }
-    public function option_saveconfig($k, $v)
+
+    public function option_saveconfig(string $k, mixed $v): void
     {
         if (is_array($v)) {
             trigger_error(sprintf("You cannot pass %s more than once", $k), E_USER_ERROR);
@@ -251,7 +266,7 @@ public function option_saveconfig($k, $v)
         }
     }
 
-    public function option_v($k, $v)
+    public function option_v(string $k, mixed $v): void
     {
         if ($k[0] === 'V') {
             $this->option_version($k, $v);
@@ -260,7 +275,7 @@ public function option_v($k, $v)
         $this->option_verbose($k, $v);
     }
 
-    public function option_verbose($k, $v)
+    public function option_verbose(string $k, mixed $v): void
     {
         static $verbose = 0;
 
@@ -282,14 +297,16 @@ public function option_verbose($k, $v)
         error_reporting($GLOBALS['olderrrep'] | $verbose);
     }
 
-    public function option_l($k, $v)
+    public function option_l(string $k, mixed $v): void
     {
-        if ($k == "L") {
-            return $this->option_lang($k, $v);
+        if ($k === "L") {
+            $this->option_lang($k, $v);
+            return;
         }
         $this->option_list($k, $v);
     }
-    public function option_list($k, $v)
+
+    public function option_list(string $k, mixed $v): never
     {
         $packageList = Config::getSupportedPackages();
 
@@ -302,18 +319,21 @@ public function option_list($k, $v)
         exit(0);
     }
 
-    public function option_lang($k, $v)
+    public function option_lang(string $k, mixed $v): void
     {
         Config::set_language($v);
     }
-    public function option_c($k, $v)
+
+    public function option_c(string $k, mixed $v): void
     {
-        if ($k == "C") {
-            return $this->option_css($k, $v);
+        if ($k === "C") {
+            $this->option_css($k, $v);
+            return;
         }
         $this->option_color($k, $v);
     }
-    public function option_color($k, $v)
+
+    public function option_color(string $k, mixed $v): void
     {
         if (is_array($v)) {
             trigger_error(sprintf("You cannot pass %s more than once", $k), E_USER_ERROR);
@@ -325,8 +345,10 @@ public function option_color($k, $v)
             trigger_error("yes/no || on/off || true/false || 1/0 expected", E_USER_ERROR);
         }
     }
-    public function option_css($k, $v) {
-        $styles = array();
+
+    public function option_css(string $k, mixed $v): void
+    {
+        $styles = [];
         foreach((array)$v as $key => $val) {
             if (!in_array($val, $styles)) {
                 $styles[] = $val;
@@ -335,12 +357,12 @@ public function option_css($k, $v) {
         Config::set_css($styles);
     }
 
-    public function option_k($k, $v)
+    public function option_k(string $k, mixed $v): void
     {
         $this->option_packagedir($k, $v);
     }
 
-    public function option_packagedir($k, $v)
+    public function option_packagedir(string $k, mixed $v): void
     {
         $packages = Config::package_dirs();
         foreach((array)$v as $key => $val) {
@@ -355,11 +377,12 @@ public function option_packagedir($k, $v)
         Config::set_package_dirs($packages);
     }
 
-    public function option_x($k, $v)
+    public function option_x(string $k, mixed $v): void
     {
-        $this->option_xinclude($k, 'true');
+        $this->option_xinclude($k, true);
     }
-    public function option_xinclude($k, $v)
+
+    public function option_xinclude(string $k, mixed $v): void
     {
         Config::set_process_xincludes(true);
     }
@@ -368,9 +391,9 @@ public function option_xinclude($k, $v)
      * Prints out the current PhD and PHP version.
      * Exits directly.
      *
-     * @return void
+     * @return never
      */
-    public function option_version($k, $v)
+    public function option_version(string $k, mixed $v): never
     {
         $color  = Config::phd_info_color();
         $output = Config::phd_info_output();
@@ -386,11 +409,12 @@ public function option_version($k, $v)
         exit(0);
     }
 
-    public function option_h($k, $v)
+    public function option_h(string $k, mixed $v): never
     {
         $this->option_help($k, $v);
     }
-    public function option_help($k, $v)
+
+    public function option_help(string $k, mixed $v): never
     {
         echo "PhD version: " .Config::VERSION;
         echo "\n" . Config::copyright() . "\n
@@ -467,33 +491,32 @@ public function option_help($k, $v)
      *
      * Returns boolean true/false on success, null on failure
      *
-     * @param string $val
-     * @return bool
+     * @param mixed $val
+     * @return ?bool
      */
-    public static function boolval($val) {
+    public static function boolval(mixed $val): ?bool
+    {
         if (!is_string($val)) {
             return null;
         }
 
         switch ($val) {
             case "on":
-                case "yes":
-                case "true":
-                case "1":
+            case "yes":
+            case "true":
+            case "1":
                 return true;
             break;
 
             case "off":
-                case "no":
-                case "false":
-                case "0":
+            case "no":
+            case "false":
+            case "0":
                 return false;
             break;
 
             default:
-            return null;
+                return null;
         }
     }
 }
-
-

From ba3e8c114c8a2b2f6947d508afdfdfdda59c86f9 Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Fri, 1 Mar 2024 18:23:51 +0100
Subject: [PATCH 6/7] Minor refactoring of Options_Parser

Fix parameter types.
Refactor handlerForOption method to always return an array.
Use strict equality operator for strings.
Cast option key passed to handlers to string.
Add short options from package handlers to short option list.
---
 phpdotnet/phd/Options/Parser.php | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/phpdotnet/phd/Options/Parser.php b/phpdotnet/phd/Options/Parser.php
index 14d54c46..7d4d96b7 100644
--- a/phpdotnet/phd/Options/Parser.php
+++ b/phpdotnet/phd/Options/Parser.php
@@ -8,7 +8,7 @@ class Options_Parser
     private array $packageHandlers = [];
 
     /**
-     * @param array<?Options_Interface> $packageHandlers
+     * @param ?array<Options_Interface> $packageHandlers
      */
     public function __construct(Options_Interface $defaultHandler, ?array $packageHandlers = []) {
         $this->defaultHandler = $defaultHandler;
@@ -17,9 +17,7 @@ public function __construct(Options_Interface $defaultHandler, ?array $packageHa
     }
 
     /**
-     * @param array<?Options_Interface> $packageHandlers
-     *
-     * @return array<?Options_Interface>
+     * @param array<Options_Interface> $packageHandlers
      */
     private function validatePackageHandlers(array $packageHandlers): void {
         foreach ($packageHandlers as $handler) {
@@ -31,9 +29,9 @@ private function validatePackageHandlers(array $packageHandlers): void {
     }
 
     /**
-     * @return ?array<Options_Interface, string>
+     * @return array<Options_Interface, string>
      */
-    private function handlerForOption(string $option): ?array {
+    private function handlerForOption(string $option): array {
         if (method_exists($this->defaultHandler, "option_{$option}")) {
             return [$this->defaultHandler, "option_{$option}"];
         }
@@ -46,7 +44,7 @@ private function handlerForOption(string $option): ?array {
                 return [$this->packageHandlers[$package], "option_{$opt[1]}"];
             }
         }
-        return null;
+        return [];
     }
 
     /**
@@ -64,7 +62,15 @@ private function getLongOptions(): array {
     }
 
     private function getShortOptions(): string {
-        return implode('', array_values($this->defaultHandler->optionList()));
+        $defaultOptions = array_values($this->defaultHandler->optionList());
+        $packageOptions = [];
+        foreach ($this->packageHandlers as $handler) {
+            foreach ($handler->optionList() as $opt) {
+                $packageOptions[] = $opt;
+            }
+        }
+        $options = array_merge($defaultOptions, $packageOptions);
+        return implode('', array_values($options));
     }
 
     /**
@@ -84,11 +90,11 @@ private function validateOptions(): void {
 
         for ($i=1; $i < $argc; $i++) {
             $checkArgv = explode('=', $argv[$i]);
-            if (substr($checkArgv[0], 0, 2) == '--') {
+            if (substr($checkArgv[0], 0, 2) === '--') {
                 if (!in_array(substr($checkArgv[0], 2), $long)) {
                     trigger_error('Invalid long option ' . $argv[$i], E_USER_ERROR);
                 }
-            } elseif (substr($checkArgv[0], 0, 1) == '-') {
+            } elseif (substr($checkArgv[0], 0, 1) === '-') {
                 if (!in_array(substr($checkArgv[0], 1), $short)) {
                     trigger_error('Invalid short option ' . $argv[$i], E_USER_ERROR);
                 }
@@ -107,7 +113,7 @@ public function getopt() {
         foreach ($args as $k => $v) {
             $handler = $this->handlerForOption((string) $k);
             if (is_callable($handler)) {
-                call_user_func($handler, $k, $v);
+                call_user_func($handler, (string) $k, $v);
             } else {
                 var_dump($k, $v);
                 trigger_error("Hmh, something weird has happend, I don't know this option", E_USER_ERROR);

From 4d7340aff504aad5db53433c2f773ca8f86a76ef Mon Sep 17 00:00:00 2001
From: haszi <haszika80@gmail.com>
Date: Sun, 3 Mar 2024 21:11:57 +0100
Subject: [PATCH 7/7] Address review comments

Remove string casts.
Change the package handlers array of the Parser constructor to a variadic Options_Interface and remove validation method.
---
 phpdotnet/phd/Options/Parser.php | 26 +++++++-------------------
 render.php                       |  2 +-
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/phpdotnet/phd/Options/Parser.php b/phpdotnet/phd/Options/Parser.php
index 7d4d96b7..a2b57722 100644
--- a/phpdotnet/phd/Options/Parser.php
+++ b/phpdotnet/phd/Options/Parser.php
@@ -5,29 +5,17 @@ class Options_Parser
 {
 
     private Options_Interface $defaultHandler;
+    /** @var array<Options_Interface> */
     private array $packageHandlers = [];
 
-    /**
-     * @param ?array<Options_Interface> $packageHandlers
-     */
-    public function __construct(Options_Interface $defaultHandler, ?array $packageHandlers = []) {
+    public function __construct(
+        Options_Interface $defaultHandler,
+        Options_Interface ...$packageHandlers
+    ) {
         $this->defaultHandler = $defaultHandler;
-        $this->validatePackageHandlers($packageHandlers);
         $this->packageHandlers = $packageHandlers;
     }
 
-    /**
-     * @param array<Options_Interface> $packageHandlers
-     */
-    private function validatePackageHandlers(array $packageHandlers): void {
-        foreach ($packageHandlers as $handler) {
-            if (!($handler instanceof Options_Interface)) {
-                trigger_error("All package handlers must implement Options_Interface", E_USER_ERROR);
-                break;
-            }
-        }
-    }
-
     /**
      * @return array<Options_Interface, string>
      */
@@ -111,9 +99,9 @@ public function getopt() {
         }
 
         foreach ($args as $k => $v) {
-            $handler = $this->handlerForOption((string) $k);
+            $handler = $this->handlerForOption($k);
             if (is_callable($handler)) {
-                call_user_func($handler, (string) $k, $v);
+                call_user_func($handler, $k, $v);
             } else {
                 var_dump($k, $v);
                 trigger_error("Hmh, something weird has happend, I don't know this option", E_USER_ERROR);
diff --git a/render.php b/render.php
index 18a21e5a..91390c02 100644
--- a/render.php
+++ b/render.php
@@ -28,7 +28,7 @@
         $packageHandlers[strtolower($package)] = $handler;
     }
 }
-$optionsParser = new Options_Parser(new Options_Handler, $packageHandlers);
+$optionsParser = new Options_Parser(new Options_Handler, ...$packageHandlers);
 $optionsParser->getopt();
 
 /* If no docbook file was passed, die */