diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 38b7dc5..0909cae 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -178,8 +178,14 @@ /tests/Polyfills/Fixtures/ValueObjectParamNotRequired\.php$ + + /tests/Polyfills/Fixtures/ValueObjectNullableReturnType\.php$ + /tests/Polyfills/Fixtures/ValueObjectUnion\.php$ + + /tests/Polyfills/Fixtures/ValueObjectUnionReturnType\.php$ + diff --git a/README.md b/README.md index 897e254..ef12396 100644 --- a/README.md +++ b/README.md @@ -391,17 +391,10 @@ This assertion expects an object to contain a comparator method in the object it The `assertObjectEquals()` assertion was introduced in PHPUnit 9.4.0. -> :information_source: Due to [limitations in how this assertion is implemented in PHPUnit] itself, it is currently not possible to create a single comparator method which will be compatible with both PHP < 7.0 and PHP 7.0 or higher. -> -> In effect two declarations of the same object would be needed to be compatible with PHP < 7.0 and PHP 7.0 and higher and still allow for testing the object using the `assertObjectEquals()` assertion. -> -> Due to this limitation, it is recommended to only use this assertion if the minimum supported PHP version of a project is PHP 7.0 or higher; or if the project does not run its tests on PHPUnit >= 9.4.0. -> -> The implementation of this assertion in the Polyfills is PHP cross-version compatible. - -[limitations in how this assertion is implemented in PHPUnit]: https://github.com/sebastianbergmann/phpunit/issues/4707 +> :information_source: In PHPUnit Polyfills 1.x and 2.x, the polyfill for this assertion had [some limitations][assertobjectequals-limitations]. These limitations are no longer present in PHPUnit Polyfills 3.x and the assertion now matches the PHPUnit native implementation. [`Assert::assertObjectEquals()`]: https://docs.phpunit.de/en/main/assertions.html#assertobjectequals +[assertobjectequals-limitations]: https://github.com/Yoast/PHPUnit-Polyfills/?tab=readme-ov-file#phpunit--940-yoastphpunitpolyfillspolyfillsassertobjectequals #### PHPUnit < 10.0.0: `Yoast\PHPUnitPolyfills\Polyfills\AssertIgnoringLineEndings` diff --git a/composer.json b/composer.json index 31e049b..4afa0cd 100644 --- a/composer.json +++ b/composer.json @@ -59,10 +59,10 @@ }, "scripts": { "lint7": [ - "@php ./vendor/php-parallel-lint/php-parallel-lint/parallel-lint . -e php --show-deprecated --exclude vendor --exclude .git --exclude tests/Polyfills/Fixtures/ValueObjectUnion.php" + "@php ./vendor/php-parallel-lint/php-parallel-lint/parallel-lint . -e php --show-deprecated --exclude vendor --exclude .git --exclude tests/Polyfills/Fixtures/ValueObjectUnion.php --exclude tests/Polyfills/Fixtures/ValueObjectUnionReturnType.php" ], "lint70": [ - "@php ./vendor/php-parallel-lint/php-parallel-lint/parallel-lint . -e php --show-deprecated --exclude vendor --exclude .git --exclude src/Exceptions/Error.php --exclude src/Exceptions/TypeError.php --exclude tests/Polyfills/Fixtures/ValueObjectParamNotRequired.php --exclude tests/Polyfills/Fixtures/ValueObjectUnion.php" + "@php ./vendor/php-parallel-lint/php-parallel-lint/parallel-lint . -e php --show-deprecated --exclude vendor --exclude .git --exclude src/Exceptions/Error.php --exclude src/Exceptions/TypeError.php --exclude tests/Polyfills/Fixtures/ValueObjectParamNotRequired.php --exclude tests/Polyfills/Fixtures/ValueObjectNullableReturnType.php --exclude tests/Polyfills/Fixtures/ValueObjectUnion.php --exclude tests/Polyfills/Fixtures/ValueObjectUnionReturnType.php" ], "lint-gte80": [ "@php ./vendor/php-parallel-lint/php-parallel-lint/parallel-lint . -e php --show-deprecated --exclude vendor --exclude .git" diff --git a/src/Polyfills/AssertObjectEquals.php b/src/Polyfills/AssertObjectEquals.php index 0b2319e..2e069bc 100644 --- a/src/Polyfills/AssertObjectEquals.php +++ b/src/Polyfills/AssertObjectEquals.php @@ -2,8 +2,6 @@ namespace Yoast\PHPUnitPolyfills\Polyfills; -use ReflectionClass; -use ReflectionException; use ReflectionNamedType; use ReflectionObject; use ReflectionType; @@ -16,7 +14,7 @@ * Introduced in PHPUnit 9.4.0. * * The polyfill implementation closely matches the PHPUnit native implementation with the exception - * of the return type check and the names of the thrown exceptions. + * of the thrown exceptions. * * @link https://github.com/sebastianbergmann/phpunit/issues/4467 * @link https://github.com/sebastianbergmann/phpunit/issues/4707 @@ -37,8 +35,7 @@ trait AssertObjectEquals { * - The method must accept exactly one argument and this argument must be required. * - This parameter must have a classname-based declared type. * - The $expected object must be compatible with this declared type. - * - The method must have a declared bool return type. (JRF: not verified in this implementation) - * - `$actual->$method($expected)` returns boolean true. + * - The method must have a declared bool return type. * * @param object $expected Expected value. * @param object $actual The value to test. @@ -103,14 +100,49 @@ final public static function assertObjectEquals( $expected, $actual, $method = ' $reflMethod = $reflObject->getMethod( $method ); /* - * As the next step, PHPUnit natively would validate the return type, - * but as return type declarations is a PHP 7.0+ feature, the polyfill - * skips this check in favour of checking the type of the actual - * returned value. - * - * Also see the upstream discussion about this: - * {@link https://github.com/sebastianbergmann/phpunit/issues/4707} + * Comparator method return type requirements validation. */ + $returnTypeError = \sprintf( + 'Comparison method %s::%s() does not declare bool return type.', + \get_class( $actual ), + $method + ); + + if ( $reflMethod->hasReturnType() === false ) { + throw new InvalidComparisonMethodException( $returnTypeError ); + } + + $returnType = $reflMethod->getReturnType(); + + if ( \class_exists( 'ReflectionNamedType' ) ) { + // PHP >= 7.1: guard against union/intersection return types. + if ( ( $returnType instanceof ReflectionNamedType ) === false ) { + throw new InvalidComparisonMethodException( $returnTypeError ); + } + } + elseif ( ( $returnType instanceof ReflectionType ) === false ) { + /* + * PHP 7.0. + * Checking for `ReflectionType` will not throw an error on union types, + * but then again union types are not supported on PHP 7.0. + */ + throw new InvalidComparisonMethodException( $returnTypeError ); + } + + if ( $returnType->allowsNull() === true ) { + throw new InvalidComparisonMethodException( $returnTypeError ); + } + + if ( \method_exists( $returnType, 'getName' ) ) { + // PHP 7.1+. + if ( $returnType->getName() !== 'bool' ) { + throw new InvalidComparisonMethodException( $returnTypeError ); + } + } + elseif ( (string) $returnType !== 'bool' ) { + // PHP 7.0. + throw new InvalidComparisonMethodException( $returnTypeError ); + } /* * Comparator method parameter requirements validation. @@ -142,55 +174,31 @@ final public static function assertObjectEquals( $expected, $actual, $method = ' $reflParameter = $reflMethod->getParameters()[0]; - if ( \method_exists( $reflParameter, 'hasType' ) ) { - // PHP >= 7.0. - $hasType = $reflParameter->hasType(); - if ( $hasType === false ) { + $hasType = $reflParameter->hasType(); + if ( $hasType === false ) { + throw new InvalidComparisonMethodException( $noDeclaredTypeError ); + } + + $type = $reflParameter->getType(); + if ( \class_exists( 'ReflectionNamedType' ) ) { + // PHP >= 7.1. + if ( ( $type instanceof ReflectionNamedType ) === false ) { throw new InvalidComparisonMethodException( $noDeclaredTypeError ); } - $type = $reflParameter->getType(); - if ( \class_exists( 'ReflectionNamedType' ) ) { - // PHP >= 7.1. - if ( ( $type instanceof ReflectionNamedType ) === false ) { - throw new InvalidComparisonMethodException( $noDeclaredTypeError ); - } - - $typeName = $type->getName(); - } - else { - /* - * PHP 7.0. - * Checking for `ReflectionType` will not throw an error on union types, - * but then again union types are not supported on PHP 7.0. - */ - if ( ( $type instanceof ReflectionType ) === false ) { - throw new InvalidComparisonMethodException( $noDeclaredTypeError ); - } - - $typeName = (string) $type; - } + $typeName = $type->getName(); } else { - // PHP < 7.0. - try { - /* - * Using `ReflectionParameter::getClass()` will trigger an autoload of the class, - * but that's okay as for a valid class type that would be triggered on the - * function call to the $method (at the end of this assertion) anyway. - */ - $hasType = $reflParameter->getClass(); - } catch ( ReflectionException $e ) { - // Class with a type declaration for a non-existent class. - throw new InvalidComparisonMethodException( $notAcceptableTypeError ); - } - - if ( ( $hasType instanceof ReflectionClass ) === false ) { - // Array or callable type. + /* + * PHP 7.0. + * Checking for `ReflectionType` will not throw an error on union types, + * but then again union types are not supported on PHP 7.0. + */ + if ( ( $type instanceof ReflectionType ) === false ) { throw new InvalidComparisonMethodException( $noDeclaredTypeError ); } - $typeName = $hasType->name; + $typeName = (string) $type; } /* @@ -209,16 +217,6 @@ final public static function assertObjectEquals( $expected, $actual, $method = ' */ $result = $actual->{$method}( $expected ); - if ( \is_bool( $result ) === false ) { - throw new InvalidComparisonMethodException( - \sprintf( - 'Comparison method %s::%s() does not return a boolean value.', - \get_class( $actual ), - $method - ) - ); - } - $msg = \sprintf( 'Failed asserting that two objects are equal. The objects are not equal according to %s::%s()', \get_class( $actual ), diff --git a/tests/Polyfills/AssertObjectEqualsTest.php b/tests/Polyfills/AssertObjectEqualsTest.php index ebb4550..3d78a90 100644 --- a/tests/Polyfills/AssertObjectEqualsTest.php +++ b/tests/Polyfills/AssertObjectEqualsTest.php @@ -20,8 +20,10 @@ use Yoast\PHPUnitPolyfills\Polyfills\ExpectExceptionMessageMatches; use Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ChildValueObject; use Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObject; +use Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObjectNullableReturnType; use Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObjectParamNotRequired; use Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObjectUnion; +use Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObjectUnionReturnType; /** * Availability test for the function polyfilled by the AssertObjectEquals trait. @@ -183,6 +185,100 @@ public function testAssertObjectEqualsFailsOnMethodNotDeclared() { $this->assertObjectEquals( $expected, $actual, 'doesNotExist' ); } + /** + * Verify that the assertObjectEquals() method throws an error when no return type is declared. + * + * @return void + */ + public function testAssertObjectEqualsFailsOnMissingReturnType() { + $msg = 'Comparison method Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObject::equalsMissingReturnType() does not declare bool return type.'; + + $exception = self::COMPARATOR_EXCEPTION; + if ( \class_exists( ComparisonMethodDoesNotDeclareBoolReturnTypeException::class ) ) { + // PHPUnit > 9.4.0. + $exception = ComparisonMethodDoesNotDeclareBoolReturnTypeException::class; + } + + $this->expectException( $exception ); + $this->expectExceptionMessage( $msg ); + + $expected = new ValueObject( 100 ); + $actual = new ValueObject( 100 ); + $this->assertObjectEquals( $expected, $actual, 'equalsMissingReturnType' ); + } + + /** + * Verify that the assertObjectEquals() method throws an error when the declared return type in a union, intersection or DNF type. + * + * @requires PHP 8.0 + * + * @return void + */ + #[RequiresPhp( '8.0' )] + public function testAssertObjectEqualsFailsOnNonNamedTypeReturnType() { + $msg = 'Comparison method Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObjectUnionReturnType::equalsUnionReturnType() does not declare bool return type.'; + + $exception = self::COMPARATOR_EXCEPTION; + if ( \class_exists( ComparisonMethodDoesNotDeclareBoolReturnTypeException::class ) ) { + // PHPUnit > 9.4.0. + $exception = ComparisonMethodDoesNotDeclareBoolReturnTypeException::class; + } + + $this->expectException( $exception ); + $this->expectExceptionMessage( $msg ); + + $expected = new ValueObjectUnionReturnType( 100 ); + $actual = new ValueObjectUnionReturnType( 100 ); + $this->assertObjectEquals( $expected, $actual, 'equalsUnionReturnType' ); + } + + /** + * Verify that the assertObjectEquals() method throws an error when the declared return type is nullable. + * + * @requires PHP 7.1 + * + * @return void + */ + #[RequiresPhp( '7.1' )] + public function testAssertObjectEqualsFailsOnNullableReturnType() { + $msg = 'Comparison method Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObjectNullableReturnType::equalsNullableReturnType() does not declare bool return type.'; + + $exception = self::COMPARATOR_EXCEPTION; + if ( \class_exists( ComparisonMethodDoesNotDeclareBoolReturnTypeException::class ) ) { + // PHPUnit > 9.4.0. + $exception = ComparisonMethodDoesNotDeclareBoolReturnTypeException::class; + } + + $this->expectException( $exception ); + $this->expectExceptionMessage( $msg ); + + $expected = new ValueObjectNullableReturnType( 100 ); + $actual = new ValueObjectNullableReturnType( 100 ); + $this->assertObjectEquals( $expected, $actual, 'equalsNullableReturnType' ); + } + + /** + * Verify that the assertObjectEquals() method throws an error when the declared return type is not boolean. + * + * @return void + */ + public function testAssertObjectEqualsFailsOnNonBooleanReturnType() { + $msg = 'Comparison method Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObject::equalsNonBooleanReturnType() does not declare bool return type.'; + + $exception = self::COMPARATOR_EXCEPTION; + if ( \class_exists( ComparisonMethodDoesNotDeclareBoolReturnTypeException::class ) ) { + // PHPUnit > 9.4.0. + $exception = ComparisonMethodDoesNotDeclareBoolReturnTypeException::class; + } + + $this->expectException( $exception ); + $this->expectExceptionMessage( $msg ); + + $expected = new ValueObject( 100 ); + $actual = new ValueObject( 100 ); + $this->assertObjectEquals( $expected, $actual, 'equalsNonBooleanReturnType' ); + } + /** * Verify that the assertObjectEquals() method throws an error when the $method accepts more than one parameter. * @@ -347,30 +443,6 @@ public function testAssertObjectEqualsFailsOnMethodParamTypeMismatch() { $this->assertObjectEquals( new stdClass(), $actual ); } - /** - * Verify that the assertObjectEquals() method throws an error when the declared return type/ - * the return value is not boolean. - * - * @return void - */ - public function testAssertObjectEqualsFailsOnNonBooleanReturnValue() { - $msg = 'Comparison method Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObject::equalsNonBooleanReturnType() does not return a boolean value.'; - - $exception = self::COMPARATOR_EXCEPTION; - if ( \class_exists( ComparisonMethodDoesNotDeclareBoolReturnTypeException::class ) ) { - // PHPUnit > 9.4.0. - $msg = 'Comparison method Yoast\PHPUnitPolyfills\Tests\Polyfills\Fixtures\ValueObject::equalsNonBooleanReturnType() does not declare bool return type.'; - $exception = ComparisonMethodDoesNotDeclareBoolReturnTypeException::class; - } - - $this->expectException( $exception ); - $this->expectExceptionMessage( $msg ); - - $expected = new ValueObject( 100 ); - $actual = new ValueObject( 100 ); - $this->assertObjectEquals( $expected, $actual, 'equalsNonBooleanReturnType' ); - } - /** * Verify that the assertObjectEquals() method fails a test when a call to method * determines that the objects are not equal. diff --git a/tests/Polyfills/Fixtures/ValueObject.php b/tests/Polyfills/Fixtures/ValueObject.php index 777a984..7d3a013 100644 --- a/tests/Polyfills/Fixtures/ValueObject.php +++ b/tests/Polyfills/Fixtures/ValueObject.php @@ -47,6 +47,28 @@ public function nonDefaultName( ValueObject $other ): bool { return ( $this->value === $other->value ); } + /** + * Comparator method: incorrectly declared - missing return type. + * + * @param self $other Object to compare. + * + * @return bool + */ + public function equalsMissingReturnType( self $other ) { + return ( $this->value === $other->value ); + } + + /** + * Comparator method: incorrectly declared - non-boolean return type/value. + * + * @param self $other Object to compare. + * + * @return bool + */ + public function equalsNonBooleanReturnType( self $other ): int { + return ( $this->value <=> $other->value ); + } + /** * Comparator method: incorrectly declared - more than one parameter. * @@ -91,15 +113,4 @@ public function equalsParamNonClassType( array $other ): bool { public function equalsParamNonExistentClassType( ClassWhichDoesntExist $other ): bool { return ( $this->value === $other->value ); } - - /** - * Comparator method: incorrectly declared - non-boolean return type/value. - * - * @param self $other Object to compare. - * - * @return bool - */ - public function equalsNonBooleanReturnType( self $other ): int { - return ( $this->value <=> $other->value ); - } } diff --git a/tests/Polyfills/Fixtures/ValueObjectNullableReturnType.php b/tests/Polyfills/Fixtures/ValueObjectNullableReturnType.php new file mode 100644 index 0000000..45c98af --- /dev/null +++ b/tests/Polyfills/Fixtures/ValueObjectNullableReturnType.php @@ -0,0 +1,39 @@ +value = $value; + } + + /** + * Comparator method: incorrectly declared - return type is nullable. + * + * @param self $other Object to compare. + * + * @return bool + */ + public function equalsNullableReturnType( self $other ): ?bool { + return ( $this->value === $other->value ); + } +} diff --git a/tests/Polyfills/Fixtures/ValueObjectUnionReturnType.php b/tests/Polyfills/Fixtures/ValueObjectUnionReturnType.php new file mode 100644 index 0000000..d937600 --- /dev/null +++ b/tests/Polyfills/Fixtures/ValueObjectUnionReturnType.php @@ -0,0 +1,39 @@ +value = $value; + } + + /** + * Comparator method: incorrectly declared - union type as return type. + * + * @param self $other Object to compare. + * + * @return bool + */ + public function equalsUnionReturnType( self $other ): bool|int { + return ( $this->value === $other->value ); + } +}