From d30c75178144edc03ed135da7112456053b136d1 Mon Sep 17 00:00:00 2001 From: "Konstantin.Myakshin" Date: Sun, 23 Jul 2017 12:48:31 +0300 Subject: [PATCH 1/4] Add some phpdocs for IDE autocompletion and better SCA --- .../Form/Extension/Core/EventListener/ResizeFormListener.php | 2 ++ src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php | 1 + .../Form/Extension/Validator/Constraints/FormValidator.php | 1 + 3 files changed, 4 insertions(+) diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php index 17d60a3d30..4b339181e9 100644 --- a/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/ResizeFormListener.php @@ -15,6 +15,7 @@ use Symfony\Component\Form\FormEvents; use Symfony\Component\Form\FormEvent; use Symfony\Component\Form\Exception\UnexpectedTypeException; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\Form\FormInterface; /** * Resize a collection form element based on the data sent from the client. @@ -146,6 +147,7 @@ class ResizeFormListener implements EventSubscriberInterface if ($this->deleteEmpty) { $previousData = $event->getForm()->getData(); + /** @var FormInterface $child */ foreach ($form as $name => $child) { $isNew = !isset($previousData[$name]); diff --git a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php index a45cb3a278..6c0c0b4295 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php @@ -120,6 +120,7 @@ class ChoiceType extends AbstractType // Reconstruct the data as mapping from child names to values $data = array(); + /** @var FormInterface $child */ foreach ($form as $child) { $value = $child->getConfig()->getOption('value'); diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php index 5b989f1a60..b5a06156c0 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php +++ b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php @@ -83,6 +83,7 @@ class FormValidator extends ConstraintValidator } else { $childrenSynchronized = true; + /** @var FormInterface $child */ foreach ($form as $child) { if (!$child->isSynchronized()) { $childrenSynchronized = false; From 636777dc611b28f81970c56afce3f8dfdaea5296 Mon Sep 17 00:00:00 2001 From: Christian Schmidt Date: Wed, 26 Jul 2017 16:58:46 +0200 Subject: [PATCH 2/4] [Debug] HTML-escape array key --- src/Symfony/Component/Debug/ExceptionHandler.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Debug/ExceptionHandler.php b/src/Symfony/Component/Debug/ExceptionHandler.php index 0c43ba6a05..e3a58393b8 100644 --- a/src/Symfony/Component/Debug/ExceptionHandler.php +++ b/src/Symfony/Component/Debug/ExceptionHandler.php @@ -94,7 +94,7 @@ class ExceptionHandler * * @param string $format The format for links to source files * - * @return string The previous file link format. + * @return string The previous file link format */ public function setFileLinkFormat($format) { @@ -418,7 +418,7 @@ EOF; $formattedValue = str_replace("\n", '', var_export($this->escapeHtml((string) $item[1]), true)); } - $result[] = is_int($key) ? $formattedValue : sprintf("'%s' => %s", $key, $formattedValue); + $result[] = is_int($key) ? $formattedValue : sprintf("'%s' => %s", $this->escapeHtml($key), $formattedValue); } return implode(', ', $result); From 31843d6f9830835fe9110d9bb4f3234e9056ee5d Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Thu, 27 Jul 2017 10:00:45 +0200 Subject: [PATCH 3/4] [ProxyManager] Cleanup fixtures --- .../Tests/LazyProxy/Dumper/PhpDumperTest.php | 68 +++---- .../LazyProxy/Fixtures/php/lazy_service.php | 185 ------------------ .../Fixtures/php/lazy_service_structure.txt | 2 +- .../Fixtures/php/lazy_service_with_hints.php | 180 ----------------- 4 files changed, 35 insertions(+), 400 deletions(-) delete mode 100644 src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service.php delete mode 100644 src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_with_hints.php diff --git a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Dumper/PhpDumperTest.php b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Dumper/PhpDumperTest.php index d6f3d0d2e8..62cc3cd38d 100644 --- a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Dumper/PhpDumperTest.php +++ b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Dumper/PhpDumperTest.php @@ -25,6 +25,39 @@ use Symfony\Component\DependencyInjection\Dumper\PhpDumper; class PhpDumperTest extends TestCase { public function testDumpContainerWithProxyService() + { + $this->assertStringMatchesFormatFile( + __DIR__.'/../Fixtures/php/lazy_service_structure.txt', + $this->dumpLazyServiceProjectServiceContainer(), + '->dump() does generate proxy lazy loading logic.' + ); + } + + /** + * Verifies that the generated container retrieves the same proxy instance on multiple subsequent requests. + */ + public function testDumpContainerWithProxyServiceWillShareProxies() + { + if (!class_exists('LazyServiceProjectServiceContainer', false)) { + eval('?>'.$this->dumpLazyServiceProjectServiceContainer()); + } + + $container = new \LazyServiceProjectServiceContainer(); + + $proxy = $container->get('foo'); + $this->assertInstanceOf('stdClass', $proxy); + $this->assertInstanceOf('ProxyManager\Proxy\LazyLoadingInterface', $proxy); + $this->assertSame($proxy, $container->get('foo')); + + $this->assertFalse($proxy->isProxyInitialized()); + + $proxy->initializeProxy(); + + $this->assertTrue($proxy->isProxyInitialized()); + $this->assertSame($proxy, $container->get('foo')); + } + + private function dumpLazyServiceProjectServiceContainer() { $container = new ContainerBuilder(); @@ -36,39 +69,6 @@ class PhpDumperTest extends TestCase $dumper->setProxyDumper(new ProxyDumper()); - $dumpedString = $dumper->dump(); - - $this->assertStringMatchesFormatFile( - __DIR__.'/../Fixtures/php/lazy_service_structure.txt', - $dumpedString, - '->dump() does generate proxy lazy loading logic.' - ); - } - - /** - * Verifies that the generated container retrieves the same proxy instance on multiple subsequent requests. - */ - public function testDumpContainerWithProxyServiceWillShareProxies() - { - // detecting ProxyManager v2 - if (class_exists('ProxyManager\ProxyGenerator\LazyLoading\MethodGenerator\StaticProxyConstructor')) { - require_once __DIR__.'/../Fixtures/php/lazy_service_with_hints.php'; - } else { - require_once __DIR__.'/../Fixtures/php/lazy_service.php'; - } - - $container = new \LazyServiceProjectServiceContainer(); - - /* @var $proxy \stdClass_c1d194250ee2e2b7d2eab8b8212368a8 */ - $proxy = $container->get('foo'); - $this->assertInstanceOf('stdClass_c1d194250ee2e2b7d2eab8b8212368a8', $proxy); - $this->assertSame($proxy, $container->get('foo')); - - $this->assertFalse($proxy->isProxyInitialized()); - - $proxy->initializeProxy(); - - $this->assertTrue($proxy->isProxyInitialized()); - $this->assertSame($proxy, $container->get('foo')); + return $dumper->dump(array('class' => 'LazyServiceProjectServiceContainer')); } } diff --git a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service.php b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service.php deleted file mode 100644 index 8971f655f4..0000000000 --- a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service.php +++ /dev/null @@ -1,185 +0,0 @@ -services = - $this->scopedServices = - $this->scopeStacks = array(); - $this->scopes = array(); - $this->scopeChildren = array(); - } - - /** - * Gets the 'foo' service. - * - * This service is shared. - * This method always returns the same instance of the service. - * - * @param bool $lazyLoad whether to try lazy-loading the service with a proxy - * - * @return stdClass A stdClass instance - */ - public function getFooService($lazyLoad = true) - { - if ($lazyLoad) { - $container = $this; - - return $this->services['foo'] = new stdClass_c1d194250ee2e2b7d2eab8b8212368a8( - function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) use ($container) { - $wrappedInstance = $container->getFooService(false); - - $proxy->setProxyInitializer(null); - - return true; - } - ); - } - - return new \stdClass(); - } -} - -class stdClass_c1d194250ee2e2b7d2eab8b8212368a8 extends \stdClass implements \ProxyManager\Proxy\LazyLoadingInterface, \ProxyManager\Proxy\ValueHolderInterface -{ - /** - * @var \Closure|null initializer responsible for generating the wrapped object - */ - private $valueHolder5157dd96e88c0 = null; - - /** - * @var \Closure|null initializer responsible for generating the wrapped object - */ - private $initializer5157dd96e8924 = null; - - /** - * @override constructor for lazy initialization - * - * @param \Closure|null $initializer - */ - public function __construct($initializer) - { - $this->initializer5157dd96e8924 = $initializer; - } - - /** - * @param string $name - */ - public function __get($name) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__get', array('name' => $name)); - - return $this->valueHolder5157dd96e88c0->$name; - } - - /** - * @param string $name - * @param mixed $value - */ - public function __set($name, $value) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__set', array('name' => $name, 'value' => $value)); - - $this->valueHolder5157dd96e88c0->$name = $value; - } - - /** - * @param string $name - * - * @return bool - */ - public function __isset($name) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__isset', array('name' => $name)); - - return isset($this->valueHolder5157dd96e88c0->$name); - } - - /** - * @param string $name - */ - public function __unset($name) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__unset', array('name' => $name)); - - unset($this->valueHolder5157dd96e88c0->$name); - } - - public function __clone() - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__clone', array()); - - $this->valueHolder5157dd96e88c0 = clone $this->valueHolder5157dd96e88c0; - } - - public function __sleep() - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__sleep', array()); - - return array('valueHolder5157dd96e88c0'); - } - - public function __wakeup() - { - } - - /** - * {@inheritdoc} - */ - public function setProxyInitializer(\Closure $initializer = null) - { - $this->initializer5157dd96e8924 = $initializer; - } - - /** - * {@inheritdoc} - */ - public function getProxyInitializer() - { - return $this->initializer5157dd96e8924; - } - - /** - * {@inheritdoc} - */ - public function initializeProxy() - { - return $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, 'initializeProxy', array()); - } - - /** - * {@inheritdoc} - */ - public function isProxyInitialized() - { - return null !== $this->valueHolder5157dd96e88c0; - } - - /** - * {@inheritdoc} - */ - public function getWrappedValueHolderValue() - { - return $this->valueHolder5157dd96e88c0; - } -} diff --git a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_structure.txt b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_structure.txt index a0e3642f49..ad9a3fe158 100644 --- a/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_structure.txt +++ b/src/Symfony/Bridge/ProxyManager/Tests/LazyProxy/Fixtures/php/lazy_service_structure.txt @@ -1,7 +1,7 @@ services = array(); - } - - /** - * Gets the 'foo' service. - * - * This service is shared. - * This method always returns the same instance of the service. - * - * @param bool $lazyLoad whether to try lazy-loading the service with a proxy - * - * @return stdClass A stdClass instance - */ - public function getFooService($lazyLoad = true) - { - if ($lazyLoad) { - $container = $this; - - return $this->services['foo'] = new stdClass_c1d194250ee2e2b7d2eab8b8212368a8( - function (&$wrappedInstance, \ProxyManager\Proxy\LazyLoadingInterface $proxy) use ($container) { - $wrappedInstance = $this->getFooService(false); - - $proxy->setProxyInitializer(null); - - return true; - } - ); - } - - return new \stdClass(); - } -} - -class stdClass_c1d194250ee2e2b7d2eab8b8212368a8 extends \stdClass implements \ProxyManager\Proxy\LazyLoadingInterface, \ProxyManager\Proxy\ValueHolderInterface -{ - /** - * @var \Closure|null initializer responsible for generating the wrapped object - */ - private $valueHolder5157dd96e88c0 = null; - - /** - * @var \Closure|null initializer responsible for generating the wrapped object - */ - private $initializer5157dd96e8924 = null; - - /** - * @override constructor for lazy initialization - * - * @param \Closure|null $initializer - */ - public function __construct($initializer) - { - $this->initializer5157dd96e8924 = $initializer; - } - - /** - * @param string $name - */ - public function __get($name) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__get', array('name' => $name)); - - return $this->valueHolder5157dd96e88c0->$name; - } - - /** - * @param string $name - * @param mixed $value - */ - public function __set($name, $value) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__set', array('name' => $name, 'value' => $value)); - - $this->valueHolder5157dd96e88c0->$name = $value; - } - - /** - * @param string $name - * - * @return bool - */ - public function __isset($name) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__isset', array('name' => $name)); - - return isset($this->valueHolder5157dd96e88c0->$name); - } - - /** - * @param string $name - */ - public function __unset($name) - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__unset', array('name' => $name)); - - unset($this->valueHolder5157dd96e88c0->$name); - } - - public function __clone() - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__clone', array()); - - $this->valueHolder5157dd96e88c0 = clone $this->valueHolder5157dd96e88c0; - } - - public function __sleep() - { - $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, '__sleep', array()); - - return array('valueHolder5157dd96e88c0'); - } - - public function __wakeup() - { - } - - /** - * {@inheritdoc} - */ - public function setProxyInitializer(\Closure $initializer = null) - { - $this->initializer5157dd96e8924 = $initializer; - } - - /** - * {@inheritdoc} - */ - public function getProxyInitializer() - { - return $this->initializer5157dd96e8924; - } - - /** - * {@inheritdoc} - */ - public function initializeProxy(): bool - { - return $this->initializer5157dd96e8924 && $this->initializer5157dd96e8924->__invoke($this->valueHolder5157dd96e88c0, $this, 'initializeProxy', array()); - } - - /** - * {@inheritdoc} - */ - public function isProxyInitialized(): bool - { - return null !== $this->valueHolder5157dd96e88c0; - } - - /** - * {@inheritdoc} - */ - public function getWrappedValueHolderValue() - { - return $this->valueHolder5157dd96e88c0; - } -} From f1aa45c5174433699604fe090ad3d8ecea4dcc62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Fri, 21 Jul 2017 12:04:43 +0200 Subject: [PATCH 4/4] [DI] Remove unused props from the PhpDumper --- .../DependencyInjection/Dumper/PhpDumper.php | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index 721c4dc68c..4d576df1bd 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -255,12 +255,11 @@ class PhpDumper extends Dumper /** * Generates the require_once statement for service includes. * - * @param string $id The service id * @param Definition $definition * * @return string */ - private function addServiceInclude($id, $definition) + private function addServiceInclude($definition) { $template = " require_once %s;\n"; $code = ''; @@ -335,9 +334,9 @@ class PhpDumper extends Dumper $code .= $this->addNewInstance($id, $sDefinition, '$'.$name, ' = '); if (!$this->hasReference($id, $sDefinition->getMethodCalls(), true) && !$this->hasReference($id, $sDefinition->getProperties(), true)) { - $code .= $this->addServiceProperties(null, $sDefinition, $name); - $code .= $this->addServiceMethodCalls(null, $sDefinition, $name); - $code .= $this->addServiceConfigurator(null, $sDefinition, $name); + $code .= $this->addServiceProperties($sDefinition, $name); + $code .= $this->addServiceMethodCalls($sDefinition, $name); + $code .= $this->addServiceConfigurator($sDefinition, $name); } $code .= "\n"; @@ -437,13 +436,12 @@ class PhpDumper extends Dumper /** * Adds method calls to a service definition. * - * @param string $id * @param Definition $definition * @param string $variableName * * @return string */ - private function addServiceMethodCalls($id, Definition $definition, $variableName = 'instance') + private function addServiceMethodCalls(Definition $definition, $variableName = 'instance') { $calls = ''; foreach ($definition->getMethodCalls() as $call) { @@ -458,7 +456,7 @@ class PhpDumper extends Dumper return $calls; } - private function addServiceProperties($id, Definition $definition, $variableName = 'instance') + private function addServiceProperties(Definition $definition, $variableName = 'instance') { $code = ''; foreach ($definition->getProperties() as $name => $value) { @@ -501,9 +499,9 @@ class PhpDumper extends Dumper } $name = (string) $this->definitionVariables->offsetGet($iDefinition); - $code .= $this->addServiceProperties(null, $iDefinition, $name); - $code .= $this->addServiceMethodCalls(null, $iDefinition, $name); - $code .= $this->addServiceConfigurator(null, $iDefinition, $name); + $code .= $this->addServiceProperties($iDefinition, $name); + $code .= $this->addServiceMethodCalls($iDefinition, $name); + $code .= $this->addServiceConfigurator($iDefinition, $name); } if ('' !== $code) { @@ -516,13 +514,12 @@ class PhpDumper extends Dumper /** * Adds configurator definition. * - * @param string $id * @param Definition $definition * @param string $variableName * * @return string */ - private function addServiceConfigurator($id, Definition $definition, $variableName = 'instance') + private function addServiceConfigurator(Definition $definition, $variableName = 'instance') { if (!$callable = $definition->getConfigurator()) { return ''; @@ -633,14 +630,14 @@ EOF; $code .= sprintf(" throw new RuntimeException('You have requested a synthetic service (\"%s\"). The DIC does not know how to construct this service.');\n }\n", $id); } else { $code .= - $this->addServiceInclude($id, $definition). + $this->addServiceInclude($definition). $this->addServiceLocalTempVariables($id, $definition). $this->addServiceInlinedDefinitions($id, $definition). $this->addServiceInstance($id, $definition). $this->addServiceInlinedDefinitionsSetup($id, $definition). - $this->addServiceProperties($id, $definition). - $this->addServiceMethodCalls($id, $definition). - $this->addServiceConfigurator($id, $definition). + $this->addServiceProperties($definition). + $this->addServiceMethodCalls($definition). + $this->addServiceConfigurator($definition). $this->addServiceReturn($id, $definition) ; }