diff --git a/generate-spec.php b/generate-spec.php index c47a4c9..29b101a 100755 --- a/generate-spec.php +++ b/generate-spec.php @@ -614,13 +614,9 @@ $requirement = $route->requirements[$urlParameter] ?? null; if (count($matchingParameters) == 1) { $parameter = $matchingParameters[array_keys($matchingParameters)[0]]; - if ($parameter?->methodParameter == null && ($route->requirements == null || !array_key_exists($urlParameter, $route->requirements))) { - Logger::error($route->name, "Unable to find parameter for '" . $urlParameter . "'"); - continue; - } $schema = $parameter->type->toArray(true); - $description = $parameter?->docType != null && $parameter->docType->description != '' ? Helpers::cleanDocComment($parameter->docType->description) : null; + $description = $parameter->type->description; } else { $schema = [ 'type' => 'string', @@ -825,8 +821,8 @@ 'name' => $queryParameter->name . ($queryParameter->type->type === 'array' ? '[]' : ''), 'in' => 'query', ]; - if ($queryParameter->docType !== null && $queryParameter->docType->description !== '') { - $parameter['description'] = Helpers::cleanDocComment($queryParameter->docType->description); + if ($queryParameter->type->description !== null && $queryParameter->type->description !== '') { + $parameter['description'] = Helpers::cleanDocComment($queryParameter->type->description); } if (!$queryParameter->type->nullable && !$queryParameter->type->hasDefaultValue) { $parameter['required'] = true; diff --git a/src/ControllerMethod.php b/src/ControllerMethod.php index f0bf920..ab05c0e 100644 --- a/src/ControllerMethod.php +++ b/src/ControllerMethod.php @@ -190,7 +190,7 @@ public static function parse(string $context, $description = ''; } // Only keep lines that don't match the status code pattern in the description - $description = implode("\n", array_filter(array_filter(explode("\n", $description), static fn (string $line) => trim($line) !== ''), static fn (string $line) => !preg_match(self::STATUS_CODE_DESCRIPTION_PATTERN, $line))); + $description = Helpers::cleanDocComment(implode("\n", array_filter(array_filter(explode("\n", $description), static fn (string $line) => trim($line) !== ''), static fn (string $line) => !preg_match(self::STATUS_CODE_DESCRIPTION_PATTERN, $line)))); if ($paramTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode && $psalmParamTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) { try { @@ -226,17 +226,24 @@ public static function parse(string $context, } elseif ($paramTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) { $type = OpenApiType::resolve($context . ': @param: ' . $methodParameterName, $definitions, $paramTag); } elseif ($allowMissingDocs) { - $type = null; + $type = OpenApiType::resolve($context . ': $' . $methodParameterName . ': ' . $methodParameterName, $definitions, $methodParameter->type); } else { Logger::error($context, "Missing doc parameter for '" . $methodParameterName . "'"); continue; } - if ($type !== null) { - $type->description = $description; + $type->description = $description; + + if ($methodParameter->default !== null) { + try { + $type->defaultValue = Helpers::exprToValue($context, $methodParameter->default); + $type->hasDefaultValue = true; + } catch (UnsupportedExprException $e) { + Logger::debug($context, $e); + } } - $param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $methodParameter, $type); + $param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $type); if (!$allowMissingDocs && $param->type->description == '') { Logger::error($context . ': @param: ' . $methodParameterName, 'Missing description'); @@ -284,9 +291,38 @@ public static function parse(string $context, if ($methodCall->var instanceof PropertyFetch && $methodCall->var->var instanceof Variable && $methodCall->var->var->name === 'this' && - $methodCall->var->name->name === 'request' && - $methodCall->name->name === 'getHeader') { - $headers[] = Helpers::exprToValue($context . ': getHeader', $methodCall->args[0]->value); + $methodCall->var->name->name === 'request') { + if ($methodCall->name->name === 'getHeader') { + $headers[] = $methodCall->args[0]->value->value; + } + if ($methodCall->name->name === 'getParam') { + $name = $methodCall->args[0]->value->value; + + if (preg_match('/^[a-zA-Z][a-zA-Z0-9_]*$/', $name)) { + Logger::error($context . ': getParam: ' . $name, 'Do not use getParam() when a controller method parameter also works. With getParam() it is not possible to add a comment and specify the parameter type, therefore it should be avoided whenever possible.'); + } + + $defaultValue = null; + $hasDefaultValue = false; + try { + $defaultValue = count($methodCall->args) > 1 ? Helpers::exprToValue($context . ': getParam: ' . $name, $methodCall->args[1]->value) : null; + $hasDefaultValue = true; + } catch (UnsupportedExprException $e) { + Logger::debug($context, $e); + } + + $type = new OpenApiType( + context: $context, + // We can not know the type, so need to fallback to object :/ + type: 'object', + // IRequest::getParam() has null as a default value, so the parameter always has a default value and allows null. + nullable: true, + hasDefaultValue: $hasDefaultValue, + defaultValue: $defaultValue, + ); + + $parameters[] = new ControllerMethodParameter($context, $definitions, $name, $type); + } } } diff --git a/src/ControllerMethodParameter.php b/src/ControllerMethodParameter.php index b824738..5bf2c2c 100644 --- a/src/ControllerMethodParameter.php +++ b/src/ControllerMethodParameter.php @@ -7,30 +7,12 @@ namespace OpenAPIExtractor; -use PhpParser\Node\Param; - class ControllerMethodParameter { - public OpenApiType $type; - public function __construct( string $context, array $definitions, public string $name, - public Param $methodParameter, - public ?OpenApiType $docType, + public OpenApiType $type, ) { - if ($docType != null) { - $this->type = $this->docType; - } else { - $this->type = OpenApiType::resolve($context . ': @param: ' . $name, $definitions, $methodParameter->type); - } - if ($methodParameter->default != null) { - try { - $this->type->defaultValue = Helpers::exprToValue($context, $methodParameter->default); - $this->type->hasDefaultValue = true; - } catch (UnsupportedExprException $e) { - Logger::debug($context, $e); - } - } } } diff --git a/src/OpenApiType.php b/src/OpenApiType.php index bfe1f11..f3169c6 100644 --- a/src/OpenApiType.php +++ b/src/OpenApiType.php @@ -104,8 +104,8 @@ enum: [0, 1], if ($this->deprecated) { $values['deprecated'] = true; } - if ($this->hasDefaultValue && $this->defaultValue !== null) { - $values['default'] = $this->type === 'object' && empty($this->defaultValue) ? new stdClass() : $this->defaultValue; + if ($this->hasDefaultValue) { + $values['default'] = $this->type === 'object' && is_array($this->defaultValue) && count($this->defaultValue) === 0 ? new stdClass() : $this->defaultValue; } if ($this->enum !== null) { $values['enum'] = $this->enum; diff --git a/tests/appinfo/routes.php b/tests/appinfo/routes.php index 800ef55..f1b1217 100644 --- a/tests/appinfo/routes.php +++ b/tests/appinfo/routes.php @@ -52,6 +52,7 @@ ['name' => 'Settings#booleanTrueFalseParameter', 'url' => '/api/{apiVersion}/boolean-true-false', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#trueFalseParameter', 'url' => '/api/{apiVersion}/true-false', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#stringValueParameter', 'url' => '/api/{apiVersion}/string-value', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#stringDefaultNullParameter', 'url' => '/api/{apiVersion}/string-default-null', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#intValueParameter', 'url' => '/api/{apiVersion}/int-value', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#numericParameter', 'url' => '/api/{apiVersion}/numeric', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#arrayListParameter', 'url' => '/api/{apiVersion}/array-list', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], @@ -86,6 +87,7 @@ ['name' => 'Settings#samePathGet', 'url' => '/api/{apiVersion}/same-path', 'verb' => 'GET', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#samePathPost', 'url' => '/api/{apiVersion}/same-path', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'Settings#requestHeader', 'url' => '/api/{apiVersion}/request-header', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], + ['name' => 'Settings#requestParams', 'url' => '/api/{apiVersion}/request-params', 'verb' => 'POST', 'requirements' => ['apiVersion' => '(v2)']], ['name' => 'V1\SubDir#subDirRoute', 'url' => '/sub-dir', 'verb' => 'GET'], ], ]; diff --git a/tests/lib/Controller/SettingsController.php b/tests/lib/Controller/SettingsController.php index d3adefb..24a8c4f 100644 --- a/tests/lib/Controller/SettingsController.php +++ b/tests/lib/Controller/SettingsController.php @@ -349,6 +349,18 @@ public function stringValueParameter(string $value): DataResponse { return new DataResponse(); } + /** + * A route with string or null + * + * @param ?string $value string or null + * @return DataResponse, array{}> + * + * 200: Admin settings updated + */ + public function stringDefaultNullParameter(?string $value = null): DataResponse { + return new DataResponse(); + } + /** * A route with int or 0 * @@ -752,4 +764,18 @@ public function requestHeader(): DataResponse { return new DataResponse(); } + + /** + * A method with a request params. + * + * @return DataResponse, array{}> + * + * 200: Admin settings updated + */ + public function requestParams(): DataResponse { + $value = $this->request->getParam('some-param'); + $value = $this->request->getParam('some-param-with-explicit-default-value', 'abc'); + + return new DataResponse(); + } } diff --git a/tests/openapi-administration.json b/tests/openapi-administration.json index bc54088..22b9bc1 100644 --- a/tests/openapi-administration.json +++ b/tests/openapi-administration.json @@ -2498,6 +2498,96 @@ } } }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/string-default-null": { + "post": { + "operationId": "settings-string-default-null-parameter", + "summary": "A route with string or null", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "value": { + "type": "string", + "nullable": true, + "default": null, + "description": "string or null" + } + } + } + } + } + }, + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/int-value": { "post": { "operationId": "settings-int-value-parameter", @@ -5431,6 +5521,100 @@ } } }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/request-params": { + "post": { + "operationId": "settings-request-params", + "summary": "A method with a request params.", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "some-param": { + "type": "object", + "nullable": true, + "default": null + }, + "some-param-with-explicit-default-value": { + "type": "object", + "nullable": true, + "default": "abc" + } + } + } + } + } + }, + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/tests/attribute-ocs/{param}": { "get": { "operationId": "routing-attributeocs-route", diff --git a/tests/openapi-full.json b/tests/openapi-full.json index 3376f31..9863762 100644 --- a/tests/openapi-full.json +++ b/tests/openapi-full.json @@ -2655,6 +2655,96 @@ } } }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/string-default-null": { + "post": { + "operationId": "settings-string-default-null-parameter", + "summary": "A route with string or null", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "value": { + "type": "string", + "nullable": true, + "default": null, + "description": "string or null" + } + } + } + } + } + }, + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/notifications/api/{apiVersion}/int-value": { "post": { "operationId": "settings-int-value-parameter", @@ -5588,6 +5678,100 @@ } } }, + "/ocs/v2.php/apps/notifications/api/{apiVersion}/request-params": { + "post": { + "operationId": "settings-request-params", + "summary": "A method with a request params.", + "description": "This endpoint requires admin access", + "tags": [ + "settings" + ], + "security": [ + { + "bearer_auth": [] + }, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": false, + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "some-param": { + "type": "object", + "nullable": true, + "default": null + }, + "some-param-with-explicit-default-value": { + "type": "object", + "nullable": true, + "default": "abc" + } + } + } + } + } + }, + "parameters": [ + { + "name": "apiVersion", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": [ + "v2" + ], + "default": "v2" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "Admin settings updated", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/tests/attribute-ocs/{param}": { "get": { "operationId": "routing-attributeocs-route",