8000 [DependencyInjection] Don't skip classes with private constructor whe… · symfony/symfony@99830f6 · GitHub
[go: up one dir, main page]
Skip to content

Commit 99830f6

Browse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Don't skip classes with private constructor when autodiscovering
1 parent 6f4f04b commit 99830f6

File tree

6 files changed

+63
-45
lines changed

6 files changed

+63
-45
lines changed

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ CHANGELOG
66

77
* Make `#[AsTaggedItem]` repeatable
88
* Support `@>` as a shorthand for `!service_closure` in yaml files
9+
* Don't skip classes with private constructor when autodiscovering
910

1011
7.2
1112
---

‎src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ protected function getConstructor(Definition $definition, bool $required): ?\Ref
181181
throw new RuntimeException(\sprintf('Invalid service "%s": class%s has no constructor.', $this->currentId, \sprintf($class !== $this->currentId ? ' "%s"' : '', $class)));
182182
}
183183
} elseif (!$r->isPublic()) {
184-
throw new RuntimeException(\sprintf('Invalid service "%s": ', $this->currentId).\sprintf($class !== $this->currentId ? 'constructor of class "%s"' : 'its constructor', $class).' must be public.');
184+
throw new RuntimeException(\sprintf('Invalid service "%s": ', $this->currentId).\sprintf($class !== $this->currentId ? 'constructor of class "%s"' : 'its constructor', $class).' must be public. Did you miss configuring a factory or a static constructor? Try using the "#[Autoconfigure(constructor: ...)]" attribute for the latter.');
185185
}
186186

187187
return $r;

‎src/Symfony/Component/DependencyInjection/Loader/FileLoader.php

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public function registerClasses(Definition $prototype, string $namespace, string
126126

127127
$autoconfigureAttributes = new RegisterAutoconfigureAttributesPass();
128128
$autoconfigureAttributes = $autoconfigureAttributes->accept($prototype) ? $autoconfigureAttributes : null;
129-
$classes = $this->findClasses($namespace, $resource, (array) $exclude, $autoconfigureAttributes, $source);
129+
$classes = $this->findClasses($namespace, $resource, (array) $exclude, $source);
130130

131131
$getPrototype = static fn () => clone $prototype;
132132
$serialized = serialize($prototype);
@@ -188,41 +188,46 @@ public function registerClasses(Definition $prototype, string $namespace, string
188188
}
189189
}
190190

