Skip to content

Commit

Permalink
Very few things are actually possibly_undefined
Browse files Browse the repository at this point in the history
  • Loading branch information
matt committed Feb 23, 2024
1 parent efe9a85 commit a884f99
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 105 deletions.
8 changes: 1 addition & 7 deletions src/InputFilter/InputFilterVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Psalm\Type\Atomic\TNonEmptyArray;
use Psalm\Type\Union;

use function array_filter;
use function array_keys;

final readonly class InputFilterVisitor implements InputFilterVisitorInterface
Expand Down Expand Up @@ -48,12 +47,7 @@ public function visit(InputFilterInterface $inputFilter, ImportType|ImportTypes
$elements[$childName] = $this->visit($child, $childTypes);
}

$elementsRequired = (bool) array_filter(
$elements,
static fn (Union $element): bool => ! $element->possibly_undefined
);

$properties = ['possibly_undefined' => ! $elementsRequired || $inputFilter instanceof OptionalInputFilter];
$properties = ['possibly_undefined' => $inputFilter instanceof OptionalInputFilter];

if ($elements === []) {
return new Union([new TArray([Type::getArrayKey(), Type::getMixed()])], $properties);
Expand Down
7 changes: 3 additions & 4 deletions src/InputFilter/InputVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ public function __construct(private array $filterVisitors, private array $valida

public function visit(InputInterface $input): Union
{
$hasFallback = $input instanceof Input && $input->hasFallback();
$possiblyUndefined = $hasFallback || ! $input->isRequired();
$union = new Union([new TNull(), new TString()]);
$hasFallback = $input instanceof Input && $input->hasFallback();
$union = new Union([new TNull(), new TString()]);

foreach ($input->getFilterChain()->getIterator() as $filter) {
if (! $filter instanceof FilterInterface) {
Expand Down Expand Up @@ -77,7 +76,7 @@ public function visit(InputInterface $input): Union
throw InputVisitorException::cannotGetInputType($input);
}

return $union->setPossiblyUndefined($possiblyUndefined);
return $union;
}

private function visitFilters(FilterInterface $filter, Union $union): Union
Expand Down
6 changes: 3 additions & 3 deletions test/Form/FieldsetVisitorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ public function testVisitReturnsFieldsetUnion(): void
{
$expected = new Union([
new TKeyedArray([
'foo' => new Union([new TString(), new TNull()], ['possibly_undefined' => true]),
'bar' => new Union([new TString(), new TNull()], ['possibly_undefined' => true]),
'foo' => new Union([new TString(), new TNull()]),
'bar' => new Union([new TString(), new TNull()]),
]),
], ['possibly_undefined' => true]);
]);
$formVisitor = new FormVisitor(new InputFilterVisitor([new InputVisitor([], [])]));
$fieldsetVisitor = new FieldsetVisitor($formVisitor);

Expand Down
4 changes: 3 additions & 1 deletion test/Form/FormCollectionSmokeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,11 @@ public function testValidationMatches(ElementInterface $element, array $data, bo

$form->setData($data);
$isFormValid = $form->isValid();
/** @var array $formData */
$formData = $form->getData();

self::assertSame($isValid, $isFormValid, "Form::isValid() returned " . ($isValid ? 'false' : 'true'));
self::assertValinorValidates($isValid, $type, $data);
self::assertValinorValidates($isValid, $type, $formData);
}

public static function validationMatchesProvider(): array
Expand Down
78 changes: 40 additions & 38 deletions test/Form/FormElementSmokeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,16 @@
use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface;

use function is_array;
use function json_encode;
use function sprintf;

#[CoversNothing]
final class FormElementSmokeTest extends TestCase
{
/**
* @param class-string<ElementInterface> $element
* @param list<list{scalar|null, bool}> $tests
* @param list<list{array|scalar|null, bool}> $tests
*/
#[DataProvider('defaultElementProvider')]
public function testDefaultElements(string $element, array $tests, string $expected): void
Expand Down Expand Up @@ -76,13 +78,13 @@ public function testDefaultElements(string $element, array $tests, string $expec

foreach ($tests as $expectation) {
[$data, $valid] = $expectation;
$formData = ['test' => $data];
$formData = is_array($data) ? $data : ['test' => $data];

$inputFilter->setData($formData);
$actual = $inputFilter->isValid();
self::assertSame($valid, $actual, sprintf(
"Validation expectation failed: %s is not %s",
$data === null ? 'null' : "'$data'",
$data === null ? 'null' : json_encode($data),
$valid ? 'valid' : 'invalid'
));
}
Expand All @@ -97,22 +99,22 @@ public static function defaultElementProvider(): array
return [
'button' => [
Button::class,
[[null, true], ['', true], [' ', false], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], [' ', false], ['a', true]],
'test: null|string',
],
'captcha' => [
Captcha::class,
[[null, false], ['', false], [' ', false], ['a', true]],
[[[], false], [null, false], ['', false], [' ', false], ['a', true]],
'test: non-empty-string',
],
'checkbox' => [
Checkbox::class,
[[null, false], ['', false], [' ', false], ['0', true], ['1', true]],
[[[], false], [null, false], ['', false], [' ', false], ['0', true], ['1', true]],
"test: '0'|'1'",
],
'color' => [
Color::class,
[[null, false], ['', false], [' ', false], ['a', false], ['#ffffff', true]],
[[[], false], [null, false], ['', false], [' ', false], ['a', false], ['#ffffff', true]],
"test: non-empty-string",
],
// 'csrf' => [
Expand All @@ -122,27 +124,27 @@ public static function defaultElementProvider(): array
// ],
'date' => [
Date::class,
[[null, false], ['', false], [' ', false], ['2024-01-28', true]],
[[[], false], [null, false], ['', false], [' ', false], ['2024-01-28', true]],
"test: non-empty-string",
],
'dateselect' => [
DateSelect::class,
[[null, true], ['', true], [' ', false], ['2024-01-28', true]],
"test?: null|string",
[[[], true], [null, true], ['', true], [' ', false], ['2024-01-28', true]],
"test: null|string",
],
'datetimelocal' => [
DateTimeLocal::class,
[[null, false], ['', false], [' ', false], ['2024-01-28T12:53', true]],
[[[], false], [null, false], ['', false], [' ', false], ['2024-01-28T12:53', true]],
"test: non-empty-string",
],
'datetimeselect' => [
DateTimeLocal::class,
[[null, false], ['', false], [' ', false], ['2024-01-28T12:54', true]],
[[[], false], [null, false], ['', false], [' ', false], ['2024-01-28T12:54', true]],
"test: non-empty-string",
],
'email' => [
Email::class,
[[null, false], ['', false], [' ', false], ['[email protected]', true]],
[[[], false], [null, false], ['', false], [' ', false], ['[email protected]', true]],
'test: non-empty-string',
],
// 'file' => [
Expand All @@ -152,87 +154,87 @@ public static function defaultElementProvider(): array
// ],
'hidden' => [
Hidden::class,
[[null, true], ['', true], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], ['a', true]],
'test: null|string',
],
'image' => [
Image::class,
[[null, true], ['', true], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], ['a', true]],
'test: null|string',
],
'month' => [
Month::class,
[[null, false], ['', false], [' ', false], ['2024-01', true]],
[[[], false], [null, false], ['', false], [' ', false], ['2024-01', true]],
"test: non-empty-string",
],
'monthselect' => [
MonthSelect::class,
[[null, true], ['', true], [' ', false], ['2024-01', true]],
"test?: null|string",
[[[], true], [null, true], ['', true], [' ', false], ['2024-01', true]],
"test: null|string",
],
'multicheckbox' => [
MultiCheckbox::class,
[[null, false], ['', false], [' ', false]],
[[[], false], [null, false], ['', false], [' ', false]],
"test: non-empty-string", // no haystack by default
],
'number' => [
Number::class,
[[null, false], ['', false], [' ', false], ['a', false], ['123', true]],
[[[], false], [null, false], ['', false], [' ', false], ['a', false], ['123', true]],
"test: numeric-string",
],
'password' => [
Password::class,
[[null, true], ['', true], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], ['a', true]],
'test: null|string',
],
'radio' => [
Radio::class,
[[null, false], ['', false], [' ', false]],
[[[], false], [null, false], ['', false], [' ', false]],
"test: non-empty-string", // no haystack by default
],
'range' => [
Range::class,
[[null, false], ['', false], [' ', false], ['a', false], ['42', true]],
[[[], false], [null, false], ['', false], [' ', false], ['a', false], ['42', true]],
"test: numeric-string",
],
'search' => [
Search::class,
[[null, true], ['', true], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], ['a', true]],
'test: null|string',
],
'select' => [
Select::class,
[[null, false], ['', false], [' ', false]],
[[[], false], [null, false], ['', false], [' ', false]],
"test: non-empty-string", // no haystack by default
],
'submit' => [
Submit::class,
[[null, true], ['', true], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], ['a', true]],
'test: null|string',
],
'tel' => [
Tel::class,
[[null, false], ['', false], [' ', false], ['a', true]],
[[[], false], [null, false], ['', false], [' ', false], ['a', true]],
'test: non-empty-string',
],
'text' => [
Text::class,
[[null, true], ['', true], [' ', false], ['a', true]],
'test?: null|string',
[[[], true], [null, true], ['', true], [' ', false], ['a', true]],
'test: null|string',
],
'time' => [
Time::class,
[[null, false], ['', false], [' ', false], ['10:25:00', true]],
[[[], false], [null, false], ['', false], [' ', false], ['10:25:00', true]],
"test: non-empty-string",
],
'url' => [
Url::class,
[[null, false], ['', false], [' ', false], ['http://example.com', true]],
[[[], false], [null, false], ['', false], [' ', false], ['http://example.com', true]],
"test: non-empty-string",
],
'week' => [
Week::class,
[[null, false], ['', false], [' ', false], ['2024-W05', true]],
[[[], false], [null, false], ['', false], [' ', false], ['2024-W05', true]],
"test: non-empty-string",
],
];
Expand Down
4 changes: 3 additions & 1 deletion test/Form/FormFieldsetSmokeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ public function testValidationMatches(array $elements, array $data, bool $isVali

$form->setData($data);
$formValid = $form->isValid();
/** @var array $formData */
$formData = $form->getData();

self::assertSame($isValid, $formValid, "Form::isValid() returned " . ($isValid ? 'false' : 'true'));
self::assertValinorValidates($isValid, $type, $data);
self::assertValinorValidates($isValid, $type, $formData);
}

