Skip to content

Commit b16b6be

Browse files
ThomasNunningerThomasNunninger
committed
Check for the correct return type in the Dispatcher (not in the individual call handlers).
1 parent 5339eae commit b16b6be

6 files changed

Lines changed: 74 additions & 118 deletions

File tree

‎src/Magic/AbstractCallHandler.php‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -99,33 +99,4 @@ protected function assertGivenParametersMatchMethodSignature(
9999

100100
// TODO: The types of the parameters need to be checked
101101
}
102-
103-
protected function assertReturnType(
104-
object $object,
105-
string $methodName,
106-
$returnValue,
107-
ClassMetadata $classMetadata
108-
)
109-
{
110-
$virtualMethodMetadata = $classMetadata->features[VirtualMethods::FEATURES_KEY][$methodName];
111-
$returnType = $virtualMethodMetadata->returnType;
112-
113-
if (! is_null($returnType->declaration())) {
114-
$isTypeValid = $returnType->validateVariable($returnValue, $object);
115-
116-
if (! $isTypeValid) {
117-
// TODO: Handle error on strict_types declaration of calling context (not file defining the class) :-/
118-
119-
throw new \TypeError(
120-
sprintf(
121-
'Return value of %s::%s() must be of the type %s, %s returned',
122-
$classMetadata->name,
123-
$methodName,
124-
$returnType->declaration(),
125-
gettype($returnValue)
126-
)
127-
);
128-
}
129-
}
130-
}
131102
}

‎src/Magic/Dispatcher.php‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212

1313
namespace ScaleUpStack\EasyObject\Magic;
1414

15+
use ScaleUpStack\EasyObject\FeatureAnalyzers\VirtualMethods;
1516
use ScaleUpStack\Metadata\Metadata\ClassMetadata;
1617
use ScaleUpStack\Metadata\Factory;
18+
use ScaleUpStack\Metadata\Metadata\VirtualMethodMetadata;
1719

1820
final class Dispatcher
1921
{
@@ -64,7 +66,10 @@ public static function invoke(
6466
$callHandler = $instance->callHandlers[$callHandlerClassName];
6567

6668
if ($callHandler->canHandle($methodName, $classMetadata, [])) {
67-
return $callHandler->execute($object, $methodName, $arguments, $classMetadata);
69+
$return = $callHandler->execute($object, $methodName, $arguments, $classMetadata);
70+
71+
$instance->assertReturnType($object, $methodName, $return, $classMetadata);
72+
return $return;
6873
}
6974
}
7075

@@ -82,4 +87,34 @@ private function classMetadata(string $className) : ClassMetadata
8287
return Factory::getMetadataForClass($className)
8388
->classMetadata[$className];
8489
}
90+
91+
private function assertReturnType(
92+
object $object,
93+
string $methodName,
94+
$returnValue,
95+
ClassMetadata $classMetadata
96+
)
97+
{
98+
/** @var VirtualMethodMetadata $virtualMethodMetadata */
99+
$virtualMethodMetadata = $classMetadata->features[VirtualMethods::FEATURES_KEY][$methodName];
100+
$returnType = $virtualMethodMetadata->returnType;
101+
102+
if (! is_null($returnType->declaration())) {
103+
$isTypeValid = $returnType->validateVariable($returnValue, $object);
104+
105+
if (! $isTypeValid) {
106+
// TODO: Handle error on strict_types declaration of calling context (not file defining the class) :-/
107+
108+
throw new \TypeError(
109+
sprintf(
110+
'Return value of %s::%s() must be of the type %s, %s returned',
111+
$classMetadata->name,
112+
$methodName,
113+
$returnType->declaration(),
114+
gettype($returnValue)
115+
)
116+
);
117+
}
118+
}
119+
}
85120
}

‎src/Magic/VirtualGetter.php‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,6 @@ public function execute(object $object, string $methodName, array $arguments, Cl
3030
$this->assertGivenParametersMatchMethodSignature($methodName, $arguments, $classMetadata);
3131

3232
$propertyName = $this->propertyName($methodName, 'get', false, $classMetadata);
33-
$propertyValue = Reflection::getPropertyValue($object, $propertyName);
34-
35-
$this->assertReturnType($object, $methodName, $propertyValue, $classMetadata);
36-
37-
return $propertyValue;
33+
return Reflection::getPropertyValue($object, $propertyName);
3834
}
3935
}

