[Config] Fix for signatures of typed properties

[tvandervorm]

Q	A
Branch?	4.3
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#32465
License	MIT
Doc PR	-

Also see the issue description, when using public typed properties (new in PHP7.4) like this:

namespace App;

class Foo {
    public int $bar;
} 

will cause $defaults['bar'] not to be set in Symfony/Component/Config/Resource/ReflectionClassResource.php::139.

This is because $bar doesn't have a default value, but does have a type hint, meaning it's default value is not null but undefined. This causes an 'undefined index' error when clearing the cache through bin/console cache:clear when running PHP7.4.

The default value is used here for the class signature, having null should be appropriate for all cases.

[nicolas-grekas]

(for 3.4)

[derrabus]

Can we please add a test? And yes, I know that that test would be skipped in our current CI setup, but still… 😅

[tvandervorm]

Can we please add a test? And yes, I know that that test would be skipped in our current CI setup, but still… sweat_smile

I'd be happy to write a testcase for this issue, but it's not clear to me how I could do this without breaking the test for PHP < 7.4 . The most straightforward way would be to add a line to testHashedSignature() in ReflectionClassResourceTest, but this will cause the currently passing PHP7.1. & PHP7.3 tests to fail.

[xabbuh]

Adding a new test case annotation with @requires PHP 7.4 should, right?

[tvandervorm]

ah, I wasn't aware one could specify PHP versions in annotations like that, will give it a go, thanks

[tvandervorm]

I added a little testcase, which I tested with both PHP7.2 & PHP 7.4. It gets skipped in the first case, passes in the second case and fails in the second case if I revert my change, so it does what it should do.

Now, I've always been taught that dependencies between tests should be avoided, which I failed to do here. However, the alternative would be to copy/paste the content of testHashedSignature(), which seems ugly to me as well. Let me know what is preferred, I'll change it if needed.

Finally, I tried whether the @requires annotation gets taken into account for @dataProvider functions, which would've been the cleanest solution, this is sadly not the case however.

[xabbuh]

What if we updated the provideHashedSignature() method instead and yield these three examples only if PHP_VERSION_ID >= 70400 is true?

[tvandervorm]

that works as well, and is a little cleaner indeed, I've updated the test.

[fabpot]

Thank you @tvandervorm.