public static function validationMatchProvider(): array
Expand Down
26 changes: 13 additions & 13 deletions test/Form/FormProcessorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,15 @@ public function testProcessFieldsetWritesAndAddsType(): void
{
$fieldsetUnion = new Union([
new TKeyedArray([
'bar' => new Union([new TString(), new TNull()], ['possibly_undefined' => true]),
'bar' => new Union([new TString(), new TNull()]),
]),
], ['possibly_undefined' => true]);
]);
$typeAlias = new TTypeAlias(TestFieldset::class, 'TTestFieldsetArray');
$formUnion = new Union([
new TKeyedArray([
'foo' => new Union([$typeAlias], ['possibly_undefined' => true]),
'foo' => new Union([$typeAlias]),
]),
], ['possibly_undefined' => true]);
]);
$expectedImport = new ImportType($typeAlias, $fieldsetUnion);

$form = new Form();
Expand Down Expand Up @@ -114,9 +114,9 @@ public function testProcessFieldsetProcessesCollection(): void
{
$fieldsetUnion = new Union([
new TKeyedArray([
'bar' => new Union([new TString(), new TNull()], ['possibly_undefined' => true]),
'bar' => new Union([new TString(), new TNull()]),
]),
], ['possibly_undefined' => true]);
]);
$typeAlias = new TTypeAlias(TestFieldset::class, 'TTestFieldsetArray');
$expected = new ImportType($typeAlias, $fieldsetUnion);