191-
if (interface_exists($class, false)) {
192-
$this->interfaces[] = $class;
193-
} else {
194-
$this->setDefinition($class, $definition = $getPrototype());
195-
if (null !== $errorMessage) {
196-
$definition->addError($errorMessage);
197-
198-
continue;
191+
$r = null === $errorMessage ? $this->container->getReflectionClass($class) : null;
192+
if ($r?->isAbstract() || $r?->isInterface()) {
193+
if ($r->isInterface()) {
194+
$this->interfaces[] = $class;
199195
}
200-
$definition->setClass($class);
196+
$autoconfigureAttributes?->processClass($this->container, $r);
197+
continue;
198+
}
201199

202-
$interfaces = [];
203-
foreach (class_implements($class, false) as $interface) {
204-
$this->singlyImplemented[$interface] = ($this->singlyImplemented[$interface] ?? $class) !== $class ? false : $class;
205-
$interfaces[] = $interface;
206-
}
200+
$this->setDefinition($class, $definition = $getPrototype());
201+
if (null !== $errorMessage) {
202+
$definition->addError($errorMessage);
207203

208-
if (!$autoconfigureAttributes) {
209-
continue;
204+
continue;
205+
}
206+
$definition->setClass($class);
207+
208+
$interfaces = [];
209+
foreach (class_implements($class, false) as $interface) {
210+
$this->singlyImplemented[$interface] = ($this->singlyImplemented[$interface] ?? $class) !== $class ? false : $class;
211+
$interfaces[] = $interface;
212+
}
213+
214+
if (!$autoconfigureAttributes) {
215+
continue;
216+
}
217+
$r = $this->container->getReflectionClass($class);
218+
$defaultAlias = 1 === \count($interfaces) ? $interfaces[0] : null;
219+
foreach ($r->getAttributes(AsAlias::class) as $attr) {
220+
/** @var AsAlias $attribute */
221+
$attribute = $attr->newInstance();
222+
$alias = $attribute->id ?? $defaultAlias;
223+
$public = $attribute->public;
224+
if (null === $alias) {
225+
throw new LogicException(\sprintf('Alias cannot be automatically determined for class "%s". If you have used the #[AsAlias] attribute with a class implementing multiple interfaces, add the interface you want to alias to the first parameter of #[AsAlias].', $class));
210226
}
211-
$r = $this->container->getReflectionClass($class);
212-
$defaultAlias = 1 === \count($interfaces) ? $interfaces[0] : null;
213-
foreach ($r->getAttributes(AsAlias::class) as $attr) {
214-
/** @var AsAlias $attribute */
215-
$attribute = $attr->newInstance();
216-
$alias = $attribute->id ?? $defaultAlias;
217-
$public = $attribute->public;
218-
if (null === $alias) {
219-
throw new LogicException(\sprintf('Alias cannot be automatically determined for class "%s". If you have used the #[AsAlias] attribute with a class implementing multiple interfaces, add the interface you want to alias to the first parameter of #[AsAlias].', $class));
220-
}
221-
if (isset($this->aliases[$alias])) {
222-
throw new LogicException(\sprintf('The "%s" alias has already been defined with the #[AsAlias] attribute in "%s".', $alias, $this->aliases[$alias]));
223-
}
224-
$this->aliases[$alias] = new Alias($class, $public);
227+
if (isset($this->aliases[$alias])) {
228+
throw new LogicException(\sprintf('The "%s" alias has already been defined with the #[AsAlias] attribute in "%s".', $alias, $this->aliases[$alias]));
225229
}
230+
$this->aliases[$alias] = new Alias($class, $public);
226231
}
227232
}
228233

@@ -304,7 +309,7 @@ protected function setDefinition(string $id, Definition $definition): void
304309
}
305310
}
306311

307-
private function findClasses(string $namespace, string $pattern, array $excludePatterns, ?RegisterAutoconfigureAttributesPass $autoconfigureAttributes, ?string $source): array
312+
private function findClasses(string $namespace, string $pattern, array $excludePatterns, ?string $source): array
308313
{
309314
$parameterBag = $this->container->getParameterBag();
310315

@@ -356,13 +361,9 @@ private function findClasses(string $namespace, string $pattern, array $excludeP
356361
throw new InvalidArgumentException(\sprintf('Expected to find class "%s" in file "%s" while importing services from resource "%s", but it was not found! Check the namespace prefix used with the resource.', $class, $path, $pattern));
357362
}
358363

359-
if ($r->isInstantiable() || $r->isInterface()) {
364+
if (!$r->isTrait()) {
360365
$classes[$class] = null;
361366
}
362-
363-
if ($autoconfigureAttributes && !$r->isInstantiable()) {
364-
$autoconfigureAttributes->processClass($this->container, $r);
365-
}
366367
}
367368

368369
// track only for new & removed files

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public function testPrivateConstructorThrowsAutowireException()
174174
$pass->process($container);
175175
$this->fail('AutowirePass should have thrown an exception');
176176
} catch (AutowiringFailedException $e) {
177-
$this->assertSame('Invalid service "private_service": constructor of class "Symfony\Component\DependencyInjection\Tests\Compiler\PrivateConstructor" must be public.', (string) $e->getMessage());
177+
$this->assertSame('Invalid service "private_service": constructor of class "Symfony\Component\DependencyInjection\Tests\Compiler\PrivateConstructor" must be public. Did you miss configuring a factory or a static constructor? Try using the "#[Autoconfigure(constructor: ...)]" attribute for the latter.', (string) $e->getMessage());
178178
}
179179
}
180180

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/StaticConstructor/PrototypeStaticConstructor.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
class PrototypeStaticConstructor implements PrototypeStaticConstructorInterface
66
{
7+
private function __construct()
8+
{
9+
}
10+
711
public static function create(): static
812
{
913
return new self();

‎src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub;
3434
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub\DeeperBaz;
3535
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\Baz;
36+
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\StaticConstructor\PrototypeStaticConstructor;
3637
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar;
3738
use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\BarInterface;
3839
use Symfony\Component\DependencyInjection\Tests\Fixtures\PrototypeAsAlias\AliasBarInterface;
@@ -380,11 +381,11 @@ public static function provideResourcesWithAsAliasAttributes(): iterable
380381
*/
381382
public function testRegisterClassesWithDuplicatedAsAlias(string $resource, string $expectedExceptionMessage)
382383
{
383-
$this->expectException(LogicException::class);
384-
$this->expectExceptionMessage($expectedExceptionMessage);
385-
386384
$container = new ContainerBuilder();
387385
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
386+
387+
$this->expectException(LogicException::class);
388+
$this->expectExceptionMessage($expectedExceptionMessage);
388389
$loader->registerClasses(
389390
(new Definition())->setAutoconfigured(true),
390391
'Symfony\Component\DependencyInjection\Tests\Fixtures\PrototypeAsAlias\\',
@@ -400,17 +401,28 @@ public static function provideResourcesWithDuplicatedAsAliasAttributes(): iterab
400401

401402
public function testRegisterClassesWithAsAliasAndImplementingMultipleInterfaces()
402403
{
403-
$this->expectException(LogicException::class);
404-
$this->expectExceptionMessage('Alias cannot be automatically determined for class "Symfony\Component\DependencyInjection\Tests\Fixtures\PrototypeAsAlias\WithAsAliasMultipleInterface". If you have used the #[AsAlias] attribute with a class implementing multiple interfaces, add the interface you want to alias to the first parameter of #[AsAlias].');
405-
406404
$container = new ContainerBuilder();
407405
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
406+
407+
$this->expectException(LogicException::class);
408+
$this->expectExceptionMessage('Alias cannot be automatically determined for class "Symfony\Component\DependencyInjection\Tests\Fixtures\PrototypeAsAlias\WithAsAliasMultipleInterface". If you have used the #[AsAlias] attribute with a class implementing multiple interfaces, add the interface you want to alias to the first parameter of #[AsAlias].');
408409
$loader->registerClasses(
409410
(new Definition())->setAutoconfigured(true),
410411
'Symfony\Component\DependencyInjection\Tests\Fixtures\PrototypeAsAlias\\',
411412
'PrototypeAsAlias/{WithAsAliasMultipleInterface,AliasBarInterface,AliasFooInterface}.php'
412413
);
413414
}
415+
416+
public function testRegisterClassesWithStaticConstructor()
417+
{
418+
$container = new ContainerBuilder();
419+
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
420+
421+
$prototype = (new Definition())->setAutoconfigured(true);
422+
$loader->registerClasses($prototype, 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\StaticConstructor\\', 'Prototype/StaticConstructor');
423+
424+
$this->assertTrue($container->has(PrototypeStaticConstructor::class));
425+
}
414426
}
415427

416428
class TestFileLoader extends FileLoader

0 commit comments

Comments
 (0)
0