Restricting internal types usages to tests/, included `psalm/plugin…

…-phpunit` This change ensures that we don't use `@internal` symbols in `src/` (too dangerous, for long-term usage), and makes the type checking much stricter, thanks to `psalm/plugin-phpunit`. Since `vimeo/psalm` has been upgraded, and vimeo/psalm#2772 no longer applies, we can also remove a lot of manually written code that was safe to clean up.
Roave · Dec 5, 2021 · ed882dd · ed882dd
1 parent f8c25b3
commit ed882dd
Show file tree

Hide file tree

Showing 41 changed files with 268 additions and 281 deletions.
diff --git a/composer.json b/composer.json
@@ -26,6 +26,7 @@
         "doctrine/coding-standard": "^9.0.0",
         "php-standard-library/psalm-plugin": "^1.1.1",
         "phpunit/phpunit": "^9.5.10",
+        "psalm/plugin-phpunit": "^0.16.1",
         "roave/infection-static-analysis-plugin": "^1.10",
         "roave/security-advisories": "dev-master",
         "squizlabs/php_codesniffer": "^3.6.1",

diff --git a/composer.lock b/composer.lock
diff --git a/psalm.xml b/psalm.xml
@@ -11,15 +11,31 @@
         <directory name="src"/>
         <directory name="test/e2e"/>
         <directory name="test/unit"/>
+        <ignoreFiles>
+            <directory name="vendor"/>
+        </ignoreFiles>
     </projectFiles>
 
     <issueHandlers>
-        <PropertyNotSetInConstructor errorLevel="suppress"/>
-        <InternalClass errorLevel="suppress"/>
-        <InternalMethod errorLevel="suppress"/>
+        <PropertyNotSetInConstructor>
+            <errorLevel type="suppress">
+                <directory name="test"/>
+            </errorLevel>
+        </PropertyNotSetInConstructor>
+        <InternalClass errorLevel="suppress">
+            <errorLevel type="suppress">
+                <directory name="test"/>
+            </errorLevel>
+        </InternalClass>
+        <InternalMethod>
+            <errorLevel type="suppress">
+                <directory name="test"/>
+            </errorLevel>
+        </InternalMethod>
     </issueHandlers>
 
     <plugins>
-        <pluginClass class="Psl\Psalm\Plugin" />
+        <pluginClass class="Psl\Psalm\Plugin"/>
+        <pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
     </plugins>
 </psalm>
