I am splitting of a side issue from #3055193: [Symfony 5] The "Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser" class is deprecated since Symfony 4.3, use "Symfony\Component\Mime\MimeTypes" instead.

There is a non-obvious aspect to Symfony 5 modernisation.

The APIs are starting to enforce parameter and return types. [ PHP7 concepts ]

In the main issue for example

When Drupal autogenerates new core/lib/Drupal/Core/ProxyClass/ classes.
Errors are being produced because the copied methods do no match the modern interface definition.

https://www.drupal.org/project/drupal/issues/3055193#comment-13537884

looking at 

+         * {@inheritdoc}
+         */
+        public function getMimeTypes(string $ext) : array
+        {
+            return $this->lazyLoadItself()->getMimeTypes($ext);
+        }

The string type and array return type are important. For compatibility.

This issue

a) Augments the ProxyBuilder to copy over types if present.
b) Modifies the ProxyBuilderTest.

CommentFileSizeAuthor
#6 interdiff-3125234-5-6.txt1.81 KBmartin107
#6 3125234-6.patch2.27 KBmartin107
#5 3125234-5.patch2.28 KBmartin107
#5 interdiff-3125234-2-5.txt1.06 KBmartin107
#2 3125234-2.patch2.26 KBmartin107
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107 created an issue. See original summary.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Assigned: martin107 » Unassigned
FileSize
3125234-2.patch2.26 KB

Here is the patch.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 3125234-2.patch, failed testing. View results

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Needs work » Needs review
FileSize
interdiff-3125234-2-5.txt1.06 KB
3125234-5.patch2.28 KB

Wow life on the bleeding edge of PHP

I was testing locally with php7.3.x

both

$reflection_method->getReturnType();

and

$parameter->getType()

trigger deprecation warnings in php 7.4

what is missing is an additional ->getName()

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
FileSize
3125234-6.patch2.27 KB
interdiff-3125234-5-6.txt1.81 KB

All credit to longwave during this review, for driving this change.

https://www.drupal.org/project/drupal/issues/3055193#comment-13544389

1) I have changed my position on this styling ...

I was going to for two spaces

method() : array

but after looking at

a) the offical php code examples ( https://www.php.net/manual/en/functions.returning-values.php )
b) The Symfony convention.

for example

vendor/symfony/serializer/Normalizer/GetSetMethodNormalizer.php:

 private function supports(string $class): bool

c) The Wordpress coding standard

They all favour the one space solution :-

method(): array

This seems to disambiguate nicely in relation to this two space situation

$y = $a ? $b : $c
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Reviewed in the other issue around return type coding style, I think this looks good now and can go in separately from that issue.

  • catch committed 66e8d08 on 9.1.x 
    Issue #3125234 by martin107, longwave: ProxyBuilder compatibility with...

  • catch committed c1cb842 on 9.0.x 
    Issue #3125234 by martin107, longwave: ProxyBuilder compatibility with...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.1.x-dev » 9.0.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.1.x/9.0.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.