Gh 2588 false not bool and show project + runtime php versions in LSP…

… status (#2591) * Add passing test for false type * Show PHP version and source in status * Fix test case name * GH-2588: Do not generalize types for return and PHP version in status * Update CL
phpactor · Mar 9, 2024 · 9e7b375 · 9e7b375
1 parent 5dc22be
commit 9e7b375
Show file tree

Hide file tree

Showing 14 changed files with 167 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,9 +21,11 @@ Improvements:
   - Remove "on develop warning" service #2533
   - Disable the processing of includes/requires, it doesn't work very well but
     it has massive performance impact on certain projects #2580
+  - Include project PHP and runtime version and LSP status
 
 Bug fixes:
 
+  - Do not generalize generated return types (i.e. false instead of bool) #2588
   - Fix diagnostic process concurrency and do not lint outdated files #2538
   - Upgrade `amp/process` to fix #2516 thanks to @gerardroche
   - Fix division by zero edge case

diff --git a/lib/CodeTransform/Adapter/WorseReflection/Transformer/UpdateReturnTypeTransformer.php b/lib/CodeTransform/Adapter/WorseReflection/Transformer/UpdateReturnTypeTransformer.php
@@ -40,7 +40,7 @@ public function transform(SourceCode $code): Promise
                 $classBuilder = $builder->class($method->class()->name()->short());
                 $methodBuilder = $classBuilder->method($method->name());
                 $replacement = $this->returnType($method);
-                $localReplacement = $replacement->toLocalType($method->scope())->generalize();
+                $localReplacement = $replacement->toLocalType($method->scope());
                 $notNullReplacement = $replacement->stripNullable();
 
                 foreach ($replacement->allTypes()->classLike() as $classType) {

diff --git a/...factor/AbstractTolerantImportNameTest.php → ...factor/AbstractTolerantImportNameCase.php b/...factor/AbstractTolerantImportNameTest.php → ...factor/AbstractTolerantImportNameCase.php
@@ -13,7 +13,7 @@
 use Phpactor\CodeTransform\Tests\Adapter\TolerantParser\TolerantTestCase;
 use Phpactor\TextDocument\TextEdits;
 
-abstract class AbstractTolerantImportNameTest extends TolerantTestCase
+abstract class AbstractTolerantImportNameCase extends TolerantTestCase
 {
     /**
      * @dataProvider provideImportClass
@@ -204,7 +204,10 @@ abstract public function provideImportFunction(): Generator;
 
     abstract protected function importName(string $source, int $offset, NameImport $nameImport, bool $importGlobals = true): TextEdits;
 
-    private function importNameFromTestFile(string $type, string $test, string $name, string $alias = null)
+    /**
+     * @return array{string,string}
+     */
+    private function importNameFromTestFile(string $type, string $test, string $name, string $alias = null): array
     {
         [$source, $expected, $offset] = $this->sourceExpectedAndOffset(__DIR__ . '/fixtures/' . $test);
         $edits = TextEdits::none();

diff --git a/lib/CodeTransform/Tests/Adapter/TolerantParser/Refactor/TolerantImportNameOnlyTest.php b/lib/CodeTransform/Tests/Adapter/TolerantParser/Refactor/TolerantImportNameOnlyTest.php
@@ -9,7 +9,7 @@
 use Phpactor\TextDocument\ByteOffset;
 use Phpactor\TextDocument\TextEdits;
 
-class TolerantImportNameOnlyTest extends AbstractTolerantImportNameTest
+class TolerantImportNameOnlyTest extends AbstractTolerantImportNameCase
 {
     public function provideImportClass(): Generator
     {

diff --git a/lib/CodeTransform/Tests/Adapter/TolerantParser/Refactor/TolerantImportNameTest.php b/lib/CodeTransform/Tests/Adapter/TolerantParser/Refactor/TolerantImportNameTest.php
@@ -9,7 +9,7 @@
 use Phpactor\TextDocument\ByteOffset;
 use Phpactor\TextDocument\TextEdits;
 
-class TolerantImportNameTest extends AbstractTolerantImportNameTest
+class TolerantImportNameTest extends AbstractTolerantImportNameCase
 {
     public function provideImportClass(): Generator
     {

diff --git a/...deTransform/Tests/Adapter/WorseReflection/Transformer/UpdateReturnTypeTransformerTest.php b/...deTransform/Tests/Adapter/WorseReflection/Transformer/UpdateReturnTypeTransformerTest.php
@@ -175,6 +175,74 @@ private function baz(): void
                 }
                 EOT
         ];
+        yield 'adds false type' => [
+            <<<'EOT'
+                <?php
+
+                class Foobar {
+                    private function foo()
+                    {
+                        return $this->baz();
+                    }
+
+                    private function baz(): string|false
+                    {
+                    }
+                }
+                EOT
+            ,
+            <<<'EOT'
+                <?php
+
+                class Foobar {
+                    private function foo(): string|false
+                    {
+                        return $this->baz();
+                    }
+
+                    private function baz(): string|false
+                    {
+                    }
+                }
+                EOT
+        ];
+        yield 'adds array shape type' => [
+            <<<'EOT'
+                <?php
+
+                class Foobar {
+                    private function foo()
+                    {
+                        return $this->baz();
+                    }
+
+                    /**
+                     * @return array{string,string}
+                     */
+                    private function baz(): array
+                    {
+                    }
+                }
+                EOT
+            ,
+            <<<'EOT'
+                <?php
+
+                class Foobar {
+                    private function foo(): array
+                    {
+                        return $this->baz();
+                    }
+
+                    /**
+                     * @return array{string,string}
+                     */
+                    private function baz(): array
+                    {
+                    }
+                }
+                EOT
+        ];
     }
 
     /**

diff --git a/lib/Extension/Php/Model/ChainResolver.php b/lib/Extension/Php/Model/ChainResolver.php
@@ -32,4 +32,22 @@ public function resolve(): ?string
             count($this->versionResolvers)
         ));
     }
+
+    public function source(): string
+    {
+        foreach ($this->versionResolvers as $versionResolver) {
+            if (!$version = $versionResolver->resolve()) {
+                continue;
+            }
+
+            return $versionResolver->name();
+        }
+
+        return 'unknown';
+    }
+
+    public function name(): string
+    {
+        return 'chain';
+    }
 }
diff --git a/lib/Extension/Php/Model/ComposerPhpVersionResolver.php b/lib/Extension/Php/Model/ComposerPhpVersionResolver.php
@@ -35,6 +35,11 @@ public function resolve(): ?string
         return null;
     }
 
+    public function name(): string
+    {
+        return 'composer';
+    }
+
     private function resolveLowestVersion(string $versionString): ?string
     {
         /** @phpstan-ignore-next-line */

diff --git a/lib/Extension/Php/Model/ConstantPhpVersionResolver.php b/lib/Extension/Php/Model/ConstantPhpVersionResolver.php
@@ -13,4 +13,9 @@ public function resolve(): ?string
     {
         return $this->version;
     }
+
+    public function name(): string
+    {
+        return 'user configured';
+    }
 }
diff --git a/lib/Extension/Php/Model/PhpVersionResolver.php b/lib/Extension/Php/Model/PhpVersionResolver.php
@@ -5,4 +5,5 @@
 interface PhpVersionResolver
 {
     public function resolve(): ?string;
+    public function name(): string;
 }
diff --git a/lib/Extension/Php/Model/RuntimePhpVersionResolver.php b/lib/Extension/Php/Model/RuntimePhpVersionResolver.php
@@ -8,4 +8,9 @@ public function resolve(): ?string
     {
         return phpversion();
     }
+
+    public function name(): string
+    {
+        return 'runtime';
+    }
 }
diff --git a/lib/Extension/Php/PhpExtension.php b/lib/Extension/Php/PhpExtension.php
@@ -11,6 +11,7 @@
 use Phpactor\Extension\Php\Model\PhpVersionResolver;
 use Phpactor\Extension\Php\Model\RuntimePhpVersionResolver;
 use Phpactor\Extension\FilePathResolver\FilePathResolverExtension;
+use Phpactor\Extension\Php\Status\PhpStatusProvider;
 use Phpactor\MapResolver\Resolver;
 
 class PhpExtension implements Extension
@@ -20,7 +21,7 @@ class PhpExtension implements Extension
 
     public function load(ContainerBuilder $container): void
     {
-        $container->register(PhpVersionResolver::class, function (Container $container) {
+        $container->register(ChainResolver::class, function (Container $container) {
             $pathResolver = $container->get(FilePathResolverExtension::SERVICE_FILE_PATH_RESOLVER);
             $composerPath = $pathResolver->resolve('%project_root%/composer.json');
 
@@ -30,6 +31,13 @@ public function load(ContainerBuilder $container): void
                 new RuntimePhpVersionResolver()
             );
         });
+        $container->register(PhpVersionResolver::class, function (Container $container) {
+            return $container->get(ChainResolver::class);
+        });
+
+        $container->register(PhpStatusProvider::class, function (Container $container) {
+            return new PhpStatusProvider($container->get(ChainResolver::class));
+        }, []);
     }
 
 

diff --git a/lib/Extension/Php/Status/PhpStatusProvider.php b/lib/Extension/Php/Status/PhpStatusProvider.php
@@ -0,0 +1,28 @@
+<?php
+
+namespace Phpactor\Extension\Php\Status;
+
+use Phpactor\Extension\LanguageServer\Status\StatusProvider;
+use Phpactor\Extension\Php\Model\ChainResolver;
+
+class PhpStatusProvider implements StatusProvider
+{
+    public function __construct(
+        private ChainResolver $chainResolver,
+    ) {
+    }
+
+    public function title(): string
+    {
+        return 'php';
+    }
+
+    public function provide(): array
+    {
+        return [
+            'project' => (string)$this->chainResolver->resolve(),
+            'source' => $this->chainResolver->source(),
+            'runtime' => phpversion(),
+        ];
+    }
+}
diff --git a/lib/WorseReflection/Tests/Inference/type/false.test b/lib/WorseReflection/Tests/Inference/type/false.test
@@ -0,0 +1,18 @@
+<?php
+
+namespace Foo;
+
+wrAssertType('string|false', possiblyFalse());
+
+function possiblyFalse(array $foo): string|false
+{
+}
+
+/**
+ * @return string|false a formatted date string. If a non-numeric value is used for
+ */
+function date()
+{
+}
+
+wrAssertType('string|false', date());