feature #28858 [DI] Deprecated using env vars with cannotBeEmpty() (r…

…o0NL)

This PR was squashed before being merged into the 4.3-dev branch (closes #28858).

Discussion
----------

[DI] Deprecated using env vars with cannotBeEmpty()

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

Continuation of #28838 for 4.2

Using environment variables for nodes marked `cannotBeEmpty()` is semantically not possible, we'll never know the value is empty yes/no during compile time. Neither we should assume one or another.

Commits
-------

397c19e [DI] Deprecated using env vars with cannotBeEmpty()

UPGRADE-4.2.md

@@ -29,6 +29,7 @@ Config
    ```
 
  * Deprecated `FileLoaderLoadException`, use `LoaderLoadException` instead.
+ * Deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()`
 
 Console
 -------

UPGRADE-5.0.md

@@ -18,6 +18,7 @@ Config
  * Added the `getChildNodeDefinitions()` method to `ParentNodeDefinitionInterface`.
  * The `Processor` class has been made final
  * Removed `FileLoaderLoadException`, use `LoaderLoadException` instead.
+ * Using environment variables with `cannotBeEmpty()` if the value is validated with `validate()` will throw an exception.
 
 Console
 -------

src/Symfony/Component/Config/CHANGELOG.md

@@ -6,6 +6,7 @@ CHANGELOG
 
  * deprecated constructing a `TreeBuilder` without passing root node information
  * renamed `FileLoaderLoadException` to `LoaderLoadException`
+ * deprecated using environment variables with `cannotBeEmpty()` if the value is validated with `validate()`
 
 4.1.0
 -----

src/Symfony/Component/Config/Definition/ScalarNode.php

@@ -48,6 +48,8 @@ protected function validateType($value)
      */
     protected function isValueEmpty($value)
     {
+        // assume environment variables are never empty (which in practice is likely to be true during runtime)
+        // not doing so breaks many configs that are valid today
         if ($this->isHandlingPlaceholder()) {
             return false;
         }

src/Symfony/Component/Config/Definition/VariableNode.php

@@ -81,6 +81,19 @@ protected function validateType($value)
      */
     protected function finalizeValue($value)
     {
+        // deny environment variables only when using custom validators
+        // this avoids ever passing an empty value to final validation closures
+        if (!$this->allowEmptyValue && $this->isHandlingPlaceholder() && $this->finalValidationClosures) {
+            @trigger_error(sprintf('Setting path "%s" to an environment variable is deprecated since Symfony 4.2. Remove "cannotBeEmpty()", "validate()" or include a prefix/suffix value instead.', $this->getPath()), E_USER_DEPRECATED);
+//            $e = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an environment variable when empty values are not allowed by definition and are validated.', $this->getPath(), json_encode($value)));
+//            if ($hint = $this->getInfo()) {
+//                $e->addHint($hint);
+//            }
+//            $e->setPath($this->getPath());
+//
+//            throw $e;
+        }
+
         if (!$this->allowEmptyValue && $this->isValueEmpty($value)) {
             $ex = new InvalidConfigurationException(sprintf('The path "%s" cannot contain an empty value, but got %s.', $this->getPath(), json_encode($value)));
             if ($hint = $this->getInfo()) {
@@ -120,6 +133,8 @@ protected function mergeValues($leftSide, $rightSide)
      * @param mixed $value
      *
      * @return bool
+     *
+     * @see finalizeValue()
      */
     protected function isValueEmpty($value)
     {

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

@@ -211,6 +211,38 @@ public function testEmptyEnvWhichCannotBeEmptyForScalarNode(): void
         $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
     }
 
+    /**
+     * NOT LEGACY (test exception in 5.0).
+     *
+     * @group legacy
+     * @expectedDeprecation Setting path "env_extension.scalar_node_not_empty_validated" to an environment variable is deprecated since Symfony 4.2. Remove "cannotBeEmpty()", "validate()" or include a prefix/suffix value instead.
+     */
+    public function testEmptyEnvWhichCannotBeEmptyForScalarNodeWithValidation(): void
+    {
+        $container = new ContainerBuilder();
+        $container->registerExtension($ext = new EnvExtension());
+        $container->prependExtensionConfig('env_extension', $expected = array(
+            'scalar_node_not_empty_validated' => '%env(SOME)%',
+        ));
+
+        $this->doProcess($container);
+
+        $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
+    }
+
+    public function testPartialEnvWhichCannotBeEmptyForScalarNode(): void
+    {
+        $container = new ContainerBuilder();
+        $container->registerExtension($ext = new EnvExtension());
+        $container->prependExtensionConfig('env_extension', $expected = array(
+            'scalar_node_not_empty_validated' => 'foo %env(SOME)% bar',
+        ));
+
+        $this->doProcess($container);
+
+        $this->assertSame($expected, $container->resolveEnvPlaceholders($ext->getConfig()));
+    }
+
     public function testEnvWithVariableNode(): void
     {
         $container = new ContainerBuilder();
@@ -282,6 +314,14 @@ public function getConfigTreeBuilder()
             ->children()
                 ->scalarNode('scalar_node')->end()
                 ->scalarNode('scalar_node_not_empty')->cannotBeEmpty()->end()
+                ->scalarNode('scalar_node_not_empty_validated')
+                    ->cannotBeEmpty()
+                    ->validate()
+                        ->always(function ($value) {
+                            return $value;
+                        })
+                    ->end()
+                ->end()
                 ->integerNode('int_node')->end()
                 ->floatNode('float_node')->end()
                 ->booleanNode('bool_node')->end()