feat: introduce unsealed shaped array syntax #510

romm · 2024-03-28T20:11:50Z

This syntax enables an extension of the shaped array type by allowing additional values that must respect a certain type.

$mapper = (new \CuyZ\Valinor\MapperBuilder())->mapper();

// Default syntax can be used like this:
$mapper->map(
    'array{foo: string, ...array<string>}',
    [
        'foo' => 'foo',
        'bar' => 'bar', // ✅ valid additional value
    ]
);

$mapper->map(
    'array{foo: string, ...array<string>}',
    [
        'foo' => 'foo',
        'bar' => 1337, // ❌ invalid value 1337
    ]
);

// Key type can be added as well:
$mapper->map(
    'array{foo: string, ...array<int, string>}',
    [
        'foo' => 'foo',
        42 => 'bar', // ✅ valid additional key
    ]
);

$mapper->map(
    'array{foo: string, ...array<int, string>}',
    [
        'foo' => 'foo',
        'bar' => 'bar' // ❌ invalid key
    ]
);

// Advanced types can be used:
$mapper->map(
    "array{
        'en_US': non-empty-string,
        ...array<non-empty-string, non-empty-string>
    }",
    [
        'en_US' => 'Hello',
        'fr_FR' => 'Salut', // ✅ valid additional value
    ]
);

$mapper->map(
    "array{
        'en_US': non-empty-string,
        ...array<non-empty-string, non-empty-string>
    }",
    [
        'en_US' => 'Hello',
        'fr_FR' => '', // ❌ invalid value
    ]
);

// If the permissive type is enabled, the following will work:
(new \CuyZ\Valinor\MapperBuilder())
    ->allowPermissiveTypes()
    ->mapper()
    ->map(
        'array{foo: string, ...}',
        ['foo' => 'foo', 'bar' => 'bar', 42 => 1337]
    ); // ✅

Fixes #438

romm · 2024-03-28T20:13:06Z

Hello @Ocramius, if you have some time I'd really appreciate if you could play with this and try to make it crash. 😛

romm · 2024-03-28T20:13:57Z

docs/pages/usage/type-reference.md

@@ -134,6 +134,15 @@ final class SomeClass

 /** @var array{string, bar: int} */
 private array $shapedArrayWithUndefinedKey,
+


I definitely need to rework this entire documentation page. 😬

Still, the additions are useful/understandable.

Ocramius

@romm I lack the low level understanding of your library to provide an implementation-level review, but based off your comments on expected behavior, this is very good!

qa/PHPStan/Extension/TreeMapperPHPStanExtension.php

Ocramius · 2024-04-01T17:20:23Z

src/Type/Parser/Exception/Iterable/ShapedArrayUnexpectedTokenAfterSealedType.php

+
+use function implode;
+
+/** @internal */


BTW, the fact that you meticulously add @internal everywhere is :chefkiss:

To be sure I don't forget to add it, I'm covered by this PHPStan rule 😁 : https://github.com/CuyZ/Valinor/blob/master/qa/PHPStan/Extension/ApiAndInternalAnnotationCheck.php

Ocramius · 2024-04-01T17:21:35Z

src/Type/Parser/Lexer/Token/ArrayToken.php

@@ -121,12 +126,50 @@ private function shapedArrayType(TokenStream $stream): ShapedArrayType

 $optional = false;

+ if ($stream->next() instanceof TripleDotsToken) {


This lexer is going to grow over time: I wonder if there's a way to declare a grammar somewhere 🤔

I have no clue about that; for now I find the current approach readable and understandable, but indeed that may change in the future.

Ocramius · 2024-04-01T17:23:13Z

tests/Functional/Type/Parser/Lexer/NativeLexerTest.php

+ public function test_unsealed_shaped_array_with_missing_closing_bracket_throws_exception(): void
+ {
+ $this->expectException(ShapedArrayClosingBracketMissing::class);
+ $this->expectExceptionCode(1631283658);
+ $this->expectExceptionMessage('Missing closing curly bracket in shaped array signature `array{0: int, ...`.');
+
+ $this->parser->parse('array{int, ...');
+ }


I would check '...' (quotes around the triple dots) in a separate test: it's just a detail, but I'm wondering if your lexer works well

Something like array{'...': '...', ...} to see what happens :D

Well that does not really need a test because that kind of cases is already covered. But because you're curious about it, here are the results 😁

(new \CuyZ\Valinor\MapperBuilder()) ->allowPermissiveTypes() ->mapper() ->map('array{"...": "...", ...array<string>}', [ '...' => '...', 'foo' => 'bar' ]); // ✅ (new \CuyZ\Valinor\MapperBuilder()) ->allowPermissiveTypes() ->mapper() ->map('array{"...": "...", ...array<string>}', [ 'foo' => 'bar' ]); // ❌ An error occurred at path ...: Cannot be empty and must be filled with a value matching type "...".

Ocramius · 2024-04-01T17:24:28Z

tests/Integration/Mapping/Other/PermissiveTypesMappingTest.php

+ } catch (MappingError $error) {
+ $this->mappingFail($error);
+ }


