Skip to content

Commit 8bb8475

Browse files
alganetalganet
committed
Fix bugs, remove dead code, and refactor Config internals
- Fix empty("0") treating "0" as null in IniLoader::parseValue - Fix pre-existing bug: matchConstructorParam no longer pollutes constructor array with null placeholders, allowing params with non-nullable defaults (e.g. EntityFactory::$style) to work - Add numeric coercion in parseConstants for strict_types compat - Add declare(strict_types=1) to Instantiator.php - Guard Autowire::cleanupParams to only autowire constructor params, not method-call arguments - Remove Container::__invoke and __call (unused by consumers) - Extract shouldCache() to replace $forceNew param in getInstance; Factory overrides to return false instead of wrapping parent call - Extract stripTrailingNulls helper, deduplicating Autowire/Instantiator - Refactor keyHasStateInstance to existingKeyFrom (no side-effect) - Cache plain callables consistently in lazyLoad - Remove func_get_args(), stripos, and dead imports - Use parent::offsetGet in has() for consistency
1 parent e744aa2 commit 8bb8475

File tree

9 files changed

+236
-239
lines changed

9 files changed

+236
-239
lines changed

‎src/Autowire.php‎

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88
use ReflectionNamedType;
99

1010
use function array_key_exists;
11-
use function end;
12-
use function key;
1311

1412
class Autowire extends Instantiator
1513
{
@@ -21,41 +19,44 @@ public function setContainer(ContainerInterface $container): void
2119
}
2220

2321
/** @inheritDoc */
24-
protected function cleanupParams(array $params): array
22+
protected function cleanupParams(array $params, bool $forConstructor = true): array
2523
{
2624
$constructor = $this->reflection()->getConstructor();
27-
if ($constructor && $this->container) {
25+
$container = $this->container;
26+
if ($forConstructor && $constructor && $container) {
2827
foreach ($constructor->getParameters() as $param) {
2928
$name = $param->getName();
3029
if (array_key_exists($name, $this->params)) {
3130
$value = $params[$name] ?? null;
3231
if ($value instanceof Ref) {
33-
$params[$name] = $this->container->get($value->id);
32+
$params[$name] = $container->get($value->id);
3433
} else {
34+
$this->propagateContainer($value);
3535
$params[$name] = $this->lazyLoad($value);
3636
}
37-
38-
continue;
39-
}
40-
41-
$type = $param->getType();
42-
if (
43-
!($type instanceof ReflectionNamedType) || $type->isBuiltin()
44-
|| !$this->container->has($type->getName())
45-
) {
46-
continue;
37+
} else {
38+
$type = $param->getType();
39+
if (
40+
$type instanceof ReflectionNamedType && !$type->isBuiltin()
41+
&& $container->has($type->getName())
42+
) {
43+
$params[$name] = $container->get($type->getName());
44+
}
4745
}
48-
49-
$params[$name] = $this->container->get($type->getName());
50-
}
51-
52-
while (end($params) === null && ($key = key($params)) !== null) {
53-
unset($params[$key]);
5446
}
5547

56-
return $params;
48+
return $this->stripTrailingNulls($params);
5749
}
5850

5951
return parent::cleanupParams($params);
6052
}
53+
54+
protected function propagateContainer(mixed $value): void
55+
{
56+
if (!$value instanceof self || $value->container !== null || $this->container === null) {
57+
return;
58+
}
59+
60+
$value->setContainer($this->container);
61+
}
6162
}

‎src/Container.php‎

Lines changed: 15 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,11 @@
77
use ArrayObject;
88
use Closure;
99
use Psr\Container\ContainerInterface;
10-
use ReflectionClass;
1110
use ReflectionException;
12-
use ReflectionFunction;
13-
use ReflectionNamedType;
1411

15-
use function array_filter;
16-
use function array_map;
17-
use function assert;
1812
use function call_user_func;
1913
use function class_exists;
20-
use function func_get_args;
21-
use function is_array;
2214
use function is_callable;
23-
use function is_string;
2415

2516
/** @extends ArrayObject<string, mixed> */
2617
class Container extends ArrayObject implements ContainerInterface
@@ -39,7 +30,7 @@ public function has(string $id): bool
3930
return false;
4031
}
4132