Expand Down Expand Up @@ -146,11 +146,11 @@ public function testProcessFieldsetSkipsLaminasFieldset(): void
new TKeyedArray([
'foo' => new Union([
new TKeyedArray([
'bar' => new Union([new TString(), new TNull()], ['possibly_undefined' => true]),
'bar' => new Union([new TString(), new TNull()]),
]),
], ['possibly_undefined' => true]),
]),
]),
], ['possibly_undefined' => true]);
]);

$form = new Form();
$fieldset = new Fieldset('foo');
Expand All @@ -173,15 +173,15 @@ public function testProcessFieldsetImportsChildFieldset(): void
{
$childUnion = new Union([
new TKeyedArray([
'baz' => new Union([new TString(), new TNull()], ['possibly_undefined' => true]),
'baz' => new Union([new TString(), new TNull()]),
]),
], ['possibly_undefined' => true]);
]);
$typeAlias = new TTypeAlias(ChildFieldset::class, 'TChildFieldsetArray');
$fieldsetUnion = new Union([
new TKeyedArray([
'bar' => new Union([$typeAlias], ['possibly_undefined' => true]),
'bar' => new Union([$typeAlias]),
]),
], ['possibly_undefined' => true]);
]);

$form = new Form();
$fieldset = new TestFieldset('foo');
Expand Down
Loading

0 comments on commit a884f99

Please sign in to comment.