#213 implemented Assert::positiveInteger() to have psalm `positive-…

…int` assertions (#214)

* Implemented #213: `Assert::positiveInteger()` to have psalm `positive-int` assertions

* #213 use `static::valueToString()` instead of `static::typeToString()` for `Assert::positiveInteger()` failures

Ref: #214 (comment)

* Added `Assert::naturalNumber` to verify that a number is `positive-int|0`

* Upgraded to latest `vimeo/psalm`

* As suggested by @muglug, dropping `@mixin` and importing a trait as `Assert` type inference mechanism

Ref: https://symfony-devs.slack.com/archives/C8SFXTD2M/p1611064766060600

Quoting:

```
ocramius  2:59 PM
can anyone help me tackle down vimeo/psalm#4381 ?
Mostly trying to figure out whether the assertions are being read differently for @mixin and concrete instances 😐

muglug  3:18 PM
mixins are a massive hack, both whenever they're used but also in Psalm
anything that relies on magic methods to work is going to be more flaky, so I'd encourage you to think of a non-mixin approach if at all possible

ocramius  3:19 PM
yeah, I'm thinking of doing that indeed, just unsure about approach yet 😐
can't they somehow be unified with traits though?
they're the same thing, no?

muglug  3:22 PM
if they were, you'd just use a trait!

ocramius  3:22 PM
ack, gonna actually try that approach for webmozart/assert then 🙂

muglug  3:24 PM
with mixins the method that's actually being called is either __call or __callStatic, whereas with traits it's still that exact method

