Skip to content

Commit 5babd74

Browse files
provokateurinprovokateurin
committed
fix(OpenApiType): Prevent making misktakes with empty JSON objects
Signed-off-by: provokateurin <[email protected]>
1 parent bb21dad commit 5babd74

File tree

4 files changed

+26
-22
lines changed

4 files changed

+26
-22
lines changed

‎src/OpenApiType.php

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ enum: [0, 1],
113113
if ($this->description !== null && $this->description !== '' && !$isParameter) {
114114
$values['description'] = Helpers::cleanDocComment($this->description);
115115
}
116-
if ($this->items instanceof \OpenAPIExtractor\OpenApiType) {
116+
if ($this->items instanceof OpenApiType) {
117117
$values['items'] = $this->items->toArray();
118118
}
119119
if ($this->minLength !== null) {
@@ -139,7 +139,7 @@ enum: [0, 1],
139139
}
140140
if ($this->properties !== null && $this->properties !== []) {
141141
$values['properties'] = array_combine(array_keys($this->properties),
142-
array_map(static fn (OpenApiType $property): array|\stdClass => $property->toArray(), array_values($this->properties)),
142+
array_map(static fn (OpenApiType $property): array|stdClass => $property->toArray(), array_values($this->properties)),
143143
);
144144
}
145145
if ($this->additionalProperties !== null) {
@@ -150,13 +150,13 @@ enum: [0, 1],
150150
}
151151
}
152152
if ($this->oneOf !== null) {
153-
$values['oneOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->oneOf);
153+
$values['oneOf'] = array_map(fn (OpenApiType $type): array|stdClass => $type->toArray(), $this->oneOf);
154154
}
155155
if ($this->anyOf !== null) {
156-
$values['anyOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->anyOf);
156+
$values['anyOf'] = array_map(fn (OpenApiType $type): array|stdClass => $type->toArray(), $this->anyOf);
157157
}
158158
if ($this->allOf !== null) {
159-
$values['allOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->allOf);
159+
$values['allOf'] = array_map(fn (OpenApiType $type): array|stdClass => $type->toArray(), $this->allOf);
160160
}
161161

162162
return $values !== [] ? $values : new stdClass();
@@ -184,9 +184,9 @@ public static function resolve(string $context, array $definitions, ParamTagValu
184184
items: self::resolve($context . ': items', $definitions, $node->type),
185185
);
186186
}
187-
if ($node instanceof GenericTypeNode && ($node->type->name === 'array' || $node->type->name === 'list' || $node->type->name === 'non-empty-list') && count($node->genericTypes) === 1) {
188-
if ($node->type->name === 'array') {
189-
Logger::error($context, "The 'array<TYPE>' syntax for arrays is forbidden due to ambiguities. Use 'list<TYPE>' for JSON arrays or 'array<string, TYPE>' for JSON objects instead.");
187+
if ($node instanceof GenericTypeNode && ($node->type->name === 'array' || $node->type->name === 'non-empty-array' || $node->type->name === 'list' || $node->type->name === 'non-empty-list') && count($node->genericTypes) === 1) {
188+
if ($node->type->name === 'array' || $node->type->name === 'non-empty-array') {
189+
Logger::error($context, "The 'array<TYPE>' and 'non-empty-array<TYPE>' syntax for arrays is forbidden due to ambiguities. Use 'list<TYPE>' for JSON arrays or 'non-empty-array<string, TYPE>' for JSON objects instead.");
190190
}
191191

192192
if ($node->genericTypes[0] instanceof IdentifierTypeNode && $node->genericTypes[0]->name === 'empty') {
@@ -232,7 +232,11 @@ public static function resolve(string $context, array $definitions, ParamTagValu
232232
);
233233
}
234234

235-
if ($node instanceof GenericTypeNode && $node->type->name === 'array' && count($node->genericTypes) === 2 && $node->genericTypes[0] instanceof IdentifierTypeNode) {
235+
if ($node instanceof GenericTypeNode && in_array($node->type->name, ['array', 'non-empty-array']) && count($node->genericTypes) === 2 && $node->genericTypes[0] instanceof IdentifierTypeNode) {
236+
if ($node->type->name !== 'non-empty-array') {
237+
Logger::error($context, 'You must ensure JSON objects are not empty using the "non-empty-array" type. To allow return empty JSON objects your code must manually check if the array is empty in order to return "new \\stdClass()" and use "non-empty-array|\\stdClass" as the type.');
238+
}
239+
236240
$allowedTypes = ['string', 'lowercase-string', 'non-empty-string', 'non-empty-lowercase-string'];
237241
if (in_array($node->genericTypes[0]->name, $allowedTypes, true)) {
238242
return new OpenApiType(
@@ -428,15 +432,15 @@ private static function mergeEnums(string $context, array $types): array {
428432
}
429433
}
430434

431-
return array_merge($nonEnums, array_map(static fn (string $type): \OpenAPIExtractor\OpenApiType => new OpenApiType(
435+
return array_merge($nonEnums, array_map(static fn (string $type): OpenApiType => new OpenApiType(
432436
context: $context,
433437
type: $type, enum: $enums[$type],
434438
), array_keys($enums)));
435439
}
436440

437441
private static function resolveIdentifier(string $context, array $definitions, string $name): OpenApiType {
438442
if ($name === 'array') {
439-
Logger::error($context, "Instead of 'array' use:\n'new stdClass()' for empty objects\n'array<string, mixed>' for non-empty objects\n'array<emtpy>' for empty lists\n'array<YourTypeHere>' for lists");
443+
Logger::error($context, "Instead of 'array' use:\n'new stdClass()' for empty objects\n'non-empty-array<string, mixed>' for non-empty objects\n'list<emtpy>' for empty lists\n'list<YourTypeHere>' for lists");
440444
}
441445
if (str_starts_with($name, '\\')) {
442446
$name = substr($name, 1);
@@ -455,7 +459,7 @@ private static function resolveIdentifier(string $context, array $definitions, s
455459
'numeric' => new OpenApiType(context: $context, type: 'number'),
456460
// https://www.php.net/manual/en/language.types.float.php: Both float and double are always stored with double precision
457461
'float', 'double' => new OpenApiType(context: $context, type: 'number', format: 'double'),
458-
'mixed', 'empty', 'array' => new OpenApiType(context: $context, type: 'object'),
462+
'mixed', 'empty' => new OpenApiType(context: $context, type: 'object'),
459463
'object', 'stdClass' => new OpenApiType(context: $context, type: 'object', additionalProperties: true),
460464
'null' => new OpenApiType(context: $context, nullable: true),
461465
default => (function () use ($context, $definitions, $name) {

‎tests/lib/Controller/ReturnArraysController.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class ReturnArraysController extends OCSController {
1717
/**
1818
* Route with array using string keys
1919
*
20-
* @return DataResponse<Http::STATUS_OK, array<string, mixed>, array{}>
20+
* @return DataResponse<Http::STATUS_OK, non-empty-array<string, mixed>|\stdClass, array{}>
2121
*
2222
* 200: OK
2323
*/
@@ -28,7 +28,7 @@ public function stringArray(): DataResponse {
2828
/**
2929
* Route with array using non-empty-string keys
3030
*
31-
* @return DataResponse<Http::STATUS_OK, array<non-empty-string, mixed>, array{}>
31+
* @return DataResponse<Http::STATUS_OK, non-empty-array<non-empty-string, mixed>|\stdClass, array{}>
3232
*
3333
* 200: OK
3434
*/
@@ -39,7 +39,7 @@ public function nonEmptyStringArray(): DataResponse {
3939
/**
4040
* Route with array using lowercase-string keys
4141
*
42-
* @return DataResponse<Http::STATUS_OK, array<lowercase-string, mixed>, array{}>
42+
* @return DataResponse<Http::STATUS_OK, non-empty-array<lowercase-string, mixed>|\stdClass, array{}>
4343
*
4444
* 200: OK
4545
*/
@@ -50,7 +50,7 @@ public function lowercaseStringArray(): DataResponse {
5050
/**
5151
* Route with array using non-empty-lowercase-string keys
5252
*
53-
* @return DataResponse<Http::STATUS_OK, array<non-empty-lowercase-string, mixed>, array{}>
53+
* @return DataResponse<Http::STATUS_OK, non-empty-array<non-empty-lowercase-string, mixed>|\stdClass, array{}>
5454
*
5555
* 200: OK
5656
*/

‎tests/lib/Controller/SettingsController.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ public function arrayListParameter(array $value = ['test']): DataResponse {
400400
/**
401401
* A route with keyed array
402402
*
403-
* @param array<string, string> $value Some array value
403+
* @param non-empty-array<string, string>|\stdClass $value Some array value
404404
* @return DataResponse<Http::STATUS_OK, list<empty>, array{}>
405405
*
406406
* 200: Admin settings updated
@@ -539,7 +539,7 @@ public function emptyArray(): DataResponse {
539539
* Route with parameter
540540
*
541541
* @param int $simple Value
542-
* @param array<string, string> $complex Values
542+
* @param non-empty-array<string, string>|\stdClass $complex Values
543543
* @return DataResponse<Http::STATUS_OK, array{test: list<empty>}, array{}>
544544
*
545545
* 200: OK
@@ -553,8 +553,8 @@ public function parameterRequestBody(int $simple, array $complex): DataResponse
553553
/**
554554
* Route with object defaults
555555
*
556-
* @param array<string, string> $empty Empty
557-
* @param array<string, string> $values Values
556+
* @param non-empty-array<string, string>|\stdClass $empty Empty
557+
* @param non-empty-array<string, string>|\stdClass $values Values
558558
* @return DataResponse<Http::STATUS_OK, array{test: list<empty>}, array{}>
559559
*
560560
* 200: OK

‎tests/lib/ResponseDefinitions.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@
3838
* link: string,
3939
* actions: list<NotificationsNotificationAction>,
4040
* subjectRich?: string,
41-
* subjectRichParameters?: array<string, mixed>,
41+
* subjectRichParameters?: non-empty-array<string, mixed>|\stdClass,
4242
* messageRich?: string,
43-
* messageRichParameters?: array<string, mixed>,
43+
* messageRichParameters?: non-empty-array<string, mixed>|\stdClass,
4444
* icon?: string,
4545
* shouldNotify?: bool,
4646
* nonEmptyList: non-empty-list<string>,

0 commit comments

Comments
 (0)