Skip to content

Commit a200bb8

Browse files
azjezzazjezz
committed
refactor(reflection): throw on absent bound/default and non-ancestor lookups
Signed-off-by: azjezz <azjezz@protonmail.com>
1 parent b9a9e0f commit a200bb8

10 files changed

Lines changed: 206 additions & 147 deletions

‎ext/reflection/php_reflection.c‎

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8439,7 +8439,9 @@ ZEND_METHOD(ReflectionGenericTypeParameter, getBound)
84398439
GET_GENERIC_PARAMETER_REFERENCE(intern, ref);
84408440

84418441
if (!ZEND_TYPE_IS_SET(ref->param->bound)) {
8442-
RETURN_NULL();
8442+
zend_throw_exception_ex(reflection_exception_ptr, 0,
8443+
"Type parameter %s has no bound", ZSTR_VAL(ref->param->name));
8444+
RETURN_THROWS();
84438445
}
84448446
zend_class_entry *declaring_class = NULL;
84458447
zend_function *declaring_fn = NULL;
@@ -8478,7 +8480,9 @@ ZEND_METHOD(ReflectionGenericTypeParameter, getDefault)
84788480
GET_GENERIC_PARAMETER_REFERENCE(intern, ref);
84798481

84808482
if (!ZEND_TYPE_IS_SET(ref->param->default_type)) {
8481-
RETURN_NULL();
8483+
zend_throw_exception_ex(reflection_exception_ptr, 0,
8484+
"Type parameter %s has no default", ZSTR_VAL(ref->param->name));
8485+
RETURN_THROWS();
84828486
}
84838487

