8000 Fix comments, improve the feature · symfony/symfony@4b3e9d4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4b3e9d4

Browse files
GuilhemNnicolas-grekas
GuilhemN
authored and
nicolas-grekas
committed
Fix comments, improve the feature
1 parent a6292b9 commit 4b3e9d4

File tree

13 files changed

+384
-419
lines changed

13 files changed

+384
-419
lines changed

‎src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

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

7+
* Added `lint:container` to check that services wiring matches type declarations
78
* Added `MailerAssertionsTrait`
89
* Deprecated support for `templating` engine in `TemplateController`, use Twig instead
910
* Deprecated the `$parser` argument of `ControllerResolver::__construct()` and `DelegatingLoader::__construct()`

‎src/Symfony/Bundle/FrameworkBundle/Command/ContainerLintCommand.php

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,21 @@
1111

1212
namespace Symfony\Bundle\FrameworkBundle\Command;
1313

14+
use Symfony\Component\Config\ConfigCache;
15+
use Symfony\Component\Config\FileLocator;
1416
use Symfony\Component\Console\Command\Command;
1517
use Symfony\Component\Console\Input\InputInterface;
1618
use Symfony\Component\Console\Input\InputOption;
1719
use Symfony\Component\Console\Output\OutputInterface;
18-
use Symfony\Component\Config\ConfigCache;
19-
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
20-
use Symfony\Component\DependencyInjection\ContainerBuilder;
21-
use Symfony\Component\DependencyInjection\Compiler\CheckTypeHintsPass;
20+
use Symfony\Component\DependencyInjection\Compiler\CheckTypeDeclarationsPass;
2221
use Symfony\Component\DependencyInjection\Compiler\PassConfig;
23-
use Symfony\Component\Config\FileLocator;
22+
use Symfony\Component\DependencyInjection\ContainerBuilder;
23+
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
2424

