Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b9cb37a

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[DependencyInjection] Don't skip classes with private constructor when autodiscovering
1 parent bd9b43e commit b9cb37a
Copy full SHA for b9cb37a

File tree

6 files changed

+63
-45
lines changed
Filter options

6 files changed

+63
-45
lines changed

‎CHANGELOG.md

Copy file name to clipboardExpand all lines: CHANGELOG.md
+1Lines 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
---

‎Compiler/AbstractRecursivePass.php

Copy file name to clipboardExpand all lines: Compiler/AbstractRecursivePass.php
+1-1Lines 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;

‎Loader/FileLoader.php

Copy file name to clipboardExpand all lines: Loader/FileLoader.php
+38-37Lines 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

‎Tests/Compiler/AutowirePassTest.php

Copy file name to clipboardExpand all lines: Tests/Compiler/AutowirePassTest.php
+1-1Lines 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

‎Tests/Fixtures/Prototype/StaticConstructor/PrototypeStaticConstructor.php

Copy file name to clipboardExpand all lines: Tests/Fixtures/Prototype/StaticConstructor/PrototypeStaticConstructor.php
+4Lines 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();

‎Tests/Loader/FileLoaderTest.php

Copy file name to clipboardExpand all lines: Tests/Loader/FileLoaderTest.php
+18-6Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.