Skip to content

Commit

Permalink
Addressing nullability issues around ReflectionClass#getMethod()
Browse files Browse the repository at this point in the history
  • Loading branch information
uzibhalepu authored and Ocramius committed Oct 10, 2022
1 parent d60f0ab commit fd4e4a1
Show file tree
Hide file tree
Showing 12 changed files with 109 additions and 27 deletions.
10 changes: 7 additions & 3 deletions src/DetectChanges/BCBreak/FunctionBased/ParameterNameChanged.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,21 @@ private function compareParameter(ReflectionParameter $fromParameter, Reflection

private function methodHasNoNamedArgumentsAnnotation(ReflectionMethod|ReflectionFunction $function): bool
{
if ($function instanceof ReflectionMethod
if (
$function instanceof ReflectionMethod
&& str_contains(
(string) $function
->getDeclaringClass()
->getDocComment(),
self::NO_NAMED_ARGUMENTS_ANNOTATION
self::NO_NAMED_ARGUMENTS_ANNOTATION,
)
) {
return true;
}

return str_contains($function->getDocComment(), self::NO_NAMED_ARGUMENTS_ANNOTATION);
$comment = $function->getDocComment();

return $comment !== null
&& str_contains($comment, self::NO_NAMED_ARGUMENTS_ANNOTATION);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ class TheClass {
array_map(
static fn (string $constant, array $errorMessages): array => [
Type\object(ReflectionClassConstant::class)
->coerce($fromClass->getReflectionConstant($constant)),
->coerce($fromClass->getConstant($constant)),
Type\object(ReflectionClassConstant::class)
->coerce($toClass->getReflectionConstant($constant)),
->coerce($toClass->getConstant($constant)),
$errorMessages,
],
array_keys($properties),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ class TheClass {
array_map(
static fn (string $constant, array $errorMessages): array => [
Type\object(ReflectionClassConstant::class)
->coerce($fromClass->getReflectionConstant($constant)),
->coerce($fromClass->getConstant($constant)),
Type\object(ReflectionClassConstant::class)
->coerce($toClass->getReflectionConstant($constant)),
->coerce($toClass->getConstant($constant)),
$errorMessages,
],
array_keys($properties),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Roave\BackwardCompatibility\Change;
use Roave\BackwardCompatibility\DetectChanges\BCBreak\FunctionBased\ParameterByReferenceChanged;
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionFunction;
use Roave\BetterReflection\Reflection\ReflectionMethod;
use Roave\BetterReflection\Reflector\DefaultReflector;
Expand Down Expand Up @@ -134,21 +135,31 @@ function changed2(& $a) {}
),
[
'N1\C::changed1' => [
$fromReflector->reflectClass('N1\C')->getMethod('changed1'),
$toReflector->reflectClass('N1\C')->getMethod('changed1'),
self::getMethod($fromReflector->reflectClass('N1\C'),'changed1'),
self::getMethod($toReflector->reflectClass('N1\C'), 'changed1'),
[
'[BC] CHANGED: The parameter $a of N1\C::changed1() changed from by-value to by-reference',

],
],
'N1\C#changed2' => [
$fromReflector->reflectClass('N1\C')->getMethod('changed2'),
$toReflector->reflectClass('N1\C')->getMethod('changed2'),
self::getMethod($fromReflector->reflectClass('N1\C'),'changed2'),
self::getMethod($toReflector->reflectClass('N1\C'),'changed2'),
[
'[BC] CHANGED: The parameter $a of N1\C#changed2() changed from by-value to by-reference',
],
],
]
);
}

/** @param non-empty-string $name */
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
{
$method = $class->getMethod($name);

\assert($method !== null);

return $method;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use function array_combine;
use function array_keys;
use function array_map;
use function assert;
use function iterator_to_array;

/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\FunctionBased\ParameterNameChanged */
Expand Down Expand Up @@ -153,6 +154,9 @@ public function theMethod(int $b) {}
$toClassReflector = new DefaultReflector($toLocator);
$fromMethod = $fromClassReflector->reflectClass('TheClass')->getMethod('theMethod');
$toMethod = $toClassReflector->reflectClass('TheClass')->getMethod('theMethod');

assert($fromMethod !== null);
assert($toMethod !== null);

$changes = (new ParameterNameChanged())($fromMethod, $toMethod);
self::assertCount(0, $changes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;

use function assert;

/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\ExcludeInternalMethod */
final class ExcludeInternalMethodTest extends TestCase
{
Expand All @@ -32,6 +34,8 @@ function method() {}
->reflectClass('A')
->getMethod('method');

assert($method !== null);

$check = $this->createMock(MethodBased::class);
$check->expects(self::once())
->method('__invoke')
Expand Down Expand Up @@ -60,6 +64,8 @@ function method() {}
)))
->reflectClass('A')
->getMethod('method');

assert($method !== null);

$check = $this->createMock(MethodBased::class);
$check->expects(self::never())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
use Roave\BackwardCompatibility\Change;
use Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodBecameFinal;
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionMethod;
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;

use function array_combine;
use function array_keys;
use function array_map;
use function assert;
use function iterator_to_array;

/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodBecameFinal */
Expand Down Expand Up @@ -124,13 +126,23 @@ private final function privateFinalToFinal() {}
array_keys($properties),
array_map(
static fn (string $methodName, array $errors): array => [
$fromClass->getMethod($methodName),
$toClass->getMethod($methodName),
self::getMethod($fromClass, $methodName),
self::getMethod($toClass, $methodName),
$errors,
],
array_keys($properties),
$properties,
),
);
}

/** @param non-empty-string $name */
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
{
$method = $class->getMethod($name);

assert($method !== null);

return $method;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
use Roave\BackwardCompatibility\Change;
use Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodConcretenessChanged;
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionMethod;
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;

use function array_combine;
use function array_keys;
use function array_map;
use function assert;
use function iterator_to_array;

/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodConcretenessChanged */
Expand Down Expand Up @@ -124,13 +126,23 @@ private abstract function privateAbstractToAbstract() {}
array_keys($properties),
array_map(
static fn (string $methodName, array $errors): array => [
$fromClass->getMethod($methodName),
$toClass->getMethod($methodName),
self::getMethod($fromClass, $methodName),
self::getMethod($toClass, $methodName),
$errors,
],
array_keys($properties),
$properties,
),
);
}

/** @param non-empty-string $name */
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
{
$method = $class->getMethod($name);

assert($method !== null);

return $method;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
use Roave\BackwardCompatibility\Change;
use Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodScopeChanged;
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionMethod;
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;

use function array_combine;
use function array_keys;
use function array_map;
use function assert;
use function iterator_to_array;

/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodScopeChanged */
Expand All @@ -40,10 +42,7 @@ public function testDiffs(
);
}

/**
* @return array<string, array<int, ReflectionMethod|array<int, string>>>
* @psalm-return array<string, array{0: ReflectionMethod, 1: ReflectionMethod, 2: list<string>}>
*/
/** @return array<string, array{0: ReflectionMethod, 1: ReflectionMethod, 2: list<string>}> */
public function propertiesToBeTested(): array
{
$astLocator = (new BetterReflection())->astLocator();
Expand Down Expand Up @@ -124,13 +123,23 @@ private static function privateStaticToStatic() {}
array_keys($properties),
array_map(
static fn (string $methodName, array $errors): array => [
$fromClass->getMethod($methodName),
$toClass->getMethod($methodName),
self::getMethod($fromClass, $methodName),
self::getMethod($toClass, $methodName),
$errors,
],
array_keys($properties),
$properties,
),
);
}

