Skip to content

Commit 541e2ef

Browse files
rodrigoprimojrfnl
andcommitted
Apply suggestions from code review
Co-authored-by: Juliette <[email protected]>
1 parent 6bb661d commit 541e2ef

File tree

2 files changed

+43
-43
lines changed

2 files changed

+43
-43
lines changed

src/Config.php

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,21 @@ class Config
170170
*/
171171
private $cliArgs = [];
172172

173+
/**
174+
* A list of valid generators.
175+
*
176+
* {@internal Once support for PHP < 5.6 is dropped, this property should be refactored into a
177+
* class constant.}
178+
*
179+
* @var array<string, string> Keys are the lowercase version of the generator name, while values
180+
* are the associated PHP generator class.
181+
*/
182+
private $validGenerators = [
183+
'text' => 'Text',
184+
'html' => 'HTML',
185+
'markdown' => 'Markdown',
186+
];
187+
173188
/**
174189
* Command line values that the user has supplied directly.
175190
*
@@ -198,23 +213,6 @@ class Config
198213
*/
199214
private static $executablePaths = [];
200215

201-
/**
202-
* A list of valid generators.
203-
*
204-
* - Keys: lowercase version of the generator name.
205-
* - Values: name of the generator PHP class.
206-
*
207-
* Note: once support for PHP < 5.6 is dropped, this property should be refactored into a class
208-
* constant.
209-
*
210-
* @var array<string, string>
211-
*/
212-
private static $validGenerators = [
213-
'text' => 'Text',
214-
'html' => 'HTML',
215-
'markdown' => 'Markdown',
216-
];
217-
218216

219217
/**
220218
* Get the value of an inaccessible property.
@@ -1253,18 +1251,20 @@ public function processLongArgument($arg, $pos)
12531251
$generatorName = substr($arg, 10);
12541252
$lowerCaseGeneratorName = strtolower($generatorName);
12551253

1256-
if (isset(self::$validGenerators[$lowerCaseGeneratorName]) === false) {
1257-
$validOptions = implode(', ', array_values(self::$validGenerators));
1258-
$error = sprintf(
1259-
'ERROR: "%s" is not a valid generator. Valid options are: %s.'.PHP_EOL.PHP_EOL,
1254+
if (isset($this->validGenerators[$lowerCaseGeneratorName]) === false) {
1255+
$lastOption = array_pop($this->validGenerators);
1256+
$validOptions = implode(', ', $this->validGenerators);
1257+
$validOptions .= ' and '.$lastOption;
1258+
$error = sprintf(
1259+
'ERROR: "%s" is not a valid generator. The following generators are supported: %s.'.PHP_EOL.PHP_EOL,
12601260
$generatorName,
12611261
$validOptions
12621262
);
1263-
$error .= $this->printShortUsage(true);
1263+
$error .= $this->printShortUsage(true);
12641264
throw new DeepExitException($error, 3);
12651265
}
12661266

1267-
$this->generator = self::$validGenerators[$lowerCaseGeneratorName];
1267+
$this->generator = $this->validGenerators[$lowerCaseGeneratorName];
12681268
self::$overriddenDefaults['generator'] = true;
12691269
} else if (substr($arg, 0, 9) === 'encoding=') {
12701270
if (isset(self::$overriddenDefaults['encoding']) === true) {

tests/Core/Config/GeneratorArgTest.php

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ final class GeneratorArgTest extends TestCase
2323
/**
2424
* Ensure that the generator property is set when the parameter is passed a valid value.
2525
*
26-
* @param string $argumentValue Generator name passed in the command line.
26+
* @param string $argumentValue Generator name passed on the command line.
2727
* @param string $expectedPropertyValue Expected value of the generator property.
2828
*
2929
* @dataProvider dataValidGeneratorNames
@@ -49,29 +49,29 @@ public function testValidGenerators($argumentValue, $expectedPropertyValue)
4949
public static function dataValidGeneratorNames()
5050
{
5151
return [
52-
[
53-
'Text',
54-
'Text',
52+
'Text generator passed' => [
53+
'argumentValue' => 'Text',
54+
'expectedPropertyValue' => 'Text',
5555
],
56-
[
57-
'HTML',
58-
'HTML',
56+
'HTML generator passed' => [
57+
'argumentValue' => 'HTML',
58+
'expectedPropertyValue' => 'HTML',
5959
],
60-
[
61-
'Markdown',
62-
'Markdown',
60+
'Markdown generator passed' => [
61+
'argumentValue' => 'Markdown',
62+
'expectedPropertyValue' => 'Markdown',
6363
],
64-
[
65-
'TEXT',
66-
'Text',
64+
'Uppercase Text generator passed' => [
65+
'argumentValue' => 'TEXT',
66+
'expectedPropertyValue' => 'Text',
6767
],
68-
[
69-
'tEXt',
70-
'Text',
68+
'Mixed case Text generator passed' => [
69+
'argumentValue' => 'tEXt',
70+
'expectedPropertyValue' => 'Text',
7171
],
72-
[
73-
'html',
74-
'HTML',
72+
'Lowercase HTML generator passed' => [
73+
'argumentValue' => 'html',
74+
'expectedPropertyValue' => 'HTML',
7575
],
7676
];
7777

@@ -110,7 +110,7 @@ public function testOnlySetOnce()
110110
public function testInvalidGenerator($generatorName)
111111
{
112112
$exception = 'PHP_CodeSniffer\Exceptions\DeepExitException';
113-
$message = 'ERROR: "'.$generatorName.'" is not a valid generator. Valid options are: Text, HTML, Markdown.';
113+
$message = 'ERROR: "'.$generatorName.'" is not a valid generator. The following generators are supported: Text, HTML and Markdown.';
114114

115115
if (method_exists($this, 'expectException') === true) {
116116
// PHPUnit 5+.

0 commit comments

Comments
 (0)