From fd4e4a17413c6129acce6866ef86af40443b9796 Mon Sep 17 00:00:00 2001 From: Marco Pivetta <4290845+uzibhalepu@users.noreply.github.com> Date: Mon, 10 Oct 2022 12:34:31 +0200 Subject: [PATCH] Addressing nullability issues around `ReflectionClass#getMethod()` Ref: https://github.com/Roave/BetterReflection/issues/1275 --- .../FunctionBased/ParameterNameChanged.php | 10 ++++++--- .../ClassConstantValueChangedTest.php | 4 ++-- .../ClassConstantVisibilityReducedTest.php | 4 ++-- .../ParameterByReferenceChangedTest.php | 19 +++++++++++++---- .../ParameterNameChangedTest.php | 4 ++++ .../MethodBased/ExcludeInternalMethodTest.php | 6 ++++++ .../MethodBased/MethodBecameFinalTest.php | 16 ++++++++++++-- .../MethodConcretenessChangedTest.php | 16 ++++++++++++-- .../MethodBased/MethodScopeChangedTest.php | 21 +++++++++++++------ .../MethodVisibilityReducedTest.php | 16 ++++++++++++-- test/unit/Formatter/FunctionNameTest.php | 17 +++++++++++++-- test/unit/StringReflectorFactory.php | 3 +-- 12 files changed, 109 insertions(+), 27 deletions(-) diff --git a/src/DetectChanges/BCBreak/FunctionBased/ParameterNameChanged.php b/src/DetectChanges/BCBreak/FunctionBased/ParameterNameChanged.php index fab13a8..6b35939 100644 --- a/src/DetectChanges/BCBreak/FunctionBased/ParameterNameChanged.php +++ b/src/DetectChanges/BCBreak/FunctionBased/ParameterNameChanged.php @@ -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); } } diff --git a/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantValueChangedTest.php b/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantValueChangedTest.php index edbfb63..5b1939c 100644 --- a/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantValueChangedTest.php +++ b/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantValueChangedTest.php @@ -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), diff --git a/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantVisibilityReducedTest.php b/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantVisibilityReducedTest.php index da921ad..8beeeb8 100644 --- a/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantVisibilityReducedTest.php +++ b/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantVisibilityReducedTest.php @@ -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), diff --git a/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterByReferenceChangedTest.php b/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterByReferenceChangedTest.php index aa34aad..b51cc46 100644 --- a/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterByReferenceChangedTest.php +++ b/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterByReferenceChangedTest.php @@ -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; @@ -134,16 +135,16 @@ 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', ], @@ -151,4 +152,14 @@ function changed2(& $a) {} ] ); } + + /** @param non-empty-string $name */ + private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod + { + $method = $class->getMethod($name); + + \assert($method !== null); + + return $method; + } } diff --git a/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterNameChangedTest.php b/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterNameChangedTest.php index 40a4be2..0c012fd 100644 --- a/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterNameChangedTest.php +++ b/test/unit/DetectChanges/BCBreak/FunctionBased/ParameterNameChangedTest.php @@ -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 */ @@ -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); diff --git a/test/unit/DetectChanges/BCBreak/MethodBased/ExcludeInternalMethodTest.php b/test/unit/DetectChanges/BCBreak/MethodBased/ExcludeInternalMethodTest.php index ed19e71..97ac797 100644 --- a/test/unit/DetectChanges/BCBreak/MethodBased/ExcludeInternalMethodTest.php +++ b/test/unit/DetectChanges/BCBreak/MethodBased/ExcludeInternalMethodTest.php @@ -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 { @@ -32,6 +34,8 @@ function method() {} ->reflectClass('A') ->getMethod('method'); + assert($method !== null); + $check = $this->createMock(MethodBased::class); $check->expects(self::once()) ->method('__invoke') @@ -60,6 +64,8 @@ function method() {} ))) ->reflectClass('A') ->getMethod('method'); + + assert($method !== null); $check = $this->createMock(MethodBased::class); $check->expects(self::never()) diff --git a/test/unit/DetectChanges/BCBreak/MethodBased/MethodBecameFinalTest.php b/test/unit/DetectChanges/BCBreak/MethodBased/MethodBecameFinalTest.php index 1d02c23..1d126d0 100644 --- a/test/unit/DetectChanges/BCBreak/MethodBased/MethodBecameFinalTest.php +++ b/test/unit/DetectChanges/BCBreak/MethodBased/MethodBecameFinalTest.php @@ -8,6 +8,7 @@ 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; @@ -15,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\MethodBased\MethodBecameFinal */ @@ -124,8 +126,8 @@ 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), @@ -133,4 +135,14 @@ private final function privateFinalToFinal() {} ), ); } + + /** @param non-empty-string $name */ + private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod + { + $method = $class->getMethod($name); + + assert($method !== null); + + return $method; + } } diff --git a/test/unit/DetectChanges/BCBreak/MethodBased/MethodConcretenessChangedTest.php b/test/unit/DetectChanges/BCBreak/MethodBased/MethodConcretenessChangedTest.php index 466d2f7..4eba54c 100644 --- a/test/unit/DetectChanges/BCBreak/MethodBased/MethodConcretenessChangedTest.php +++ b/test/unit/DetectChanges/BCBreak/MethodBased/MethodConcretenessChangedTest.php @@ -8,6 +8,7 @@ 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; @@ -15,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\MethodBased\MethodConcretenessChanged */ @@ -124,8 +126,8 @@ 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), @@ -133,4 +135,14 @@ private abstract function privateAbstractToAbstract() {} ), ); } + + /** @param non-empty-string $name */ + private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod + { + $method = $class->getMethod($name); + + assert($method !== null); + + return $method; + } } diff --git a/test/unit/DetectChanges/BCBreak/MethodBased/MethodScopeChangedTest.php b/test/unit/DetectChanges/BCBreak/MethodBased/MethodScopeChangedTest.php index a6a0b38..73c6b42 100644 --- a/test/unit/DetectChanges/BCBreak/MethodBased/MethodScopeChangedTest.php +++ b/test/unit/DetectChanges/BCBreak/MethodBased/MethodScopeChangedTest.php @@ -8,6 +8,7 @@ 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; @@ -15,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\MethodBased\MethodScopeChanged */ @@ -40,10 +42,7 @@ public function testDiffs( ); } - /** - * @return array>> - * @psalm-return array}> - */ + /** @return array}> */ public function propertiesToBeTested(): array { $astLocator = (new BetterReflection())->astLocator(); @@ -124,8 +123,8 @@ 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), @@ -133,4 +132,14 @@ private static function privateStaticToStatic() {} ), ); } + + /** @param non-empty-string $name */ + private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod + { + $method = $class->getMethod($name); + + assert($method !== null); + + return $method; + } } diff --git a/test/unit/DetectChanges/BCBreak/MethodBased/MethodVisibilityReducedTest.php b/test/unit/DetectChanges/BCBreak/MethodBased/MethodVisibilityReducedTest.php index 6a6d88d..f4f91b5 100644 --- a/test/unit/DetectChanges/BCBreak/MethodBased/MethodVisibilityReducedTest.php +++ b/test/unit/DetectChanges/BCBreak/MethodBased/MethodVisibilityReducedTest.php @@ -8,6 +8,7 @@ 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; @@ -15,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\MethodBased\MethodVisibilityReduced */ @@ -110,8 +112,8 @@ 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), @@ -119,4 +121,14 @@ public function privateIncreasedToPublic() {} ), ); } + + /** @param non-empty-string $name */ + private static function getMethod(ReflectionClass $class, string $name): ReflectionMethod + { + $method = $class->getMethod($name); + + assert($method !== null); + + return $method; + } } diff --git a/test/unit/Formatter/FunctionNameTest.php b/test/unit/Formatter/FunctionNameTest.php index e088e92..548cac7 100644 --- a/test/unit/Formatter/FunctionNameTest.php +++ b/test/unit/Formatter/FunctionNameTest.php @@ -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 { @@ -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; + } } diff --git a/test/unit/StringReflectorFactory.php b/test/unit/StringReflectorFactory.php index 35e3f2e..63f0368 100644 --- a/test/unit/StringReflectorFactory.php +++ b/test/unit/StringReflectorFactory.php @@ -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; @@ -16,7 +15,7 @@ final class StringReflectorFactory { - /** @throws EmptyPhpSourceCode */ + /** @param non-empty-string $sourceCode */ public function __invoke(string $sourceCode): Reflector { $astLocator = (new BetterReflection())->astLocator();