ocramius  3:24 PM
yes, I was just wondering if it could import the method as if it was a trait (at type level)
that would squash out *a lot* of code, but it's also true that a mixin does not have a trait definition
I think it makes sense to use the language feature for this 🙂
```

The `@mixin` support in `vimeo/psalm` is a massive hack, which also requires other tooling to increase complexity
in the static analysis parsing capabilities. In order to reduce that complexity, we instead rely on `Assert`
importing `Mixin` as a trait, which is much simpler and much more stable long-term.

While this indeed leads to `Mixin` being changed from an `interface` to a `trait`, that is not really a BC
break, as `Mixin` was explicitly documented to be used only as a type system aid, and not as an usable
symbol.

* Removed `Assert::naturalNumber()`, since we already have `Assert::natural()` doing the same thing

Note: we refined the assertion on `Assert::natural()` to correctly restrict to `positive-int|0`,
but we had to remove some minor test on `Assert::allNaturalNumber()` due to `@psalm-assert iterable<positive-int|0>`
not yet working in upstream.

See vimeo/psalm#5052

* Adjusted mixin generator to comply with current coding standards

* Upgraded `vimeo/psalm` to `4.6.1` to get a green build (note: `4.6.2` broken as per vimeo/psalm#5310)

While there is no guarantee that `vimeo/psalm:4.6.3` will fix the issue, we know for sure that
`vimeo/psalm:4.6.2` is broken, as per vimeo/psalm#5310, and we cannot
therefore rely on its static analysis there until upstream issue is fixed.

Meanwhile, this allows for installation of `webmozart/assert` with `vimeo/psalm:4.6.1` and `vimeo/psalm:>4.6.2`,
hoping for a brighter future.

README.md

@@ -89,6 +89,7 @@ Method                                                   | Description
 `stringNotEmpty($value, $message = '')`                  | Check that a value is a non-empty string
 `integer($value, $message = '')`                         | Check that a value is an integer
 `integerish($value, $message = '')`                      | Check that a value casts to an integer
+`positiveInteger($value, $message = '')`                 | Check that a value is a positive (non-zero) integer
 `float($value, $message = '')`                           | Check that a value is a float
 `numeric($value, $message = '')`                         | Check that a value is numeric
 `natural($value, $message= ''')`                         | Check that a value is a non-negative integer

bin/src/MixinGenerator.php

@@ -50,16 +50,19 @@ final class MixinGenerator
 
     public function generate(): string
     {
-        $file = "<?php\n\n";
-        $file .= $this->fileComment()."\n\n";
-        $file .= $this->namespace()."\n";
+        return \sprintf(
+            <<<'PHP'
+<?php
 
-        return $file;
-    }
+/**
+ * provides type inference and auto-completion for magic static methods of Assert.
+ */
 
-    private function fileComment(): string
-    {
-        return $this->phpdoc(['provides type inference and auto-completion for magic static methods of Assert.'], 0);
+%s
+PHP
+            ,
+            $this->namespace()
+        );
     }
 
     private function namespace(): string
@@ -74,16 +77,15 @@ private function namespace(): string
         $namespace .= sprintf("use %s;\n", Throwable::class);
         $namespace .= "\n";
 
-        $namespace .= $this->interface($assert);
+        $namespace .= $this->trait($assert);
 
         return $namespace;
     }
 
-    private function interface(ReflectionClass $assert): string
+    private function trait(ReflectionClass $assert): string
     {
         $staticMethods = $this->getMethods($assert);
 
-        $phpdocMethods = [];
         $declaredMethods = [];
 
         foreach ($staticMethods as $method) {
@@ -98,18 +100,21 @@ private function interface(ReflectionClass $assert): string
             }
         }
 
-        $interface = '';
-
-        if (count($phpdocMethods) > 0) {
-            $interface = $this->phpdoc($phpdocMethods, 0)."\n";
-        }
-
-        $interface .= "interface Mixin\n{\n";
-        $interface .= implode("\n\n", $declaredMethods);
-
-        $interface .= "\n}";
+        return \sprintf(
+            <<<'PHP'
+/**
+ * This trait aids static analysis tooling in introspecting assertion magic methods.
+ * Do not use this trait directly: it will change, and is not designed for reuse.
+ */
+trait Mixin
+{
+%s
+}
 
-        return $interface;
+PHP
+            ,
+            implode("\n\n", $declaredMethods)
+        );
     }
 
     /**
@@ -329,7 +334,15 @@ private function staticMethod(string $name, array $parameters, array $defaultVal
         $indentation = str_repeat(' ', $indent);
 
         $staticFunction = $this->phpdoc($phpdocLines, $indent)."\n";
-        $staticFunction .= $indentation.'public static function '.$name.$this->functionParameters($parameters, $defaultValues).';';
+        $staticFunction .= $indentation.'public static function '.$name.$this->functionParameters($parameters, $defaultValues)."\n"
+            .$indentation."{\n"
+            .$indentation.$indentation.'static::__callStatic('
+            .'\'' . $name . '\', array('
+            .implode(', ', \array_map(static function (string $parameter): string {
+                return '$'. $parameter;
+            }, $parameters))
+            ."));\n"
+            .$indentation.'}';
 
         return $staticFunction;
     }
@@ -382,7 +395,9 @@ private function phpdoc(array $lines, int $indent): string
             $phpdoc .= "\n".$indentation.rtrim(' * '.$line);
         }
 
-        $phpdoc .= "\n".$indentation.' */';
+        $phpdoc .= "\n".$indentation.' *'
+            . "\n".$indentation.' * @return void'
+            . "\n".$indentation.' */';
 
         return $phpdoc;
     }
@@ -452,6 +467,11 @@ private function getMethods(ReflectionClass $assert): array
                 continue;
             }
 
+            if ($staticMethod->getFileName() !== $assert->getFileName()) {
+                // Skip this method - was imported by generated mixin
+                continue;
+            }
+
             if ('__callStatic' === $staticMethod->name) {
                 continue;
             }

ci/composer.json

@@ -1,5 +1,5 @@
 {
     "require": {
-        "vimeo/psalm": "dev-master#61f5a06"
+        "vimeo/psalm": "4.6.1"
     }
 }