From 8a547a4894174f9c6026b02f915a5c7ec22e13f0 Mon Sep 17 00:00:00 2001
From: Ralf Lang <ralf.lang@ralf-lang.de>
Date: Thu, 2 Nov 2023 07:40:14 +0100
Subject: [PATCH 1/4] Add ArgvWrapper implementation and unit tests along with
 the shell of a truely signature/object driven draft of the parser.

---
 src/ArgvParser.php       |  9 ++++++++
 src/ArgvWrapper.php      | 50 ++++++++++++++++++++++++++++++++++++++++
 src/ImmutableParser.php  | 11 +++++++++
 src/ParserArgument.php   | 11 +++++++++
 src/ParserBuilder.php    | 17 ++++++++++++++
 src/ParserOption.php     | 13 +++++++++++
 src/ParserResult.php     | 10 ++++++++
 test/AllTests.php        |  6 +++++
 test/ArgvWrapperTest.php | 28 ++++++++++++++++++++++
 test/bootstrap.php       | 13 +++++++++++
 10 files changed, 168 insertions(+)
 create mode 100644 src/ArgvParser.php
 create mode 100644 src/ArgvWrapper.php
 create mode 100644 src/ImmutableParser.php
 create mode 100644 src/ParserArgument.php
 create mode 100644 src/ParserBuilder.php
 create mode 100644 src/ParserOption.php
 create mode 100644 src/ParserResult.php
 create mode 100644 test/AllTests.php
 create mode 100644 test/ArgvWrapperTest.php
 create mode 100644 test/bootstrap.php

