Skip to content

Fix some return type annotations #33007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 7, 2019
Merged

Fix some return type annotations #33007

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,6 @@ protected function describeContainerParameter($parameter, array $options = [])

/**
* Writes data as json.
*
* @return array|string
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 This method does not return anything.

*/
private function writeData(array $data, array $options)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/XmlDescriptor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,6 @@ protected function describeContainerParameter($parameter, array $options = [])

/**
* Writes DOM document.
*
* @return \DOMDocument|string
*/
private function writeDocument(\DOMDocument $dom)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Component/Console/Descriptor/JsonDescriptor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ protected function describeApplication(Application $application, array $options

/**
* Writes data as json.
*
* @return array|string
*/
private function writeData(array $data, array $options)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Symfony/Component/Console/Descriptor/XmlDescriptor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,6 @@ private function appendDocument(\DOMNode $parentNode, \DOMNode $importedParent)

/**
* Writes DOM document.
*
* @return \DOMDocument|string
*/
private function writeDocument(\DOMDocument $dom)
{
Expand Down
32 changes: 16 additions & 16 deletions src/Symfony/Component/DomCrawler/Crawler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public function addNode(\DOMNode $node)
*
* @param int $position The position
*
* @return self
* @return static
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Most methods in this class return the result of createSubCrawler() which is already (correctly) annotated with @return static.

*/
public function eq($position)
{
Expand Down Expand Up @@ -377,7 +377,7 @@ public function each(\Closure $closure)
* @param int $offset
* @param int $length
*
* @return self
* @return static
*/
public function slice($offset = 0, $length = null)
{
Expand All @@ -391,7 +391,7 @@ public function slice($offset = 0, $length = null)
*
* @param \Closure $closure An anonymous function
*
* @return self
* @return static
*/
public function reduce(\Closure $closure)
{
Expand All @@ -408,7 +408,7 @@ public function reduce(\Closure $closure)
/**
* Returns the first node of the current selection.
*
* @return self
* @return static
*/
public function first()
{
Expand All @@ -418,7 +418,7 @@ public function first()
/**
* Returns the last node of the current selection.
*
* @return self
* @return static
*/
public function last()
{
Expand All @@ -428,7 +428,7 @@ public function last()
/**
* Returns the siblings nodes of the current selection.
*
* @return self
* @return static
*
* @throws \InvalidArgumentException When current node is empty
*/
Expand All @@ -444,7 +444,7 @@ public function siblings()
/**
* Returns the next siblings nodes of the current selection.
*
* @return self
* @return static
*
* @throws \InvalidArgumentException When current node is empty
*/
Expand All @@ -460,7 +460,7 @@ public function nextAll()
/**
* Returns the previous sibling nodes of the current selection.
*
* @return self
* @return static
*
* @throws \InvalidArgumentException
*/
Expand All @@ -476,7 +476,7 @@ public function previousAll()
/**
* Returns the parents nodes of the current selection.
*
* @return self
* @return static
*
* @throws \InvalidArgumentException When current node is empty
*/
Expand All @@ -501,7 +501,7 @@ public function parents()
/**
* Returns the children nodes of the current selection.
*
* @return self
* @return static
*
* @throws \InvalidArgumentException When current node is empty
*/
Expand Down Expand Up @@ -664,7 +664,7 @@ public function extract($attributes)
*
* @param string $xpath An XPath expression
*
* @return self
* @return static
*/
public function filterXPath($xpath)
{
Expand All @@ -685,7 +685,7 @@ public function filterXPath($xpath)
*
* @param string $selector A CSS selector
*
* @return self
* @return static
*
* @throws \RuntimeException if the CssSelector Component is not available
*/
Expand All @@ -706,7 +706,7 @@ public function filter($selector)
*
* @param string $value The link text
*
* @return self
* @return static
*/
public function selectLink($value)
{
Expand All @@ -721,7 +721,7 @@ public function selectLink($value)
*
* @param string $value The image alt
*
* @return self A new instance of Crawler with the filtered list of nodes
* @return static A new instance of Crawler with the filtered list of nodes
*/
public function selectImage($value)
{
Expand All @@ -735,7 +735,7 @@ public function selectImage($value)
*
* @param string $value The button text
*
* @return self
* @return static
*/
public function selectButton($value)
{
Expand Down Expand Up @@ -937,7 +937,7 @@ public static function xpathLiteral($s)
*
* @param string $xpath
*
* @return self
* @return static
*/
private function filterRelativeXPath($xpath)
{
Expand Down
6 changes: 2 additions & 4 deletions src/Symfony/Component/Form/NativeRequestHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ private static function getRequestMethod()
* This method is identical to {@link \Symfony\Component\HttpFoundation\FileBag::fixPhpFilesArray}
* and should be kept as such in order to port fixes quickly and easily.
*
* @return array
* @return mixed
*/
private static function fixPhpFilesArray($data)
{
Expand Down Expand Up @@ -228,9 +228,7 @@ private static function fixPhpFilesArray($data)
/**
* Sets empty uploaded files to NULL in the given uploaded files array.
*
* @param mixed $data The file upload data
*
* @return array|null Returns the stripped upload data
* @return mixed Returns the stripped upload data
*/
private static function stripEmptyFiles($data)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/HttpKernel/HttpCache/Store.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,13 +350,13 @@ private function doPurge($url)
*
* @param string $key The store key
*
* @return string The data associated with the key
* @return string|null The data associated with the key
*/
private function load($key)
{
$path = $this->getPath($key);

return file_exists($path) ? file_get_contents($path) : false;
return file_exists($path) ? file_get_contents($path) : null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As suggested by @Tobion in #32993 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_get_contents can also return false that we should cast to null. this will otherwise break the type hints later

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we talking about the file behind $path being deleted between the file_exists() and the file_get_contents() call or are there other reasons why file_get_contents could return false in this case?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

permissions, race conditions and more probably

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, something like that?

return file_exists($path) && false !== $contents = file_get_contents($path) ? $contents : null;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#33016

}

/**
Expand Down