I've seen this in many mapper integration tests: I think this try-catch flow should be abstracted away in the test suite

MyTestUtility::useMapper($this->mapper) would be sufficient, for example.

Alternatively:

final class TestMapper implements TreeMapper { private TreeMapper $realMapper; // add try-catch logic here }

Yeah I already thought about something like this, at some point I'll just do it… I hope 😁

romm · 2024-04-01T20:54:03Z

@Ocramius I was actually just asking for real tests on your end, but thanks for the review it's really much appreciated. If you are done we can merge it. 😉

Ocramius · 2024-04-01T20:55:17Z

Nothing immediate in userland projects: I think it used to crash in a project last year, due to parsing failing

This prevents PHPStan from crashing when giving a type it does not understand to `\CuyZ\Valinor\Mapper\TreeMapper::map()`.

This syntax enables an extension of the shaped array type by allowing additional values that must respect a certain type. ```php $mapper = (new \CuyZ\Valinor\MapperBuilder())->mapper(); // Default syntax can be used like this: $mapper->map( 'array{foo: string, ...array<string>}', [ 'foo' => 'foo', 'bar' => 'bar', // ✅ valid additional value ] ); $mapper->map( 'array{foo: string, ...array<string>}', [ 'foo' => 'foo', 'bar' => 1337, // ❌ invalid value 1337 ] ); // Key type can be added as well: $mapper->map( 'array{foo: string, ...array<int, string>}', [ 'foo' => 'foo', 42 => 'bar', // ✅ valid additional key ] ); $mapper->map( 'array{foo: string, ...array<int, string>}', [ 'foo' => 'foo', 'bar' => 'bar' // ❌ invalid key ] ); // Advanced types can be used: $mapper->map( "array{ 'en_US': non-empty-string, ...array<non-empty-string, non-empty-string> }", [ 'en_US' => 'Hello', 'fr_FR' => 'Salut', // ✅ valid additional value ] ); $mapper->map( "array{ 'en_US': non-empty-string, ...array<non-empty-string, non-empty-string> }", [ 'en_US' => 'Hello', 'fr_FR' => '', // ❌ invalid value ] ); // If the permissive type is enabled, the following will work: (new \CuyZ\Valinor\MapperBuilder()) ->allowPermissiveTypes() ->mapper() ->map( 'array{foo: string, ...}', ['foo' => 'foo', 'bar' => 'bar', 42 => 1337] ); // ✅ ```

romm · 2024-04-02T16:10:51Z

Let's go for it then! 💪

romm commented Mar 28, 2024

View reviewed changes

Ocramius approved these changes Apr 1, 2024

View reviewed changes

romm added 3 commits April 2, 2024 18:10

qa: catch type parser exception in PHPStan extension

c617def

This prevents PHPStan from crashing when giving a type it does not understand to `\CuyZ\Valinor\Mapper\TreeMapper::map()`.

qa: add documentation comment on PHPStan extension

4b6faa2

romm force-pushed the feat/unsealed-array-shape branch from b0ddcf1 to 4b6faa2 Compare April 2, 2024 16:10

romm enabled auto-merge (rebase) April 2, 2024 16:11

romm merged commit c8174bf into CuyZ:master Apr 2, 2024
11 checks passed

romm deleted the feat/unsealed-array-shape branch April 2, 2024 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce unsealed shaped array syntax #510

feat: introduce unsealed shaped array syntax #510

romm commented Mar 28, 2024

romm commented Mar 28, 2024

romm Mar 28, 2024

Ocramius Apr 1, 2024

Ocramius left a comment

Ocramius Apr 1, 2024

romm Apr 1, 2024

Ocramius Apr 1, 2024

romm Apr 1, 2024

Ocramius Apr 1, 2024

Ocramius Apr 1, 2024

romm Apr 1, 2024

Ocramius Apr 1, 2024

Ocramius Apr 1, 2024

romm Apr 1, 2024

romm commented Apr 1, 2024

Ocramius commented Apr 1, 2024

romm commented Apr 2, 2024

		@@ -134,6 +134,15 @@ final class SomeClass

		/** @var array{string, bar: int} */
		private array $shapedArrayWithUndefinedKey,

		@@ -121,12 +126,50 @@ private function shapedArrayType(TokenStream $stream): ShapedArrayType

		$optional = false;

		if ($stream->next() instanceof TripleDotsToken) {

feat: introduce unsealed shaped array syntax #510

feat: introduce unsealed shaped array syntax #510

Conversation

romm commented Mar 28, 2024

romm commented Mar 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ocramius left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romm commented Apr 1, 2024

Ocramius commented Apr 1, 2024

romm commented Apr 2, 2024