Psalm refactoring

Author: danog

README.md

@@ -271,7 +271,14 @@ Overriding the following methods in your assertion class allows you to change th
 ## Static analysis support
 
 Where applicable, assertion functions are annotated to support Psalm's
-[Assertion syntax](https://psalm.dev/docs/annotating_code/assertion_syntax/).
+[Assertion syntax](https://psalm.dev/docs/annotating_code/assertion_syntax/).  
+
+A native Psalm plugin can be enabled to also add type inference for return types (new in 2.x):
+
+```php
+vendor/bin/psalm-plugin enable webmozart/assert
+```
+
 A dedicated [PHPStan Plugin](https://github.com/phpstan/phpstan-webmozart-assert) is
 required for proper type support.
 

bin/generate.php

@@ -8,9 +8,11 @@
  */
 
 use Webmozart\Assert\Bin\MixinGenerator;
+use Webmozart\Assert\Bin\StaticAnalysisNonReturnGenerator;
 
 require_once __DIR__.'/../vendor/autoload.php';
 
 file_put_contents(__DIR__.'/../src/Mixin.php', (new MixinGenerator())->generate());
+(new StaticAnalysisNonReturnGenerator(__DIR__.'/../tests/static-analysis'))->generate();
 
 echo "Done.";

bin/src/MixinGenerator.php

@@ -76,6 +76,7 @@ private function namespace(): string
         $assert = new ReflectionClass(Assert::class);
 
         $namespace = sprintf("namespace %s;\n\n", $assert->getNamespaceName());
+        $namespace .= sprintf("use %s;\n", \Closure::class);
         $namespace .= sprintf("use %s;\n", ArrayAccess::class);
         $namespace .= sprintf("use %s;\n", Countable::class);
         $namespace .= sprintf("use %s;\n", Throwable::class);
@@ -231,11 +232,11 @@ private function assertion(ReflectionMethod $method, string $methodNameTemplate,
             }
 
             if ($parameterReflection->hasType()) {
-                if ($parameterReflection->name === 'value') {
+                if (count($parameters) === 1) {
                     $parameterTypes[$parameterReflection->name] = 'mixed';
 
                     $nativeReturnType = match ($typeTemplate) {
-                        '%s|null' => $this->reduceParameterType($parameterReflection->getType()),
+                        '%s|null' => $this->nullableReturnType($method->getReturnType()),
                         'iterable<%s>' => 'iterable',
                         'iterable<%s|null>' => 'iterable',
                     };
@@ -245,6 +246,12 @@ private function assertion(ReflectionMethod $method, string $methodNameTemplate,
             }
         }
 
+        // Ensure @template comes before @param, and @param values match function signature order
+        $parsedComment = $this->reorderAnnotations($parsedComment);
+        if (isset($parsedComment['param'])) {
+            $parsedComment['param'] = $this->reorderParamsBySignature($parsedComment['param'], $parameters);
+        }
+
         if (in_array($newMethodName, $this->skipMethods, true)) {
             return null;
         }
@@ -253,6 +260,13 @@ private function assertion(ReflectionMethod $method, string $methodNameTemplate,
 
         $phpdocReturnType = 'mixed';
 
+        $templateTypeNames = [];
+        if (isset($parsedComment['template'])) {
+            foreach ($parsedComment['template'] as $template) {
+                $templateTypeNames[] = explode(' ', $template)[0];
+            }
+        }
+
         $phpdocLines = [];
         foreach ($parsedComment as $key => $values) {
             if ($this->shouldSkipAnnotation($newMethodName, $key)) {
@@ -275,14 +289,24 @@ private function assertion(ReflectionMethod $method, string $methodNameTemplate,
 
             foreach ($values as $i => $value) {
                 $parts = $this->splitDocLine($value);
-                if (('param' === $key || 'psalm-param' === $key) && isset($parts[1]) && isset($parameters[0]) && $parts[1] === '$'.$parameters[0] && 'mixed' !== $parts[0]) {
-                    $parts[0] = $this->applyTypeTemplate($parts[0], $typeTemplate);
+                if ('param' === $key && isset($parts[1]) && isset($parameters[0]) && $parts[1] === '$'.$parameters[0] && 'mixed' !== $parts[0]) {
+                    $parts[0] = $this->applyTypeTemplate($parts[0], $typeTemplate, $templateTypeNames);
 
                     $values[$i] = \implode(' ', $parts);
+
+                    if ('mixed' === $phpdocReturnType) {
+                        $phpdocReturnType = $parts[0];
+                    }
                 }
             }
 
-            if ('psalm-return' === $key || 'return' === $key) {
+            if ('return' === $key) {
+                foreach ($values as $value) {
+                    $parts = $this->splitDocLine($value);
+                    if ('mixed' !== $parts[0]) {
+                        $phpdocReturnType = $this->applyTypeTemplate($parts[0], $typeTemplate, $templateTypeNames);
+                    }
+                }
                 continue;
             }
 
@@ -294,8 +318,12 @@ private function assertion(ReflectionMethod $method, string $methodNameTemplate,
                 $parts = $this->splitDocLine($value);
                 $type = $parts[0];
 
+                if ('template' === $key && 'iterable<%s|null>' === $typeTemplate) {
+                    $type = preg_replace('/^(\S+\s+(?:of|as)\s+)(.+)$/', '$1$2|null', $type) ?? $type;
+                }
+
                 if ('psalm-assert' === $key) {
-                    $type = $this->applyTypeTemplate($type, $typeTemplate);
+                    $type = $this->applyTypeTemplate($type, $typeTemplate, $templateTypeNames);
 
                     $phpdocReturnType = $type;
                 }
@@ -322,6 +350,20 @@ private function assertion(ReflectionMethod $method, string $methodNameTemplate,
             }
         }
 
+        if ('mixed' === $phpdocReturnType) {
+            $returnType = $method->getReturnType();
+            if ($returnType !== null) {
+                $returnTypeStr = $this->reduceParameterType($returnType);
+                if ('mixed' !== $returnTypeStr) {
+                    $phpdocReturnType = $this->applyTypeTemplate($returnTypeStr, $typeTemplate, $templateTypeNames);
+                }
+            }
+        }
+
+        if ('mixed' === $phpdocReturnType && 'mixed' !== $nativeReturnType) {
+            $phpdocReturnType = $nativeReturnType;
+        }
+
         $phpdocLines[] = '@return '.$phpdocReturnType;
         $phpdocLines[] = '';
 
@@ -351,7 +393,13 @@ private function reduceParameterType(ReflectionType $type): string
             return \implode('|', \array_map([$this, 'reduceParameterType'], $type->getTypes()));
         }
 
-        $type = Assert::isInstanceOf($type, ReflectionNamedType::class);
+        if (!$type instanceof ReflectionNamedType) {
+            throw new RuntimeException(sprintf(
+                'Expected a "%s" instance, got "%s".',
+                ReflectionNamedType::class,
+                get_debug_type($type)
+            ));
+        }
 
         if ($type->getName() === 'mixed') {
             return $type->getName();
@@ -360,8 +408,25 @@ private function reduceParameterType(ReflectionType $type): string
         return ($type->allowsNull() ? '?' : '') . $type->getName();
     }
 
-    private function applyTypeTemplate(string $type, string $typeTemplate): string
+    private function nullableReturnType(?ReflectionType $type): string
+    {
+        if ($type === null) {
+            return 'mixed';
+        }
+        $typeStr = $this->reduceParameterType($type);
+        if ($typeStr === 'mixed') {
+            return 'mixed';
+        }
+
+        return $typeStr.'|null';
+    }
+
+    private function applyTypeTemplate(string $type, string $typeTemplate, array $templateTypeNames = []): string
     {
+        if (in_array($type, $templateTypeNames, true) && str_contains($typeTemplate, 'iterable') && str_contains($typeTemplate, '|null')) {
+            $typeTemplate = str_replace('|null', '', $typeTemplate);
+        }
+
         $combinedType = sprintf($typeTemplate, $type);
 
         if ('empty|null' === $combinedType) {
@@ -377,7 +442,7 @@ private function shouldSkipAnnotation(string $newMethodName, string $key): bool
             return false;
         }
 
-        return 'psalm-assert' === $key || 'psalm-return' === $key;
+        return 'psalm-assert' === $key;
     }
 
     /**
@@ -557,6 +622,70 @@ private function splitDocLine(string $line): array
         return [trim($matches[1]), $matches[2], $matches[3] ?? null];
     }
 
+    /**
+     * Ensures @template annotations appear before @param annotations.
+     *
+     * @param array<string, list<string>> $annotations
+     *
+     * @return array<string, list<string>>
+     */
+    private function reorderAnnotations(array $annotations): array
+    {
+        $keys = array_keys($annotations);
+        $templatePos = array_search('template', $keys, true);
+        $paramPos = array_search('param', $keys, true);
+
+        if ($templatePos === false || $paramPos === false || $templatePos < $paramPos) {
+            return $annotations;
+        }
+
+        $result = [];
+        foreach ($annotations as $key => $values) {
+            if ($key === 'param') {
+                $result['template'] = $annotations['template'];
+            }
+            if ($key !== 'template') {
+                $result[$key] = $values;
+            }
+        }
+
+        return $result;
+    }
+
+    /**
+     * Reorders @param doc entries to match the function signature parameter order.
+     *
+     * @param list<string> $paramDocs
+     * @param list<string> $parameterNames
+     *
+     * @return list<string>
+     */
+    private function reorderParamsBySignature(array $paramDocs, array $parameterNames): array
+    {
+        $byVarName = [];
+        $withoutVarName = [];
+
+        foreach ($paramDocs as $doc) {
+            $parts = $this->splitDocLine($doc);
+            if (isset($parts[1])) {
+                $byVarName[$parts[1]] = $doc;
+            } else {
+                $withoutVarName[] = $doc;
+            }
+        }
+
+        $ordered = [];
+        foreach ($parameterNames as $name) {
+            $key = '$'.$name;
+            if (isset($byVarName[$key])) {
+                $ordered[] = $byVarName[$key];
+                unset($byVarName[$key]);
+            }
+        }
+
+        return array_merge($ordered, array_values($byVarName), $withoutVarName);
+    }
+
     /**
      * @psalm-return list<ReflectionMethod>
      *

bin/src/StaticAnalysisNonReturnGenerator.php

@@ -0,0 +1,117 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Webmozart\Assert\Bin;
+
+use RuntimeException;
+
+final class StaticAnalysisNonReturnGenerator
+{
+    private const SOURCE_NAMESPACE = 'namespace Webmozart\\Assert\\Tests\\StaticAnalysis;';
+    private const TARGET_NAMESPACE = 'namespace Webmozart\\Assert\\Tests\\StaticAnalysis\\NonReturn;';
+
+    public function __construct(
+        private string $staticAnalysisDirectory
+    ) {
+    }
+
+    public function generate(): void
+    {
+        $targetDirectory = $this->targetDirectory();
+
+        if (!is_dir($targetDirectory) && !mkdir($targetDirectory, 0777, true) && !is_dir($targetDirectory)) {
+            throw new RuntimeException(sprintf('Could not create "%s".', $targetDirectory));
+        }
+
+        foreach ($this->generatedFiles() as $targetFile => $content) {
+            if (false === file_put_contents($targetFile, $content)) {
+                throw new RuntimeException(sprintf('Could not write "%s".', $targetFile));
+            }
+        }
+    }
+
+    /**
+     * @return array<string, string>
+     */
+    public function generatedFiles(): array
+    {
+        $generatedFiles = [];
+
+        foreach ($this->sourceFiles() as $sourceFile) {
+            $content = file_get_contents($sourceFile);
+            if (false === $content) {
+                throw new RuntimeException(sprintf('Could not read "%s".', $sourceFile));
+            }
+
+            $generatedFiles[$this->targetDirectory().'/'.basename($sourceFile)] = $this->generateFileContent($content, $sourceFile);
+        }
+
+        return $generatedFiles;
+    }
+
+    public function generateFileContent(string $content, string $sourceFile): string
+    {
+        $content = str_replace(self::SOURCE_NAMESPACE, self::TARGET_NAMESPACE, $content);
+
+        $replacementCount = 0;
+        $generatedContent = preg_replace_callback(
+            '/^(\s*)return Assert::([A-Za-z0-9_]+)\((\$[A-Za-z_][A-Za-z0-9_]*)([^;\n]*)\);$/m',
+            static function (array $matches): string {
+                return sprintf(
+                    "%sAssert::%s(%s%s);\n\n%sreturn %s;",
+                    $matches[1],
+                    $matches[2],
+                    $matches[3],
+                    $matches[4],
+                    $matches[1],
+                    $matches[3]
+                );
+            },
+            $content,
+            -1,
+            $replacementCount
+        );
+
+        if (null === $generatedContent) {
+            throw new RuntimeException(sprintf('Could not generate non-return static analysis test from "%s".', $sourceFile));
+        }
+
+        $generatedContent = preg_replace("/;\n{3,}([ \t]+return )/", ";\n\n$1", $generatedContent);
+        if (null === $generatedContent) {
+            throw new RuntimeException(sprintf('Could not normalize non-return static analysis test from "%s".', $sourceFile));
+        }
+
+        $expectedReplacementCount = substr_count($content, 'return Assert::');
+        if ($replacementCount !== $expectedReplacementCount) {
+            throw new RuntimeException(sprintf(
+                'Expected to replace %d return assertions in "%s", replaced %d.',
+                $expectedReplacementCount,
+                $sourceFile,
+                (int) $replacementCount
+            ));
+        }
+
+        return $generatedContent;
+    }
+
+    /**
+     * @return list<string>
+     */
+    private function sourceFiles(): array
+    {
+        $files = glob($this->staticAnalysisDirectory.'/assert-*.php');
+        if (false === $files) {
+            throw new RuntimeException(sprintf('Could not scan "%s".', $this->staticAnalysisDirectory));
+        }
+
+        sort($files);
+
+        return $files;
+    }
+
+    private function targetDirectory(): string
+    {
+        return $this->staticAnalysisDirectory.'/non-return';
+    }
+}