Return validated value

[dbalabka]

I've faced the fact that it is required to create temporary variables to validate values:

$firstName = getParam('firstName');
$lastName = getParam('lastName');
Assert::notNull($firstName);
Assert::notNull($lastName);

createCustomer(
    $firstName,
    $lastName
);

We can rid of temporary variables if the assertion function returns a valid value:

createCustomer(
    Assert::notNull(getParam('firstName')),
    Assert::notNull(getParam('lastName'))
);

For some of the assertion functions, we can simply return the valid value. The changes should be BC-safe.

Possible implementation: https://github.com/webmozarts/assert/pull/281 Psalm support demonstration: https://psalm.dev/r/a439359976

References:

1. Similar Java implementation: https://commons.apache.org/proper/commons-lang/javadocs/api-3.1/org/apache/commons/lang3/Validate.html#notNull(T)

[frankdekker]

I'm looking for this feature as well, and this would be helpful for every method. Your example is precisely my use case. The fluent notation makes it much more cleaner to use. Your current PR is only the notNull method. Is your intention to add this to all methods? Otherwise I could also extend on this PR.

[dbalabka]

@frankdekker, I will add support for other methods as well if the maintainer accepts that such a feature is required. We should wait for @webmozart's reply.

[shadowhand]

I think that Assert should be used inside the createCustomer method:

public function createCustomer(mixed $firstName, mixed $lastName): void
{
    Assert::stringNotEmpty($firstName);
    Assert::stringNotEmpty($lastName);

    // And then database write, or whatever
}

The usage suggested by OP promotes using Assert (and thus, Throwable) as a validation tool, which is not how this package was intended to be used. Throws are far too costly in PHP to be used for validation.

[dbalabka]

@shadowhand , that is an interesting idea. Unfortunately, the createCustomer method is a part of the library, and there is no easy way to extend it. Furthermore, createCustomer is strictly typed and does not allow nullable values to be passed as a parameter. Therefore, the only way to make static analysis tools happy is to add type guards before calling the method.

[ruudk]

We had a similar need in our project, when working with poorly typed things (mostly vendor code). We decided to introduce an AssertReturn class which looks like this:

final class AssertReturn
{
    /**
     * @return array<mixed>
     */
    public static function array(mixed $input) : array
    {
        Assert::isArray($input);

        return $input;
    }

    /**
     * @return list<string>
     */
    public static function listOfString(mixed $input) : array
    {
        Assert::isList($input);
        Assert::allString($input);

        return $input;
    }

    public static function string(mixed $input) : string
    {
        Assert::string($input);

        return $input;
    }

    public static function boolean(mixed $input) : bool
    {
        Assert::boolean($input);

        return $input;
    }

    // ... and so on

I wish I didn't have to create this, and could just use echo Assert::string($input); instead.

[shadowhand]

While I like this idea, it is very hard to implement with full static analysis. Or at least, I am not smart enough to understand how to correctly designate the change from, e.g. a mixed to a non-empty-string correctly. Simply setting the @return non-empty-string will cause all kinds of internal oddities in my experience.

All of which is to say, I am currently setting up a v2.0 branch, and I like this idea, but I need help implementing.

[frankdekker]

@shadowhand In the meantime we setup our own assert class, and so far we didn't run into much issues with phpstan. For ideas you can maybe take a look at our notation: https://github.com/123inkt/utils/blob/master/src/Assert.php#L500

[kkevindev]

For the meantime, I've created a simple package where it just simply returns the validated value.

https://github.com/kkevindev/assert-return-value / composer require kkevindev/assert-return-value

This package will be obsoleted when this issue is resolved & a version is tagged.

[shadowhand]

The current master branch is for version 2.0, which is unreleased and accepting BC breaking changes. Feel free to submit a PR with these changes.

[kkevindev]

The current master branch is for version 2.0, which is unreleased and accepting BC breaking changes. Feel free to submit a PR with these changes.

@shadowhand

Will do, what would be your ideal minimum accepted PHP version? Do you want a proper native PHP approach (where possible) which would require I think PHP 8.0 (for union types) and enrich it with docblock @return or do you only want to use @return to keep the PHP 7.2 supported?

Do you want to keep supporting PHP 7 or do we want to "force" users to update their EOL PHP versions to the 'Active support' / 'Security fixes only' versions (currently 8.1, 8.2, 8.3, 8.4).

[shadowhand]

@kkevindev the minimum PHP version has already been bumped to PHP 8.2 for v2.