84848488
zend_class_entry *declaring_class = NULL;
@@ -8675,10 +8679,6 @@ ZEND_METHOD(ReflectionNamedType, getGenericArguments)
86758679
static void reflection_build_named_args_list(zval *return_value, const zend_type *boxed,
86768680
zend_class_entry *declaring_class)
86778681
{
8678-
if (!ZEND_TYPE_HAS_NAMED_WITH_ARGS(*boxed)) {
8679-
RETURN_NULL();
8680-
}
8681-
86828682
zend_type_named_with_args *named = ZEND_TYPE_NAMED_WITH_ARGS(*boxed);
86838683
array_init_size(return_value, named->count);
86848684
for (uint32_t i = 0; i < named->count; i++) {
@@ -8698,11 +8698,20 @@ ZEND_METHOD(ReflectionClass, getGenericArgumentsForParentClass)
86988698
ZEND_PARSE_PARAMETERS_NONE();
86998699
GET_REFLECTION_OBJECT_PTR(ce);
87008700

8701-
if (!ce->generic_types || !ce->generic_types->extends) {
8702-
RETURN_NULL();
8701+
bool has_parent = (ce->ce_flags & ZEND_ACC_LINKED)
8702+
? ce->parent != NULL
8703+
: ce->parent_name != NULL;
8704+
if (!has_parent) {
8705+
zend_throw_exception_ex(reflection_exception_ptr, 0,
8706+
"Class %s has no parent class", ZSTR_VAL(ce->name));
8707+
RETURN_THROWS();
87038708
}
87048709

8705-
reflection_build_named_args_list(return_value, ce->generic_types->extends, ce);
8710+
if (ce->generic_types && ce->generic_types->extends) {
8711+
reflection_build_named_args_list(return_value, ce->generic_types->extends, ce);
8712+
return;
8713+
}
8714+
RETURN_EMPTY_ARRAY();
87068715
}
87078716

87088717
ZEND_METHOD(ReflectionClass, getGenericArgumentsForParentInterface)
@@ -8716,22 +8725,40 @@ ZEND_METHOD(ReflectionClass, getGenericArgumentsForParentInterface)
87168725
ZEND_PARSE_PARAMETERS_END();
87178726
GET_REFLECTION_OBJECT_PTR(ce);
87188727

8719-
if (!ce->generic_types || !ce->generic_types->implements) {
8720-
RETURN_NULL();
8728+
bool is_ancestor = false;
8729+
if (ce->ce_flags & ZEND_ACC_LINKED) {
8730+
for (uint32_t i = 0; i < ce->num_interfaces; i++) {
8731+
if (zend_string_equals_ci(ce->interfaces[i]->name, name)) {
8732+
is_ancestor = true;
8733+
break;
8734+
}
8735+
}
8736+
} else {
8737+
for (uint32_t i = 0; i < ce->num_interfaces; i++) {
8738+
if (zend_string_equals_ci(ce->interface_names[i].name, name)) {
8739+
is_ancestor = true;
8740+
break;
8741+
}
8742+
}
8743+
}
8744+
if (!is_ancestor) {
8745+
zend_throw_exception_ex(reflection_exception_ptr, 0,
8746+
"%s is not an ancestor interface of %s", ZSTR_VAL(name), ZSTR_VAL(ce->name));
8747+
RETURN_THROWS();
87218748
}
87228749

8723-
zval *zv;
8724-
ZEND_HASH_FOREACH_VAL(ce->generic_types->implements, zv) {
8725-
zend_type *boxed = (zend_type *) Z_PTR_P(zv);
8726-
if (ZEND_TYPE_HAS_NAMED_WITH_ARGS(*boxed)) {
8750+
if (ce->generic_types && ce->generic_types->implements) {
8751+
zval *zv;
8752+
ZEND_HASH_FOREACH_VAL(ce->generic_types->implements, zv) {
8753+
zend_type *boxed = (zend_type *) Z_PTR_P(zv);
87278754
zend_type_named_with_args *named = ZEND_TYPE_NAMED_WITH_ARGS(*boxed);
87288755
if (zend_string_equals_ci(named->name, name)) {
87298756
reflection_build_named_args_list(return_value, boxed, ce);
87308757
return;
87318758
}
8732-
}
8733-
} ZEND_HASH_FOREACH_END();
8734-
RETURN_NULL();
8759+
} ZEND_HASH_FOREACH_END();
8760+
}
8761+
RETURN_EMPTY_ARRAY();
87358762
}
87368763

87378764
ZEND_METHOD(ReflectionClass, getGenericArgumentsForUsedTrait)
@@ -8745,22 +8772,31 @@ ZEND_METHOD(ReflectionClass, getGenericArgumentsForUsedTrait)
87458772
ZEND_PARSE_PARAMETERS_END();
87468773
GET_REFLECTION_OBJECT_PTR(ce);
87478774

8748-
if (!ce->generic_types || !ce->generic_types->trait_uses) {
8749-
RETURN_NULL();
8775+
bool is_used = false;
8776+
for (uint32_t i = 0; i < ce->num_traits; i++) {
8777+
if (zend_string_equals_ci(ce->trait_names[i].name, name)) {
8778+
is_used = true;
8779+
break;
8780+
}
8781+
}
8782+
if (!is_used) {
8783+
zend_throw_exception_ex(reflection_exception_ptr, 0,
8784+
"%s is not a trait used by %s", ZSTR_VAL(name), ZSTR_VAL(ce->name));
8785+
RETURN_THROWS();
87508786
}
87518787

8752-
zval *zv;
8753-
ZEND_HASH_FOREACH_VAL(ce->generic_types->trait_uses, zv) {
8754-
zend_type *boxed = (zend_type *) Z_PTR_P(zv);
8755-
if (ZEND_TYPE_HAS_NAMED_WITH_ARGS(*boxed)) {
8788+
if (ce->generic_types && ce->generic_types->trait_uses) {
8789+
zval *zv;
8790+
ZEND_HASH_FOREACH_VAL(ce->generic_types->trait_uses, zv) {
8791+
zend_type *boxed = (zend_type *) Z_PTR_P(zv);
87568792
zend_type_named_with_args *named = ZEND_TYPE_NAMED_WITH_ARGS(*boxed);
87578793
if (zend_string_equals_ci(named->name, name)) {
87588794
reflection_build_named_args_list(return_value, boxed, ce);
87598795
return;
87608796
}
8761-
}
8762-
} ZEND_HASH_FOREACH_END();
8763-
RETURN_NULL();
8797+
} ZEND_HASH_FOREACH_END();
8798+
}
8799+
RETURN_EMPTY_ARRAY();
87648800
}
87658801

87668802
PHP_MINIT_FUNCTION(reflection) /* {{{ */

‎ext/reflection/php_reflection.stub.php‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -449,32 +449,33 @@ public function getGenericParameters(): array {}
449449

450450
/**
451451
* Returns the type arguments this class supplies at the parent-class extends
452-
* site, in source order. Returns null if there is no parent class, or if
453-
* the extends clause specified no type arguments.
452+
* site, in source order. Returns an empty array if the extends clause
453+
* specified no type arguments.
454454
*
455-
* @return list<ReflectionType>|null
455+
* @return list<ReflectionType>
456+
* @throws ReflectionException if this class has no parent class
456457
*/
457-
public function getGenericArgumentsForParentClass(): ?array {}
458+
public function getGenericArgumentsForParentClass(): array {}
458459

459460
/**
460-
* Returns the type arguments this class supplies for the named parent
461-
* interface (a directly-listed `implements` entry, or, for an interface
462-
* declaration, a directly-listed `extends` entry), in source order.
463-
* Returns null if $name is not a directly-listed parent interface, or if
464-
* no type arguments were specified at the use site.
461+
* Returns the type arguments this class supplies for the named ancestor
462+
* interface, in source order. Returns an empty array if no type arguments
463+
* were specified at the use site.
465464
*
466-
* @return list<ReflectionType>|null
465+
* @return list<ReflectionType>
466+
* @throws ReflectionException if $name is not an ancestor interface
467467
*/
468-
public function getGenericArgumentsForParentInterface(string $name): ?array {}
468+
public function getGenericArgumentsForParentInterface(string $name): array {}
469469

470470
/**
471471
* Returns the type arguments this class supplies at the use site for trait
472-
* $name, in source order. Returns null if $name is not a directly-used
473-
* trait, or if no type arguments were specified at the use site.
472+
* $name, in source order. Returns an empty array if no type arguments were
473+
* specified at the use site.
474474
*
475-
* @return list<ReflectionType>|null
475+
* @return list<ReflectionType>
476+
* @throws ReflectionException if $name is not a directly-used trait
476477
*/
477-
public function getGenericArgumentsForUsedTrait(string $name): ?array {}
478+
public function getGenericArgumentsForUsedTrait(string $name): array {}
478479
}
479480

480481
class ReflectionObject extends ReflectionClass
@@ -844,11 +845,13 @@ public function getVariance(): ReflectionGenericVariance {}
844845

845846
public function hasBound(): bool {}
846847

847-
public function getBound(): ?ReflectionType {}
848+
/** @throws ReflectionException if this type parameter has no bound; check hasBound() first */
849+
public function getBound(): ReflectionType {}
848850

849851
public function hasDefault(): bool {}
850852

851-
public function getDefault(): ?ReflectionType {}
853+
/** @throws ReflectionException if this type parameter has no default; check hasDefault() first */
854+
public function getDefault(): ReflectionType {}
852855

853856
public function getDeclaringEntity(): ReflectionClass|ReflectionFunctionAbstract {}
854857

‎ext/reflection/php_reflection_arginfo.h‎

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/reflection/php_reflection_decl.h‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎ext/reflection/tests/generics/ancestor_args_no_generics_class.phpt‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Reflection: classes with no generic content return null from all three accessors
2+
Reflection: classes with no generic content — empty array when ancestor without args, throw when not an ancestor
33
--FILE--
44
<?php
55
class Plain {}
@@ -9,18 +9,39 @@ class WithIface implements IPlain {}
99
trait TPlain { public function noop(): void {} }
1010
class WithTrait { use TPlain; }
1111

12-
foreach (['Plain', 'WithParent', 'WithIface', 'WithTrait'] as $cls) {
12+
function show(string $cls): void {
1313
$rc = new ReflectionClass($cls);
14-
$p = $rc->getGenericArgumentsForParentClass();
15-
$i = $rc->getGenericArgumentsForParentInterface('IPlain');
16-
$t = $rc->getGenericArgumentsForUsedTrait('TPlain');
17-
echo $cls, ": parent=", ($p === null ? "null" : "non-null"),
18-
" iface=", ($i === null ? "null" : "non-null"),
19-
" trait=", ($t === null ? "null" : "non-null"), "\n";
14+
15+
try {
16+
$p = $rc->getGenericArgumentsForParentClass();
17+
echo $cls, ": parent=", json_encode($p);
18+
} catch (ReflectionException $e) {
19+
echo $cls, ": parent=throw(", $e->getMessage(), ")";
20+
}
21+
22+
try {
23+
$i = $rc->getGenericArgumentsForParentInterface('IPlain');
24+
echo " iface=", json_encode($i);
25+
} catch (ReflectionException $e) {
26+
echo " iface=throw(", $e->getMessage(), ")";
27+
}
28+
29+
try {
30+
$t = $rc->getGenericArgumentsForUsedTrait('TPlain');
31+
echo " trait=", json_encode($t);
32+
} catch (ReflectionException $e) {
33+
echo " trait=throw(", $e->getMessage(), ")";
34+
}
35+
36+
echo "\n";
37+
}
38+
39+
foreach (['Plain', 'WithParent', 'WithIface', 'WithTrait'] as $cls) {
40+
show($cls);
2041
}
2142
?>
2243
--EXPECT--
23-
Plain: parent=null iface=null trait=null
24-
WithParent: parent=null iface=null trait=null
25-
WithIface: parent=null iface=null trait=null
26-
WithTrait: parent=null iface=null trait=null
44+
Plain: parent=throw(Class Plain has no parent class) iface=throw(IPlain is not an ancestor interface of Plain) trait=throw(TPlain is not a trait used by Plain)
45+
WithParent: parent=[] iface=throw(IPlain is not an ancestor interface of WithParent) trait=throw(TPlain is not a trait used by WithParent)
46+
WithIface: parent=throw(Class WithIface has no parent class) iface=[] trait=throw(TPlain is not a trait used by WithIface)
47+
WithTrait: parent=throw(Class WithTrait has no parent class) iface=throw(IPlain is not an ancestor interface of WithTrait) trait=[]

‎ext/reflection/tests/generics/ancestor_args_parent_class.phpt‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
--TEST--
2-
Reflection: getGenericArgumentsForParentClass returns args from extends clause
2+
Reflection: getGenericArgumentsForParentClass returns args from extends clause, throws when no parent
33
--FILE--
44
<?php
55
class A<T> {}
@@ -12,15 +12,20 @@ class Multi extends B<int, float> {}
1212

1313
$cases = [
1414
'WithArgs' => ['string'],
15-
'NoArgs' => null,
16-
'NoParent' => null,
15+
'NoArgs' => [],
16+
'NoParent' => 'throw',
1717
'Multi' => ['int', 'float'],
1818
];
1919

2020
foreach ($cases as $cls => $want) {
21-
$args = (new ReflectionClass($cls))->getGenericArgumentsForParentClass();
22-
if ($want === null) {
23-
echo $cls, ": ", $args === null ? "null OK" : "FAIL (got non-null)", "\n";
21+
try {
22+
$args = (new ReflectionClass($cls))->getGenericArgumentsForParentClass();
23+
} catch (ReflectionException $e) {
24+
echo $cls, ": ", $want === 'throw' ? "throw OK ({$e->getMessage()})" : "FAIL (unexpected throw)", "\n";
25+
continue;
26+
}
27+
if ($want === 'throw') {
28+
echo $cls, ": FAIL (expected throw)\n";
2429
} else {
2530
$got = array_map(fn($t) => $t->getName(), $args);
2631
echo $cls, ": ", $got === $want ? "OK" : ("FAIL got " . implode(",", $got)), "\n";
@@ -29,6 +34,6 @@ foreach ($cases as $cls => $want) {
2934
?>
3035
--EXPECT--
3136
WithArgs: OK
32-
NoArgs: null OK
33-
NoParent: null OK
37+
NoArgs: OK
38+
NoParent: throw OK (Class NoParent has no parent class)
3439
Multi: OK

0 commit comments

Comments
 (0)