Skip to content

Commit

Permalink
[HttpFoundation] fix return type declarations
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Aug 26, 2019
1 parent 1f72501 commit ef5ead0
Show file tree
Hide file tree
Showing 20 changed files with 31 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpFoundation/BinaryFileResponse.php
Expand Up @@ -327,12 +327,12 @@ public function setContent($content)
if (null !== $content) {
throw new \LogicException('The content cannot be set on a BinaryFileResponse instance.');
}

return $this;
}

/**
* {@inheritdoc}
*
* @return false
*/
public function getContent()
{
Expand Down
8 changes: 3 additions & 5 deletions src/Symfony/Component/HttpFoundation/FileBag.php
Expand Up @@ -75,8 +75,8 @@ protected function convertFileInformation($file)
return $file;
}

$file = $this->fixPhpFilesArray($file);
if (\is_array($file)) {
$file = $this->fixPhpFilesArray($file);
$keys = array_keys($file);
sort($keys);

Expand Down Expand Up @@ -109,14 +109,12 @@ protected function convertFileInformation($file)
* It's safe to pass an already converted array, in which case this method
* just returns the original array unmodified.
*
* @param array $data
*
* @return array
*/
protected function fixPhpFilesArray($data)
{
if (!\is_array($data)) {
return $data;
}

$keys = array_keys($data);
sort($keys);

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/HeaderBag.php
Expand Up @@ -121,7 +121,7 @@ public function get($key, $default = null, $first = true)
}

if ($first) {
return \count($headers[$key]) ? $headers[$key][0] : $default;
return \count($headers[$key]) ? (string) $headers[$key][0] : $default;
}

return $headers[$key];
Expand Down
6 changes: 5 additions & 1 deletion src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -528,6 +528,10 @@ public function __toString()
try {
$content = $this->getContent();
} catch (\LogicException $e) {
if (\PHP_VERSION_ID >= 70400) {
throw $e;
}

return trigger_error($e, E_USER_ERROR);
}

Expand Down Expand Up @@ -912,7 +916,7 @@ public function getClientIps()
* ("Client-Ip" for instance), configure it via the $trustedHeaderSet
* argument of the Request::setTrustedProxies() method instead.
*
* @return string The client IP address
* @return string|null The client IP address
*
* @see getClientIps()
* @see https://wikipedia.org/wiki/X-Forwarded-For
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/Response.php
Expand Up @@ -407,7 +407,7 @@ public function setContent($content)
/**
* Gets the current response content.
*
* @return string Content
* @return string|false

This comment has been minimized.

Copy link
@gmponos

gmponos Aug 27, 2019

Contributor

@nicolas-grekas how is this possible to be false? I can see on the related PR #33332 that this changed to string|null

  1. Not sure how this reached to be string|false here on 4.3.4?
  2. on version 4.3 how can this be string|null ? I can see that the content is always set using setContent which always casts the content to string

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 27, 2019

Author Member

Because of child classes: see StreamResponse + BinaryFilrResponse. If 4.3 says |null, that's a merge issue. LSP rulez here.

This comment has been minimized.

Copy link
@gmponos

gmponos Aug 27, 2019

Contributor

Oh.. ok I see.. no 4.3 and 3.4 is string|false.

I think the PR was string|null.

Nevermind...

This comment has been minimized.

Copy link
@Tobion

Tobion Aug 27, 2019

Member

Then the bug is in the child classes to me. What's the point of returning false?

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 27, 2019

Author Member

We can try changing this in 5.0 (string|null would be better for me too), but changing this historical behavior in 3.4 would be inappropriate. PR welcome as usual, branch 4.4 :)

*/
public function getContent()
{
Expand Down
Expand Up @@ -97,7 +97,7 @@ public function remove($name)
* @param string $name Key name
* @param bool $writeContext Write context, default false
*
* @return array
* @return array|null
*/
protected function &resolveAttributePath($name, $writeContext = false)
{
Expand Down
Expand Up @@ -40,9 +40,6 @@ class MemcachedSessionHandler extends AbstractSessionHandler
* * prefix: The prefix to use for the memcached keys in order to avoid collision
* * expiretime: The time to live in seconds.
*
* @param \Memcached $memcached A \Memcached instance
* @param array $options An associative array of Memcached options
*
* @throws \InvalidArgumentException When unsupported options are passed
*/
public function __construct(\Memcached $memcached, array $options = [])
Expand All @@ -58,7 +55,7 @@ public function __construct(\Memcached $memcached, array $options = [])
}

/**
* {@inheritdoc}
* @return bool
*/
public function close()
{
Expand All @@ -74,7 +71,7 @@ protected function doRead($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function updateTimestamp($sessionId, $data)
{
Expand Down Expand Up @@ -102,7 +99,7 @@ protected function doDestroy($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function gc($maxlifetime)
{
Expand Down
Expand Up @@ -67,7 +67,7 @@ protected function doDestroy($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function gc($maxlifetime)
{
Expand Down
Expand Up @@ -286,7 +286,7 @@ public function read($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function gc($maxlifetime)
{
Expand Down
Expand Up @@ -94,7 +94,7 @@ public function close()
}

/**
* {@inheritdoc}
* @return bool
*/
public function gc($maxlifetime)
{
Expand Down
Expand Up @@ -31,7 +31,7 @@ abstract class AbstractProxy
/**
* Gets the session.save_handler name.
*
* @return string
* @return string|null
*/
public function getSaveHandlerName()
{
Expand Down
Expand Up @@ -76,7 +76,7 @@ public function destroy($sessionId)
}

/**
* {@inheritdoc}
* @return bool
*/
public function gc($maxlifetime)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Component/HttpFoundation/StreamedResponse.php
Expand Up @@ -136,8 +136,6 @@ public function setContent($content)

/**
* {@inheritdoc}
*
* @return false
*/
public function getContent()
{
Expand Down
Expand Up @@ -107,7 +107,7 @@ public function testRequests($requestRange, $offset, $length, $responseRange)

$this->assertEquals(206, $response->getStatusCode());
$this->assertEquals($responseRange, $response->headers->get('Content-Range'));
$this->assertSame($length, $response->headers->get('Content-Length'));
$this->assertSame((string) $length, $response->headers->get('Content-Length'));
}

/**
Expand Down
Expand Up @@ -144,7 +144,8 @@ public function testUpdateTimestamp()
{
$mock = $this->getMockBuilder(['SessionHandlerInterface', 'SessionUpdateTimestampHandlerInterface'])->getMock();
$mock->expects($this->once())
->method('updateTimestamp');
->method('updateTimestamp')
->willReturn(false);

$proxy = new SessionHandlerProxy($mock);
$proxy->updateTimestamp('id', 'data');
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/HttpKernel/Tests/KernelTest.php
Expand Up @@ -22,6 +22,7 @@
use Symfony\Component\HttpKernel\Config\EnvParametersResource;
use Symfony\Component\HttpKernel\DependencyInjection\ResettableServicePass;
use Symfony\Component\HttpKernel\DependencyInjection\ServicesResetter;
use Symfony\Component\HttpKernel\HttpKernel;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\HttpKernel\Tests\Fixtures\KernelForOverrideName;
Expand Down
3 changes: 3 additions & 0 deletions src/Symfony/Component/HttpKernel/Tests/Logger.php
Expand Up @@ -22,6 +22,9 @@ public function __construct()
$this->clear();
}

/**
* @return array
*/
public function getLogs($level = false)
{
return false === $level ? $this->logs : $this->logs[$level];
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Process/Process.php
Expand Up @@ -71,7 +71,7 @@ class Process implements \IteratorAggregate
private $status = self::STATUS_READY;
private $incrementalOutputOffset = 0;
private $incrementalErrorOutputOffset = 0;
private $tty;
private $tty = false;
private $pty;
private $inheritEnv = false;

Expand Down
Expand Up @@ -46,7 +46,7 @@ interface RememberMeServicesInterface
* make sure to throw an AuthenticationException as this will consequentially
* result in a call to loginFail() and therefore an invalidation of the cookie.
*
* @return TokenInterface
* @return TokenInterface|null
*/
public function autoLogin(Request $request);

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/VarDumper/Dumper/AbstractDumper.php
Expand Up @@ -35,7 +35,7 @@ abstract class AbstractDumper implements DataDumperInterface, DumperInterface
protected $indentPad = ' ';
protected $flags;

private $charset;
private $charset = '';

/**
* @param callable|resource|string|null $output A line dumper callable, an opened stream or an output path, defaults to static::$defaultOutput
Expand Down

0 comments on commit ef5ead0

Please sign in to comment.