From 4b2d0f66195476ea6c14f7de60222ebce10fdf41 Mon Sep 17 00:00:00 2001 From: provokateurin Date: Tue, 12 Nov 2024 15:49:30 +0100 Subject: [PATCH] refactor: Improve the code quality Signed-off-by: provokateurin --- generate-spec.php | 58 +++++++++++++++++++--------------------- merge-specs.php | 5 ++-- rector.php | 1 + src/ControllerMethod.php | 15 ++++++----- src/Helpers.php | 6 ++--- src/OpenApiType.php | 26 ++++++++---------- src/ResponseType.php | 6 +---- 7 files changed, 54 insertions(+), 63 deletions(-) diff --git a/generate-spec.php b/generate-spec.php index e6c1952..754157a 100755 --- a/generate-spec.php +++ b/generate-spec.php @@ -84,11 +84,7 @@ $appIsCore = false; $appID = (string)$xml->id; - if ($xml->namespace) { - $readableAppID = (string)$xml->namespace; - } else { - $readableAppID = Helpers::generateReadableAppID($appID); - } + $readableAppID = $xml->namespace ? (string)$xml->namespace : Helpers::generateReadableAppID($appID); $appSummary = (string)$xml->summary; $appVersion = (string)$xml->version; $appLicence = (string)$xml->licence; @@ -182,8 +178,8 @@ * @var Class_ $node */ foreach ($nodeFinder->findInstanceOf($astParser->parse(file_get_contents($path)), Class_::class) as $node) { - $implementsCapability = count(array_filter($node->implements, fn (Name $name): bool => $name->getLast() == 'ICapability')) > 0; - $implementsPublicCapability = count(array_filter($node->implements, fn (Name $name): bool => $name->getLast() == 'IPublicCapability')) > 0; + $implementsCapability = array_filter($node->implements, fn (Name $name): bool => $name->getLast() === 'ICapability') !== []; + $implementsPublicCapability = array_filter($node->implements, fn (Name $name): bool => $name->getLast() === 'IPublicCapability') !== []; if (!$implementsCapability && !$implementsPublicCapability) { continue; } @@ -327,7 +323,7 @@ } } -if (count($parsedRoutes) === 0) { +if ($parsedRoutes === []) { Logger::warning('Routes', 'No routes were loaded'); } @@ -343,7 +339,7 @@ foreach ($value as $route) { $routeName = $route['name']; - $postfix = array_key_exists('postfix', $route) ? $route['postfix'] : null; + $postfix = $route['postfix'] ?? null; $verb = array_key_exists('verb', $route) ? $route['verb'] : 'GET'; $requirements = array_key_exists('requirements', $route) ? $route['requirements'] : []; $defaults = array_key_exists('defaults', $route) ? $route['defaults'] : []; @@ -358,7 +354,7 @@ $url = $pathPrefix . $root . $url; $methodName = lcfirst(str_replace('_', '', ucwords(explode('#', (string)$routeName)[1], '_'))); - if ($methodName == 'preflightedCors') { + if ($methodName === 'preflightedCors') { continue; } @@ -390,7 +386,7 @@ $controllerScopes = Helpers::getOpenAPIAttributeScopes($controllerClass, $routeName); if (Helpers::classMethodHasAnnotationOrAttribute($controllerClass, 'IgnoreOpenAPI')) { - if (count($controllerScopes) === 0 || (in_array('ignore', $controllerScopes, true) && count($controllerScopes) === 1)) { + if ($controllerScopes === [] || (in_array('ignore', $controllerScopes, true) && count($controllerScopes) === 1)) { Logger::debug($routeName, "Controller '" . $controllerName . "' ignored because of IgnoreOpenAPI attribute"); continue; } @@ -409,24 +405,24 @@ $tagName = implode('_', array_map(fn (string $s) => strtolower($s), Helpers::splitOnUppercaseFollowedByNonUppercase($controllerName))); $doc = $controllerClass->getDocComment()?->getText(); - if ($doc != null && count(array_filter($tags, fn (array $tag): bool => $tag['name'] == $tagName)) == 0) { + if ($doc != null && count(array_filter($tags, fn (array $tag): bool => $tag['name'] === $tagName)) == 0) { $classDescription = []; $docNodes = $phpDocParser->parse(new TokenIterator($lexer->tokenize($doc)))->children; foreach ($docNodes as $docNode) { if ($docNode instanceof PhpDocTextNode) { $block = Helpers::cleanDocComment($docNode->text); - if ($block == '') { + if ($block === '') { continue; } $classDescription[] = $block; } } - if (count($classDescription) > 0) { + if ($classDescription !== []) { $tags[] = [ 'name' => $tagName, - 'description' => join("\n", $classDescription), + 'description' => implode("\n", $classDescription), ]; } } @@ -454,7 +450,7 @@ $scopes = Helpers::getOpenAPIAttributeScopes($classMethod, $routeName); if ($isIgnored) { - if (count($scopes) === 0 || (in_array('ignore', $scopes, true) && count($scopes) === 1)) { + if ($scopes === [] || (in_array('ignore', $scopes, true) && count($scopes) === 1)) { Logger::debug($routeName, 'Route ignored because of IgnoreOpenAPI attribute'); continue; } @@ -503,7 +499,7 @@ } $classMethodInfo = ControllerMethod::parse($routeName, $definitions, $methodFunction, $isAdmin, $isDeprecated, $isPasswordConfirmation); - if (count($classMethodInfo->returns) > 0) { + if ($classMethodInfo->returns !== []) { Logger::error($routeName, 'Returns an invalid response'); continue; } @@ -527,7 +523,7 @@ $docStatusCodes = array_map(fn (ControllerMethodResponse $response): int => $response->statusCode, array_filter($classMethodInfo->responses, fn (?ControllerMethodResponse $response): bool => $response != null)); $missingDocStatusCodes = array_unique(array_filter(array_diff($codeStatusCodes, $docStatusCodes), fn (int $code): bool => $code < 500)); - if (count($missingDocStatusCodes) > 0) { + if ($missingDocStatusCodes !== []) { Logger::error($routeName, 'Returns undocumented status codes: ' . implode(', ', $missingDocStatusCodes)); continue; } @@ -593,7 +589,7 @@ foreach ($urlParameters as $urlParameter) { $matchingParameters = array_filter($route->controllerMethod->parameters, fn (ControllerMethodParameter $param): bool => $param->name == $urlParameter); - $requirement = array_key_exists($urlParameter, $route->requirements) ? $route->requirements[$urlParameter] : null; + $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))) { @@ -615,7 +611,7 @@ $requirement = '^' . $requirement; } if (!str_ends_with((string)$requirement, '$')) { - $requirement = $requirement . '$'; + $requirement .= '$'; } } @@ -677,7 +673,7 @@ $mergedResponses = []; foreach (array_unique(array_map(fn (ControllerMethodResponse $response): int => $response->statusCode, array_filter($route->controllerMethod->responses, fn (?ControllerMethodResponse $response): bool => $response != null))) as $statusCode) { - if ($firstStatusCode && count($mergedResponses) > 0) { + if ($firstStatusCode && $mergedResponses !== []) { break; } @@ -691,14 +687,14 @@ $mergedContentTypeResponses = []; foreach (array_unique(array_map(fn (ControllerMethodResponse $response): ?string => $response->contentType, array_filter($statusCodeResponses, fn (ControllerMethodResponse $response): bool => $response->contentType != null))) as $contentType) { - if ($firstContentType && count($mergedContentTypeResponses) > 0) { + if ($firstContentType && $mergedContentTypeResponses !== []) { break; } /** @var ControllerMethodResponse[] $contentTypeResponses */ $contentTypeResponses = array_values(array_filter($statusCodeResponses, fn (ControllerMethodResponse $response): bool => $response->contentType == $contentType)); - $hasEmpty = count(array_filter($contentTypeResponses, fn (ControllerMethodResponse $response): bool => $response->type == null)) > 0; + $hasEmpty = array_filter($contentTypeResponses, fn (ControllerMethodResponse $response): bool => $response->type == null) !== []; $uniqueResponses = array_values(array_intersect_key($contentTypeResponses, array_unique(array_map(fn (ControllerMethodResponse $response): array|\stdClass => $response->type->toArray(), array_filter($contentTypeResponses, fn (ControllerMethodResponse $response): bool => $response->type != null)), SORT_REGULAR))); if (count($uniqueResponses) == 1) { if ($hasEmpty) { @@ -722,7 +718,7 @@ $response = [ 'description' => array_key_exists($statusCode, $route->controllerMethod->responseDescription) ? $route->controllerMethod->responseDescription[$statusCode] : '', ]; - if (count($headers) > 0) { + if ($headers !== []) { $response['headers'] = array_combine( array_keys($headers), array_map( @@ -733,7 +729,7 @@ ), ); } - if (count($mergedContentTypeResponses) > 0) { + if ($mergedContentTypeResponses !== []) { $response['content'] = $mergedContentTypeResponses; } $mergedResponses[$statusCode] = $response; @@ -757,7 +753,7 @@ if ($route->controllerMethod->summary !== null) { $operation['summary'] = $route->controllerMethod->summary; } - if (count($route->controllerMethod->description) > 0) { + if ($route->controllerMethod->description !== []) { $operation['description'] = implode("\n", $route->controllerMethod->description); } if ($route->controllerMethod->isDeprecated) { @@ -768,7 +764,7 @@ } $operation['security'] = $security; - if (count($bodyParameters) > 0) { + if ($bodyParameters !== []) { $requiredBodyParameters = []; foreach ($bodyParameters as $bodyParameter) { @@ -778,7 +774,7 @@ } } - $required = count($requiredBodyParameters) > 0; + $required = $requiredBodyParameters !== []; $schema = [ 'type' => 'object', @@ -829,7 +825,7 @@ ], ]; } - if (count($parameters) > 0) { + if ($parameters !== []) { $operation['parameters'] = $parameters; } @@ -925,7 +921,7 @@ if (!$hasSingleScope) { $scopePaths['full'] = []; -} elseif (count($scopePaths) === 0) { +} elseif ($scopePaths === []) { if (isset($schemas['Capabilities']) || isset($schemas['PublicCapabilities'])) { Logger::debug('app', 'Generating default scope without routes to populate capabilities'); $scopePaths['default'] = []; @@ -994,7 +990,7 @@ $scopedSchemas['PublicCapabilities'] = $schemas['PublicCapabilities']; } - if (count($scopedSchemas) === 0) { + if ($scopedSchemas === []) { $scopedSchemas = new stdClass(); } else { ksort($scopedSchemas); diff --git a/merge-specs.php b/merge-specs.php index 53a77ed..38de5af 100755 --- a/merge-specs.php +++ b/merge-specs.php @@ -129,7 +129,7 @@ function rewriteSchemaNames(array $spec): array { $schemas = $spec['components']['schemas']; $readableAppID = Helpers::generateReadableAppID($spec['info']['title']); return array_combine( - array_map(fn (string $key): string => $key == 'OCSMeta' ? $key : $readableAppID . $key, array_keys($schemas)), + array_map(fn (string $key): string => $key === 'OCSMeta' ? $key : $readableAppID . $key, array_keys($schemas)), array_values($schemas), ); } @@ -164,7 +164,8 @@ function rewriteOperations(array $spec): array { $operation['responses'] = [$value => $operation['responses'][$value]]; } if (array_key_exists('security', $operation)) { - for ($i = 0; $i < count($operation['security']); $i++) { + $counter = count($operation['security']); + for ($i = 0; $i < $counter; $i++) { if (count($operation['security'][$i]) == 0) { $operation['security'][$i] = new stdClass(); // When reading {} will be converted to [], so we have to fix it } diff --git a/rector.php b/rector.php index 1548127..e590488 100644 --- a/rector.php +++ b/rector.php @@ -15,6 +15,7 @@ ->withPhpSets() ->withPreparedSets( deadCode: true, + codeQuality: true, typeDeclarations: true, strictBooleans: true, ); diff --git a/src/ControllerMethod.php b/src/ControllerMethod.php index c198b6f..7b17b58 100644 --- a/src/ControllerMethod.php +++ b/src/ControllerMethod.php @@ -54,13 +54,14 @@ public static function parse(string $context, array $definitions, ClassMethod $m foreach ($docNodes as $docNode) { if ($docNode instanceof PhpDocTextNode) { $block = Helpers::cleanDocComment($docNode->text); - if ($block == '') { + if ($block === '') { continue; } - $pattern = '/([0-9]{3}): /'; + $pattern = '/(\d{3}): /'; if (preg_match($pattern, $block)) { $parts = preg_split($pattern, $block, -1, PREG_SPLIT_DELIM_CAPTURE | PREG_SPLIT_NO_EMPTY); - for ($i = 0; $i < count($parts); $i += 2) { + $counter = count($parts); + for ($i = 0; $i < $counter; $i += 2) { $statusCode = intval($parts[$i]); $responseDescriptions[$statusCode] = trim($parts[$i + 1]); } @@ -109,7 +110,7 @@ public static function parse(string $context, array $definitions, ClassMethod $m if (!$allowMissingDocs) { foreach (array_unique(array_map(fn (ControllerMethodResponse $response): int => $response->statusCode, array_filter($responses, fn (?ControllerMethodResponse $response): bool => $response != null))) as $statusCode) { - if ($statusCode < 500 && (!array_key_exists($statusCode, $responseDescriptions) || $responseDescriptions[$statusCode] == '')) { + if ($statusCode < 500 && (!array_key_exists($statusCode, $responseDescriptions) || $responseDescriptions[$statusCode] === '')) { Logger::error($context, 'Missing description for status code ' . $statusCode); } } @@ -136,7 +137,7 @@ public static function parse(string $context, array $definitions, ClassMethod $m } } - if ($paramTag !== null && $psalmParamTag !== null) { + if ($paramTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode && $psalmParamTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) { // Use all the type information from @psalm-param because it is more specific, // but pull the description from @param and @psalm-param because usually only one of them has it. if ($psalmParamTag->description !== '') { @@ -176,10 +177,10 @@ public static function parse(string $context, array $definitions, ClassMethod $m } $param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $methodParameter, $type); - } elseif ($psalmParamTag !== null) { + } elseif ($psalmParamTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) { $type = OpenApiType::resolve($context . ': @param: ' . $methodParameterName, $definitions, $psalmParamTag); $param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $methodParameter, $type); - } elseif ($paramTag !== null) { + } elseif ($paramTag instanceof \PHPStan\PhpDocParser\Ast\PhpDoc\ParamTagValueNode) { $type = OpenApiType::resolve($context . ': @param: ' . $methodParameterName, $definitions, $paramTag); $param = new ControllerMethodParameter($context, $definitions, $methodParameterName, $methodParameter, $type); } elseif ($allowMissingDocs) { diff --git a/src/Helpers.php b/src/Helpers.php index 4fb0e73..a1adb17 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -73,7 +73,7 @@ public static function mergeSchemas(array $schemas): mixed { if (!in_array(true, array_map(fn ($schema): bool => is_array($schema), $schemas))) { $results = array_values(array_unique($schemas)); if (count($results) > 1) { - throw new Exception('Incompatibles types: ' . join(', ', $results)); + throw new Exception('Incompatibles types: ' . implode(', ', $results)); } return $results[0]; } @@ -205,7 +205,7 @@ public static function getOpenAPIAttributeScopes(ClassMethod|Class_|Node $node, foreach ($node->attrGroups as $attrGroup) { foreach ($attrGroup->attrs as $attr) { if ($attr->name->getLast() === self::OPENAPI_ATTRIBUTE_CLASSNAME) { - if (empty($attr->args)) { + if ($attr->args === []) { $scopes[] = 'default'; continue; } @@ -230,7 +230,7 @@ public static function getOpenAPIAttributeTagsByScope(ClassMethod|Class_|Node $n foreach ($node->attrGroups as $attrGroup) { foreach ($attrGroup->attrs as $attr) { if ($attr->name->getLast() === self::OPENAPI_ATTRIBUTE_CLASSNAME) { - if (empty($attr->args)) { + if ($attr->args === []) { $tags[$defaultScope] = [$defaultTag]; continue; } diff --git a/src/OpenApiType.php b/src/OpenApiType.php index 8105742..63ce147 100644 --- a/src/OpenApiType.php +++ b/src/OpenApiType.php @@ -67,7 +67,7 @@ public function toArray(bool $isParameter = false): array|stdClass { type: 'integer', nullable: $this->nullable, hasDefaultValue: $this->hasDefaultValue, - defaultValue: !$this->hasDefaultValue ? null : ($this->defaultValue === true ? 1 : 0), + defaultValue: $this->hasDefaultValue ? ($this->defaultValue === true ? 1 : 0) : (null), description: $this->description, enum: [0, 1], ))->toArray($isParameter); @@ -99,11 +99,7 @@ enum: [0, 1], $values['nullable'] = true; } if ($this->hasDefaultValue && $this->defaultValue !== null) { - if ($this->type === 'object' && empty($this->defaultValue)) { - $values['default'] = new stdClass(); - } else { - $values['default'] = $this->defaultValue; - } + $values['default'] = $this->type === 'object' && empty($this->defaultValue) ? new stdClass() : $this->defaultValue; } if ($this->enum !== null) { $values['enum'] = $this->enum; @@ -111,7 +107,7 @@ enum: [0, 1], if ($this->description !== null && $this->description !== '' && !$isParameter) { $values['description'] = Helpers::cleanDocComment($this->description); } - if ($this->items !== null) { + if ($this->items instanceof \OpenAPIExtractor\OpenApiType) { $values['items'] = $this->items->toArray(); } if ($this->minLength !== null) { @@ -135,7 +131,7 @@ enum: [0, 1], if ($this->required !== null) { $values['required'] = $this->required; } - if ($this->properties !== null && count($this->properties) > 0) { + if ($this->properties !== null && $this->properties !== []) { $values['properties'] = array_combine(array_keys($this->properties), array_map(static fn (OpenApiType $property): array|\stdClass => $property->toArray(), array_values($this->properties)), ); @@ -157,7 +153,7 @@ enum: [0, 1], $values['allOf'] = array_map(fn (OpenApiType $type): array|\stdClass => $type->toArray(), $this->allOf); } - return count($values) > 0 ? $values : new stdClass(); + return $values !== [] ? $values : new stdClass(); } public static function resolve(string $context, array $definitions, ParamTagValueNode|NodeAbstract|TypeNode $node): OpenApiType { @@ -220,7 +216,7 @@ public static function resolve(string $context, array $definitions, ParamTagValu context: $context, type: 'object', properties: $properties, - required: count($required) > 0 ? $required : null, + required: $required !== [] ? $required : null, ); } @@ -263,14 +259,14 @@ public static function resolve(string $context, array $definitions, ParamTagValu $isUnion = $node instanceof UnionTypeNode || $node instanceof UnionType; $isIntersection = $node instanceof IntersectionTypeNode || $node instanceof IntersectionType; - if ($isUnion && count($node->types) == count(array_filter($node->types, fn ($type): bool => $type instanceof ConstTypeNode && $type->constExpr instanceof ConstExprStringNode))) { + if ($isUnion && count($node->types) === count(array_filter($node->types, fn ($type): bool => $type instanceof ConstTypeNode && $type->constExpr instanceof ConstExprStringNode))) { $values = []; /** @var ConstTypeNode $type */ foreach ($node->types as $type) { $values[] = $type->constExpr->value; } - if (count(array_filter($values, fn (string $value): bool => $value == '')) > 0) { + if (array_filter($values, fn (string $value): bool => $value === '') !== []) { // Not a valid enum return new OpenApiType( context: $context, @@ -284,14 +280,14 @@ public static function resolve(string $context, array $definitions, ParamTagValu enum: $values, ); } - if ($isUnion && count($node->types) == count(array_filter($node->types, fn ($type): bool => $type instanceof ConstTypeNode && $type->constExpr instanceof ConstExprIntegerNode))) { + if ($isUnion && count($node->types) === count(array_filter($node->types, fn ($type): bool => $type instanceof ConstTypeNode && $type->constExpr instanceof ConstExprIntegerNode))) { $values = []; /** @var ConstTypeNode $type */ foreach ($node->types as $type) { $values[] = (int)$type->constExpr->value; } - if (count(array_filter($values, fn (string $value): bool => $value == '')) > 0) { + if (array_filter($values, fn (string $value): bool => $value === '') !== []) { // Not a valid enum return new OpenApiType( context: $context, @@ -427,7 +423,7 @@ private static function mergeEnums(string $context, array $types): array { } private static function resolveIdentifier(string $context, array $definitions, string $name): OpenApiType { - if ($name == 'array') { + if ($name === 'array') { Logger::error($context, "Instead of 'array' use:\n'new stdClass()' for empty objects\n'array' for non-empty objects\n'array' for empty lists\n'array' for lists"); } if (str_starts_with($name, '\\')) { diff --git a/src/ResponseType.php b/src/ResponseType.php index c49ad2d..8c3492c 100644 --- a/src/ResponseType.php +++ b/src/ResponseType.php @@ -243,11 +243,7 @@ public static function resolve(string $context, TypeNode $obj): array { if (array_key_exists('Content-Type', $headers)) { /** @var OpenApiType $value */ $values = $headers['Content-Type']; - if ($values->oneOf != null) { - $values = $values->oneOf; - } else { - $values = [$values]; - } + $values = $values->oneOf != null ? $values->oneOf : [$values]; foreach ($values as $value) { if ($value->type == 'string' && $value->enum != null) {