From 2311437c9f20d6bd1eb57b8bd221d129fb6299cb Mon Sep 17 00:00:00 2001 From: Roland Franssen Date: Mon, 2 Jul 2018 20:21:13 +0200 Subject: [PATCH] [DI] Deprecate non-string default envs --- UPGRADE-4.3.md | 17 ++++++++++ UPGRADE-5.0.md | 17 ++++++++++ .../DependencyInjection/CHANGELOG.md | 1 + .../EnvPlaceholderParameterBag.php | 13 ++++++-- .../ValidateEnvPlaceholdersPassTest.php | 28 +++++++++++++--- .../EnvPlaceholderParameterBagTest.php | 32 +++++++++++++++++++ 6 files changed, 102 insertions(+), 6 deletions(-) diff --git a/UPGRADE-4.3.md b/UPGRADE-4.3.md index 5bd139fc9311..e60f75789dea 100644 --- a/UPGRADE-4.3.md +++ b/UPGRADE-4.3.md @@ -21,6 +21,23 @@ Config * Deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` +DependencyInjection +------------------- + + * Deprecated support for non-string default env() parameters + + Before: + ```yaml + parameters: + env(NAME): 1.5 + ``` + + After: + ```yaml + parameters: + env(NAME): '1.5' + ``` + EventDispatcher --------------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index f886a94e68fa..ec6f60951ec6 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -72,6 +72,23 @@ EventDispatcher * The `TraceableEventDispatcherInterface` has been removed. * The signature of the `EventDispatcherInterface::dispatch()` method has been updated to `dispatch($event, string $eventName = null)` +DependencyInjection +------------------- + + * Removed support for non-string default env() parameters + + Before: + ```yaml + parameters: + env(NAME): 1.5 + ``` + + After: + ```yaml + parameters: + env(NAME): '1.5' + ``` + Filesystem ---------- diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index 9935b8a58052..edc6ddf11577 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -13,6 +13,7 @@ CHANGELOG * added `ReverseContainer`: a container that turns services back to their ids * added ability to define an index for a tagged collection * added ability to define an index for services in an injected service locator argument + * deprecated support for non-string default env() parameters 4.2.0 ----- diff --git a/src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php b/src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php index 1a1579338063..200351bf4255 100644 --- a/src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php +++ b/src/Symfony/Component/DependencyInjection/ParameterBag/EnvPlaceholderParameterBag.php @@ -49,8 +49,11 @@ public function get($name) if ($this->has($name)) { $defaultValue = parent::get($name); - if (null !== $defaultValue && !is_scalar($defaultValue)) { + if (null !== $defaultValue && !is_scalar($defaultValue)) { // !is_string in 5.0 + //throw new RuntimeException(sprintf('The default value of an env() parameter must be a string or null, but "%s" given to "%s".', \gettype($defaultValue), $name)); throw new RuntimeException(sprintf('The default value of an env() parameter must be scalar or null, but "%s" given to "%s".', \gettype($defaultValue), $name)); + } elseif (is_scalar($defaultValue) && !\is_string($defaultValue)) { + @trigger_error(sprintf('A non-string default value of an env() parameter is deprecated since 4.3, cast "%s" to string instead.', $name), E_USER_DEPRECATED); } } @@ -147,9 +150,15 @@ public function resolve() continue; } if (is_numeric($default = $this->parameters[$name])) { + if (!\is_string($default)) { + @trigger_error(sprintf('A non-string default value of env parameter "%s" is deprecated since 4.3, cast it to string instead.', $env), E_USER_DEPRECATED); + } $this->parameters[$name] = (string) $default; - } elseif (null !== $default && !is_scalar($default)) { + } elseif (null !== $default && !is_scalar($default)) { // !is_string in 5.0 + //throw new RuntimeException(sprintf('The default value of env parameter "%s" must be a string or null, %s given.', $env, \gettype($default))); throw new RuntimeException(sprintf('The default value of env parameter "%s" must be scalar or null, %s given.', $env, \gettype($default))); + } elseif (is_scalar($default) && !\is_string($default)) { + @trigger_error(sprintf('A non-string default value of env parameter "%s" is deprecated since 4.3, cast it to string instead.', $env), E_USER_DEPRECATED); } } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php index 22950f34550a..cd1a47a1076b 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php @@ -27,13 +27,14 @@ public function testEnvsAreValidatedInConfig() { $container = new ContainerBuilder(); $container->setParameter('env(NULLED)', null); - $container->setParameter('env(FLOATISH)', 3.2); + $container->setParameter('env(FLOATISH)', '3.2'); $container->registerExtension($ext = new EnvExtension()); $container->prependExtensionConfig('env_extension', $expected = [ 'scalar_node' => '%env(NULLED)%', 'scalar_node_not_empty' => '%env(FLOATISH)%', 'int_node' => '%env(int:FOO)%', 'float_node' => '%env(float:BAR)%', + 'string_node' => '%env(UNDEFINED)%', ]); $this->doProcess($container); @@ -41,6 +42,26 @@ public function testEnvsAreValidatedInConfig() $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig())); } + /** + * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException + * @expectedExceptionMessage Invalid configuration for path "env_extension.string_node": "fail" is not a valid string + */ + public function testDefaultEnvIsValidatedInConfig() + { + $container = new ContainerBuilder(); + $container->setParameter('env(STRING)', 'fail'); + $container->registerExtension($ext = new EnvExtension()); + $container->prependExtensionConfig('env_extension', $expected = [ + 'string_node' => '%env(STRING)%', + ]); + + $this->doProcess($container); + } + + /** + * @group legacy + * @expectedDeprecation A non-string default value of an env() parameter is deprecated since 4.3, cast "env(FLOATISH)" to string instead. + */ public function testDefaultEnvWithoutPrefixIsValidatedInConfig() { $container = new ContainerBuilder(); @@ -48,7 +69,6 @@ public function testDefaultEnvWithoutPrefixIsValidatedInConfig() $container->registerExtension($ext = new EnvExtension()); $container->prependExtensionConfig('env_extension', $expected = [ 'float_node' => '%env(FLOATISH)%', - 'string_node' => '%env(UNDEFINED)%', ]); $this->doProcess($container); @@ -357,9 +377,9 @@ public function getConfigTreeBuilder() ->scalarNode('string_node') ->validate() ->ifTrue(function ($value) { - return !\is_string($value); + return !\is_string($value) || 'fail' === $value; }) - ->thenInvalid('%s is not a string') + ->thenInvalid('%s is not a valid string') ->end() ->end() ->end(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/ParameterBag/EnvPlaceholderParameterBagTest.php b/src/Symfony/Component/DependencyInjection/Tests/ParameterBag/EnvPlaceholderParameterBagTest.php index e7c88d2bb58e..b245823f670d 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ParameterBag/EnvPlaceholderParameterBagTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ParameterBag/EnvPlaceholderParameterBagTest.php @@ -111,6 +111,10 @@ public function testMergeWithDifferentIdentifiersForPlaceholders() $this->assertCount(2, $merged[$envName]); } + /** + * @group legacy + * @expectedDeprecation A non-string default value of env parameter "INT_VAR" is deprecated since 4.3, cast it to string instead. + */ public function testResolveEnvCastsIntToString() { $bag = new EnvPlaceholderParameterBag(); @@ -120,6 +124,34 @@ public function testResolveEnvCastsIntToString() $this->assertSame('2', $bag->all()['env(INT_VAR)']); } + /** + * @group legacy + * @expectedDeprecation A non-string default value of an env() parameter is deprecated since 4.3, cast "env(INT_VAR)" to string instead. + * @expectedDeprecation A non-string default value of env parameter "INT_VAR" is deprecated since 4.3, cast it to string instead. + */ + public function testGetDefaultScalarEnv() + { + $bag = new EnvPlaceholderParameterBag(); + $bag->set('env(INT_VAR)', 2); + $this->assertStringMatchesFormat('env_%s_INT_VAR_%s', $bag->get('env(INT_VAR)')); + $this->assertSame(2, $bag->all()['env(INT_VAR)']); + $bag->resolve(); + $this->assertStringMatchesFormat('env_%s_INT_VAR_%s', $bag->get('env(INT_VAR)')); + $this->assertSame('2', $bag->all()['env(INT_VAR)']); + } + + public function testGetDefaultEnv() + { + $bag = new EnvPlaceholderParameterBag(); + $this->assertStringMatchesFormat('env_%s_INT_VAR_%s', $bag->get('env(INT_VAR)')); + $bag->set('env(INT_VAR)', '2'); + $this->assertStringMatchesFormat('env_%s_INT_VAR_%s', $bag->get('env(INT_VAR)')); + $this->assertSame('2', $bag->all()['env(INT_VAR)']); + $bag->resolve(); + $this->assertStringMatchesFormat('env_%s_INT_VAR_%s', $bag->get('env(INT_VAR)')); + $this->assertSame('2', $bag->all()['env(INT_VAR)']); + } + public function testResolveEnvAllowsNull() { $bag = new EnvPlaceholderParameterBag();