diff --git a/src/DetectChanges/BCBreak/FunctionBased/FunctionBased.php b/src/DetectChanges/BCBreak/FunctionBased/FunctionBased.php
@@ -11,10 +11,10 @@
 interface FunctionBased
 {
     /**
-     * @template T of ReflectionMethod|ReflectionFunction
-     *
      * @param T $fromFunction
      * @param T $toFunction
+     *
+     * @template T of ReflectionMethod|ReflectionFunction
      */
     public function __invoke(
         ReflectionMethod|ReflectionFunction $fromFunction,

diff --git a/src/DetectChanges/BCBreak/FunctionBased/MultipleChecksOnAFunction.php b/src/DetectChanges/BCBreak/FunctionBased/MultipleChecksOnAFunction.php
@@ -27,12 +27,12 @@ public function __invoke(
     }
 
     /**
-     * @template T of ReflectionMethod|ReflectionFunction
-     *
      * @param T $fromFunction
      * @param T $toFunction
      *
      * @return iterable<int, Change>
+     *
+     * @template T of ReflectionMethod|ReflectionFunction
      */
     private function multipleChecks(
         ReflectionMethod|ReflectionFunction $fromFunction,

diff --git a/src/DetectChanges/BCBreak/PropertyBased/PropertyTypeChanged.php b/src/DetectChanges/BCBreak/PropertyBased/PropertyTypeChanged.php
@@ -5,16 +5,13 @@
 namespace Roave\BackwardCompatibility\DetectChanges\BCBreak\PropertyBased;
 
 use Psl\Str;
-use Psl\Vec;
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\Changes;
 use Roave\BackwardCompatibility\DetectChanges\Variance\TypeIsContravariant;
 use Roave\BackwardCompatibility\DetectChanges\Variance\TypeIsCovariant;
 use Roave\BackwardCompatibility\Formatter\ReflectionPropertyName;
 use Roave\BetterReflection\Reflection\ReflectionProperty;
 
-use function array_merge;
-
 /**
  * Type declarations for properties are invariant: you can't restrict the type because the consumer may
  * write invalid values to it, and you cannot widen the type because the consumer may expect a specific

diff --git a/src/Git/CheckedOutRepository.php b/src/Git/CheckedOutRepository.php
@@ -9,20 +9,15 @@
 
 final class CheckedOutRepository
 {
-    private string $path;
-
-    private function __construct()
+    private function __construct(private string $path)
     {
     }
 
     public static function fromPath(string $path): self
     {
         Psl\invariant(Filesystem\is_directory($path . '/.git'), 'Directory "%s" is not a GIT repository.', $path);
 
-        $instance       = new self();
-        $instance->path = $path;
-
-        return $instance;
+        return new self($path);
     }
 
     public function __toString(): string

diff --git a/src/Git/Revision.php b/src/Git/Revision.php
@@ -7,23 +7,23 @@
 use Psl;
 use Psl\Regex;
 use Psl\Str;
+use Psl\Type;
 
 final class Revision
 {
-    private string $sha1;
-
-    private function __construct()
+    /** @param non-empty-string $sha1 */
+    private function __construct(private string $sha1)
     {
     }
 
     public static function fromSha1(string $sha1): self
     {
         Psl\invariant(Regex\matches($sha1, '/^[a-zA-Z0-9]{40}$/'), 'Invalid SHA1 hash.');
 
-        $instance       = new self();
-        $instance->sha1 = Str\trim_right($sha1);
-
-        return $instance;
+        return new self(
+            Type\non_empty_string()
+                ->assert(Str\trim_right($sha1))
+        );
     }
 
     public function __toString(): string

diff --git a/test/unit/DetectChanges/BCBreak/ClassBased/AncestorRemovedTest.php b/test/unit/DetectChanges/BCBreak/ClassBased/AncestorRemovedTest.php
@@ -132,14 +132,11 @@ class ClassWithInvertedInterfaceNames implements IG, IE {}
         return array_combine(
             array_keys($classes),
             array_map(
-                /** @psalm-param list<string> $errors https://github.com/vimeo/psalm/issues/2772 */
-                static function (string $className, array $errors) use ($fromReflector, $toReflector): array {
-                    return [
-                        $fromReflector->reflectClass($className),
-                        $toReflector->reflectClass($className),
-                        $errors,
-                    ];
-                },
+                static fn (string $className, array $errors): array => [
+                    $fromReflector->reflectClass($className),
+                    $toReflector->reflectClass($className),
+                    $errors,
+                ],
                 array_keys($classes),
                 $classes
             )

diff --git a/test/unit/DetectChanges/BCBreak/ClassBased/ClassBecameAbstractTest.php b/test/unit/DetectChanges/BCBreak/ClassBased/ClassBecameAbstractTest.php
@@ -99,14 +99,11 @@ interface AbstractToInterface {}
         return array_combine(
             array_keys($classes),
             array_map(
-                /** @psalm-param list<string> $errors https://github.com/vimeo/psalm/issues/2772 */
-                static function (string $className, array $errors) use ($fromReflector, $toReflector): array {
-                    return [
-                        $fromReflector->reflectClass($className),
-                        $toReflector->reflectClass($className),
-                        $errors,
-                    ];
-                },
+                static fn (string $className, array $errors): array => [
+                    $fromReflector->reflectClass($className),
+                    $toReflector->reflectClass($className),
+                    $errors,
+                ],
                 array_keys($classes),
                 $classes
             )

diff --git a/test/unit/DetectChanges/BCBreak/ClassBased/ClassBecameInterfaceTest.php b/test/unit/DetectChanges/BCBreak/ClassBased/ClassBecameInterfaceTest.php
@@ -105,14 +105,11 @@ trait TraitToTrait {}
         return array_combine(
             array_keys($classes),
             array_map(
-                /** @psalm-param list<string> $errors https://github.com/vimeo/psalm/issues/2772 */
-                static function (string $className, array $errors) use ($fromReflector, $toReflector): array {
-                    return [
-                        $fromReflector->reflectClass($className),
-                        $toReflector->reflectClass($className),
-                        $errors,
-                    ];
-                },
+                static fn (string $className, array $errors): array => [
+                    $fromReflector->reflectClass($className),
+                    $toReflector->reflectClass($className),
+                    $errors,
+                ],
                 array_keys($classes),
                 $classes
             )

diff --git a/test/unit/DetectChanges/BCBreak/ClassBased/ClassBecameTraitTest.php b/test/unit/DetectChanges/BCBreak/ClassBased/ClassBecameTraitTest.php
@@ -108,14 +108,11 @@ trait TraitToTrait {}
         return array_combine(
             array_keys($classes),
             array_map(
-                /** @psalm-param list<string> $errors https://github.com/vimeo/psalm/issues/2772 */
-                static function (string $className, array $errors) use ($fromReflector, $toReflector): array {
-                    return [
-                        $fromReflector->reflectClass($className),
-                        $toReflector->reflectClass($className),
-                        $errors,
-                    ];
-                },
+                static fn (string $className, array $errors): array => [
+                    $fromReflector->reflectClass($className),
+                    $toReflector->reflectClass($className),
+                    $errors,
+                ],
                 array_keys($classes),
                 $classes
             )

diff --git a/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantValueChangedTest.php b/test/unit/DetectChanges/BCBreak/ClassConstantBased/ClassConstantValueChangedTest.php
@@ -5,6 +5,7 @@
 namespace RoaveTest\BackwardCompatibility\DetectChanges\BCBreak\ClassConstantBased;
 
 use PHPUnit\Framework\TestCase;
+use Psl\Type;
 use Roave\BackwardCompatibility\Change;
 use Roave\BackwardCompatibility\DetectChanges\BCBreak\ClassConstantBased\ClassConstantValueChanged;
 use Roave\BetterReflection\BetterReflection;
@@ -25,7 +26,7 @@ final class ClassConstantValueChangedTest extends TestCase
     /**
      * @param string[] $expectedMessages
      *
-     * @dataProvider propertiesToBeTested
+     * @dataProvider constantsToBeTested
      */
     public function testDiffs(
         ReflectionClassConstant $fromConstant,
@@ -43,10 +44,13 @@ public function testDiffs(
     }
 
     /**
-     * @return array<string, array<int, ReflectionClassConstant|array<int, string>>>
-     * @psalm-return array<string, array{0: ReflectionClassConstant|null, 1: ReflectionClassConstant|null, 2: list<string>}>
+     * @return array<string, array{
+     *     0: ReflectionClassConstant,
+     *     1: ReflectionClassConstant,
+     *     2: list<string>
+     * }>
      */
-    public function propertiesToBeTested(): array
+    public function constantsToBeTested(): array
     {
         $astLocator = (new BetterReflection())->astLocator();
 
@@ -123,14 +127,13 @@ class TheClass {
         return array_combine(
             array_keys($properties),
             array_map(
-                /** @psalm-param list<string> $errorMessages https://github.com/vimeo/psalm/issues/2772 */
-                static function (string $constant, array $errorMessages) use ($fromClass, $toClass): array {
-                    return [
-                        $fromClass->getReflectionConstant($constant),
-                        $toClass->getReflectionConstant($constant),
-                        $errorMessages,
-                    ];
-                },
+                static fn (string $constant, array $errorMessages): array => [
+                    Type\object(ReflectionClassConstant::class)
+                        ->coerce($fromClass->getReflectionConstant($constant)),
+                    Type\object(ReflectionClassConstant::class)
+                        ->coerce($toClass->getReflectionConstant($constant)),
+                    $errorMessages,
+                ],
                 array_keys($properties),
                 $properties
             )