Skip to content

Commit 3b15bb8

Browse files
dmetznerdmetzner
authored
[PHP] Enhance Symfony generator (#12532)
* Enhance Symfony generator - Add missing typehints - Add missing use statements - Simplify model ctor - Change fallthrough Exception to Throwable - prevent storing result of void methods - fix validate method - add default null values to model properties - simplify API interface return values - fix/rework default value implementation - fix optional params deprecation warnings - .. For more details check out the PR: #12532 * Enhance Symfony generator Tests - Skip risky tests - Fix type hint error - Fix class exists tests - Fix broken doc block - Enhance annotations - Update phpunit.xml.dist - Fix test config resource location
1 parent 286a31c commit 3b15bb8

56 files changed

Lines changed: 862 additions & 558 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/PhpSymfonyServerCodegen.java‎

Lines changed: 44 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -373,23 +373,15 @@ public void processOpts() {
373373

374374
// Type-hintable primitive types
375375
// ref: http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration
376-
if (phpLegacySupport) {
377-
typeHintable = new HashSet<>(
378-
Arrays.asList(
379-
"array"
380-
)
381-
);
382-
} else {
383-
typeHintable = new HashSet<>(
384-
Arrays.asList(
385-
"array",
386-
"bool",
387-
"float",
388-
"int",
389-
"string"
390-
)
391-
);
392-
}
376+
typeHintable = new HashSet<>(
377+
Arrays.asList(
378+
"array",
379+
"bool",
380+
"float",
381+
"int",
382+
"string"
383+
)
384+
);
393385
}
394386

395387
@Override
@@ -419,32 +411,24 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
419411
}
420412

421413
// Create a variable to display the correct data type in comments for interfaces
422-
param.vendorExtensions.put("x-comment-type", "\\" + param.dataType);
414+
param.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(param.dataType));
423415
if (param.isContainer) {
424-
param.vendorExtensions.put("x-comment-type", "\\" + param.dataType + "[]");
416+
param.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(param.dataType) + "[]");
425417
}
426418
}
427419

428420
// Create a variable to display the correct return type in comments for interfaces
429421
if (op.returnType != null) {
430-
op.vendorExtensions.put("x-comment-type", "\\" + op.returnType);
422+
op.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(op.returnType));
431423
if ("array".equals(op.returnContainer)) {
432-
op.vendorExtensions.put("x-comment-type", "\\" + op.returnType + "[]");
424+
op.vendorExtensions.put("x-comment-type", prependSlashToDataTypeOnlyIfNecessary(op.returnType) + "[]");
433425
}
434426
} else {
435427
op.vendorExtensions.put("x-comment-type", "void");
436428
}
437429
// Create a variable to add typing for return value of interface
438430
if (op.returnType != null) {
439-
if ("array".equals(op.returnContainer)) {
440-
op.vendorExtensions.put("x-return-type", "iterable");
441-
} else {
442-
if (defaultIncludes.contains(op.returnType)) {
443-
op.vendorExtensions.put("x-return-type", "array|" + op.returnType);
444-
} else {
445-
op.vendorExtensions.put("x-return-type", "array|\\" + op.returnType);
446-
}
447-
}
431+
op.vendorExtensions.put("x-return-type", "array|object|null");
448432
} else {
449433
op.vendorExtensions.put("x-return-type", "void");
450434
}
@@ -476,26 +460,26 @@ public ModelsMap postProcessModels(ModelsMap objs) {
476460
if (var.dataType != null) {
477461
// Determine if the parameter type is supported as a type hint and make it available
478462
// to the templating engine
479-
String typeHint = getTypeHint(var.dataType);
480-
if (!typeHint.isEmpty()) {
481-
var.vendorExtensions.put("x-parameter-type", typeHint);
482-
}
483-
463+
var.vendorExtensions.put("x-parameter-type", getTypeHintNullable(var.dataType));
464+
var.vendorExtensions.put("x-comment-type", getTypeHintNullableForComments(var.dataType));
484465
if (var.isContainer) {
485-
var.vendorExtensions.put("x-parameter-type", getTypeHint(var.dataType + "[]"));
486-
}
487-
488-
// Create a variable to display the correct data type in comments for models
489-
var.vendorExtensions.put("x-comment-type", var.dataType);
490-
if (var.isContainer) {
491-
var.vendorExtensions.put("x-comment-type", var.dataType + "[]");
466+
var.vendorExtensions.put("x-parameter-type", getTypeHintNullable(var.dataType + "[]"));
467+
var.vendorExtensions.put("x-comment-type", getTypeHintNullableForComments(var.dataType + "[]"));
492468
}
493469
}
494470
}
495471

