Fix some return type annotations #33007

derrabus · 2019-08-07T08:42:50Z

derrabus · 2019-08-07T08:44:19Z

src/Symfony/Bundle/FrameworkBundle/Console/Descriptor/JsonDescriptor.php

@@ -179,8 +179,6 @@ protected function describeContainerParameter($parameter, array $options = [])

    /**
     * Writes data as json.
-     *
-     * @return array|string


💡 This method does not return anything.

derrabus · 2019-08-07T08:45:53Z

src/Symfony/Component/DomCrawler/Crawler.php

@@ -334,7 +334,7 @@ public function addNode(\DOMNode $node)
     *
     * @param int $position The position
     *
-     * @return self
+     * @return static


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

src/Symfony/Component/Form/NativeRequestHandler.php

derrabus · 2019-08-07T09:18:35Z

derrabus · 2019-08-07T12:11:31Z

src/Symfony/Component/HttpKernel/HttpCache/Store.php

     */
    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;


As suggested by @Tobion in #32993 (comment)

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

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?

permissions, race conditions and more probably

So, something like that?

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

nicolas-grekas · 2019-08-07T12:24:00Z

derrabus requested a review from xabbuh as a code owner August 7, 2019 08:42

carsonbot added Status: Needs Review Bug labels Aug 7, 2019

derrabus commented Aug 7, 2019

View reviewed changes

src/Symfony/Component/Form/NativeRequestHandler.php Show resolved Hide resolved

derrabus force-pushed the bugfix/return-types branch from eab81e2 to d531326 Compare August 7, 2019 09:15

xabbuh approved these changes Aug 7, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Aug 7, 2019

xabbuh added this to the 3.4 milestone Aug 7, 2019

derrabus mentioned this pull request Aug 7, 2019

Turned return type annotations of private methods into php return types #32993

Merged

Fix some return type annotations.

0a78dc0

derrabus force-pushed the bugfix/return-types branch from d531326 to 0a78dc0 Compare August 7, 2019 12:10

derrabus commented Aug 7, 2019

View reviewed changes

nicolas-grekas merged commit 0a78dc0 into symfony:3.4 Aug 7, 2019

derrabus deleted the bugfix/return-types branch August 7, 2019 12:38

derrabus mentioned this pull request Aug 7, 2019

[HttpKernel] Resilience against file_get_contents() race conditions #33016

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix some return type annotations #33007

Fix some return type annotations #33007

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	N/A
License	MIT
Doc PR	N/A

Uh oh!

Fix some return type annotations #33007

Fix some return type annotations #33007

Uh oh!

Conversation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!