refactor: fix the latent LSP violations in HTTP (#10463)

* refactor: fix the latent LSP violations in `HTTP`

* add upgrade instructions

* call `File::__construct` in `UploadedFile`'s constructor

Author: paulbalandan

system/HTTP/Files/UploadedFile.php

@@ -123,7 +123,7 @@ public function __construct(string $path, string $originalName, ?string $mimeTyp
      * @param bool        $overwrite  State for indicating whether to overwrite the previously generated file with the same
      *                                name or not.
      *
-     * @return bool
+     * @return static
      */
     public function move(string $targetPath, ?string $name = null, bool $overwrite = false)
     {
@@ -165,7 +165,9 @@ public function move(string $targetPath, ?string $name = null, bool $overwrite =
         $this->path = $targetPath;
         $this->name = basename($destination);
 
-        return true;
+        parent::__construct($destination, false);
+
+        return $this;
     }
 
     /**

system/HTTP/Files/UploadedFileInterface.php

@@ -62,7 +62,7 @@ public function __construct(string $path, string $originalName, ?string $mimeTyp
      * @param string      $targetPath Path to which to move the uploaded file.
      * @param string|null $name       the name to rename the file to.
      *
-     * @return bool
+     * @return static
      *
      * @throws InvalidArgumentException if the $path specified is invalid.
      * @throws RuntimeException         on the second or subsequent call to the method.

system/HTTP/IncomingRequest.php

@@ -56,8 +56,6 @@ class IncomingRequest extends Request
      * everything this cares about (and the router, etc) is the portion
      * AFTER the baseURL. So, if hosted in a sub-folder this will
      * appear different than actual URI path. If you need that use getPath().
-     *
-     * @var URI
      */
     protected $uri;
 

system/HTTP/OutgoingRequest.php

@@ -30,7 +30,7 @@ class OutgoingRequest extends Message implements OutgoingRequestInterface
     /**
      * A URI instance.
      *
-     * @var URI|null
+     * @var URI
      */
     protected $uri;
 
@@ -41,7 +41,7 @@ class OutgoingRequest extends Message implements OutgoingRequestInterface
      */
     public function __construct(
         string $method,
-        ?URI $uri = null,
+        URI $uri,
         array $headers = [],
         $body = null,
         string $version = '1.1',
@@ -110,7 +110,7 @@ public function withMethod($method)
     /**
      * Retrieves the URI instance.
      *
-     * @return URI|null
+     * @return URI
      */
     public function getUri()
     {

tests/system/HTTP/Files/FileMovingTest.php

@@ -215,6 +215,34 @@ public function testMoved(): void
         $this->assertTrue($file->hasMoved());
     }
 
+    public function testPathnameAfterMoveReflectsTheMovedFile(): void
+    {
+        $finalFilename = 'fileA.txt';
+        service('superglobals')->setFilesArray([
+            'userfile1' => [
+                'name'     => $finalFilename,
+                'type'     => 'text/plain',
+                'size'     => 124,
+                'tmp_name' => '/tmp/fileA.txt',
+                'error'    => 0,
+            ],
+        ]);
+
+        $collection = new FileCollection();
+
+        $destination = $this->destination;
+
+        if (! is_dir($destination)) {
+            mkdir($destination, 0777, true);
+        }
+
+        $file = $collection->getFile('userfile1');
+        $this->assertInstanceOf(UploadedFile::class, $file);
+
+        $file->move($destination, $file->getName(), false);
+        $this->assertSame($destination . '/' . $finalFilename, $file->getPathname());
+    }
+
     public function testStore(): void
     {
         $finalFilename = 'fileA';

user_guide_src/source/changelogs/v4.8.0.rst

@@ -55,6 +55,7 @@ update your implementations to include the new methods or method changes to ensu
 - **Cache:** ``CodeIgniter\Cache\CacheInterface::remember()`` now accepts a TTL callable. Custom implementations of ``CacheInterface`` must update the ``$ttl`` parameter type from ``int`` to ``callable|int``.
 - **Database:** ``CodeIgniter\Database\ConnectionInterface`` now requires the ``afterCommit()``, ``afterRollback()``, ``inTransaction()``, and ``transaction()`` methods.
 - **HTTP:** ``CodeIgniter\HTTP\ResponseInterface`` now requires the ``stream()`` and ``eventStream()`` methods, which create streaming and SSE responses. See :ref:`streaming-responses`.
+- **HTTP:** ``CodeIgniter\HTTP\Files\UploadedFileInterface::move()`` now returns ``static`` instead of ``bool``. The previous ``bool`` return was incompatible with ``CodeIgniter\Files\File::move()``, which ``UploadedFile`` extends, so no implementation could satisfy both. See :doc:`../installation/upgrade_480` for the migration.
 - **Logging:** ``CodeIgniter\Log\Handlers\HandlerInterface::handle()`` now requires a third parameter ``array $context = []``. Any custom log handler that overrides ``handle()`` - whether implementing ``HandlerInterface`` directly or extending a built-in handler class - must add the parameter to its ``handle()`` method signature.
 - **Security:** The ``SecurityInterface``'s ``verify()`` method now has a native return type of ``static``.
 - **Validation:** ``CodeIgniter\Validation\ValidationInterface`` now requires the ``getValidatedInput()`` method, which returns a ``CodeIgniter\Input\ValidatedInput`` instance.
@@ -73,6 +74,7 @@ Method Signature Changes
 - **Config:** ``CodeIgniter\Config\Services::request()`` no longer accepts any parameter.
 - **Database:** The following methods have had their signatures updated to remove deprecated parameters:
     - ``CodeIgniter\Database\Forge::_createTable()`` no longer accepts the deprecated ``$ifNotExists`` parameter. The method signature is now ``_createTable(string $table, array $attributes)``.
+- **HTTP:** ``CodeIgniter\HTTP\OutgoingRequest::__construct()`` now requires the ``$uri`` parameter, which was previously ``?URI $uri = null``. Passing ``null`` only worked when a ``Host`` header was supplied in the same call, since the constructor otherwise dereferences the URI to set that header. Consequently ``OutgoingRequest::getUri()`` now returns ``URI`` instead of ``URI|null``, matching ``OutgoingRequestInterface``. See :doc:`../installation/upgrade_480` for the migration.
 - **Model:** ``CodeIgniter\BaseModel`` now requires the ``chunkRows()``, ``chunkById()``, and ``chunkRowsById()`` methods. Custom classes extending ``BaseModel`` directly must implement them.
 - **Session:** The ``$max_lifetime`` parameter of the following ``gc()`` methods now has the native ``int`` type, matching ``SessionHandlerInterface``: ``ArrayHandler::gc()``, ``DatabaseHandler::gc()``, ``FileHandler::gc()``, ``MemcachedHandler::gc()``, ``PostgreHandler::gc()``, ``RedisHandler::gc()``.
 

user_guide_src/source/installation/upgrade_480.rst

@@ -27,6 +27,61 @@ Previously, returning a non-integer value from a command run through ``spark`` w
 Starting with v4.8.0, this behavior is still supported but will trigger a deprecation notice. Commands should now return an integer exit code
 to ensure proper behavior across all platforms.
 
+Uploaded File Move Return Type
+==============================
+
+``CodeIgniter\HTTP\Files\UploadedFileInterface::move()`` now returns ``static``
+instead of ``bool``, matching ``CodeIgniter\Files\File::move()`` which
+``UploadedFile`` extends.
+
+If you have a custom implementation of ``UploadedFileInterface``, or a class
+extending ``UploadedFile`` that overrides ``move()``, return the instance
+instead of ``true``:
+
+.. code-block:: php
+
+    // Before
+    public function move(string $targetPath, ?string $name = null, bool $overwrite = false)
+    {
+        // ...
+
+        return true;
+    }
+
+    // After
+    public function move(string $targetPath, ?string $name = null, bool $overwrite = false)
+    {
+        // ...
+
+        return $this;
+    }
+
+Calling code that only tests the result, such as ``if ($file->move($path))``,
+needs no change because the returned instance is truthy. Code comparing the
+result strictly against ``true`` must be updated.
+
+Outgoing Request Constructor
+============================
+
+``CodeIgniter\HTTP\OutgoingRequest::__construct()`` now requires the ``$uri``
+parameter, which was previously ``?URI $uri = null``. Consequently
+``OutgoingRequest::getUri()`` now returns ``URI`` instead of ``URI|null``.
+
+Passing ``null`` only worked when a ``Host`` header was supplied in the same
+call, because the constructor's host check short-circuits before dereferencing
+the URI. Such calls must now pass a ``URI``:
+
+.. code-block:: php
+
+    // Before
+    $request = new OutgoingRequest('GET', null, ['Host' => 'example.com']);
+
+    // After
+    $request = new OutgoingRequest('GET', new URI('http://example.com'), ['Host' => 'example.com']);
+
+Any other call that omitted ``$uri`` or passed ``null`` already failed with
+``Call to a member function getHost() on null``, so it needs no migration.
+
 *********************
 Breaking Enhancements
 *********************

utils/phpstan-baseline/loader.neon

@@ -1,4 +1,4 @@
-# total 692 errors
+# total 689 errors
 
 includes:
     - argument.type.neon
@@ -8,7 +8,6 @@ includes:
     - deadCode.unreachable.neon
     - function.resultUnused.neon
     - method.childParameterType.neon
-    - method.childReturnType.neon
     - method.notFound.neon
     - missingType.callable.neon
     - missingType.iterableValue.neon

utils/phpstan-baseline/method.childReturnType.neon

@@ -1,4 +1,4 @@
-# total 22 errors
+# total 21 errors
 
 parameters:
     ignoreErrors:
@@ -77,11 +77,6 @@ parameters:
             count: 1
             path: ../../system/HTTP/Exceptions/RedirectException.php
 
-        -
-            message: '#^PHPDoc type CodeIgniter\\HTTP\\URI of property CodeIgniter\\HTTP\\IncomingRequest\:\:\$uri is not the same as PHPDoc type CodeIgniter\\HTTP\\URI\|null of overridden property CodeIgniter\\HTTP\\OutgoingRequest\:\:\$uri\.$#'
-            count: 1
-            path: ../../system/HTTP/IncomingRequest.php
-
         -
             message: '#^PHPDoc type string of property CodeIgniter\\Session\\Handlers\\FileHandler\:\:\$savePath is not the same as PHPDoc type array\<string, mixed\>\|string of overridden property CodeIgniter\\Session\\Handlers\\BaseHandler\:\:\$savePath\.$#'
             count: 1