feature #26509 [BrowserKit] Avoid nullable values in some Client's methods (ossinkine)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[BrowserKit] Avoid nullable values in some Client's methods

| Q             | A
| ------------- | ---
| Branch?       | master | Bug fix?      | yes/no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes <!-- please add some, will be required by reviewers -->
| License | MIT

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->
I suggest the some methods in `Client` should not return `null` for simplify usage.
If you are trying to get response from client when `request` method was not called, it seems an exception should be throwed.

Commits
-------

c2c285355b Avoid nullable values in some Client's methods

src/Symfony/Component/BrowserKit/Client.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\BrowserKit;
 
+use Symfony\Component\BrowserKit\Exception\BadMethodCallException;
 use Symfony\Component\DomCrawler\Crawler;
 use Symfony\Component\DomCrawler\Link;
 use Symfony\Component\DomCrawler\Form;
@@ -183,20 +184,30 @@ abstract class Client
     /**
      * Returns the current Crawler instance.
      *
-     * @return Crawler|null A Crawler instance
+     * @return Crawler A Crawler instance
      */
     public function getCrawler()
     {
+        if (null === $this->crawler) {
+            @trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
+            // throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
+        }
+
         return $this->crawler;
     }
 
     /**
      * Returns the current BrowserKit Response instance.
      *
-     * @return Response|null A BrowserKit Response instance
+     * @return Response A BrowserKit Response instance
      */
     public function getInternalResponse()
     {
+        if (null === $this->internalResponse) {
+            @trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
+            // throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
+        }
+
         return $this->internalResponse;
     }
 
@@ -206,22 +217,32 @@ abstract class Client
      * The origin response is the response instance that is returned
      * by the code that handles requests.
      *
-     * @return object|null A response instance
+     * @return object A response instance
      *
      * @see doRequest()
      */
     public function getResponse()
     {
+        if (null === $this->response) {
+            @trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
+            // throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
+        }
+
         return $this->response;
     }
 
     /**
      * Returns the current BrowserKit Request instance.
      *
-     * @return Request|null A BrowserKit Request instance
+     * @return Request A BrowserKit Request instance
      */
     public function getInternalRequest()
     {
+        if (null === $this->internalRequest) {
+            @trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
+            // throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
+        }
+
         return $this->internalRequest;
     }
 
@@ -231,12 +252,17 @@ abstract class Client
      * The origin request is the request instance that is sent
      * to the code that handles requests.
      *
-     * @return object|null A Request instance
+     * @return object A Request instance
      *
      * @see doRequest()
      */
     public function getRequest()
     {
+        if (null === $this->request) {
+            @trigger_error(sprintf('Calling the "%s()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.', __METHOD__), E_USER_DEPRECATED);
+            // throw new BadMethodCallException(sprintf('The "request()" method must be called before the "%s()" one', __METHOD__));
+        }
+
         return $this->request;
     }
 

src/Symfony/Component/BrowserKit/Exception/BadMethodCallException.php

@@ -0,0 +1,16 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\BrowserKit\Exception;
+
+class BadMethodCallException extends \BadMethodCallException
+{
+}

src/Symfony/Component/BrowserKit/Tests/ClientTest.php

@@ -94,6 +94,16 @@ class ClientTest extends TestCase
         $this->assertEquals('http://example.com/', $client->getRequest()->getUri(), '->getCrawler() returns the Request of the last request');
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getRequest()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
+     */
+    public function testGetRequestNull()
+    {
+        $client = new TestClient();
+        $this->assertNull($client->getRequest());
+    }
+
     public function testGetRequestWithXHR()
     {
         $client = new TestClient();
@@ -124,6 +134,16 @@ class ClientTest extends TestCase
         $this->assertInstanceOf('Symfony\Component\BrowserKit\Response', $client->getResponse(), '->getCrawler() returns the Response of the last request');
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getResponse()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
+     */
+    public function testGetResponseNull()
+    {
+        $client = new TestClient();
+        $this->assertNull($client->getResponse());
+    }
+
     public function testGetInternalResponse()
     {
         $client = new TestClient();
@@ -135,6 +155,16 @@ class ClientTest extends TestCase
         $this->assertInstanceOf('Symfony\Component\BrowserKit\Tests\SpecialResponse', $client->getResponse());
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getInternalResponse()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
+     */
+    public function testGetInternalResponseNull()
+    {
+        $client = new TestClient();
+        $this->assertNull($client->getInternalResponse());
+    }
+
     public function testGetContent()
     {
         $json = '{"jsonrpc":"2.0","method":"echo","id":7,"params":["Hello World"]}';
@@ -153,6 +183,16 @@ class ClientTest extends TestCase
         $this->assertSame($crawler, $client->getCrawler(), '->getCrawler() returns the Crawler of the last request');
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getCrawler()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
+     */
+    public function testGetCrawlerNull()
+    {
+        $client = new TestClient();
+        $this->assertNull($client->getCrawler());
+    }
+
     public function testRequestHttpHeaders()
     {
         $client = new TestClient();
@@ -720,6 +760,10 @@ class ClientTest extends TestCase
         $this->assertInstanceOf('Symfony\Component\BrowserKit\Request', $client->getInternalRequest());
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Calling the "Symfony\Component\BrowserKit\Client::getInternalRequest()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.
+     */
     public function testInternalRequestNull()
     {
         $client = new TestClient();

src/Symfony/Component/HttpKernel/Client.php

@@ -25,8 +25,8 @@ use Symfony\Component\HttpFoundation\Response;
  *
  * @author Fabien Potencier <fabien@symfony.com>
  *
- * @method Request|null  getRequest()  A Request instance
+ * @method Request  getRequest()  A Request instance
- * @method Response|null getResponse() A Response instance
+ * @method Response getResponse() A Response instance
  */
 class Client extends BaseClient
 {