Skip to content

Commit

Permalink
Documented callable parameter structure for Assert::callback()
Browse files Browse the repository at this point in the history
`Assert::callback()` accepts a `$callable` that, **AT MOST**, accepts **1** parameter and
returns `bool`.

The number of times I've written `->with(self::callback(function ($a, $b, $c) : void {`
is very high:

 * I generally forget that `->with()` is variadic (that's OK)
 * I forget that `self::callback()` applies to a single parameter
 * I forget that the `callable` must return a `bool` in order to operate

In order to avoid these mistakes, the `Callback` constraint is now templated, and the
given `callable` is enforced to require at most one parameter, and to always return
`bool`.

As a counter-example to verify that this type signature works, I've run this against
`tests/static-analysis/TestUsingCallbacks.php` with following snippet:

```php
    public function testInvalid(): void
    {
        $mock = $this->createMock(SayHello::class);

        $mock
            ->expects(self::once())
            ->method('hey')
            ->with(self::callback(static function (string $a, string $b): bool {
                return true;
            }))
            ->willReturn('Hey Joe!');

        self::assertSame('Hey Joe!', $mock->hey('Joe'));
    }
```

The above now raises:

```
ERROR: InvalidArgument - tests/static-analysis/TestUsingCallbacks.php:62:35 - Argument 1 of PHPUnit\StaticAnalysis\TestUsingCallbacks::callback expects callable(mixed):bool, pure-Closure(string, string):true provided (see https://psalm.dev/004)
            ->with(self::callback(static function (string $a, string $b): bool {
                return true;
            }))
```

This produces
  • Loading branch information
Ocramius authored and sebastianbergmann committed Jan 27, 2021
1 parent b912a34 commit eee79a1
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 3 deletions.
7 changes: 7 additions & 0 deletions src/Framework/Assert.php
Original file line number Diff line number Diff line change
Expand Up @@ -3114,6 +3114,13 @@ public static function isTrue(): IsTrue
return new IsTrue;
}

/**
* @psalm-template CallbackInput of mixed
*
* @psalm-param callable(mixed $callback): bool $callback
*
* @psalm-return Callback<CallbackInput>
*/
public static function callback(callable $callback): Callback
{
return new Callback($callback);
Expand Down
11 changes: 8 additions & 3 deletions src/Framework/Constraint/Callback.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,21 @@
*/
namespace PHPUnit\Framework\Constraint;

use function call_user_func;

/**
* Constraint that evaluates against a specified closure.
*
* @psalm-template CallbackInput of mixed
*/
final class Callback extends Constraint
{
/**
* @var callable
*
* @psalm-var callable(CallbackInput $input): bool
*/
private $callback;

/** @psalm-param callable(CallbackInput $input): bool $callback */
public function __construct(callable $callback)
{
$this->callback = $callback;
Expand All @@ -39,9 +42,11 @@ public function toString(): string
* constraint is met, false otherwise.
*
* @param mixed $other value or object to evaluate
*
* @psalm-param CallbackInput $other
*/
protected function matches($other): bool
{
return call_user_func($this->callback, $other);
return ($this->callback)($other);
}
}
54 changes: 54 additions & 0 deletions tests/static-analysis/TestUsingCallbacks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php declare(strict_types=1);
/*
* This file is part of PHPUnit.
*
* (c) Sebastian Bergmann <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace PHPUnit\StaticAnalysis;

use PHPUnit\Framework\TestCase;

/** @see https://www.youtube.com/watch?v=rXwMrBb2x1Q */
interface SayHello
{
public function hey(string $toPerson): string;
}

/** @small */
final class TestUsingCallbacks extends TestCase
{
public function testWillSayHelloAndCheckCallbackInput(): void
{
$mock = $this->createMock(SayHello::class);

$mock
->expects(self::once())
->method('hey')
->with(self::callback(static function (string $input): bool {
self::assertStringContainsString('Joe', $input);

return true;
}))
->willReturn('Hey Joe!');

self::assertSame('Hey Joe!', $mock->hey('Joe'));
}

public function testWillSayHelloAndCheckCallbackWithoutAnyInput(): void
{
$mock = $this->createMock(SayHello::class);

$mock
->expects(self::once())
->method('hey')
->with(self::callback(static function (): bool {
return true;
}))
->willReturn('Hey Joe!');

self::assertSame('Hey Joe!', $mock->hey('Joe'));
}
}

0 comments on commit eee79a1

Please sign in to comment.