should be remove as it doesn't add any value when considering the method signature

I added it because it looks like most of the docblocks for methods in the Symfony core either use{@inheritdoc} or an explicit description of parameters. You're right (and I'm sure the clean code principles would agree with you) that it doesn't add anything by looking at it, although inside an IDE like PHPStorm people will use the quick documentation feature which makes it a little bit more understandable what the type parameter type is, but of course I can remove it - just let me know

We generally prefer removing those, eventhough the code base is not always consistent on this aspect.

Sure, I'll change it then. I'll wait a bit for your response to the other part before committing and pushing.

What about this? should do the same isn't it?
$this->envPlaceholders = array_map('array_unique', array_merge_recursive($this->envPlaceholders, $bag->getEnvPlaceholders()));

or what about another approach like this?

@@ -30,7 +30,9 @@ class EnvPlaceholderParameterBag extends ParameterBag             $env = substr($name, 4, -1);             if (isset($this->envPlaceholders[$env])) {-                return $this->envPlaceholders[$env][0];+                foreach ($this->envPlaceholders[$env] as $placeholder) {+                    return $placeholder;+                }             }             if (preg_match('/\W/', $env)) {                 throw new InvalidArgumentException(sprintf('Invalid %s name: only "word" characters are allowed.', $name));@@ -43,8 +45,9 @@ class EnvPlaceholderParameterBag extends ParameterBag                     throw new RuntimeException(sprintf('The default value of an env() parameter must be scalar, but "%s" given to "%s".', gettype($defaultValue), $name));                 }             }+            $placeholder = sprintf('env_%s_%s', $env, md5($name.uniqid(mt_rand(), true)));-            return $this->envPlaceholders[$env][] = sprintf('env_%s_%s', $env, md5($name.uniqid(mt_rand(), true)));+            return $this->envPlaceholders[$env][$placeholder] = $placeholder;         }         return parent::get($name);

About thearray_map, it works and it cuts some lines of code, but it is needless use of recursive functions and maybe not as clear, is it really needed?

For the changes insideget(), why would you change from returning the 0th index to using foreach? If you think this should be changed how about something more readable like

return array_values($this->envPlaceholders[$placeholder])[0];

Finally for the changes in setting$this->envPlaceholders I don't see the point in an array that will always have the structure

[  'apple' => 'apple',  'orange' => 'orange']

Plus (and I know it's going outside the scope of this PR) I don't think that assignment in a return statement is very clear and would prefer something like:

$uniqueName = md5($name.uniqid(mt_rand(), true));$placeholder = sprintf('env_%s_%s', $environmentVariableName, $uniqueName);$this->envPlaceholders[$environmentVariableName][] = $placeholder;return $placeholder;

I made some naming changes locally because I also foundenv confusing as it conflicts with Symfony environments but these don't have to be included in any PR changes.

Sorry to sound like a dick and that I don't want to change it, but I think I should at least be sure about the changes before I make them. I think it's a really cool feature and the only reason I'm here is because when I was working on changing my own small project to use theSYMFONY__ENV variable style I came across this and thought it looks much better.

needless use of recursive functions

not needless at all: the array is recursive (width two depth levels)

array_values($this->envPlaceholders[$placeholder])[0]

this would create a full copy of the entire array just to get the first element. foreach is the most efficient way to do so when not knowing the first key.

don't see the point in an array that will always have the structure

the point is using keys as a precomputed deduplication index, so that e.g. array_unique (slow compared to indexes) is not needed anymore.

I don't think that assignment in a return statement

no pb for that change, go for it

I can see your point for most of those, but really using a 3 lineforeach to get the first element of an array seems like overkill. How about

return reset($this->envPlaceholders[$env]); // return first result

reset has two drawbacks: it works by reference, which I prefer to avoid, and it changes the internal state of the array, which is a not wanted side effect. Even if these could be of no importance in this case, I consider using it a worse practice...

this change is not needed anymore: now that keys are strings, they are already unique

assertCount()

current()?

see#20199 (comment), same applies tocurrent