42-
$entry = $this[$id];
33+
$entry = parent::offsetGet($id);
4334
if ($entry instanceof Instantiator) {
4435
return class_exists($entry->getClassName());
4536
}
@@ -49,12 +40,13 @@ public function has(string $id): bool
4940

5041
public function getItem(string $name, bool $raw = false): mixed
5142
{
52-
if (!isset($this[$name])) {
43+
if (!parent::offsetExists($name)) {
5344
throw new NotFoundException('Item ' . $name . ' not found');
5445
}
5546

56-
if ($raw || !is_callable($this[$name])) {
57-
return $this[$name];
47+
$value = $this[$name];
48+
if ($raw || !is_callable($value)) {
49+
return $value;
5850
}
5951

6052
try {
@@ -86,76 +78,27 @@ public function set(string $name, mixed $value): void
8678
protected function lazyLoad(string $name): mixed
8779
{
8880
$callback = $this[$name];
89-
if ($callback instanceof Instantiator && !$callback instanceof Factory) {
90-
return $this[$name] = $callback();
81+
if ($callback instanceof Instantiator) {
82+
$result = $callback();
83+
if ($callback->shouldCache()) {
84+
$this[$name] = $result;
85+
}
86+
87+
return $result;
9188
}
9289

9390
if ($callback instanceof Closure) {
9491
return $this[$name] = $callback($this);
9592
}
9693

97-
return call_user_func($callback);
94+
return $this[$name] = call_user_func($callback);
9895
}
9996

10097
public function __isset(string $name): bool
10198
{
10299
return $this->has($name);
103100
}
104101

105-
public function __invoke(mixed $spec): mixed
106-
{
107-
if (is_callable($spec)) {
108-
if (is_array($spec)) {
109-
[$class, $method] = $spec;
110-
$class = new ReflectionClass($class);
111-
$object = $class->newInstance();
112-
$mirror = $class->getMethod($method);
113-
} else {
114-
$object = false;
115-
assert($spec instanceof Closure || is_string($spec));
116-
$mirror = new ReflectionFunction($spec);
117-
}
118-
119-
$container = $this;
120-
$arguments = array_map(
121-
static function ($param) use ($container) {
122-
$paramClass = $param->getType();
123-
if ($paramClass instanceof ReflectionNamedType) {
124-
return $container->getItem($paramClass->getName());
125-
}
126-
127-
return null;
128-
},
129-
$mirror->getParameters(),
130-
);
131-
if ($object) {
132-
return $mirror->invokeArgs($object, $arguments);
133-
}
134-
135-
return $mirror->invokeArgs($arguments);
136-
}
137-
138-
if ((bool) array_filter(func_get_args(), 'is_object')) {
139-
foreach (func_get_args() as $dependency) {
140-
$this[$dependency::class] = $dependency;
141-
}
142-
}
143-
144-
foreach ($spec as $name => $item) {
145-
parent::offsetSet($name, $item);
146-
}
147-
148-
return $this;
149-
}
150-
151-
/** @param array<mixed> $dict */
152-
public function __call(string $name, array $dict): mixed
153-
{
154-
$this->__invoke($dict[0]);
155-
156-
return $this->getItem($name);
157-
}
158-
159102
public function __get(string $name): mixed
160103
{
161104
return $this->getItem($name);
@@ -164,6 +107,8 @@ public function __get(string $name): mixed
164107
public function __set(string $name, mixed $value): void
165108
{
166109
if (isset($this[$name]) && $this[$name] instanceof Instantiator) {
110+
// Update the Instantiator's cached instance so other Instantiators
111+
// holding a reference to it resolve to the new value
167112
$this[$name]->setInstance($value);
168113
}
169114

‎src/Factory.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@
66

77
class Factory extends Instantiator
88
{
9-
public function getInstance(bool $forceNew = false): mixed
9+
public function shouldCache(): bool
1010
{
11-
$this->instance = null;
12-
13-
return parent::getInstance(true);
11+
return false;
1412
}
1513
}

‎src/IniLoader.php‎

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,21 @@
77
use Closure;
88
use InvalidArgumentException;
99

10-
use function array_filter;
1110
use function constant;
1211
use function count;
13-
use function current;
1412
use function defined;
1513
use function explode;
1614
use function file_exists;
1715
use function is_array;
18-
use function is_object;
16+
use function is_numeric;
1917
use function is_string;
2018
use function parse_ini_file;
2119
use function parse_ini_string;
2220
use function preg_match;
2321
use function preg_replace;
2422
use function preg_replace_callback;
2523
use function str_contains;
24+
use function strstr;
2625
use function trim;
2726

2827
class IniLoader
@@ -82,14 +81,24 @@ public function fromFile(string $configurator): Container
8281
/** @param array<string, mixed> $configurator */
8382
public function fromArray(array $configurator): Container
8483
{
85-
foreach ($this->state() + $configurator as $key => $value) {
84+
foreach ($configurator as $key => $value) {
85+
$stringKey = (string) $key;
86+
87+
// State takes precedence: use existing non-Instantiator value instead
88+
if (
89+
$this->container->offsetExists($stringKey)
90+
&& !$this->container[$stringKey] instanceof Instantiator
91+
) {
92+
$value = $this->container[$stringKey];
93+
}
94+
8695
if ($value instanceof Closure) {
87-
$this->container->offsetSet((string) $key, $value);
96+
$this->container->offsetSet($stringKey, $value);
8897
continue;
8998
}
9099

91100
if ($value instanceof Instantiator) {
92-
$this->container->offsetSet((string) $key, $value);
101+
$this->container->offsetSet($stringKey, $value);
93102
continue;
94103
}
95104

@@ -99,18 +108,11 @@ public function fromArray(array $configurator): Container
99108
return $this->container;
100109
}
101110

102-
/** @return array<string, mixed> */
103-
protected function state(): array
111+
protected function existingKeyFrom(string $key): string|false
104112
{
105-
return array_filter(
106-
$this->container->getArrayCopy(),
107-
static fn($v): bool => !is_object($v) || !$v instanceof Instantiator,
108-
);
109-
}
113+
$k = strstr($key, ' ', true) ?: $key;
110114

111-
protected function keyHasStateInstance(string $key, mixed &$k): bool
112-
{
113-
return $this->container->offsetExists($k = current(explode(' ', $key)));
115+
return $this->container->offsetExists($k) ? $k : false;
114116
}
115117

116118
protected function keyHasInstantiator(string $key): bool
@@ -122,8 +124,9 @@ protected function parseItem(string|int $key, mixed $value): void
122124
{
123125
$key = trim((string) $key);
124126
if ($this->keyHasInstantiator($key)) {
125-
if ($this->keyHasStateInstance($key, $k)) {
126-
$this->container->offsetSet($key, $this->container[$k]);
127+
$existingKey = $this->existingKeyFrom($key);
128+
if ($existingKey !== false) {
129+
$this->container->offsetSet($key, $this->container[$existingKey]);
127130
} else {
128131
$this->parseInstantiator($key, $value);
129132
}
@@ -146,7 +149,7 @@ protected function parseSubValues(array &$value): array
146149
return $value;
147150
}
148151

149-
protected function parseStandardItem(string $key, mixed &$value): void
152+
protected function parseStandardItem(string $key, mixed $value): void
150153
{
151154
if (is_array($value)) {
152155
$this->parseSubValues($value);
@@ -159,6 +162,10 @@ protected function parseStandardItem(string $key, mixed &$value): void
159162

160163
protected function removeDuplicatedSpaces(string $string): string
161164
{
165+
if (!str_contains($string, ' ')) {
166+
return $string;
167+
}
168+
162169
return (string) preg_replace('/\s+/', ' ', $string);
163170
}
164171

@@ -211,7 +218,7 @@ protected function parseValue(mixed $value): mixed
211218
return $this->parseSubValues($value);
212219
}
213220

214-
if (empty($value)) {
221+
if ($value === '' || $value === null) {
215222
return null;
216223
}
217224

@@ -243,6 +250,12 @@ protected function parseConstants(string $value): mixed
243250
return constant($value);
244251
}
245252

253+
if (is_numeric($value)) {
254+
$isFloat = str_contains($value, '.') || str_contains($value, 'e') || str_contains($value, 'E');
255+
256+
return $isFloat ? (float) $value : (int) $value;
257+
}
258+
246259
return $value;
247260
}
248261

0 commit comments

Comments
 (0)