25-
class ContainerLintCommand extends Command
25+
final class ContainerLintCommand extends Command
2626
{
27+
protected static $defaultName = 'lint:container';
28+
2729
/**
2830
* @var ContainerBuilder
2931
*/
@@ -35,37 +37,33 @@ class ContainerLintCommand extends Command
3537
protected function configure()
3638
{
3739
$this
38-
->setDescription('Lints container for services arguments type hints')
39-
->setHelp('This command will parse all your defined services and check that you are injecting service without type error based on type hints.')
40-
->addOption('only-used-services', 'o', InputOption::VALUE_NONE, 'Check only services that are used in your application')
40+
->setDescription('Ensures that arguments injected into services match type declarations')
41+
->setHelp('This command parses service definitions and ensures that injected values match the type declarations of each services\' class.')
42+
->addOption('ignore-unused-services', 'o', InputOption::VALUE_NONE, 'Ignore unused services')
4143
;
4244
}
4345

4446
/**
4547
* {@inheritdoc}
4648
*/
47-
protected function execute(InputInterface $input, OutputInterface $output)
49+
protected function execute(InputInterface $input, OutputInterface $output): int
4850
{
4951
$container = $this->getContainerBuilder();
5052

5153
$container->setParameter('container.build_id', 'lint_container');
5254

5355
$container->addCompilerPass(
54-
new CheckTypeHintsPass(),
55-
$input->getOption('only-used-services') ? PassConfig::TYPE_AFTER_REMOVING : PassConfig::TYPE_BEFORE_OPTIMIZATION
56+
new CheckTypeDeclarationsPass(true),
57+
$input->getOption('ignore-unused-services') ? PassConfig::TYPE_AFTER_REMOVING : PassConfig::TYPE_OPTIMIZE,
58+
-5
5659
);
5760

5861
$container->compile();
62+
63+
return 0;
5964
}
6065

61-
/**
62-
* Loads the ContainerBuilder from the cache.
63-
*
64-
* @return ContainerBuilder
65-
*
66-
* @throws \LogicException
67-
*/
68-
protected function getContainerBuilder()
66+
private function getContainerBuilder(): ContainerBuilder
6967
{
7068
if ($this->containerBuilder) {
7169
return $this->containerBuilder;
@@ -74,10 +72,9 @@ protected function getContainerBuilder()
7472
$kernel = $this->getApplication()->getKernel();
7573

7674
if (!$kernel->isDebug() || !(new ConfigCache($kernel->getContainer()->getParameter('debug.container.dump'), true))->isFresh()) {
77-
$buildContainer = \Closure::bind(function () { return $this->buildContainer(); }, $kernel, get_class($kernel));
75+
$buildContainer = \Closure::bind(function () { return $this->buildContainer(); }, $kernel, \get_class($kernel));
7876
$container = $buildContainer();
79-
$container->getCompilerPassConfig()->setRemovingPasses(array());
80-
$container->compile();
77+
$container->getCompilerPassConfig()->setRemovingPasses([]);
8178
} else {
8279
(new XmlFileLoader($container = new ContainerBuilder(), new FileLocator()))->load($kernel->getContainer()->getParameter('debug.container.dump'));
8380
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CHANGELOG
44
4.4.0
55
-----
66

7-
* added `CheckTypeHintsPass` to check injected parameters type during compilation
7+
* added `CheckTypeDeclarationsPass` to check injected parameters type during compilation
88
* added support for opcache.preload by generating a preloading script in the cache folder
99
* added support for dumping the container in one file instead of many files
1010
* deprecated support for short factories and short configurators in Yaml

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

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\DependencyInjection\Compiler;
13+
14+
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
15+
use Symfony\Component\DependencyInjection\Definition;
16+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
17+
use Symfony\Component\DependencyInjection\Exception\InvalidParameterTypeException;
18+
use Symfony\Component\DependencyInjection\Parameter;
19+
use Symfony\Component\DependencyInjection\Reference;
20+
use Symfony\Component\DependencyInjection\ServiceLocator;
21+
22+
/**
23+
* Checks whether injected parameters are compatible with type declarations.
24+
*
25+
* This pass should be run after all optimization passes.
26+
*
27+
* It can be added either:
28+
* * before removing passes to check all services even if they are not currently used,
29+
* * after removing passes to check only services are used in the app.
30+
*
31+
* @author Nicolas Grekas <p@tchwork.com>
32+
* @author Julien Maulny <jmaulny@darkmira.fr>
33+
*/
34+
final class CheckTypeDeclarationsPass extends AbstractRecursivePass
35+
{
36+
private const SCALAR_TYPES = ['int', 'float', 'bool', 'string'];
37+
38+
private $autoload;
39+
40+
/**
41+
* @param bool $autoload Whether services who's class in not loaded should be checked or not.
42+
* Defaults to false to save loading code during compilation.
43+
*/
44+
public function __construct(bool $autoload = false)
45+
{
46+
$this->autoload = $autoload;
47+
}
48+
49+
/**
50+
* {@inheritdoc}
51+
*/
52+
protected function processValue($value, $isRoot = false)
53+
{
54+
if (!$value instanceof Definition) {
55+
return parent::processValue($value, $isRoot);
56+
}
57+
58+
if (!$this->autoload && !class_exists($class = $value->getClass(), false) && !interface_exists($class, false)) {
59+
return parent::processValue($value, $isRoot);
60+
}
61+
62+
if (ServiceLocator::class === $value->getClass()) {
63+
return parent::processValue($value, $isRoot);
64+
}
65+
66+
if ($constructor = $this->getConstructor($value, false)) {
67+
$this->checkTypeDeclarations($value, $constructor, $value->getArguments());
68+
}
69+
70+
foreach ($value->getMethodCalls() as $methodCall) {
71+
$reflectionMethod = $this->getReflectionMethod($value, $methodCall[0]);
72+
73+
$this->checkTypeDeclarations($value, $reflectionMethod, $methodCall[1]);
74+
}
75+
76+
return parent::processValue($value, $isRoot);
77+
}
78+
79+
/**
80+
* @throws InvalidArgumentException When not enough parameters are defined for the method
81+
*/
82+
private function checkTypeDeclarations(Definition $checkedDefinition, \ReflectionFunctionAbstract $reflectionFunction, array $configurationArguments): void
83+
{
84+
$numberOfRequiredParameters = $reflectionFunction->getNumberOfRequiredParameters();
85+
86+
if (\count($configurationArguments) < $numberOfRequiredParameters) {
87+
throw new InvalidArgumentException(sprintf('Invalid definition for service "%s": "%s::%s()" requires %d arguments, %d passed.', $this->currentId, $reflectionFunction->class, $reflectionFunction->name, $numberOfRequiredParameters, \count($configurationArguments)));
88+
}
89+
90+
$reflectionParameters = $reflectionFunction->getParameters();
91+
$checksCount = min($reflectionFunction->getNumberOfParameters(), \count($configurationArguments));
92+
93+
for ($i = 0; $i < $checksCount; ++$i) {
94+
if (!$reflectionParameters[$i]->hasType() || $reflectionParameters[$i]->isVariadic()) {
95+
continue;
96+
}
97+
98+
$this->checkType($checkedDefinition, $configurationArguments[$i], $reflectionParameters[$i]);
99+
}
100+
101+
if ($reflectionFunction->isVariadic() && ($lastParameter = end($reflectionParameters))->hasType()) {
102+
$variadicParameters = \array_slice($configurationArguments, $lastParameter->getPosition());
103+
104+
foreach ($variadicParameters as $variadicParameter) {
105+
$this->checkType($checkedDefinition, $variadicParameter, $lastParameter);
106+
}
107+
}
108+
}
109+
110+
/**
111+
* @throws InvalidParameterTypeException When a parameter is not compatible with the declared type
112+
*/
113+
private function checkType(Definition $checkedDefinition, $configurationArgument, \ReflectionParameter $parameter): void
114+
{
115+
$parameterTypeName = $parameter->getType()->getName();
116+
117+
$referencedDefinition = $configurationArgument;
118+
119+
if ($referencedDefinition instanceof Reference) {
120+
if (!$this->container->has($referencedDefinition)) {
121+
return;
122+
}
123+
124+
$referencedDefinition = $this->container->findDefinition((string) $referencedDefinition);
125+
}
126+
127+
if ('self' === $parameterTypeName) {
128+
$parameterTypeName = $parameter->getDeclaringClass()->getName();
129+
}
130+
if ('static' === $parameterTypeName) {
131+
$parameterTypeName = $checkedDefinition->getClass();
132+
}
133+
134+
if ($referencedDefinition instanceof Definition) {
135+
$class = $referencedDefinition->getClass();
136+
137+
if (!$class || (!$this->autoload && !class_exists($class, false) && !interface_exists($class, false))) {
138+
return;
139+
}
140+
141+
if (!is_a($class, $parameterTypeName, true)) {
142+
throw new InvalidParameterTypeException($this->currentId, $class, $parameter);
143+
}
144+
} else {
145+
if (null === $configurationArgument && $parameter->allowsNull()) {
146+
return;
147+
}
148+
149+
if (\in_array($parameterTypeName, self::SCALAR_TYPES, true) && is_scalar($configurationArgument)) {
150+
return;
151+
}
152+
153+
if ('iterable' === $parameterTypeName && $configurationArgument instanceof IteratorArgument) {
154+
return;
155+
}
156+
157+
if ('Traversable' === $parameterTypeName && $configurationArgument instanceof IteratorArgument) {
158+
return;
159+
}
160+
161+
if ($configurationArgument instanceof Parameter) {
162+
return;
163+
}
164+
165+
$checkFunction = sprintf('is_%s', $parameter->getType()->getName());
166+
167+
if (!$parameter->getType()->isBuiltin() || !$checkFunction($configurationArgument)) {
168+
throw new InvalidParameterTypeException($this->currentId, \gettype($configurationArgument), $parameter);
169+
}
170+
}
171+
}
172+
}

0 commit comments

Comments
 (0)
0