Allow type-safe calls to any callable?

[romm]

Currently, this library allows to map a raw imput to an object.

(new \CuyZ\Valinor\MapperBuilder())
    ->mapper()
    ->map(SomeClass::class, [/* … */]);

While this is useful in a lot of cases, it came to my mind that all the node system behind this system could also be used for something else; what if we could do this:

(new \CuyZ\Valinor\MapperBuilder())
    ->caller()
    ->call(
        function(string $foo, int $bar) { /* … */ },
        ['foo' => 'foo!', 'bar' => 42]
    )

Knowing that the first argument could be any callable — for instance a first-class callable — this would allow developers to easily get type-safe calls to a function that implements the logic behind the input.

One of the biggest pro is that there would be no need to declare a full value-object anymore — in cases where the data doesn't need to passed along several functions — the typed arguments would guarantee the validity of the data. I believe this would prevent having to write a lot of boilerplate code (the value object class) for simple cases.

---

Before going further on this path, I'd love to hear the inputs of some people that showed interested for the project in the first place. If you have some time, I'd be glad to read your suggestions @Ocramius, @brandonsavage, @shulard, @brayniverse (and others of course 😊).

[shulard]

At first, this seems to be a good idea.

However since latest version of PHP simplified a lot simple object definition, the main argument is not the best for me… Do you think it can help in others cases ?

Also is it really in the project scope to move that way ? Hydrating object is already a complex task, if you handle edge cases it can add complexity (maybe…).

[romm]

I agree that object definition has been simplified a lot in last PHP versions, but my point was more about no having to write PHP files that do not add that much value to the codebase.

Also is it really in the project scope to move that way ? Hydrating object is already a complex task, if you handle edge cases it can add complexity (maybe…).

Well this looks a lot like object hydration to me, but instead of an object this is more like a typed shaped array that is mapped then sent to a callable. Something like:

$callable = function(string $foo, int $bar) { /* … */ };

$shapedArray = (new \CuyZ\Valinor\MapperBuilder())
    ->mapper()
    ->map('array{foo: string, bar: int}', [/* … */]);

$callable(...$shapedArray);

Or maybe instead of directly calling the callable, this library could add some helper to fetch a callable's parameters signature?

$callable = function(string $foo, int $bar) { /* … */ };

$mapperBuilder = new \CuyZ\Valinor\MapperBuilder();

$signature = $mapperBuilder->signatureFor($callable); // returns `array{foo: string, bar: int}`

$mapperBuilder->map($signature, [/* … */]);

[shulard]

Ok thank you for the additional details !

If it’s the same logic I think it’ll be a nice addition to allow mapping a callable definition. I love the signature idea and I think it allow better to keep the responsibilities of each item when reading the code.

[romm]

After giving it some thoughts I made #50 that gives a first shot of what would eventually enable this feature.

WDYT?

[shulard]

This look awesome ^^. Having the ability to map any type of data can help using the mapper in a lot of different contexts !

[romm]

Indeed, let's see how it goes. 🙂

[azjezz]

A small note: first-class closure syntax won't be that useful.

e.g:

/**
 * @param positive-int $foo
 * @param non-empty-string $baz
 */
function foo(int $foo, string $baz): void
{
  if ($foo < 0 || $baz === '') {
    throw new Exception('read the docblock!!!!');
  }
}

(new \CuyZ\Valinor\MapperBuilder())
    ->caller()
    ->call(foo(...), ['foo' => 0, 'baz' => '']);

This example will fail, the mapper can't read the docblock of foo function to determine the type of the input since foo(...) returns a Closure ( same as static fn(int $foo, string $baz) => foo($foo, $baz); ).

[romm]

Hi @azjezz, I just tried this locally, it seems there is no issue reading the docblock of a closure, maybe I'm missing something?

/**
 * @param non-empty-string $foo
 */
function foo(string $foo) { }

$reflection = new ReflectionFunction(foo(...));

var_dump($reflection->getDocComment()); // outputs the actual docblock

[azjezz]

Oh nice, didn't know using that syntax inherits the docblock as well.

in that case, ignore me 😛

[romm]

Thank you for taking part of the discussion, it would have been an instant nogo on this feature if you were right so… 🙂

Anyway, any chance we can have your thoughts on the original proposal? 😁

[brandonsavage]

Adding this behavior seems like an interesting idea and certainly useful, but there is a question for me as to whether or not this is something you want to maintain alongside the work you're presently doing and whether or not it would potentially alter the trajectory of this project.

I would let the idea percolate a little bit before implementing it, and see how it settles over time. Perhaps let this issue stay open for a few months? Adding it in is simple. Taking it out is REALLY hard in the future if you learn you don't like it.

[romm]

This is a good advice indeed.

As I stated in #41 (reply in thread) the original idea could be changed to something like this:

$callable = function(string $foo, int $bar) { /* … */ };

$mapperBuilder = new \CuyZ\Valinor\MapperBuilder();

$signature = $mapperBuilder->signatureFor($callable); // returns `array{foo: string, bar: int}`

$mapperBuilder->map($signature, [/* … */]);

I believe this is a more accurate way of implementing this kind of feature, without adding heavy maintenance burden.

Anyway, I'll wait a bit to see where this leads. 🙂

[romm]

Hi guys, it's been almost a year and the library is almost ready for the first 1.0 stable release.

Before that, I want to ship the feature discussed here; this is, I believe, a really interesting feature that allows for instance to map a request's arguments to a controller function/method.

I see two ways of doing it, and I'd love to have your feedback on it:

1. Change the current TreeMapper signature

This is a breacking change, but we can do it without too much impact (IMO) before the 1.0 release.

Current signature:

/**
 * @template T of object
 *
 * @param string|class-string<T> $signature
 * @return (
 *     $signature is class-string<T>
 *         ? T
 *         : mixed
 * )
 *
 * @throws MappingError
 */
public function map(string $signature, mixed $source): mixed;

New signature:

/**
 * @template T of object
 *
 * @param string|class-string<T>|callable $signature
 * @return (
 *     $signature is class-string<T>
 *         ? T
 *         : mixed
 * )
 *
 * @throws MappingError
 */
public function map(string|callable $signature, mixed $source): mixed;

Usage:

function some_controller_action(string $foo, int $bar) {
    echo "$foo : $bar";
}

$arguments = (new \CuyZ\Valinor\MapperBuilder())
    ->mapper()
    ->map(some_controller_action(...), [
        'foo' => 'foo',
        'bar' => 42,
    ]);

some_controller_action(...$arguments); // prints "foo : 42"

2. Introduce a new method in MapperBuilder

A new interface:

interface CallableTreeMapper
{
    /**
     * @return array<string, mixed>
     *
     * @throws MappingError
     */
    public function map(callable $callable, mixed $source): array;
}

Usage:

function some_controller_action(string $foo, int $bar) {
    echo "$foo : $bar";
}

$arguments = (new \CuyZ\Valinor\MapperBuilder())
    ->callableMapper() // Maybe another name?
    ->map(some_controller_action(...), [
        'foo' => 'foo',
        'bar' => 42,
    ]);

some_controller_action(...$arguments); // prints "foo : 42"

---

So the question is: which one should be implemented? Personnaly I'm having trouble taking a decision here, as the two options look reliable. The second one might make more sense because of the type of $source which is a string in one case and a callable in the other.

Any opinion? 🙂

[Ocramius]

IMO this should not be on ->map(), and should have VERY clear naming.

A separate interface is needed, and it should not be called ->map(), but rather ->forFirstParameterOf($input, callable(T)): T.

[romm]

Closing the thread, as this feature is part of the 1.0.0 release. 🎉