Skip to content

Commit 95f41d5

Browse files
thekidthekid
authored
Merge pull request #48 from xp-framework/fix/mixed-named-parameters-php7
Fix mixing named and unnamed parameters in PHP 7
2 parents ad1ad91 + de91fff commit 95f41d5

File tree

4 files changed

+133
-18
lines changed

4 files changed

+133
-18
lines changed

‎src/main/php/lang/reflection/Routine.class.php‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
use Error, ReflectionException, ReflectionUnionType, ReflectionIntersectionType;
44
use lang\{Reflection, Type, TypeUnion};
55

6-
/** Base class for methods and constructors */
6+
/**
7+
* Base class for methods and constructors
8+
*
9+
* @test lang.reflection.unittest.ArgumentPassingTest
10+
*/
711
abstract class Routine extends Member {
812

913
/** @return [:var] */
@@ -90,23 +94,29 @@ public function parameters(): Parameters {
9094

9195
/** Support named arguments for PHP 7.X */
9296
public static function pass($reflect, $args) {
93-
if (is_string(key($args))) {
94-
$pass= [];
95-
foreach ($reflect->getParameters() as $param) {
96-
if (isset($args[$param->name])) {
97-
$pass[]= $args[$param->name];
98-
} else if ($param->isOptional()) {
99-
$pass[]= $param->getDefaultValue();
100-
} else {
101-
throw new ReflectionException('Missing parameter $'.$param->name);
102-
}
97+
$pass= [];
98+
foreach ($reflect->getParameters() as $i => $param) {
99+
if ($param->isVariadic()) {
100+
while ($args) $pass[]= array_shift($args);
101+
break;
102+
} else if (array_key_exists($param->name, $args)) {
103+
$pass[]= $args[$param->name];
103104
unset($args[$param->name]);
105+
} else if (array_key_exists($i, $args)) {
106+
$pass[]= $args[$i];
107+
unset($args[$i]);
108+
} else if ($param->isOptional()) {
109+
$pass[]= $param->getDefaultValue();
110+
} else {
111+
throw new ReflectionException('Missing parameter $'.$param->name);
104112
}
105-
if ($args) {
106-
throw new Error('Unknown named parameter $'.key($args));
107-
}
108-
return $pass;
109113
}
110-
return $args;
114+
115+
// Check for excess named parameters
116+
if ($args && is_string($excess= key($args))) {
117+
throw new Error('Unknown named parameter $'.$excess);
118+
}
119+
120+
return $pass;
111121
}
112122
}

‎src/main/php/lang/reflection/Type.class.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ public function newInstance(... $args) {
204204
$constructor= $this->reflect->hasMethod('__construct');
205205
try {
206206
if ($constructor) {
207-
$pass= PHP_VERSION_ID < 80000 && $args ? Routine::pass($constructor, $args) : $args;
207+
$pass= PHP_VERSION_ID < 80000 && $args ? Routine::pass($this->reflect->getMethod('__construct'), $args) : $args;
208208
return $this->reflect->newInstanceArgs($pass);
209209
} else {
210210
return $this->reflect->newInstance();

‎src/test/php/lang/reflection/unittest/AnnotationTest.class.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ public function instantiation() {
273273
Assert::instance(Annotated::class, $t->annotation(Annotated::class)->newInstance());
274274
}
275275

276-
#[Test, Values(['#[Parameterized(1, 2)]', '#[Parameterized(a: 1, b: 2)]', '#[Parameterized(b: 2, a: 1)]'])]
276+
#[Test, Values(['#[Parameterized(1, 2)]', '#[Parameterized(1, b: 2)]', '#[Parameterized(a: 1, b: 2)]', '#[Parameterized(b: 2, a: 1)]'])]
277277
public function parameterized_instantiation($declaration) {
278278
$t= $this->declare('{}', $declaration);
279279
Assert::equals(new Parameterized(1, 2), $t->annotation(Parameterized::class)->newInstance());

‎src/test/php/lang/reflection/unittest/ArgumentPassingTest.class.php‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
<?php namespace lang\reflection\unittest;
2+
3+
use ReflectionFunction;
4+
use lang\Error;
5+
use lang\reflection\Routine;
6+
use test\{Assert, Expect, Test};
7+
8+
class ArgumentPassingTest {
9+
10+
#[Test]
11+
public function pass_ordered() {
12+
$f= new ReflectionFunction(fn($a, $b) => null);
13+
Assert::equals([1, 2], Routine::pass($f, [1, 2]));
14+
}
15+
16+
#[Test]
17+
public function pass_ordered_null() {
18+
$f= new ReflectionFunction(fn($a, $b) => null);
19+
Assert::equals([null, 2], Routine::pass($f, [null, 2]));
20+
}
21+
22+
#[Test, Expect(class: Error::class, message: 'Missing parameter $a')]
23+
public function missing() {
24+
$f= new ReflectionFunction(fn($a, $b) => null);
25+
Routine::pass($f, []);
26+
}
27+
28+
#[Test, Expect(class: Error::class, message: 'Missing parameter $b')]
29+
public function missing_ordered() {
30+
$f= new ReflectionFunction(fn($a, $b) => null);
31+
Routine::pass($f, [1]);
32+
}
33+
34+
#[Test]
35+
public function pass_named() {
36+
$f= new ReflectionFunction(fn($a, $b) => null);
37+
Assert::equals([1, 2], Routine::pass($f, ['a' => 1, 'b' => 2]));
38+
}
39+
40+
#[Test]
41+
public function pass_named_null() {
42+
$f= new ReflectionFunction(fn($a, $b) => null);
43+
Assert::equals([null, 2], Routine::pass($f, ['a' => null, 'b' => 2]));
44+
}
45+
46+
#[Test]
47+
public function pass_named_out_of_order() {
48+
$f= new ReflectionFunction(fn($a, $b) => null);
49+
Assert::equals([1, 2], Routine::pass($f, ['b' => 2, 'a' => 1]));
50+
}
51+
52+
#[Test]
53+
public function omit_optional() {
54+
$f= new ReflectionFunction(fn($a, $b= 0, $c= 0) => null);
55+
Assert::equals([1, 0, 2], Routine::pass($f, ['a' => 1, 'c' => 2]));
56+
}
57+
58+
#[Test, Expect(class: Error::class, message: 'Missing parameter $b')]
59+
public function missing_named() {
60+
$f= new ReflectionFunction(fn($a, $b) => null);
61+
Routine::pass($f, ['a' => 1]);
62+
}
63+
64+
#[Test, Expect(class: Error::class, message: 'Unknown named parameter $unknown')]
65+
public function unknown_named() {
66+
$f= new ReflectionFunction(fn($a, $b) => null);
67+
Routine::pass($f, ['a' => 1, 'b' => 2, 'unknown' => null]);
68+
}
69+
70+
#[Test]
71+
public function pass_named_and_ordered() {
72+
$f= new ReflectionFunction(fn($a, $b) => null);
73+
Assert::equals([1, 2], Routine::pass($f, [1, 'b' => 2]));
74+
}
75+
76+
#[Test]
77+
public function pass_too_many() {
78+
$f= new ReflectionFunction(fn($a, $b) => null);
79+
Assert::equals([1, 2], Routine::pass($f, [1, 2, 3]));
80+
}
81+
82+
#[Test]
83+
public function pass_optional() {
84+
$f= new ReflectionFunction(fn($a, $b= 0) => null);
85+
Assert::equals([1, 2], Routine::pass($f, [1, 2]));
86+
}
87+
88+
#[Test]
89+
public function pass_without_optional() {
90+
$f= new ReflectionFunction(fn($a, $b= 0) => null);
91+
Assert::equals([1, 0], Routine::pass($f, [1]));
92+
}
93+
94+
#[Test]
95+
public function pass_variadic() {
96+
$f= new ReflectionFunction(fn(... $a) => null);
97+
Assert::equals([1, 2], Routine::pass($f, [1, 2]));
98+
}
99+
100+
#[Test]
101+
public function pass_variadic_after() {
102+
$f= new ReflectionFunction(fn($a, ... $b) => null);
103+
Assert::equals([1, 2, 3], Routine::pass($f, [1, 2, 3]));
104+
}
105+
}

0 commit comments

Comments
 (0)