‎tests/PhpUnit/Magic/AbstractCallHandlerTest.php‎

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -166,63 +166,4 @@ public function it_throws_an_exception_on_assertGivenParametersMatchMethodSignat
166166
$this->classMetadata
167167
);
168168
}
169-
170-
/**
171-
* @test
172-
* @covers ::assertReturnType()
173-
*/
174-
public function it_returns_void_on_assertReturnType_when_return_value_is_of_correct_type()
175-
{
176-
// given a mocked AbstractCallHandler, and ClassMetadata of some object as provided in setUp()
177-
// and some object, a method name, and a return value
178-
$object = new ClassForAbstractCallHandlerTesting();
179-
$methodName = 'getSomeProperty';
180-
$returnValue = 42;
181-
182-
// when asserting the return type of the return value
183-
$result = Reflection::methodOfClass(AbstractCallHandler::class, 'assertReturnType')
184-
->invoke(
185-
$this->callHandler,
186-
$object,
187-
$methodName,
188-
$returnValue,
189-
$this->classMetadata
190-
);
191-
192-
// then the $result is null
193-
$this->assertNull($result);
194-
}
195-
196-
/**
197-
* @test
198-
* @covers ::assertReturnType()
199-
*/
200-
public function it_throws_an_exception_on_assertReturnType_when_return_value_is_of_invalid_type()
201-
{
202-
// given a mocked AbstractCallHandler, and ClassMetadata of some object as provided in setUp()
203-
// and some object, a method name, and some return value
204-
$object = new ClassForAbstractCallHandlerTesting();
205-
$methodName = 'getSomeProperty';
206-
$returnValue = 'not an int';
207-
208-
// when asserting the return type of the return value
209-
// then an exception is thrown
210-
$this->expectException(\TypeError::class);
211-
$this->expectExceptionMessage(
212-
sprintf(
213-
'Return value of %s::%s() must be of the type int, string returned',
214-
ClassForAbstractCallHandlerTesting::class,
215-
$methodName
216-
)
217-
);
218-
219-
$result = Reflection::methodOfClass(AbstractCallHandler::class, 'assertReturnType')
220-
->invoke(
221-
$this->callHandler,
222-
$object,
223-
$methodName,
224-
$returnValue,
225-
$this->classMetadata
226-
);
227-
}
228169
}

‎tests/PhpUnit/Magic/DispatcherTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use ScaleUpStack\EasyObject\Magic\VirtualGetter;
1717
use ScaleUpStack\EasyObject\Tests\Resources\Magic\ClassForDispatcherTesting;
1818
use ScaleUpStack\EasyObject\Tests\Resources\TestCase;
19+
use ScaleUpStack\Reflection\Reflection;
1920

2021
/**
2122
* @coversDefaultClass \ScaleUpStack\EasyObject\Magic\Dispatcher
@@ -28,6 +29,7 @@ final class DispatcherTest extends TestCase
2829
* @covers ::__construct()
2930
* @covers ::classMetadata()
3031
* @covers ::invoke()
32+
* @covers ::assertReturnType()
3133
*/
3234
public function it_invokes_a_virtual_method_on_some_object_via_some_call_handler()
3335
{
@@ -49,6 +51,41 @@ public function it_invokes_a_virtual_method_on_some_object_via_some_call_handler
4951
$this->assertSame(42, $result);
5052
}
5153

54+
/**
55+
* @test
56+
* @covers ::assertReturnType()
57+
*/
58+
public function it_throws_an_exception_when_the_return_type_is_invalid()
59+
{
60+
// given an object, and a list of supported call handlers
61+
$object = new ClassForDispatcherTesting();
62+
$supportedCallHandlers = [
63+
VirtualGetter::class
64+
];
65+
// and an invalid type of the property (according to the virtual getter)
66+
Reflection::setPropertyValue($object, 'someProperty', 'not an int as expected');
67+
68+
// when invoking an allowed method that would return a wrong return type
69+
// then an exception is thrown
70+
$this->expectException(\TypeError::class);
71+
$this->expectExceptionMessage(
72+
sprintf(
73+
'Return value of %s::%s() must be of the type %s, %s returned',
74+
ClassForDispatcherTesting::class,
75+
'getSomeProperty',
76+
'int',
77+
'string'
78+
)
79+
);
80+
81+
$result = Dispatcher::invoke(
82+
$object,
83+
'getSomeProperty',
84+
[],
85+
$supportedCallHandlers
86+
);
87+
}
88+
5289
/**
5390
* @test
5491
* @covers ::invoke()

‎tests/PhpUnit/Magic/VirtualGetterTest.php‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -110,30 +110,6 @@ public function it_executes_a_virtual_getter($returnType)
110110
$this->assertSame(42, $result);
111111
}
112112

113-
/**
114-
* @test
115-
* @covers ::execute()
116-
*/
117-
public function it_throws_an_exception_when_the_returned_value_is_of_wrong_type()
118-
{
119-
// given a VirtualGetter call handler, an object, and the object's ClassMetadata
120-
$handler = new VirtualGetter();
121-
$object = new ClassForMagicTesting();
122-
$metadata = $this->getClassMetadata(
123-
'method',
124-
'string getSomeProperty()'
125-
);
126-
127-
// when executing the method
128-
// then an exception is thrown
129-
$this->expectException(\TypeError::class);
130-
$this->expectExceptionMessage(
131-
'Return value of ScaleUpStack\EasyObject\Tests\Resources\Magic\ClassForMagicTesting::getSomeProperty() must be of the type string, integer returned'
132-
);
133-
134-
$handler->execute($object, 'getSomeProperty', [], $metadata);
135-
}
136-
137113
/**
138114
* @test
139115
* @covers ::execute()

0 commit comments

Comments
 (0)