496472
return objs;
497473
}
498474

475+
public String prependSlashToDataTypeOnlyIfNecessary(String dataType) {
476+
if (dataType.equals("array") || dataType.equals("bool") || dataType.equals("float") || dataType.equals("int") || dataType.equals("string") || dataType.equals("object") || dataType.equals("iterable") || dataType.equals("mixed")) {
477+
return dataType;
478+
}
479+
480+
return "\\" + dataType;
481+
}
482+
499483
/**
500484
* Output the Getter name for boolean property, e.g. isActive
501485
*
@@ -645,6 +629,24 @@ protected String toSymfonyService(String name) {
645629
return prefix + name;
646630
}
647631

632+
protected String getTypeHintNullable(String type) {
633+
String typeHint = getTypeHint(type);
634+
if (!typeHint.equals("")) {
635+
return "?" + typeHint;
636+
}
637+
638+
return typeHint;
639+
}
640+
641+
protected String getTypeHintNullableForComments(String type) {
642+
String typeHint = getTypeHint(type);
643+
if (!typeHint.equals("")) {
644+
return typeHint + "|null";
645+
}
646+
647+
return typeHint;
648+
}
649+
648650
protected String getTypeHint(String type) {
649651
// Type hint array types
650652
if (type.endsWith("[]")) {

‎modules/openapi-generator/src/main/resources/php-symfony/ApiServer.mustache‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
namespace {{apiPackage}};
2121

22+
use Symfony\Component\DependencyInjection\Reference;
23+
2224
/**
2325
* ApiServer Class Doc Comment
2426
*
@@ -35,15 +37,15 @@ class ApiServer
3537
/**
3638
* @var array
3739
*/
38-
private $apis = array();
40+
private array $apis = array();
3941
4042
/**
4143
* Adds an API handler to the server.
4244
*
4345
* @param string $api An API name of the handle
4446
* @param mixed $handler A handler to set for the given API
4547
*/
46-
public function addApiHandler($api, $handler)
48+
public function addApiHandler(string $api, $handler): void
4749
{
4850
if (isset($this->apis[$api])) {
4951
throw new \InvalidArgumentException('API has already a handler: '.$api);
@@ -59,7 +61,7 @@ class ApiServer
5961
* @return mixed Returns a handler
6062
* @throws \InvalidArgumentException When no such handler exists
6163
*/
62-
public function getApiHandler($api)
64+
public function getApiHandler(string $api)
6365
{
6466
if (!isset($this->apis[$api])) {
6567
throw new \InvalidArgumentException('No handler for '.$api.' implemented.');

‎modules/openapi-generator/src/main/resources/php-symfony/Controller.mustache‎

Lines changed: 46 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919

2020
namespace {{controllerPackage}};
2121

22+
use {{apiPackage}}\ApiServer;
2223
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
2324
use Symfony\Component\HttpFoundation\Request;
2425
use Symfony\Component\HttpFoundation\Response;
2526
use Symfony\Component\HttpKernel\Exception\HttpException;
27+
use Symfony\Component\Validator\ConstraintViolation;
2628
use {{servicePackage}}\SerializerInterface;
2729
use {{servicePackage}}\ValidatorInterface;
2830

@@ -36,23 +38,29 @@ use {{servicePackage}}\ValidatorInterface;
3638
*/
3739
class Controller extends AbstractController
3840
{
39-
protected $validator;
40-
protected $serializer;
41-
protected $apiServer;
41+
protected ValidatorInterface $validator;
42+
protected SerializerInterface $serializer;
43+
protected ApiServer $apiServer;
4244
43-
public function setValidator(ValidatorInterface $validator)
45+
public function setValidator(ValidatorInterface $validator): self
4446
{
4547
$this->validator = $validator;
48+
49+
return $this;
4650
}
4751

48-
public function setSerializer(SerializerInterface $serializer)
52+
public function setSerializer(SerializerInterface $serializer): self
4953
{
5054
$this->serializer = $serializer;
55+
56+
return $this;
5157
}
5258

53-
public function setApiServer($server)
59+
public function setApiServer(ApiServer $server): self
5460
{
5561
$this->apiServer = $server;
62+
63+
return $this;
5664
}
5765

5866
/**
@@ -63,7 +71,7 @@ class Controller extends AbstractController
6371
*
6472
* @return Response
6573
*/
66-
public function createBadRequestResponse($message = 'Bad Request.')
74+
public function createBadRequestResponse(string $message = 'Bad Request.'): Response
6775
{
6876
return new Response($message, 400);
6977
}
@@ -76,7 +84,7 @@ class Controller extends AbstractController
7684
*
7785
* @return Response
7886
*/
79-
public function createErrorResponse(HttpException $exception)
87+
public function createErrorResponse(HttpException $exception): Response
8088
{
8189
$statusCode = $exception->getStatusCode();
8290
$headers = array_merge($exception->getHeaders(), ['Content-Type' => 'application/json']);
@@ -91,48 +99,59 @@ class Controller extends AbstractController
9199
* Serializes data to a given type format.
92100
*
93101
* @param mixed $data The data to serialize.
94-
* @param string $class The source data class.
95102
* @param string $format The target serialization format.
96103
*
97104
* @return string A serialized data string.
98105
*/
99-
protected function serialize($data, $format)
106+
protected function serialize($data, string $format): string
100107
{
101108
return $this->serializer->serialize($data, $format);
102109
}
103110

104111
/**
105112
* Deserializes data from a given type format.
106113
*
107-
* @param string $data The data to deserialize.
114+
* @param mixed $data The data to deserialize.
108115
* @param string $class The target data class.
109116
* @param string $format The source serialization format.
110117
*
111118
* @return mixed A deserialized data.
112119
*/
113-
protected function deserialize($data, $class, $format)
120+
protected function deserialize($data, string $class, string $format)
114121
{
115122
return $this->serializer->deserialize($data, $class, $format);
116123
}
117124

118-
protected function validate($data, $asserts = null)
125+
/**
126+
* @param mixed $data
127+
* @param mixed $asserts
128+
*
129+
* @return Response|null
130+
*/
131+
protected function validate($data, $asserts = null): ?Response
119132
{
120133
$errors = $this->validator->validate($data, $asserts);
121134
122135
if (count($errors) > 0) {
123-
$errorsString = (string)$errors;
136+
$errorsString = '';
137+
/** @var ConstraintViolation $violation */
138+
foreach ($errors as $violation) {
139+
$errorsString .= $violation->getMessage()."\n";
140+
}
124141
return $this->createBadRequestResponse($errorsString);
125142
}
143+
144+
return null;
126145
}
127146

128147
/**
129148
* Converts an exception to a serializable array.
130149
*
131-
* @param \Exception|null $exception
150+
* @param \Throwable|null $exception
132151
*
133-
* @return array
152+
* @return array|null
134153
*/
135-
private function exceptionToArray(\Exception $exception = null)
154+
private function exceptionToArray(\Throwable $exception = null): ?array
136155
{
137156
if (null === $exception) {
138157
return null;
@@ -151,7 +170,15 @@ class Controller extends AbstractController
151170
];
152171
}
153172

154-
protected function getOutputFormat($accept, array $produced)
173+
/**
174+
* Converts an exception to a serializable array.
175+
*
176+
* @param string $accept
177+
* @param array $produced
178+
*
179+
* @return ?string
180+
*/
181+
protected function getOutputFormat(string $accept, array $produced): ?string
155182
{
156183
// Figure out what the client accepts
157184
$accept = preg_split("/[\s,]+/", $accept);
@@ -186,7 +213,7 @@ class Controller extends AbstractController
186213
*
187214
* @return bool Returns true if Content-Type supported otherwise false.
188215
*/
189-
public static function isContentTypeAllowed(Request $request, array $consumes = [])
216+
public static function isContentTypeAllowed(Request $request, array $consumes = []): bool
190217
{
191218
if (!empty($consumes) && $consumes[0] !== '*/*') {
192219
$currentFormat = $request->getContentType();

‎modules/openapi-generator/src/main/resources/php-symfony/Extension.mustache‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
3434
*/
3535
class {{bundleExtensionName}} extends Extension
3636
{
37-
public function load(array $configs, ContainerBuilder $container)
37+
public function load(array $configs, ContainerBuilder $container): void
3838
{
3939
$loader = new YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
4040
$loader->load('services.yml');

‎modules/openapi-generator/src/main/resources/php-symfony/README.mustache‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class {{baseName}}Api implements {{classname}} // An interface is autogenerated
106106
/**
107107
* Implementation of {{classname}}#{{operationId}}
108108
*/
109-
public function {{operationId}}({{#allParams}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}{{^required}} = {{#defaultValue}}'{{{.}}}'{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}, &$responseCode, array &$responseHeaders): {{#isArray}}iterable{{/isArray}}{{^isArray}}array|{{{vendorExtensions.x-comment-type}}}{{/isArray}}
109+
public function {{operationId}}({{#allParams}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}{{^required}} = {{#defaultValue}}'{{{.}}}'{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}}{{/required}}{{^-last}}, {{/-last}}{{/allParams}}, int &$responseCode, array &$responseHeaders): {{{vendorExtensions.x-return-type}}}
110110
{
111111
// Implement the operation ...
112112
}

‎modules/openapi-generator/src/main/resources/php-symfony/api.mustache‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ interface {{classname}}
3838
/**
3939
* Sets authentication method {{name}}
4040
*
41-
* @param string $value Value of the {{name}} authentication method.
41+
* @param string|null $value Value of the {{name}} authentication method.
4242
*
4343
* @return void
4444
*/
45-
public function set{{name}}($value);
45+
public function set{{name}}(?string $value): void;
4646
{{/authMethods}}
4747
{{#operation}}
4848

@@ -58,16 +58,17 @@ interface {{classname}}
5858
*
5959
{{/description}}
6060
{{#allParams}}
61-
* @param {{vendorExtensions.x-comment-type}} ${{paramName}} {{description}} {{#required}}(required){{/required}}{{^required}}(optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}){{/required}}{{#isDeprecated}} (deprecated){{/isDeprecated}}
61+
* @param {{vendorExtensions.x-parameter-type}}{{^required}}{{^defaultValue}}|null{{/defaultValue}}{{/required}} ${{paramName}} {{description}} {{#required}}(required){{/required}}{{^required}}(optional{{#defaultValue}}, default to {{{.}}}{{/defaultValue}}){{/required}}{{#isDeprecated}} (deprecated){{/isDeprecated}}
6262
{{/allParams}}
63-
* @param \array $responseHeaders Additional HTTP headers to return with the response ()
63+
* @param int &$responseCode The HTTP Response Code
64+
* @param array $responseHeaders Additional HTTP headers to return with the response ()
6465
*
65-
* @return {{{vendorExtensions.x-comment-type}}}
66+
* @return {{{vendorExtensions.x-return-type}}}
6667
{{#isDeprecated}}
6768
* @deprecated
6869
{{/isDeprecated}}
6970
*/
70-
public function {{operationId}}({{#allParams}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}{{^required}} = {{{defaultValue}}}{{^defaultValue}}null{{/defaultValue}}{{/required}}, {{/allParams}}&$responseCode, array &$responseHeaders): {{{vendorExtensions.x-return-type}}};
71+
public function {{operationId}}({{#allParams}}{{^required}}{{^defaultValue}}?{{/defaultValue}}{{/required}}{{#vendorExtensions.x-parameter-type}}{{vendorExtensions.x-parameter-type}} {{/vendorExtensions.x-parameter-type}}${{paramName}}, {{/allParams}}int &$responseCode, array &$responseHeaders): {{{vendorExtensions.x-return-type}}};
7172

7273
{{/operation}}
7374
}

0 commit comments

Comments
 (0)