Skip to content

Commit

Permalink
Update Hook::safe_offset() to handle closures (#183)
Browse files Browse the repository at this point in the history
## Summary

Reimplements #174 after implementing PhpStan and code styles in WP_Mock.
Tests have been reworked in PhpUnit.

Quote from OP:

> WP_Mock\Functions::type('callable') was causing a mismatch between the
safe_offset values returned for the Mockery matcher and the actual
closure being passed to add_action. The matcher was getting
spl_object_hash'd, while the Closure was getting `"__CLOSURE__"`. Now,
safe_offset() treats Mockery matchers with type closure as
`"__CLOSURE__"` too.

### Closes: #89

## Test

- [x] Unit tests pass


## Open questions

I think we could rename `safe_offset()` into something more meaningful
to make it easier to understand what it does. Seems this is used solely
within the `Hook` abstract and Filter/Action implementations to get a
string representations of the hook arguments. We could rename it using
camel case and soft deprecate the former method.

As predicted, PhpStan will start spitting errors as tests, return types
and such start to change. I covered some of the issues in this PR, but
had to update the baseline to fix several unrelated positives which we
can address in subsequent PRs.

Co-authored-by: Ashley Gibson <[email protected]>
  • Loading branch information
unfulvio-godaddy and agibson-godaddy authored Jan 3, 2023
1 parent 9d16132 commit 4ea29a9
Show file tree
Hide file tree
Showing 10 changed files with 318 additions and 120 deletions.
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,19 @@ If the actual instance of an expected class cannot be passed, `AnyInstance` can
\WP_Mock::expectFilterAdded( 'the_content', array( new \WP_Mock\Matcher\AnyInstance( Special::class ), 'the_content' ) );
```

#### Asserting that closures have been added as hook callbacks

Sometimes it's handy to add a [Closure](https://secure.php.net/manual/en/class.closure.php) as a WordPress hook instead of defining a function in the global namespace. To assert that such a hook has been added, you can perform assertions referencing the Closure class or a `callable` type:

```php
public function test_anonymous_function_hook() {
\WP_Mock::expectActionAdded('save_post', \WP_Mock\Functions::type('callable'));
\WP_Mock::expectActionAdded('save_post', \WP_Mock\Functions::type(Closure::class));
\WP_Mock::expectFilterAdded('the_content', \WP_Mock\Functions::type('callable'));
\WP_Mock::expectFilterAdded('the_content', \WP_Mock\Functions::type(Closure::class));
}
```

#### Asserting that actions and filters are applied

Now that we're testing whether or not we're adding actions and/or filters, the next step is to ensure our code is calling those hooks when expected.
Expand Down
12 changes: 5 additions & 7 deletions php/WP_Mock/Action.php
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
<?php
/**
* Mock WordPress actions by substituting each action with an advanced object
* capable of intercepting calls and returning predictable behavior.
*
* @package WP_Mock
* @subpackage Hooks
*/

namespace WP_Mock;

/**
* Mock representation of a WordPress action as an object.
*
* Mocks WordPress actions by substituting each action with an object capable of intercepting calls and returning predictable behavior.
*/
class Action extends Hook
{
public function react($args)
Expand Down
12 changes: 5 additions & 7 deletions php/WP_Mock/Filter.php
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
<?php
/**
* Mock WordPress filters by substituting each filter with an advanced object
* capable of intercepting calls and returning predictable data.
*
* @package WP_Mock
* @subpackage Hooks
*/

namespace WP_Mock;

/**
* Mock representation of a WordPress filter as an object.
*
* Mocks WordPress filters by substituting each filter with an object capable of intercepting calls and returning predictable behavior.
*/
class Filter extends Hook
{
/**
Expand Down
50 changes: 34 additions & 16 deletions php/WP_Mock/Hook.php
Original file line number Diff line number Diff line change
@@ -1,48 +1,64 @@
<?php
/**
* Abstract Hook interface for both actions and filters.
*
* @package WP_Mock
* @subpackage Hooks
*/

namespace WP_Mock;

use Closure;
use PHPUnit\Framework\ExpectationFailedException;
use WP_Mock;
use WP_Mock\Matcher\AnyInstance;

/**
* Abstract mock representation of a WordPress hook.
*
* @see Action for mocking WordPress action hooks
* @see Filter for mocking WordPress filter hooks
*/
abstract class Hook
{
/** @var string Hook name */
/** @var string hook name */
protected $name;

/** @var array Collection of processors */
protected $processors = array();
/** @var array<mixed> collection of processors */
protected $processors = [];

public function __construct($name)
/**
* Hook constructor.
*
* @param string $name hook name
*/
public function __construct(string $name)
{
$this->name = $name;
}

protected function safe_offset($value)
/**
* Gets a string representation of a value.
*
* @param mixed $value
* @return string
*/
protected function safe_offset($value): string
{
if (is_null($value)) {
if (null === $value) {
return 'null';
/** the following is to prevent a possible return mismatch when {@see \WP_Mock\Functions::type()} is used with 'callable' */
} elseif ($value instanceof Closure || Closure::class === $value || (is_string($value) && '<CLOSURE>' === strtoupper($value))) {
return '__CLOSURE__';
} elseif (is_scalar($value)) {
return $value;
return (string) $value;
} elseif ($value instanceof AnyInstance) {
return (string) $value;
} elseif (is_object($value)) {
return spl_object_hash($value);
} elseif (is_array($value)) {
$return = '';
$parsed = '';

foreach ($value as $k => $v) {
$k = is_numeric($k) ? '' : $k;
$return .= $k . $this->safe_offset($v);
$parsed .= $k.$this->safe_offset($v);
}

return $return;
return $parsed;
}

return '';
Expand Down Expand Up @@ -92,6 +108,8 @@ protected function strict_check(): void
}

/**
* Gets the message to output when the strict mode exception is thrown.
*
* @return string
*/
abstract protected function get_strict_mode_message();
Expand Down
9 changes: 0 additions & 9 deletions php/WP_Mock/HookedCallback.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,6 @@ protected function new_responder()
return new HookedCallbackResponder();
}

protected function safe_offset($value)
{
if ($value instanceof \Closure) {
$value = '__CLOSURE__';
}

return parent::safe_offset($value);
}

/**
* Converts a callable to a string
*
Expand Down
6 changes: 5 additions & 1 deletion php/WP_Mock/Tools/TestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,22 @@
use PHPUnit\Util\Test as TestUtil;
use ReflectionException;
use ReflectionMethod;
use PHPUnit\Framework\TestCase as PhpUnitTestCase;
use RuntimeException;
use WP_Mock;
use WP_Mock\Tools\Constraints\ExpectationsMet;
use WP_Mock\Tools\Constraints\IsEqualHtml;
use WP_Mock\Traits\AccessInaccessibleClassMembersTrait;

/**
* WP_Mock test case.
*
* Projects using WP_Mock can extend this class in their unit tests.
*/
abstract class TestCase extends \PHPUnit\Framework\TestCase
abstract class TestCase extends PhpUnitTestCase
{
use AccessInaccessibleClassMembersTrait;

/** @var array<string, Mockery\Mock> */
protected $mockedStaticMethods = [];

Expand Down
75 changes: 75 additions & 0 deletions php/WP_Mock/Traits/AccessInaccessibleClassMembersTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?php

namespace WP_Mock\Traits;

use ReflectionClass;
use ReflectionException;
use ReflectionMethod;
use ReflectionProperty;

/**
* Trait for accessing inaccessible class members through reflection.
*/
trait AccessInaccessibleClassMembersTrait
{
/**
* Gets the given inaccessible method for the given class.
*
* Allows for calling protected and private methods on a class.
*
* @param class-string|object $class the class name or instance
* @param string $methodName the method name
* @return ReflectionMethod
* @throws ReflectionException
*/
public function getInaccessibleMethod($class, string $methodName): ReflectionMethod
{
$class = new ReflectionClass($class);

$method = $class->getMethod($methodName);
$method->setAccessible(true);

return $method;
}

/**
* Gets the given inaccessible property for the given class.
*
* Allows for calling protected and private properties on a class.
*
* @param class-string|object $class the class name or instance
* @param string $propertyName the property name
* @return ReflectionProperty
* @throws ReflectionException
*/
public function getInaccessibleProperty($class, string $propertyName): ReflectionProperty
{
$class = new ReflectionClass($class);

$property = $class->getProperty($propertyName);
$property->setAccessible(true);

return $property;
}

/**
* Allows for setting private or protected properties in a class.
*
* @param object|null $instance class instance or null for static classes
* @param class-string $class
* @param string $property
* @param mixed $value
* @return ReflectionProperty
* @throws ReflectionException
*/
public function setInaccessibleProperty($instance, string $class, string $property, $value): ReflectionProperty
{
$class = new ReflectionClass($class);

$property = $class->getProperty($property);
$property->setAccessible(true);
$property->setValue($instance, $value);

return $property;
}
}
Loading

0 comments on commit 4ea29a9

Please sign in to comment.