Skip to content

Commit 0bf17d2

Browse files
alexandre-dauboisalexandre-daubois
committed
Address review
1 parent 07aa0c8 commit 0bf17d2

File tree

9 files changed

+23
-25
lines changed

9 files changed

+23
-25
lines changed

‎src/Symfony/Component/Config/Builder/ArrayShapeGenerator.php‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
use Symfony\Component\Config\Definition\PrototypedArrayNode;
2323
use Symfony\Component\Config\Definition\ScalarNode;
2424
use Symfony\Component\Config\Definition\StringNode;
25-
use Symfony\Component\Config\Definition\VariableNode;
2625

2726
/**
2827
* @author Alexandre Daubois <[email protected]>
@@ -45,19 +44,19 @@ private static function doGeneratePhpDoc(NodeInterface $node, int $nestingLevel
4544
$node instanceof NumericNode => self::handleNumericNode($node),
4645
$node instanceof EnumNode => $node->getPermissibleValues('|'),
4746
$node instanceof ScalarNode => 'scalar|null',
48-
$node instanceof VariableNode => 'mixed',
49-
default => throw new \LogicException('Unsupported node type: '.$node::class),
47+
default => 'mixed',
5048
};
5149
}
5250

5351
if ($node instanceof PrototypedArrayNode) {
5452
$isHashmap = (bool) $node->getKeyAttribute();
53+
$arrayType = ($isHashmap ? 'array<string, ' : 'list<').self::doGeneratePhpDoc($node->getPrototype(), 1 + $nestingLevel).'>';
5554

56-
return ($isHashmap ? 'array<string, ' : 'list<').self::doGeneratePhpDoc($node->getPrototype(), 1 + $nestingLevel).'>';
55+
return $node->hasDefaultValue() && null === $node->getDefaultValue() ? $arrayType.'|null' : $arrayType;
5756
}
5857

5958
if (!($children = $node->getChildren()) && !$node->getParent() instanceof PrototypedArrayNode) {
60-
return 'array<mixed>';
59+
return $node->hasDefaultValue() && null === $node->getDefaultValue() ? 'array<mixed>|null' : 'array<mixed>';
6160
}
6261

6362
$arrayShape = \sprintf("array{%s\n", self::generateInlinePhpDocForNode($node));
@@ -67,8 +66,8 @@ private static function doGeneratePhpDoc(NodeInterface $node, int $nestingLevel
6766

6867
if ($child instanceof PrototypedArrayNode) {
6968
$isHashmap = (bool) $child->getKeyAttribute();
70-
71-
$arrayShape .= ($isHashmap ? 'array<string, ' : 'list<').self::doGeneratePhpDoc($child->getPrototype(), 1 + $nestingLevel).'>';
69+
$childArrayType = ($isHashmap ? 'array<string, ' : 'list<').self::doGeneratePhpDoc($child->getPrototype(), 1 + $nestingLevel).'>';
70+
$arrayShape .= $child->hasDefaultValue() && null === $child->getDefaultValue() ? $childArrayType.'|null' : $childArrayType;
7271
} else {
7372
$arrayShape .= self::doGeneratePhpDoc($child, 1 + $nestingLevel);
7473
}
@@ -80,7 +79,9 @@ private static function doGeneratePhpDoc(NodeInterface $node, int $nestingLevel
8079
$arrayShape .= str_repeat(' ', $nestingLevel)."...<mixed>\n";
8180
}
8281

83-
return $arrayShape.str_repeat(' ', $nestingLevel - 1).'}';
82+
$arrayShape = $arrayShape.str_repeat(' ', $nestingLevel - 1).'}';
83+
84+
return $node->hasDefaultValue() && null === $node->getDefaultValue() ? $arrayShape.'|null' : $arrayShape;
8485
}
8586

8687
private static function dumpNodeKey(NodeInterface $node): string
@@ -114,22 +115,19 @@ private static function handleNumericNode(NumericNode $node): string
114115

115116
private static function generateInlinePhpDocForNode(BaseNode $node): string
116117
{
117-
$parts = [];
118-
118+
$comment = '';
119119
if ($node->isDeprecated()) {
120-
$parts[] = 'Deprecated: '.$node->getDeprecation($node->getName(), $node->getPath())['message'];
120+
$comment .= ' // Deprecated: '.$node->getDeprecation($node->getName(), $node->getPath())['message'];
121121
}
122122

123123
if ($info = $node->getInfo()) {
124-
$parts[] = $info;
124+
$comment .= ' // '.$info;
125125
}
126126

127127
if ($node->hasDefaultValue()) {
128-
$parts[] = 'Default: '.json_encode($node->getDefaultValue(), \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE | \JSON_PRESERVE_ZERO_FRACTION);
128+
$comment .= ' // Default: '.json_encode($node->getDefaultValue(), \JSON_UNESCAPED_SLASHES | \JSON_UNESCAPED_UNICODE | \JSON_PRESERVE_ZERO_FRACTION);
129129
}
130130

131-
$comment = implode(' | ', $parts);
132-
133-
return $comment ? ' // '.rtrim(preg_replace('/\s+/', ' ', $comment)) : '';
131+
return rtrim(preg_replace('/\s+/', ' ', $comment));
134132
}
135133
}

‎src/Symfony/Component/Config/Tests/Builder/ArrayShapeGeneratorTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public function testPhpDocHandleAdditionalDocumentation()
115115
$root = new ArrayNode('root');
116116
$root->addChild($child);
117117

118-
$this->assertStringContainsString('node?: bool, // Deprecated: The "node" option is deprecated. | This is a boolean node. | Default: true', ArrayShapeGenerator::generate($root));
118+
$this->assertStringContainsString('node?: bool, // Deprecated: The "node" option is deprecated. // This is a boolean node. // Default: true', ArrayShapeGenerator::generate($root));
119119
}
120120

121121
public function testPhpDocHandleMultilineDoc()
@@ -128,7 +128,7 @@ public function testPhpDocHandleMultilineDoc()
128128
$root = new ArrayNode('root');
129129
$root->addChild($child);
130130

131-
$this->assertStringContainsString('node?: bool, // Deprecated: The "node" option is deprecated. | This is a boolean node. Set to true to enable it. Set to false to disable it. | Default: true', ArrayShapeGenerator::generate($root));
131+
$this->assertStringContainsString('node?: bool, // Deprecated: The "node" option is deprecated. // This is a boolean node. Set to true to enable it. Set to false to disable it. // Default: true', ArrayShapeGenerator::generate($root));
132132
}
133133

134134
public function testPhpDocShapeSingleLevel()

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToList/Messenger/ReceivingConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function color($value): static
4545
* translator?: array{
4646
* fallbacks?: list<scalar|null>,
4747
* sources?: array<string, scalar|null>,
48-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
48+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
4949
* page?: list<array{
5050
* number?: int<min, max>,
5151
* content?: scalar|null,

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToList/Messenger/RoutingConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function senders(ParamConfigurator|array $value): static
3131
* translator?: array{
3232
* fallbacks?: list<scalar|null>,
3333
* sources?: array<string, scalar|null>,
34-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
34+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
3535
* page?: list<array{
3636
* number?: int<min, max>,
3737
* content?: scalar|null,

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToList/MessengerConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public function receiving(array $value = []): \Symfony\Config\AddToList\Messenge
4040
* translator?: array{
4141
* fallbacks?: list<scalar|null>,
4242
* sources?: array<string, scalar|null>,
43-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
43+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
4444
* page?: list<array{
4545
* number?: int<min, max>,
4646
* content?: scalar|null,

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToList/Translator/Books/PageConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public function content($value): static
4545
* translator?: array{
4646
* fallbacks?: list<scalar|null>,
4747
* sources?: array<string, scalar|null>,
48-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
48+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
4949
* page?: list<array{
5050
* number?: int<min, max>,
5151
* content?: scalar|null,

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToList/Translator/BooksConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function page(array $value = []): \Symfony\Config\AddToList\Translator\Bo
3030
* translator?: array{
3131
* fallbacks?: list<scalar|null>,
3232
* sources?: array<string, scalar|null>,
33-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
33+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
3434
* page?: list<array{
3535
* number?: int<min, max>,
3636
* content?: scalar|null,

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToList/TranslatorConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public function books(array $value = []): \Symfony\Config\AddToList\Translator\B
6262
* translator?: array{
6363
* fallbacks?: list<scalar|null>,
6464
* sources?: array<string, scalar|null>,
65-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
65+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
6666
* page?: list<array{
6767
* number?: int<min, max>,
6868
* content?: scalar|null,

‎src/Symfony/Component/Config/Tests/Builder/Fixtures/AddToList/Symfony/Config/AddToListConfig.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public function getExtensionAlias(): string
5050
* translator?: array{
5151
* fallbacks?: list<scalar|null>,
5252
* sources?: array<string, scalar|null>,
53-
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. | looks for translation in old fashion way
53+
* books?: array{ // Deprecated: The child node "books" at path "add_to_list.translator.books" is deprecated. // looks for translation in old fashion way
5454
* page?: list<array{
5555
* number?: int<min, max>,
5656
* content?: scalar|null,

0 commit comments

Comments
 (0)