Skip to content

Commit a6ae1eb

Browse files
author
Chirag Aggarwal
committed
fix: preserve empty-string sentinel in coerce()
Optional params with `--flag=` (or default `''`) bypass `validate()` entirely, so they reach `coerce()` un-validated. The previous version ran them through `filter_var($value, FILTER_VALIDATE_BOOLEAN)`, which silently returns `false` for empty strings -- a behaviour change for callers (e.g. Cloud's `Patch.php`) that use `''` as a three-state "not set" sentinel before resolving to `null`. Now coerce returns early for empty strings, and uses `FILTER_NULL_ON_FAILURE` for everything else so any unrecognised input falls back to the original value rather than getting silently downgraded to `false`. Spotted by Greptile review on PR #52.
1 parent 8686789 commit a6ae1eb

2 files changed

Lines changed: 34 additions & 5 deletions

File tree

src/CLI/CLI.php

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -424,15 +424,18 @@ protected function validate(string $key, array $param, $value): void
424424
* to happen here at the dispatch boundary.
425425
*
426426
* Only string inputs are coerced; bool defaults are passed through
427-
* untouched.
427+
* untouched. Strings that `filter_var` doesn't recognise (including the
428+
* empty string, which bypasses `validate()` for optional params) are
429+
* passed through unchanged so callers that use `''` as a sentinel for
430+
* "not set" keep working.
428431
*
429432
* @param Validator|callable $validator
430433
* @param mixed $value
431434
* @return mixed
432435
*/
433436
protected function coerce(Validator|callable $validator, mixed $value): mixed
434437
{
435-
if (! is_string($value)) {
438+
if (! is_string($value) || $value === '') {
436439
return $value;
437440
}
438441

@@ -444,11 +447,13 @@ protected function coerce(Validator|callable $validator, mixed $value): mixed
444447
$validator = $validator->getValidator();
445448
}
446449

447-
if ($validator instanceof Boolean) {
448-
return \filter_var($value, FILTER_VALIDATE_BOOLEAN);
450+
if (! ($validator instanceof Boolean)) {
451+
return $value;
449452
}
450453

451-
return $value;
454+
$coerced = \filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);
455+
456+
return $coerced === null ? $value : $coerced;
452457
}
453458

454459
public function setContainer(Container $container): self

tests/CLI/CLITest.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,30 @@ public function testBooleanParamCoercionUnwrapsNullable(): void
364364
$this->assertFalse($captured);
365365
}
366366

367+
/**
368+
* Empty-string params bypass `validate()` when optional, so they reach
369+
* `coerce()` un-validated. We must NOT silently turn them into `false`
370+
* (callers like Cloud's `Patch.php` use `''` as a "not set" sentinel and
371+
* later resolve it to `null`/three-state).
372+
*/
373+
public function testBooleanParamPreservesEmptyStringSentinel(): void
374+
{
375+
$captured = 'untouched';
376+
377+
$cli = new CLI(new Generic(), ['test.php', 'build']);
378+
379+
$cli
380+
->task('build')
381+
->param('commit', '', new Boolean(true), 'Commit changes', true)
382+
->action(function ($commit) use (&$captured) {
383+
$captured = $commit;
384+
});
385+
386+
$cli->run();
387+
388+
$this->assertSame('', $captured);
389+
}
390+
367391
public function testNonBooleanValidatorPassesValueThroughUnchanged(): void
368392
{
369393
$captured = null;

0 commit comments

Comments
 (0)