Fixup some psalm return types

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

@@ -27,6 +27,8 @@ final class MixinGenerator
     private array $unsupportedMethods = [
         'nullOrNotInstanceOf',  // not supported by psalm (https://github.com/vimeo/psalm/issues/3443)
         'allNotInstanceOf',     // not supported by psalm (https://github.com/vimeo/psalm/issues/3443)
+        'allIsNotInstanceOfAny',
+        'allNullOrIsNotInstanceOfAny',
         'nullOrNotEmpty',       // not supported by psalm (https://github.com/vimeo/psalm/issues/3443)
         'allNotEmpty',          // not supported by psalm (https://github.com/vimeo/psalm/issues/3443)
         'allNotNull',           // not supported by psalm (https://github.com/vimeo/psalm/issues/3443)
@@ -76,6 +78,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 +234,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 +248,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;
         }
@@ -275,14 +284,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]) {
+                if ('param' === $key && isset($parts[1]) && isset($parameters[0]) && $parts[1] === '$'.$parameters[0] && 'mixed' !== $parts[0]) {
                     $parts[0] = $this->applyTypeTemplate($parts[0], $typeTemplate);
 
                     $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);
+                    }
+                }
                 continue;
             }
 
@@ -322,6 +341,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);
+                }
+            }
+        }
+
+        if ('mixed' === $phpdocReturnType && 'mixed' !== $nativeReturnType) {
+            $phpdocReturnType = $nativeReturnType;
+        }
+
         $phpdocLines[] = '@return '.$phpdocReturnType;
         $phpdocLines[] = '';
 
@@ -351,7 +384,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,6 +399,19 @@ private function reduceParameterType(ReflectionType $type): string
         return ($type->allowsNull() ? '?' : '') . $type->getName();
     }
 
+    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): string
     {
         $combinedType = sprintf($typeTemplate, $type);
@@ -377,7 +429,7 @@ private function shouldSkipAnnotation(string $newMethodName, string $key): bool
             return false;
         }
 
-        return 'psalm-assert' === $key || 'psalm-return' === $key;
+        return 'psalm-assert' === $key;
     }
 
     /**
@@ -557,6 +609,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';
+    }
+}

composer.json

@@ -40,8 +40,12 @@
         }
     },
     "extra": {
+        "psalm": {
+            "pluginClass": "Webmozart\\Assert\\PsalmPlugin"
+        },
         "branch-alias": {
-            "dev-feature/2-0": "2.0-dev"
+            "dev-feature/2-0": "2.0-dev",
+            "dev-master": "2.0-dev"
         }
     },
     "scripts": {
@@ -63,5 +67,8 @@
         "cs-fix": "./tools/php-cs-fixer/vendor/bin/php-cs-fixer fix",
         "static-analysis": "./tools/psalm/vendor/bin/psalm --threads=4 --root=$(pwd)",
         "test": "./tools/phpunit/vendor/bin/phpunit"
+    },
+    "require-dev": {
+        "vimeo/psalm": "^6"
     }
 }

psalm.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
 <psalm
-    errorLevel="8"
+    errorLevel="1"
     resolveFromConfigFile="true"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
     xmlns="https://getpsalm.org/schema/config"
@@ -10,4 +10,8 @@
         <directory name="bin"/>
         <directory name="tests/static-analysis" />
     </projectFiles>
+
+    <plugins>
+        <plugin filename="src/PsalmPlugin.php" />
+    </plugins>
 </psalm>