diff --git a/src/ArgvParser.php b/src/ArgvParser.php
new file mode 100644
index 0000000..d3f27be
--- /dev/null
+++ b/src/ArgvParser.php
@@ -0,0 +1,9 @@
+<?php
+namespace Horde\Argv;
+
+/**
+ * Generic interface of command line arguments
+ */
+interface ArgvParser
+{
+}
\ No newline at end of file
diff --git a/src/ArgvWrapper.php b/src/ArgvWrapper.php
new file mode 100644
index 0000000..f739289
--- /dev/null
+++ b/src/ArgvWrapper.php
@@ -0,0 +1,50 @@
+<?php
+declare(strict_types=1);
+namespace Horde\Argv;
+
+use InvalidArgumentException;
+use IteratorAggregate;
+use RuntimeException;
+use Traversable;
+use Countable;
+/**
+ * Wrap a copy of Argv into a simple, typed object for DI
+ */
+class ArgvWrapper implements IteratorAggregate, Countable
+{
+    private readonly array $argv;
+
+    public function __construct(array $argv)
+    {
+        $argvCopy = [];
+        // TODO: Check if the array is actually argv-like
+        foreach ($argv as $pos => $argument) {
+            if (!is_string($argument)) {
+                throw new InvalidArgumentException('All members of argv must be strings.');
+            }
+
+            $argvCopy[] = $argument;
+        }
+        $this->argv = $argvCopy;
+    }
+
+    public function getIterator(): Traversable
+    {
+        return new \ArrayIterator($this->argv);
+    }
+
+    public function count(): int
+    {
+        return count($this->argv);
+    }
+
+    public static function fromGlobal()
+    {
+        if (empty($GLOBALS['argv']))
+        {
+            // Argv always contains at least the binary's name so this indicates a severe error
+            throw new RuntimeException("Argv Global is not available or in invalid state");
+        }
+        return new ArgvWrapper($GLOBALS['argv']);
+    }
+}
\ No newline at end of file
diff --git a/src/ImmutableParser.php b/src/ImmutableParser.php
new file mode 100644
index 0000000..fc9ab54
--- /dev/null
+++ b/src/ImmutableParser.php
@@ -0,0 +1,11 @@
+<?php
+namespace Horde\Argv\Parser;
+
+/**
+ * An immutable parser can only configured once through its constructor
+ * and provides no interface for later modification.
+ */
+class ImmutableParser implements ArgvParser
+{
+
+}
\ No newline at end of file
diff --git a/src/ParserArgument.php b/src/ParserArgument.php
new file mode 100644
index 0000000..65e9773
--- /dev/null
+++ b/src/ParserArgument.php
@@ -0,0 +1,11 @@
+<?php
+namespace Horde\Argv\Parser;
+
+/**
+ * Generic interface of command line arguments
+ */
+interface ParserArgument
+{
+    public function getNames();
+
+}
\ No newline at end of file
diff --git a/src/ParserBuilder.php b/src/ParserBuilder.php
new file mode 100644
index 0000000..b4b96a8
--- /dev/null
+++ b/src/ParserBuilder.php
@@ -0,0 +1,17 @@
+<?php
+namespace Horde\Argv\Parser;
+
+/**
+ * The ParserBuilder implements the fluent builder pattern.
+ *
+ * It is technically a usable, mutable parser
+ * Eject to an ImmutableParser to seal it against modification
+ *
+ */
+class ParserBuilder implements Parser
+{
+    public function withArgument(ParserArgument $argument)
+    {
+
+    }
+}
\ No newline at end of file
diff --git a/src/ParserOption.php b/src/ParserOption.php
new file mode 100644
index 0000000..f893e2b
--- /dev/null
+++ b/src/ParserOption.php
@@ -0,0 +1,13 @@
+<?php
+namespace Horde\Argv\Parser;
+
+/**
+ * Generic interface of an option
+ */
+interface Option
+{
+    /**
+     * Return a list of all names the option has
+     */
+    public function getNames(): iterable;
+}
\ No newline at end of file
diff --git a/src/ParserResult.php b/src/ParserResult.php
new file mode 100644
index 0000000..509ec98
--- /dev/null
+++ b/src/ParserResult.php
@@ -0,0 +1,10 @@
+<?php
+namespace Horde\Argv\Parser;
+
+/**
+ * A class holding the result of the parsing process
+ */
+class ParserResult
+{
+
+}
\ No newline at end of file
diff --git a/test/AllTests.php b/test/AllTests.php
new file mode 100644
index 0000000..87bf3d7
--- /dev/null
+++ b/test/AllTests.php
@@ -0,0 +1,6 @@
+<?php
+
+if (!class_exists(\Horde_Test_AllTests::class)) {
+    require_once 'Horde/Test/AllTests.php';
+}
+Horde_Test_AllTests::init(__FILE__)->run();
diff --git a/test/ArgvWrapperTest.php b/test/ArgvWrapperTest.php
new file mode 100644
index 0000000..445426f
--- /dev/null
+++ b/test/ArgvWrapperTest.php
@@ -0,0 +1,28 @@
+<?php
+/**
+ * @author     Ralf Lang <ralf.lang@ralf-lang.de>
+ */
+namespace Horde\Argv\Test;
+
+use PHPUnit\Framework\TestCase;
+use Horde\Argv\ArgvWrapper;
+
+class ArgvWrapperTest extends TestCase
+{
+    public function testCountArguments()
+    {
+        $argv = new ArgvWrapper(['foo', 'bar']);
+        $this->assertEquals(count($argv), 2);
+        $this->assertEquals($argv->count(), 2);
+        $this->assertCount(2, $argv, 'Object wrapping two values counts two.');
+    }
+
+    public function testCastingToArray()
+    {
+        $inArray = ['foo', 'bar'];
+        $argv = new ArgvWrapper($inArray);
+        $outArray = iterator_to_array($argv);
+        $this->assertEquals($inArray, $outArray, 'Wrapper can be cast back to array');
+
+    }
+}
diff --git a/test/bootstrap.php b/test/bootstrap.php
new file mode 100644
index 0000000..88f2d9b
--- /dev/null
+++ b/test/bootstrap.php
@@ -0,0 +1,13 @@
+<?php
+
+$candidates = [
+    dirname(__FILE__, 2) . '/vendor/autoload.php',
+    dirname(__FILE__, 4) . '/autoload.php',
+];
+// Cover root case and library case
+foreach ($candidates as $candidate) {
+    if (file_exists($candidate)) {
+        require_once $candidate;
+    }
+}
+\Horde_Test_Bootstrap::bootstrap(dirname(__FILE__));

From b231d77b42af66b896c33daba21b953bc31726f1 Mon Sep 17 00:00:00 2001
From: Ralf Lang <ralf.lang@ralf-lang.de>
Date: Fri, 2 May 2025 11:07:37 +0200
Subject: [PATCH 2/4] Mark value object as stdclass. It has dynamic properties
 by design

