[Config] Allow to always use config classes #51544
Conversation
|
Thanks for working on this, it's related to the limitation I found in #51273 |
nicolas-grekas
left a comment
nicolas-grekas
left a comment
There was a problem hiding this comment.
The changes on the Config components are BC breaks. Can we find a way that doesn't break BC?
| if ($this->default) { | ||
| if (!\is_array($this->defaultValue)) { | ||
| throw new \InvalidArgumentException(sprintf('%s: the default value of an array node has to be an array.', $node->getPath())); | ||
| throw new \InvalidArgumentException(sprintf('"%s": the default value of an array node has to be an array.', $node->getPath())); |
There was a problem hiding this comment.
should be reverted, that's a false-positive from fabbot I suppose
|
|
||
| protected $node; | ||
|
|
||
| public $allowedTypes; |
There was a problem hiding this comment.
removing consts and public properties is a BC break, we need to figure out a way that doesn't break BC
- undo changes from symfony#44166
alexndlm
force-pushed
the
feature/config-allow-to-always-use-config-classes
branch
from
f4bb242 to
72e1fd0
Compare
alexndlm
requested review from
OskarStark,
chalasr,
dunglas,
jderusse,
kbond,
lyrixx,
xabbuh and
yceruto
as code owners
| $parameterTypes = $this->getParameterTypes($node); | ||
| $comment .= ' * @param ParamConfigurator|'.implode('|', $parameterTypes).' $value'."\n"; | ||
| $parameterType = $this->getParameterType($node); | ||
| if (null === $parameterType || '' === $parameterType) { |
There was a problem hiding this comment.
why returning an empty string to mean mixed instead of returning mixed ?
| * @template TValue | ||
| * @param TValue $value | ||
| * @return \Symfony\Config\NodeInitialValues\MessengerConfig|$this | ||
| * @psalm-return (TValue is array ? \Symfony\Config\NodeInitialValues\MessengerConfig : static) |
There was a problem hiding this comment.
why should passing an array return a MessengerConfig while passing another kind of value would be fluent ? What does this correspond to ?
There was a problem hiding this comment.
I wondered about the same and I think I've got the answer:
- When you pass a non-array, you likely won't want to configure anything else nested in the tree you're configuring. That's what happens for other config formats, structurally.
- When you pass an array, you can configure nested options, so that then it might make sense to return the nested config-builder.
I'm just wondering about the check: should we return the config-builder when the array is non empty?
Or should we do this?
| * @psalm-return (TValue is array ? \Symfony\Config\NodeInitialValues\MessengerConfig : static) | |
| * @psalm-return (TValue is non-empty-array ? $this :\Symfony\Config\NodeInitialValues\MessengerConfig) |
| * @return $this | ||
| */ | ||
| public function keyedArray(string $name, ParamConfigurator|string|array $value): static | ||
| public function keyedArray(string $name, mixed $value): static |
There was a problem hiding this comment.
this type becoming wider looks like a regression to me.
|
Let me close as stalled and superseded by #51273 |
5 participants
When the config node doesn't have normalization, using the config object is impossible.
When the config node has some array normalization (like
http_codesin\Symfony\Bundle\FrameworkBundle\DependencyInjection\Configuration::addHttpClientRetrySection), it is not possible to use the config object.This PR relates to changes introduced in: