Add rector and apply some basic rules

zircote · Apr 30, 2024 · 6d825ea · 6d825ea
1 parent cc21aaf
commit 6d825ea
Show file tree

Hide file tree

Showing 26 changed files with 106 additions and 96 deletions.
diff --git a/composer.json b/composer.json
@@ -62,6 +62,7 @@
         "friendsofphp/php-cs-fixer": "^3.48",
         "phpstan/phpstan": "^1.6",
         "phpunit/phpunit": "^9.0",
+        "rector/rector": "^1.0",
         "vimeo/psalm": "^4.30"
     },
     "suggest": {
@@ -79,6 +80,7 @@
     },
     "scripts-descriptions": {
         "cs": "Fix all codestyle issues",
+        "rector": "Automatic refactoring",
         "lint": "Test codestyle",
         "test": "Run all non-legacy and codestyle tests",
         "testlegacy": "Run tests using the legacy TokenAnalyser",
@@ -93,7 +95,11 @@
     },
     "scripts": {
         "cs": "export XDEBUG_MODE=off && php-cs-fixer fix --allow-risky=yes",
-        "lint": "@cs --dry-run",
+        "rector": "rector process src",
+        "lint": [
+            "@cs --dry-run",
+            "@rector --dry-run"
+        ],
         "test": [
             "export XDEBUG_MODE=off && phpunit",
             "@lint"

diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -65,11 +65,6 @@ parameters:
 			count: 1
 			path: src/Annotations/Schema.php
 
-		-
-			message: "#^Variable \\$logLine in empty\\(\\) always exists and is not falsy\\.$#"
-			count: 1
-			path: src/Loggers/ConsoleLogger.php
-
 		-
 			message: "#^Call to function is_array\\(\\) with bool\\|OpenApi\\\\Annotations\\\\AdditionalProperties will always evaluate to false\\.$#"
 			count: 1

diff --git a/rector.php b/rector.php
@@ -0,0 +1,34 @@
+<?php
+
+use Rector\CodeQuality\Rector\ClassMethod\ExplicitReturnNullRector;
+use Rector\CodeQuality\Rector\For_\ForRepeatedCountToOwnVariableRector;
+use Rector\CodeQuality\Rector\If_\CombineIfRector;
+use Rector\CodeQuality\Rector\If_\ExplicitBoolCompareRector;
+use Rector\CodeQuality\Rector\If_\ShortenElseIfRector;
+use Rector\Config\RectorConfig;
+use Rector\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector;
+use Rector\DeadCode\Rector\If_\RemoveDeadInstanceOfRector;
+use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromStrictConstructorRector;
+use Rector\ValueObject\PhpVersion;
+
+return RectorConfig::configure()
+    ->withRules([
+        TypedPropertyFromStrictConstructorRector::class
+    ])
+    ->withSkip([
+        CombineIfRector::class,
+        ExplicitBoolCompareRector::class,
+        ExplicitReturnNullRector::class => [
+            __DIR__ . '/src/Analysers/TokenAnalyser.php',
+        ],
+        ForRepeatedCountToOwnVariableRector::class,
+        RemoveAlwaysTrueIfConditionRector::class => [
+            __DIR__ . '/src/Processors/ExpandEnums.php',
+        ] ,
+        RemoveDeadInstanceOfRector::class => [
+            __DIR__ . '/src/Processors/ExpandEnums.php',
+        ],
+        ShortenElseIfRector::class,
+    ])
+    ->withPreparedSets(true, true)
+    ->withPhpVersion(PhpVersion::PHP_74);
diff --git a/src/Analysers/AttributeAnnotationFactory.php b/src/Analysers/AttributeAnnotationFactory.php
@@ -123,7 +123,7 @@ public function build(\Reflector $reflector, Context $context): array
             }
 
             // Property can be nested...
-            return $annotation->getRoot() != $possibleParent->getRoot()
+            return $annotation->getRoot() !== $possibleParent->getRoot()
                 && ($explicitParent || ($isAttachable && $isParentAllowed));
         };
 

diff --git a/src/Analysers/ReflectionAnalyser.php b/src/Analysers/ReflectionAnalyser.php
@@ -24,14 +24,13 @@ class ReflectionAnalyser implements AnalyserInterface
     use GeneratorAwareTrait;
 
     /** @var AnnotationFactoryInterface[] */
-    protected array $annotationFactories;
+    protected array $annotationFactories = [];
 
     /**
      * @param array<AnnotationFactoryInterface> $annotationFactories
      */
     public function __construct(array $annotationFactories = [])
     {
-        $this->annotationFactories = [];
         foreach ($annotationFactories as $annotationFactory) {
             if ($annotationFactory->isSupported()) {
                 $this->annotationFactories[] = $annotationFactory;
@@ -115,7 +114,7 @@ protected function analyzeFqdn(string $fqdn, Analysis $analysis, array $details)
         if ($parentClass = $rc->getParentClass()) {
             $definition['extends'] = $normaliseClass($parentClass->getName());
         }
-        $definition[$contextType == 'class' ? 'implements' : 'extends'] = array_map($normaliseClass, $details['interfaces']);
+        $definition[$contextType === 'class' ? 'implements' : 'extends'] = array_map($normaliseClass, $details['interfaces']);
         $definition['traits'] = array_map($normaliseClass, $details['traits']);
 
         foreach ($this->annotationFactories as $annotationFactory) {

diff --git a/src/Analysers/TokenAnalyser.php b/src/Analysers/TokenAnalyser.php
@@ -160,7 +160,9 @@ protected function fromTokens(array $tokens, Context $parseContext): Analysis
 
                 if ($token[0] === T_IMPLEMENTS) {
                     $schemaContext->implements = $this->parseNamespaceList($tokens, $token, $parseContext);
-                    $classDefinition['implements'] = array_map([$schemaContext, 'fullyQualifiedName'], $schemaContext->implements);
+                    $classDefinition['implements'] = array_map(function (?string $source) use ($schemaContext): string {
+                        return $schemaContext->fullyQualifiedName($source);
+                    }, $schemaContext->implements);
                 }
 
                 if ($comment) {
@@ -201,7 +203,9 @@ protected function fromTokens(array $tokens, Context $parseContext): Analysis
 
                 if ($token[0] === T_EXTENDS) {
                     $schemaContext->extends = $this->parseNamespaceList($tokens, $token, $parseContext);
-                    $interfaceDefinition['extends'] = array_map([$schemaContext, 'fullyQualifiedName'], $schemaContext->extends);
+                    $interfaceDefinition['extends'] = array_map(function (?string $source) use ($schemaContext): string {
+                        return $schemaContext->fullyQualifiedName($source);
+                    }, $schemaContext->extends);
                 }
 
                 if ($comment) {

diff --git a/src/Analysers/TokenScanner.php b/src/Analysers/TokenScanner.php
@@ -66,7 +66,7 @@ protected function scanTokens(array $tokens): array
                         break;
                     case '}':
                         array_pop($stack);
-                        if (count($stack) == $unitLevel) {
+                        if (count($stack) === $unitLevel) {
                             $currentName = null;
                         }
                         break;
@@ -76,7 +76,7 @@ protected function scanTokens(array $tokens): array
 
             switch ($token[0]) {
                 case T_ABSTRACT:
-                    if (count($stack)) {
+                    if ($stack !== []) {
                         $isAbstractFunction = true;
                     }
                     break;
@@ -115,7 +115,7 @@ protected function scanTokens(array $tokens): array
                     // unless ...
                     if (is_string($token) && ($token === '(' || $token === '{')) {
                         // new class[()] { ... }
-                        if ('{' == $token) {
+                        if ('{' === $token) {
                             prev($tokens);
                         }
                         break;
@@ -254,7 +254,7 @@ protected function resolveFQN(array $names, string $namespace, array $uses): arr
     protected function skipTo(array &$tokens, string $char, bool $prev = false): void
     {
         while (false !== ($token = next($tokens))) {
-            if (is_string($token) && $token == $char) {
+            if (is_string($token) && $token === $char) {
                 if ($prev) {
                     prev($tokens);
                 }

diff --git a/src/Analysis.php b/src/Analysis.php
@@ -134,7 +134,7 @@ public function addAnalysis(Analysis $analysis): void
         $this->interfaces = array_merge($this->interfaces, $analysis->interfaces);
         $this->traits = array_merge($this->traits, $analysis->traits);
         $this->enums = array_merge($this->enums, $analysis->enums);
-        if ($this->openapi === null && $analysis->openapi !== null) {
+        if (!$this->openapi instanceof OA\OpenApi && $analysis->openapi instanceof OA\OpenApi) {
             $this->openapi = $analysis->openapi;
         }
     }
@@ -331,7 +331,7 @@ public function getAnnotationForSource(string $fqdn, string $class): ?OA\Abstrac
                 if (is_iterable($definition['context']->annotations)) {
                     /** @var OA\AbstractAnnotation $annotation */
                     foreach (array_reverse($definition['context']->annotations) as $annotation) {
-                        if (is_a($annotation, $class) && $annotation->isRoot($class) && !$annotation->_context->is('generated')) {
+                        if ($annotation instanceof $class && $annotation->isRoot($class) && !$annotation->_context->is('generated')) {
                             return $annotation;
                         }
                     }
@@ -363,7 +363,7 @@ public function getContext(object $annotation): ?Context
      */
     public function merged(): Analysis
     {
-        if ($this->openapi === null) {
+        if (!$this->openapi instanceof OA\OpenApi) {
             throw new \Exception('No openapi target set. Run the MergeIntoOpenApi processor');
         }
         $unmerged = $this->openapi->_unmerged;
@@ -420,7 +420,7 @@ public function process($processors = null): void
 
     public function validate(): bool
     {
-        if ($this->openapi !== null) {
+        if ($this->openapi instanceof OA\OpenApi) {
             return $this->openapi->validate();
         }
 

diff --git a/src/Annotations/AbstractAnnotation.php b/src/Annotations/AbstractAnnotation.php
@@ -37,7 +37,7 @@ abstract class AbstractAnnotation implements \JsonSerializable
     /**
      * @var Context|null
      */
-    public $_context = null;
+    public $_context;
 
     /**
      * Annotations that couldn't be merged by mapping or postprocessing.
@@ -240,7 +240,7 @@ public function mergeProperties($object): void
                 $identity = method_exists($object, 'identity') ? $object->identity() : get_class($object);
                 $context1 = $this->_context;
                 $context2 = property_exists($object, '_context') ? $object->_context : 'unknown';
-                if (is_object($this->{$property}) && $this->{$property} instanceof AbstractAnnotation) {
+                if ($this->{$property} instanceof AbstractAnnotation) {
                     $context1 = $this->{$property}->_context;
                 }
                 $this->_context->logger->error('Multiple definitions for ' . $identity . '->' . $property . "\n     Using: " . $context1 . "\n  Skipping: " . $context2);
@@ -303,7 +303,7 @@ public function jsonSerialize()
 
         // Correct empty array to empty objects.
         foreach (static::$_types as $property => $type) {
-            if ($type === 'object' && is_array($data->{$property}) && empty($data->{$property})) {
+            if ($type === 'object' && is_array($data->{$property}) && $data->{$property} === []) {
                 $data->{$property} = new \stdClass();
             }
         }
@@ -334,11 +334,7 @@ public function jsonSerialize()
                 } else {
                     $key = $item->{$keyField};
                     if (!Generator::isDefault($key) && empty($object->{$key})) {
-                        if ($item instanceof \JsonSerializable) {
-                            $object->{$key} = $item->jsonSerialize();
-                        } else {
-                            $object->{$key} = $item;
-                        }
+                        $object->{$key} = $item instanceof \JsonSerializable ? $item->jsonSerialize() : $item;
                         unset($object->{$key}->{$keyField});
                     }
                 }
@@ -370,7 +366,7 @@ public function jsonSerialize()
 
                 // preserve other properties
                 foreach (get_object_vars($this) as $property => $value) {
-                    if ('_' == $property[0] || in_array($property, ['ref', 'nullable'])) {
+                    if ('_' === $property[0] || in_array($property, ['ref', 'nullable'])) {
                         continue;
                     }
                     if (!Generator::isDefault($value)) {
@@ -499,7 +495,7 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
         }
 
         if (property_exists($this, 'ref') && !Generator::isDefault($this->ref) && is_string($this->ref)) {
-            if (substr($this->ref, 0, 2) === '#/' && count($stack) > 0 && $stack[0] instanceof OpenApi) {
+            if (substr($this->ref, 0, 2) === '#/' && $stack !== [] && $stack[0] instanceof OpenApi) {
                 // Internal reference
                 try {
                     $stack[0]->ref($this->ref);
@@ -558,7 +554,7 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
             }
         }
 
-        return self::_validate($this, $stack, $skip, $ref, $context) ? $valid : false;
+        return self::_validate($this, $stack, $skip, $ref, $context) && $valid;
     }
 
     /**
@@ -664,7 +660,7 @@ public function getRoot(): string
      */
     public function isRoot(string $rootClass): bool
     {
-        return get_class($this) == $rootClass || $this->getRoot() == $rootClass;
+        return get_class($this) === $rootClass || $this->getRoot() === $rootClass;
     }
 
     /**
@@ -696,7 +692,7 @@ private function validateType(string $type, $value): bool
                 return false;
             }
             $itemType = substr($type, 1, -1);
-            foreach ($value as $i => $item) {
+            foreach ($value as $item) {
                 if ($this->validateType($itemType, $item) === false) {
                     return false;
                 }
@@ -760,7 +756,7 @@ private function validateArrayType($value): bool
             return false;
         }
         $count = 0;
-        foreach ($value as $i => $item) {
+        foreach (array_keys($value) as $i) {
             // not a array, but a hash/map
             if ($count !== $i) {
                 return false;

diff --git a/src/Annotations/Flow.php b/src/Annotations/Flow.php
@@ -97,7 +97,7 @@ class Flow extends AbstractAnnotation
     #[\ReturnTypeWillChange]
     public function jsonSerialize()
     {
-        if (is_array($this->scopes) && empty($this->scopes)) {
+        if ($this->scopes === []) {
             $this->scopes = new \stdClass();
         }
 

diff --git a/src/Annotations/OpenApi.php b/src/Annotations/OpenApi.php
@@ -178,11 +178,7 @@ public function saveAs(string $filename, string $format = 'auto'): void
             $format = strtolower(substr($filename, -5)) === '.json' ? 'json' : 'yaml';
         }
 
-        if (strtolower($format) === 'json') {
-            $content = $this->toJson();
-        } else {
-            $content = $this->toYaml();
-        }
+        $content = strtolower($format) === 'json' ? $this->toJson() : $this->toYaml();
 
         if (file_put_contents($filename, $content) === false) {
             throw new \Exception('Failed to saveAs("' . $filename . '", "' . $format . '")');
@@ -229,11 +225,9 @@ private static function resolveRef(string $ref, string $resolved, $container, ar
                 return $container->{$property};
             }
             $mapping = [];
-            if ($container instanceof AbstractAnnotation) {
-                foreach ($container::$_nested as $nestedClass => $nested) {
-                    if (is_string($nested) === false && count($nested) === 2 && $nested[0] === $property) {
-                        $mapping[$nestedClass] = $nested[1];
-                    }
+            foreach ($container::$_nested as $nestedClass => $nested) {
+                if (is_string($nested) === false && count($nested) === 2 && $nested[0] === $property) {
+                    $mapping[$nestedClass] = $nested[1];
                 }
             }
 

diff --git a/src/Annotations/Operation.php b/src/Annotations/Operation.php
@@ -224,7 +224,7 @@ public function validate(array $stack = [], array $skip = [], string $ref = '',
 
         if (!Generator::isDefault($this->responses)) {
             foreach ($this->responses as $response) {
-                if (!Generator::isDefault($response->response) && $response->response !== 'default' && preg_match('/^([12345]{1}[0-9]{2})|([12345]{1}XX)$/', (string) $response->response) === 0) {
+                if (!Generator::isDefault($response->response) && $response->response !== 'default' && preg_match('/^([12345]{1}\d{2})|([12345]{1}XX)$/', (string) $response->response) === 0) {
                     $this->_context->logger->warning('Invalid value "' . $response->response . '" for ' . $response->_identity([]) . '->response, expecting "default", a HTTP Status Code or HTTP Status Code range definition in ' . $response->_context);
                     $valid = false;
                 }

diff --git a/src/Context.php b/src/Context.php
@@ -96,7 +96,7 @@ public function with(string $property): ?Context
         if ($this->is($property)) {
             return $this;
         }
-        if ($this->parent !== null) {
+        if ($this->parent instanceof Context) {
             return $this->parent->with($property);
         }
 
@@ -108,7 +108,7 @@ public function with(string $property): ?Context
      */
     public function root(): Context
     {
-        if ($this->parent !== null) {
+        if ($this->parent instanceof Context) {
             return $this->parent->root();
         }
 
@@ -168,7 +168,7 @@ public function getDebugLocation(): string
      */
     public function __get(string $property)
     {
-        if ($this->parent !== null) {
+        if ($this->parent instanceof Context) {
             return $this->parent->{$property};
         }
 
@@ -213,7 +213,7 @@ public static function detect(int $index = 0): Context
         if (isset($caller['class'])) {
             $fqn = explode('\\', $caller['class']);
             $context->class = array_pop($fqn);
-            if (count($fqn)) {
+            if ($fqn !== []) {
                 $context->namespace = implode('\\', $fqn);
             }
         }
@@ -231,12 +231,7 @@ public function fullyQualifiedName(?string $source): string
             return '';
         }
 
-        if ($this->namespace) {
-            $namespace = str_replace('\\\\', '\\', '\\' . $this->namespace . '\\');
-        } else {
-            // global namespace
-            $namespace = '\\';
-        }
+        $namespace = $this->namespace ? str_replace('\\\\', '\\', '\\' . $this->namespace . '\\') : '\\';
 
         $thisSource = $this->class ?? $this->interface ?? $this->trait;
         if ($thisSource && strcasecmp($source, $thisSource) === 0) {