/** @param non-empty-string $name */
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
{
$method = $class->getMethod($name);

assert($method !== null);

return $method;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
use Roave\BackwardCompatibility\Change;
use Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodVisibilityReduced;
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionMethod;
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;

use function array_combine;
use function array_keys;
use function array_map;
use function assert;
use function iterator_to_array;

/** @covers \Roave\BackwardCompatibility\DetectChanges\BCBreak\MethodBased\MethodVisibilityReduced */
Expand Down Expand Up @@ -110,13 +112,23 @@ public function privateIncreasedToPublic() {}
array_keys($properties),
array_map(
static fn (string $methodName, array $errors): array => [
$fromClass->getMethod($methodName),
$toClass->getMethod($methodName),
self::getMethod($fromClass, $methodName),
self::getMethod($toClass, $methodName),
$errors,
],
array_keys($properties),
$properties,
),
);
}

/** @param non-empty-string $name */
private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod
{
$method = $class->getMethod($name);

assert($method !== null);

return $method;
}
}
17 changes: 15 additions & 2 deletions test/unit/Formatter/FunctionNameTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@
use PHPUnit\Framework\TestCase;
use Roave\BackwardCompatibility\Formatter\FunctionName;
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflection\ReflectionClass;
use Roave\BetterReflection\Reflection\ReflectionFunction;
use Roave\BetterReflection\Reflection\ReflectionMethod;
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\SourceLocator\Type\StringSourceLocator;

use function assert;

/** @covers \Roave\BackwardCompatibility\Formatter\FunctionName */
final class FunctionNameTest extends TestCase
{
Expand Down Expand Up @@ -64,13 +67,23 @@ function e() {}
'N1\b()',
],
'N2\C::d' => [
$reflector->reflectClass('N2\C')->getMethod('d'),
$this->getMethod($reflector->reflectClass('N2\C'), 'd'),
'N2\C::d()',
],
'N2\C#e' => [
$reflector->reflectClass('N2\C')->getMethod('e'),
$this->getMethod($reflector->reflectClass('N2\C'), 'e'),
'N2\C#e()',
],
];
}

/** @param non-empty-string $name */
private function getMethod(ReflectionClass $class, string $name): ReflectionMethod
{
$method = $class->getMethod($name);

assert($method !== null);

return $method;
}
}
3 changes: 1 addition & 2 deletions test/unit/StringReflectorFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Roave\BetterReflection\BetterReflection;
use Roave\BetterReflection\Reflector\DefaultReflector;
use Roave\BetterReflection\Reflector\Reflector;
use Roave\BetterReflection\SourceLocator\Exception\EmptyPhpSourceCode;
use Roave\BetterReflection\SourceLocator\SourceStubber\ReflectionSourceStubber;
use Roave\BetterReflection\SourceLocator\Type\AggregateSourceLocator;
use Roave\BetterReflection\SourceLocator\Type\EvaledCodeSourceLocator;
Expand All @@ -16,7 +15,7 @@

final class StringReflectorFactory
{
/** @throws EmptyPhpSourceCode */
/** @param non-empty-string $sourceCode */
public function __invoke(string $sourceCode): Reflector
{
$astLocator = (new BetterReflection())->astLocator();
Expand Down

0 comments on commit fd4e4a1

Please sign in to comment.