---
 src/Values.php | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/Values.php b/src/Values.php
index d60d187..717bb34 100644
--- a/src/Values.php
+++ b/src/Values.php
@@ -19,9 +19,12 @@
 use ArrayAccess;
 use Countable;
 use IteratorAggregate;
+use stdClass;
 
 /**
  * Result hash for Horde_Argv_Parser
+ * 
+ * This is a value object and inherently uses dynamic properties. Do not try to "fix" this.
  *
  * @category  Horde
  * @package   Argv
@@ -30,7 +33,7 @@
  * @copyright 2010-2017 Horde LLC
  * @license   http://www.horde.org/licenses/bsd BSD
  */
-class Values implements IteratorAggregate, ArrayAccess, Countable
+class Values extends stdClass implements IteratorAggregate, ArrayAccess, Countable
 {
     public function __construct($defaults = array())
     {

From a472e711cea50f3328105b9cd52cc838083462ad Mon Sep 17 00:00:00 2001
From: Ralf Lang <ralf.lang@ralf-lang.de>
Date: Fri, 2 May 2025 11:27:01 +0200
Subject: [PATCH 3/4] feat(!): Use mixed return type - bump minimum required
 PHP version to 8.0

Add many missing public properties
readonly attribute needs type
OptionContainer->addOption adds a property Option itself is not aware of
more builtin interface method compat
---
 .horde.yml                |  2 +-
 lib/Horde/Argv/Option.php |  2 +-
 src/HelpFormatter.php     | 13 +++++++++++++
 src/Option.php            | 16 ++++++++++++++++
 src/OptionGroup.php       |  3 ++-
 src/Parser.php            | 15 ++++++++++++---
 src/Values.php            | 25 +++++++++++++------------
 7 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/.horde.yml b/.horde.yml
index 6cec020..5027946 100644
--- a/.horde.yml
+++ b/.horde.yml
@@ -38,7 +38,7 @@ license:
   uri: http://www.horde.org/licenses/bsd
 dependencies:
   required:
-    php: ^7.4 || ^8
+    php: ^8
     composer:
       horde/cli: ^3
       horde/exception: ^3
diff --git a/lib/Horde/Argv/Option.php b/lib/Horde/Argv/Option.php
index 850d404..958f45c 100644
--- a/lib/Horde/Argv/Option.php
+++ b/lib/Horde/Argv/Option.php
@@ -277,7 +277,7 @@ public function checkChoice($opt, $value)
     public $callbackArgs;
     public $help;
     public $metavar;
-    public $container; 
+    public $container;
 
     /**
      * Constructor.
diff --git a/src/HelpFormatter.php b/src/HelpFormatter.php
index e37d8a9..17191c9 100644
--- a/src/HelpFormatter.php
+++ b/src/HelpFormatter.php
@@ -76,6 +76,19 @@ abstract class HelpFormatter
     const NO_DEFAULT_VALUE = 'none';
 
     public $parser = null;
+    public $_color;
+    public $indent_increment;
+    public $max_help_position;
+    public $help_position;
+    public $width;
+    public $level;
+    public $current_indent;
+    public $help_width;
+    public $default_tag;
+    public $option_strings;
+    public $_short_opt_fmt;
+    public $_long_opt_fmt;
+    public $short_first;
 
     public function __construct(
         $indent_increment, $max_help_position, $width = null,
diff --git a/src/Option.php b/src/Option.php
index f165016..43a8f4a 100644
--- a/src/Option.php
+++ b/src/Option.php
@@ -155,6 +155,7 @@ public function checkChoice($opt, $value)
         'help',
         'metavar',
     );
+    
 
     /**
      * The set of actions allowed by option parsers.  Explicitly listed here so
@@ -267,14 +268,29 @@ public function checkChoice($opt, $value)
 
     public $shortOpts = array();
     public $longOpts = array();
+    // These should probably be made readonly once we are sure we *really* usually set them through a constructor
+    public $action;
+    public $type;
     public $dest;
     public $default;
+    public $nargs;
+    public $const;
+    public $choices;
+    public $callback;
+    public $callbackArgs;
+    public $help;
+    // TODO: Where is this even used?
+    public $metavar;
+    // Used in OptionContainer->addOption
+    public $container; 
 
     /**
      * Constructor.
      */
     public function __construct()
     {
+        // TODO: Refactor this to use optional constructor properties
+
         // The last argument to this function is an $attrs hash, if it
         // is present and an array. All other arguments are $opts.
         $opts = func_get_args();
diff --git a/src/OptionGroup.php b/src/OptionGroup.php
index eac7af9..564ccab 100644
--- a/src/OptionGroup.php
+++ b/src/OptionGroup.php
@@ -30,8 +30,9 @@
 class OptionGroup extends OptionContainer
 {
     protected $_title;
+    public readonly Parser $parser;
 
-    public function __construct($parser, $title, $description = null)
+    public function __construct(Parser $parser, $title, $description = null)
     {
         $this->parser = $parser;
         parent::__construct($parser->optionClass, $parser->conflictHandler, $description);
diff --git a/src/Parser.php b/src/Parser.php
index 409f208..8b6fc60 100644
--- a/src/Parser.php
+++ b/src/Parser.php
@@ -84,10 +84,19 @@
  */
 class Parser extends OptionContainer
 {
-    public $standardOptionList = array();
-
+    public $standardOptionList = [];
     protected $_usage;
-    public $optionGroups = array();
+    public $prog;
+    public $epilog;
+    public $optionGroups = [];
+    public $allowInterspersedArgs;
+    public $ignoreUnknownArgs;
+    public $rargs;
+    public $largs;
+    public $values;
+    public $formatter;
+    public $version;
+    public $allowUnknownArgs;
 
     public function __construct($args = array())
     {
diff --git a/src/Values.php b/src/Values.php
index 717bb34..70dae83 100644
--- a/src/Values.php
+++ b/src/Values.php
@@ -18,6 +18,7 @@
 use ArrayIterator;
 use ArrayAccess;
 use Countable;
+use Iterator;
 use IteratorAggregate;
 use stdClass;
 
@@ -42,7 +43,7 @@ public function __construct($defaults = array())
         }
     }
 
-    public function __toString()
+    public function __toString(): string
     {
         $str = array();
         foreach ($this as $attr => $val) {
@@ -51,37 +52,37 @@ public function __toString()
         return implode(', ', $str);
     }
 
-    public function offsetExists($attr)
+    public function offsetExists(mixed $offset): bool
     {
-        return isset($this->$attr) && !is_null($this->$attr);
+        return isset($this->$offset) && !is_null($this->$offset);
     }
 
-    public function offsetGet($attr)
+    public function offsetGet(mixed $offset): mixed
     {
-        return $this->$attr;
+        return $this->$offset;
     }
 
-    public function offsetSet($attr, $val)
+    public function offsetSet(mixed $offset, mixed $value): void
     {
-        $this->$attr = $val;
+        $this->$offset = $value;
     }
 
-    public function offsetUnset($attr)
+    public function offsetUnset(mixed $offset): void
     {
-        unset($this->$attr);
+        unset($this->$offset);
     }
 
-    public function getIterator()
+    public function getIterator(): Iterator
     {
         return new ArrayIterator(get_object_vars($this));
     }
 
-    public function count()
+    public function count(): int
     {
         return count(get_object_vars($this));
     }
 
-    public function ensureValue($attr, $value)
+    public function ensureValue(mixed $attr, mixed $value): mixed
     {
         if (is_null($this->$attr)) {
             $this->$attr = $value;

From 0d030914d2d5114f424812506012424c166802e8 Mon Sep 17 00:00:00 2001
From: Ralf Lang <ralf.lang@ralf-lang.de>
Date: Sat, 3 May 2025 07:10:36 +0200
Subject: [PATCH 4/4] feat: phpstan compatibility improvements level 1

---
 src/ImmutableParser.php |  2 +-
 src/Option.php          | 58 +++++++++++++++++++++++++----------------
 src/OptionException.php |  1 +
 src/Parser.php          |  4 +--
 4 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/src/ImmutableParser.php b/src/ImmutableParser.php
index fc9ab54..63d8332 100644
--- a/src/ImmutableParser.php
+++ b/src/ImmutableParser.php
@@ -1,5 +1,5 @@
 <?php
-namespace Horde\Argv\Parser;
+namespace Horde\Argv;
 
 /**
  * An immutable parser can only configured once through its constructor
diff --git a/src/Option.php b/src/Option.php
index 43a8f4a..c34bce2 100644
--- a/src/Option.php
+++ b/src/Option.php
@@ -15,6 +15,10 @@
  * @package  Argv
  */
 namespace Horde\Argv;
+use Iterator;
+use InvalidArgumentException;
+use RuntimeException;
+
 
 /**
  * Defines the Option class and some standard value-checking functions.
@@ -131,8 +135,8 @@ public function checkChoice($opt, $value)
                 $choices[] = (string)$choice;
             }
             $choices = "'" . implode("', '", $choices) . "'";
-            throw new Horde_Argv_OptionValueException(sprintf(
-                Horde_Argv_Translation::t("option %s: invalid choice: '%s' (choose from %s)"),
+            throw new OptionValueException(sprintf(
+                Translation::t("option %s: invalid choice: '%s' (choose from %s)"),
                 $opt, $value, $choices));
         }
     }
@@ -339,17 +343,17 @@ protected function _setOptStrings($opts)
             $opt = (string)$opt;
 
             if (strlen($opt) < 2) {
-                throw new Horde_Argv_OptionException(sprintf("invalid option string '%s': must be at least two characters long", $opt), $this);
+                throw new OptionException(sprintf("invalid option string '%s': must be at least two characters long", $opt), $this);
             } elseif (strlen($opt) == 2) {
                 if (!($opt[0] == '-' && $opt[1] != '-')) {
-                    throw new Horde_Argv_OptionException(sprintf(
+                    throw new OptionException(sprintf(
                         "invalid short option string '%s': " .
                         "must be of the form -x, (x any non-dash char)", $opt), $this);
                 }
                 $this->shortOpts[] = $opt;
             } else {
                 if (!(substr($opt, 0, 2) == '--' && $opt[2] != '-')) {
-                    throw new Horde_Argv_OptionException(sprintf(
+                    throw new OptionException(sprintf(
                         "invalid long option string '%s': " .
                         "must start with --, followed by non-dash", $opt), $this);
                 }
@@ -376,7 +380,7 @@ protected function _setAttrs($attrs)
         if ($attrs) {
             $attrs = array_keys($attrs);
             sort($attrs);
-            throw new Horde_Argv_OptionException(sprintf(
+            throw new OptionException(sprintf(
                 'invalid keyword arguments: %s', implode(', ', $attrs)), $this);
         }
     }
@@ -389,7 +393,7 @@ public function _checkAction()
         if (is_null($this->action)) {
             $this->action = 'store';
         } elseif (!in_array($this->action, $this->ACTIONS)) {
-            throw new Horde_Argv_OptionException(sprintf("invalid action: '%s'", $this->action), $this);
+            throw new OptionException(sprintf("invalid action: '%s'", $this->action), $this);
         }
     }
 
@@ -411,11 +415,11 @@ public function _checkType()
             }
 
             if (!in_array($this->type, $this->TYPES)) {
-                throw new Horde_Argv_OptionException(sprintf("invalid option type: '%s'", $this->type), $this);
+                throw new OptionException(sprintf("invalid option type: '%s'", $this->type), $this);
             }
 
             if (!in_array($this->action, $this->TYPED_ACTIONS)) {
-                throw new Horde_Argv_OptionException(sprintf(
+                throw new OptionException(sprintf(
                     "must not supply a type for action '%s'", $this->action), $this);
             }
         }
@@ -425,15 +429,15 @@ public function _checkChoice()
     {
         if ($this->type == 'choice') {
             if (is_null($this->choices)) {
-                throw new Horde_Argv_OptionException(
+                throw new OptionException(
                     "must supply a list of choices for type 'choice'", $this);
             } elseif (!(is_array($this->choices) || $this->choices instanceof Iterator)) {
-                throw new Horde_Argv_OptionException(sprintf(
+                throw new OptionException(sprintf(
                     "choices must be a list of strings ('%s' supplied)",
                     gettype($this->choices)), $this);
             }
         } elseif (!is_null($this->choices)) {
-            throw new Horde_Argv_OptionException(sprintf(
+            throw new OptionException(sprintf(
                 "must not supply choices for type '%s'", $this->type), $this);
         }
     }
@@ -459,7 +463,7 @@ public function _checkDest()
     public function _checkConst()
     {
         if (!in_array($this->action, $this->CONST_ACTIONS) && !is_null($this->const)) {
-            throw new Horde_Argv_OptionException(sprintf(
+            throw new OptionException(sprintf(
                 "'const' must not be supplied for action '%s'", $this->action),
                 $this);
         }
@@ -472,7 +476,7 @@ public function _checkNargs()
                 $this->nargs = 1;
             }
         } elseif (!is_null($this->nargs)) {
-            throw new Horde_Argv_OptionException(sprintf(
+            throw new OptionException(sprintf(
                 "'nargs' must not be supplied for action '%s'", $this->action),
                 $this);
         }
@@ -480,30 +484,38 @@ public function _checkNargs()
 
     public function _checkCallback()
     {
+        // if action is callback, callback must exist and be valid. If not, callback must be null or exist and be valid
         if ($this->action == 'callback') {
+            // Callback must be a callable or an array with object as first item and method name as second item OR a string in format CLASS#method
             if (!is_callable($this->callback)) {
-                $callback_name = is_array($this->callback) ?
-                    is_object($this->callback[0]) ? get_class($this->callback[0] . '#' . $this->callback[1]) : implode('#', $this->callback) :
-                    $this->callback;
-                throw new Horde_Argv_OptionException(sprintf(
-                    "callback not callable: '%s'", $callback_name), $this);
+                $callback_name = '';
+                if (is_array($this->callback)) {
+                    if (is_object($this->callback[0])) {
+                        $callback_name = get_class($this->callback[0]) . '#' .  $this->callback[1];
+                    } else {
+                        $callback_name = implode('#', $this->callback);
+                        }
+                } else {
+                    throw new OptionException(sprintf(
+                        "callback not callable: '%s'", $callback_name), $this);
+                }
             }
             if (!is_null($this->callbackArgs) && !is_array($this->callbackArgs)) {
-                throw new Horde_Argv_OptionException(sprintf(
+                throw new OptionException(sprintf(
                     "callbackArgs, if supplied, must be an array: not '%s'",
                     $this->callbackArgs), $this);
             }
         } else {
             if (!is_null($this->callback)) {
                 $callback_name = is_array($this->callback) ?
-                    is_object($this->callback[0]) ? get_class($this->callback[0] . '#' . $this->callback[1]) : implode('#', $this->callback) :
+                    is_object($this->callback[0]) ? get_class($this->callback[0]) . '#' . $this->callback[1] : implode('#', $this->callback) :
                     $this->callback;
-                throw new Horde_Argv_OptionException(sprintf(
+                throw new OptionException(sprintf(
                     "callback supplied ('%s') for non-callback option",
                     $callback_name), $this);
             }
             if (!is_null($this->callbackArgs)) {
-                throw new Horde_Argv_OptionException(
+                throw new OptionException(
                     'callbackArgs supplied for non-callback option', $this);
             }
         }
diff --git a/src/OptionException.php b/src/OptionException.php
index 373b8f3..6bc2153 100644
--- a/src/OptionException.php
+++ b/src/OptionException.php
@@ -29,6 +29,7 @@
  */
 class OptionException extends Exception
 {
+    public string $optionId;
     public function __construct($msg, $option = null)
     {
         $this->optionId = (string)$option;
diff --git a/src/Parser.php b/src/Parser.php
index 8b6fc60..f901433 100644
--- a/src/Parser.php
+++ b/src/Parser.php
@@ -82,7 +82,7 @@
  * @copyright 2010-2017 Horde LLC
  * @license   http://www.horde.org/licenses/bsd BSD
  */
-class Parser extends OptionContainer
+class Parser extends OptionContainer implements ArgvParser
 {
     public $standardOptionList = [];
     protected $_usage;
@@ -478,7 +478,7 @@ protected function _processLongOpt(&$rargs, &$values)
                 throw $e;
             }
         }
-
+        $value = null;
         if ($option->takesValue()) {
             $nargs = $option->nargs;
             if (count($rargs) < $nargs) {