From 541e2efb0e5505262a78ca6c27101e231b4ad729 Mon Sep 17 00:00:00 2001
From: Rodrigo Primo <rodrigosprimo@gmail.com>
Date: Thu, 19 Dec 2024 19:22:35 -0300
Subject: [PATCH] Apply suggestions from code review

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
---
 src/Config.php                         | 46 +++++++++++++-------------
 tests/Core/Config/GeneratorArgTest.php | 40 +++++++++++-----------
 2 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/src/Config.php b/src/Config.php
index cfbae896b9..7e3714555c 100644
--- a/src/Config.php
+++ b/src/Config.php
@@ -170,6 +170,21 @@ class Config
      */
     private $cliArgs = [];
 
+    /**
+     * A list of valid generators.
+     *
+     * {@internal Once support for PHP < 5.6 is dropped, this property should be refactored into a
+     * class constant.}
+     *
+     * @var array<string, string> Keys are the lowercase version of the generator name, while values
+     *                            are the associated PHP generator class.
+     */
+    private $validGenerators = [
+        'text'     => 'Text',
+        'html'     => 'HTML',
+        'markdown' => 'Markdown',
+    ];
+
     /**
      * Command line values that the user has supplied directly.
      *
@@ -198,23 +213,6 @@ class Config
      */
     private static $executablePaths = [];
 
-    /**
-     * A list of valid generators.
-     *
-     * - Keys: lowercase version of the generator name.
-     * - Values: name of the generator PHP class.
-     *
-     * Note: once support for PHP < 5.6 is dropped, this property should be refactored into a class
-     * constant.
-     *
-     * @var array<string, string>
-     */
-    private static $validGenerators = [
-        'text'     => 'Text',
-        'html'     => 'HTML',
-        'markdown' => 'Markdown',
-    ];
-
 
     /**
      * Get the value of an inaccessible property.
@@ -1253,18 +1251,20 @@ public function processLongArgument($arg, $pos)
                 $generatorName          = substr($arg, 10);
                 $lowerCaseGeneratorName = strtolower($generatorName);
 
-                if (isset(self::$validGenerators[$lowerCaseGeneratorName]) === false) {
-                    $validOptions = implode(', ', array_values(self::$validGenerators));
-                    $error        = sprintf(
-                        'ERROR: "%s" is not a valid generator. Valid options are: %s.'.PHP_EOL.PHP_EOL,
+                if (isset($this->validGenerators[$lowerCaseGeneratorName]) === false) {
+                    $lastOption    = array_pop($this->validGenerators);
+                    $validOptions  = implode(', ', $this->validGenerators);
+                    $validOptions .= ' and '.$lastOption;
+                    $error         = sprintf(
+                        'ERROR: "%s" is not a valid generator. The following generators are supported: %s.'.PHP_EOL.PHP_EOL,
                         $generatorName,
                         $validOptions
                     );
-                    $error       .= $this->printShortUsage(true);
+                    $error        .= $this->printShortUsage(true);
                     throw new DeepExitException($error, 3);
                 }
 
-                $this->generator = self::$validGenerators[$lowerCaseGeneratorName];
+                $this->generator = $this->validGenerators[$lowerCaseGeneratorName];
                 self::$overriddenDefaults['generator'] = true;
             } else if (substr($arg, 0, 9) === 'encoding=') {
                 if (isset(self::$overriddenDefaults['encoding']) === true) {
diff --git a/tests/Core/Config/GeneratorArgTest.php b/tests/Core/Config/GeneratorArgTest.php
index 72949409e9..f0fffd033f 100644
--- a/tests/Core/Config/GeneratorArgTest.php
+++ b/tests/Core/Config/GeneratorArgTest.php
@@ -23,7 +23,7 @@ final class GeneratorArgTest extends TestCase
     /**
      * Ensure that the generator property is set when the parameter is passed a valid value.
      *
-     * @param string $argumentValue         Generator name passed in the command line.
+     * @param string $argumentValue         Generator name passed on the command line.
      * @param string $expectedPropertyValue Expected value of the generator property.
      *
      * @dataProvider dataValidGeneratorNames
@@ -49,29 +49,29 @@ public function testValidGenerators($argumentValue, $expectedPropertyValue)
     public static function dataValidGeneratorNames()
     {
         return [
-            [
-                'Text',
-                'Text',
+            'Text generator passed'            => [
+                'argumentValue'         => 'Text',
+                'expectedPropertyValue' => 'Text',
             ],
-            [
-                'HTML',
-                'HTML',
+            'HTML generator passed'            => [
+                'argumentValue'         => 'HTML',
+                'expectedPropertyValue' => 'HTML',
             ],
-            [
-                'Markdown',
-                'Markdown',
+            'Markdown generator passed'        => [
+                'argumentValue'         => 'Markdown',
+                'expectedPropertyValue' => 'Markdown',
             ],
-            [
-                'TEXT',
-                'Text',
+            'Uppercase Text generator passed'  => [
+                'argumentValue'         => 'TEXT',
+                'expectedPropertyValue' => 'Text',
             ],
-            [
-                'tEXt',
-                'Text',
+            'Mixed case Text generator passed' => [
+                'argumentValue'         => 'tEXt',
+                'expectedPropertyValue' => 'Text',
             ],
-            [
-                'html',
-                'HTML',
+            'Lowercase HTML generator passed'  => [
+                'argumentValue'         => 'html',
+                'expectedPropertyValue' => 'HTML',
             ],
         ];
 
@@ -110,7 +110,7 @@ public function testOnlySetOnce()
     public function testInvalidGenerator($generatorName)
     {
         $exception = 'PHP_CodeSniffer\Exceptions\DeepExitException';
-        $message   = 'ERROR: "'.$generatorName.'" is not a valid generator. Valid options are: Text, HTML, Markdown.';
+        $message   = 'ERROR: "'.$generatorName.'" is not a valid generator. The following generators are supported: Text, HTML and Markdown.';
 
         if (method_exists($this, 'expectException') === true) {
             // PHPUnit 5+.