Skip to content

Commit 9b54e76

Browse files
cseufertthisispiersTRPBlkrms
authored
Merge upstream changes (#6)
* Support nullable class type hints (Level-2#182) * Remove PHP 8 deprecation warning * Level-2#195 - stop some parameters getting counted twice * Fix issue where 'shareInstances' resolve but fail to propagate (Level-2#201) * Level-2#200 - Expand shareInstances test to replicate propagation issue * Level-2#200 - Fix issue where 'shareInstances' resolve but fail to propagate Because `$share` is passed by reference to `matchParam()`, and `matchParam()` removes matching objects from its `$search` array, instances may be removed from `$share` before it is passed to `expand()` or `create()`. Depending on the order of constructor parameters and the relative placement of `shareInstances` dependencies in the object tree, this may result in multiple instances of these dependencies being created. Fixed by passing a copy of the `$share` array to `matchParam()`. - Updated CI to use github actions Co-authored-by: thisispiers <piers@c1h.co.uk> Co-authored-by: Tom Butler <tom@r.je> Co-authored-by: Luke Arms <luke@linacreative.com>
1 parent 0abde11 commit 9b54e76

20 files changed

Lines changed: 165 additions & 49 deletions

‎.coveralls.yml‎

Lines changed: 0 additions & 3 deletions
This file was deleted.

‎.github/workflows/ci.yml‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
name: Dice CI
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
9+
jobs:
10+
build:
11+
runs-on: ubuntu-latest
12+
13+
strategy:
14+
matrix:
15+
php: [ '7.1', '7.4', '8.0', '8.1' ]
16+
17+
steps:
18+
- name: Setup PHP with PECL extension
19+
uses: shivammathur/setup-php@v2
20+
with:
21+
php-version: ${{ matrix.php }}
22+
coverage: xdebug
23+
ini-file: development
24+
tools: composer
25+
- uses: actions/checkout@v2
26+
- name: Install PHP Dependancies
27+
run: composer install
28+
- name: Run tests with coverage
29+
env:
30+
XDEBUG_MODE: coverage
31+
run: composer test-coverage
32+
- name: Upload coverage results to Coveralls
33+
uses: nick-invision/retry@v2
34+
env:
35+
COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
36+
COVERALLS_PARALLEL: true
37+
COVERALLS_FLAG_NAME: "PHP${{ matrix.php }}"
38+
with:
39+
timeout_seconds: 60
40+
max_attempts: 3
41+
command: php vendor/bin/php-coveralls -x clover.xml -o coveralls-upload.json -v

‎src/Dice.php‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* @copyright 2012-2020 Tom Butler <tom@r.je> | https://r.je/dice
55
* @license http://www.opensource.org/licenses/bsd-license.php BSD License
66
*/
7+
78
namespace Dice;
89
class Dice
910
{
@@ -112,12 +113,10 @@ public function getRule(string $name): array
112113
/**
113114
* Returns a fully constructed object based on $name using $args and $share as constructor arguments if supplied
114115
* @template T
115-
116116
* @param string $name The name of the class to instantiate
117117
* @psalm-param class-string<T> $name
118118
* @param array $args An array with any additional arguments to be passed into the constructor upon instantiation
119119
* @param array $share a list of defined in shareInstances for objects higher up the object graph, should only be used internally
120-
121120
* @return object A fully constructed object based on the specified input arguments
122121
* @psalm-return T
123122
*/
@@ -204,7 +203,7 @@ private function getClosure(string $name, array $rule)
204203
//Internal classes may not be able to be constructed without calling the constructor and will not suffer from #7, construct them normally.
205204
if ($class->isInternal()) {
206205
$this->instances[$name] = $this->instances[
207-
"\\" . $name
206+
"\\" . $class->name
208207
] = $closure($args, $share);
209208
} else {
210209
//Otherwise, create the class without calling the constructor (and write to \$name and $name, see issue #68)
@@ -331,11 +330,11 @@ private function expand(
331330
? $this->create($param)
332331
: $param;
333332
}
334-
/**
335-
* Looks through the array $search for any object which can be used to fulfil $param
336-
The original array $search is modifed so must be passed by reference.
337333

338-
*/
334+
/**
335+
* Looks through the array $search for any object which can be used to fulfil $param
336+
* The original array $search is modifed so must be passed by reference.
337+
*/
339338
private function matchParam(
340339
\ReflectionParameter $param,
341340
$class,
@@ -353,6 +352,7 @@ private function matchParam(
353352
}
354353
return false;
355354
}
355+
356356
/**
357357
* Returns a closure that generates arguments for $method based on $rule and any $args passed into the closure
358358
* @param object $method An instance of ReflectionMethod (see: {@link http:// php.net/manual/en/class.reflectionmethod.php})
@@ -366,9 +366,7 @@ private function getParams(\ReflectionMethod $method, array $rule)
366366
foreach ($method->getParameters() as $param) {
367367
$pType = $param->getType();
368368
$class =
369-
$pType &&
370-
!$pType->isBuiltin() &&
371-
$pType instanceof \ReflectionNamedType
369+
$pType instanceof \ReflectionNamedType && !$pType->isBuiltin()
372370
? $pType->getName()
373371
: null;
374372
$paramInfo[] = [
@@ -407,22 +405,26 @@ private function getParams(\ReflectionMethod $method, array $rule)
407405
}
408406
// Do the same with $share
409407
elseif (
410-
$share &&
411-
($match = $this->matchParam($param, $class, $share)) !==
408+
($copy = $share) &&
409+
($match = $this->matchParam($param, $class, $copy)) !==
412410
false
413411
) {
414412
$parameters[] = $match;
415413
}
416414
// When nothing from $args or $share matches but a class is type hinted, create an instance to use, using a substitution if set
417415
elseif ($class) {
418416
try {
419-
$parameters[] = $sub
420-
? $this->expand(
417+
if ($sub) {
418+
$parameters[] = $this->expand(
421419
$rule["substitutions"][$class],
422420
$share,
423421
true
424-
)
425-
: $this->create($class, [], $share);
422+
);
423+
} else {
424+
$parameters[] = !$param->allowsNull()
425+
? $this->create($class, [], $share)
426+
: null;
427+
}
426428
} catch (\InvalidArgumentException $e) {
427429
}
428430
}

‎tests/BasicTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ public function testPassGlobals()
185185
$_GET["foo"] = "bar";
186186

187187
$dice = $this->dice->addRule("CheckConstructorArgs", [
188-
"constructParams" => [[\Dice\Dice::GLOBAL => "_GET"]],
188+
"constructParams" => [[Dice\Dice::GLOBAL => "_GET"]],
189189
]);
190190

191191
$obj = $dice->create("CheckConstructorArgs");
@@ -218,7 +218,7 @@ public function testImmutability()
218218
public function testPassSelf()
219219
{
220220
$dice = $this->dice->addRule("CheckConstructorArgs", [
221-
"constructParams" => [[\Dice\Dice::INSTANCE => \Dice\Dice::SELF]],
221+
"constructParams" => [[Dice\Dice::INSTANCE => Dice\Dice::SELF]],
222222
]);
223223

224224
$obj = $dice->create("CheckConstructorArgs");

‎tests/CallTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/* @description Dice - A minimal Dependency Injection Container for PHP *
34
* @author Tom Butler tom@r.je *
45
* @copyright 2012-2018 Tom Butler <tom@r.je> | https:// r.je/dice.html *
@@ -71,8 +72,8 @@ public function testCallChain()
7172
$rules = [
7273
"TestCallImmutable" => [
7374
"call" => [
74-
["call1", ["foo"], \Dice\Dice::CHAIN_CALL],
75-
["call2", ["bar"], \Dice\Dice::CHAIN_CALL],
75+
["call1", ["foo"], Dice\Dice::CHAIN_CALL],
76+
["call2", ["bar"], Dice\Dice::CHAIN_CALL],
7677
],
7778
],
7879
];

‎tests/ChainTest.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/* @description Dice - A minimal Dependency Injection Container for PHP *
34
* @author Tom Butler tom@r.je *
45
* @copyright 2012-2018 Tom Butler <tom@r.je> | https:// r.je/dice.html *
@@ -26,8 +27,8 @@ public function testMultipleChainCall()
2627
'$someClass' => [
2728
"instanceOf" => "Factory",
2829
"call" => [
29-
["get", [], \Dice\Dice::CHAIN_CALL],
30-
["getBar", [], \Dice\Dice::CHAIN_CALL],
30+
["get", [], Dice\Dice::CHAIN_CALL],
31+
["getBar", [], Dice\Dice::CHAIN_CALL],
3132
],
3233
],
3334
]);
@@ -43,7 +44,7 @@ public function testChainCallShared()
4344
'$someClass' => [
4445
"shared" => true,
4546
"instanceOf" => "Factory",
46-
"call" => [["get", [], \Dice\Dice::CHAIN_CALL]],
47+
"call" => [["get", [], Dice\Dice::CHAIN_CALL]],
4748
],
4849
]);
4950

@@ -57,7 +58,7 @@ public function testChainCallInject()
5758
$dice = $this->dice->addRules([
5859
"FactoryDependency" => [
5960
"instanceOf" => "Factory",
60-
"call" => [["get", [], \Dice\Dice::CHAIN_CALL]],
61+
"call" => [["get", [], Dice\Dice::CHAIN_CALL]],
6162
],
6263
]);
6364

@@ -72,7 +73,7 @@ public function testChainCallInjectShared()
7273
"FactoryDependency" => [
7374
"shared" => true,
7475
"instanceOf" => "Factory",
75-
"call" => [["get", [], \Dice\Dice::CHAIN_CALL]],
76+
"call" => [["get", [], Dice\Dice::CHAIN_CALL]],
7677
],
7778
]);
7879

‎tests/ConstructParamsTest.php‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/* @description Dice - A minimal Dependency Injection Container for PHP *
34
* @author Tom Butler tom@r.je *
45
* @copyright 2012-2018 Tom Butler <tom@r.je> | https:// r.je/dice.html *
@@ -57,7 +58,7 @@ public function testInternalClassExtendedConstructor()
5758
public function testDefaultNullAssigned()
5859
{
5960
$rule = [];
60-
$rule["constructParams"] = [[\Dice\Dice::INSTANCE => "A"], null];
61+
$rule["constructParams"] = [[Dice\Dice::INSTANCE => "A"], null];
6162
$dice = $this->dice->addRule("MethodWithDefaultNull", $rule);
6263
$obj = $dice->create("MethodWithDefaultNull");
6364
$this->assertNull($obj->b);
@@ -138,4 +139,11 @@ public function testNullScalarNested()
138139
$obj = $dice->create("NullScalarNested");
139140
$this->assertEquals(null, $obj->nullScalar->string);
140141
}
142+
143+
public function testNullableClassTypeHint()
144+
{
145+
$nullableClassTypeHint = $this->dice->create("NullableClassTypeHint");
146+
147+
$this->assertEquals(null, $nullableClassTypeHint->obj);
148+
}
141149
}

‎tests/CreateArgsTest.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/* @description Dice - A minimal Dependency Injection Container for PHP *
34
* @author Tom Butler tom@r.je *
45
* @copyright 2012-2018 Tom Butler <tom@r.je> | https:// r.je/dice.html *
@@ -85,15 +86,18 @@ public function testTwoDefaultNullClass()
8586
{
8687
$obj = $this->dice->create("MethodWithTwoDefaultNullC");
8788
$this->assertNull($obj->a);
88-
$this->assertInstanceOf("NB", $obj->b);
89+
$this->assertNull($obj->b);
90+
// $this->assertInstanceOf("NB", $obj->b);
8991
}
9092

9193
public function testTwoDefaultNullClassClass()
9294
{
9395
$obj = $this->dice->create("MethodWithTwoDefaultNullCC");
9496
$this->assertNull($obj->a);
95-
$this->assertInstanceOf("NB", $obj->b);
96-
$this->assertInstanceOf("NC", $obj->c);
97+
$this->assertNull($obj->b);
98+
$this->assertNull($obj->c);
99+
// $this->assertInstanceOf("NB", $obj->b);
100+
// $this->assertInstanceOf("NC", $obj->c);
97101
}
98102

99103
public function testScalarConstructorArgs()

‎tests/DiceTest.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/* @description Dice - A minimal Dependency Injection Container for PHP *
34
* @author Tom Butler tom@r.je *
45
* @copyright 2012-2018 Tom Butler <tom@r.je> | https:// r.je/dice.html *
@@ -34,7 +35,7 @@ public function autoload($class)
3435
protected function setUp(): void
3536
{
3637
parent::setUp();
37-
$this->dice = new \Dice\Dice();
38+
$this->dice = new Dice\Dice();
3839
}
3940

4041
protected function tearDown(): void

‎tests/NamedInstancesTest.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/* @description Dice - A minimal Dependency Injection Container for PHP *
34
* @author Tom Butler tom@r.je *
45
* @copyright 2012-2018 Tom Butler <tom@r.je> | https:// r.je/dice.html *
@@ -24,8 +25,8 @@ public function testMultipleSharedInstancesByNameMixed()
2425

2526
$rule = [];
2627
$rule["constructParams"] = [
27-
[\Dice\Dice::INSTANCE => "Y"],
28-
[\Dice\Dice::INSTANCE => "[Y2]"],
28+
[Dice\Dice::INSTANCE => "Y"],
29+
[Dice\Dice::INSTANCE => "[Y2]"],
2930
];
3031

3132
$dice = $dice->addRule("Z", $rule);
@@ -42,7 +43,7 @@ public function testNonSharedComponentByNameA()
4243
$dice = $this->dice->addRule('$B', $rule);
4344

4445
$rule = [];
45-
$rule["constructParams"][] = [\Dice\Dice::INSTANCE => '$B'];
46+
$rule["constructParams"][] = [Dice\Dice::INSTANCE => '$B'];
4647
$dice = $dice->addRule("A", $rule);
4748

4849
$a = $dice->create("A");
@@ -63,7 +64,7 @@ public function testNonSharedComponentByName()
6364

6465
$rule = [];
6566

66-
$rule["constructParams"][] = [\Dice\Dice::INSTANCE => '$Y2'];
67+
$rule["constructParams"][] = [Dice\Dice::INSTANCE => '$Y2'];
6768
$dice = $dice->addRule("Y1", $rule);
6869

6970
$y1 = $dice->create("Y1");
@@ -77,7 +78,7 @@ public function testSubstitutionByName()
7778
$dice = $this->dice->addRule('$B', $rule);
7879

7980
$rule = [];
80-
$rule["substitutions"]["B"] = [\Dice\Dice::INSTANCE => '$B'];
81+
$rule["substitutions"]["B"] = [Dice\Dice::INSTANCE => '$B'];
8182

8283
$dice = $dice->addRule("A", $rule);
8384
$a = $dice->create("A");
@@ -99,8 +100,8 @@ public function testMultipleSubstitutions()
99100

100101
$rule = [];
101102
$rule["constructParams"] = [
102-
[\Dice\Dice::INSTANCE => '$Y2A'],
103-
[\Dice\Dice::INSTANCE => '$Y2B'],
103+
[Dice\Dice::INSTANCE => '$Y2A'],
104+
[Dice\Dice::INSTANCE => '$Y2B'],
104105
];
105106
$dice = $dice->addRule("HasTwoSameDependencies", $rule);
106107

0 commit comments

Comments
 (0)