feature #27808 [DI] Deprecate non-string default envs (ro0NL)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[DI] Deprecate non-string default envs

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes-ish
| New feature?  | yes
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes
| Tests pass?   | no    <!-- please add some, will be required by reviewers -->
| Fixed tickets | #27680, #27470 (comment)
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

This is a failing test to further clarify the issues raised in #27680

So given #27680 (comment)

> We should be sure this solves a real-world issue.

I think it solves a real bug :)

Commits
-------

2311437 [DI] Deprecate non-string default envs

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
 ---------------
 

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
 ----------
 

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
 -----

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);
             }
         }
     }

src/Symfony/Component/DependencyInjection/Tests/Compiler/ValidateEnvPlaceholdersPassTest.php

@@ -27,28 +27,48 @@ 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);
 
         $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();
         $container->setParameter('env(FLOATISH)', 3.2);
         $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();

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();