Use PHP 7.4 syntax in Sylius components #13012

Zales0123 · 2021-08-26T23:35:04Z

Q	A
Branch?	1.10
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Related tickets	related to #12891
License	MIT

I know, it's probably unreviewable (but bundles will be worse 😄). In tests we trust 🖖 🚀

vvasiloi · 2021-08-27T06:01:28Z

src/Sylius/Component/Attribute/Model/Attribute.php


-    /** @var string */
-    protected $type = TextAttributeType::TYPE;
+    protected ?string $type = TextAttributeType::TYPE;

    /** @var array */
    protected $configuration = [];


Suggested change

protected $configuration = [];

protected array $configuration = [];

vvasiloi · 2021-08-27T06:01:41Z

src/Sylius/Component/Attribute/Model/Attribute.php


-    /** @var string */
-    protected $type = TextAttributeType::TYPE;
+    protected ?string $type = TextAttributeType::TYPE;

    /** @var array */


Suggested change

/** @var array */

vvasiloi · 2021-08-27T06:05:24Z

src/Sylius/Component/Channel/Context/CachedPerRequestChannelContext.php


-    /** @var RequestStack */
-    private $requestStack;
+    private RequestStack $requestStack;

    /** @var \SplObjectStorage<Request, ChannelInterface> */
    private $requestToChannelMap;


Suggested change

private $requestToChannelMap;

private \SplObjectStorage $requestToChannelMap;

There's also private \SplObjectStorage $requestToExceptionMap; below, but can't suggest a change there...

vvasiloi · 2021-08-27T06:15:30Z

src/Sylius/Component/Channel/spec/Context/CachedPerRequestChannelContextSpec.php


-                /** @var ChannelInterface */
-                private $channel;
+                private ?ChannelInterface $channel;


Suggested change

private ?ChannelInterface $channel;

private ChannelInterface $channel;

vvasiloi · 2021-08-27T06:18:05Z

src/Sylius/Component/Core/Cart/Context/ShopBasedCartContext.php


-    /** @var OrderInterface|null */
-    private $cart;
+    private ?\Sylius\Component\Core\Model\OrderInterface $cart = null;


Suggested change

private ?\Sylius\Component\Core\Model\OrderInterface $cart = null;

private ?OrderInterface $cart = null;

vvasiloi · 2021-08-27T06:21:25Z

src/Sylius/Component/Core/Dashboard/SalesDataProvider.php

@@ -22,8 +22,7 @@
 */
 final class SalesDataProvider implements SalesDataProviderInterface
 {
-    /** @var EntityRepository */
-    private $orderRepository;
+    private EntityRepository $orderRepository;


Unrelated, but why an interface is not used here and in the constructor?

vvasiloi · 2021-08-27T06:25:08Z

src/Sylius/Component/Core/Model/ChannelPricing.php

@@ -15,20 +15,15 @@

 class ChannelPricing implements ChannelPricingInterface
 {
-    /** @var int|null */
-    protected $id;
+    protected ?int $id = null;


Suggested change

protected ?int $id = null;

protected $id;

Revert and change /** @var int|null */ to /** @var mixed */

vvasiloi · 2021-08-27T06:27:58Z

src/Sylius/Component/Core/Model/Customer.php


    /**
     * @var Collection|AddressInterface[]
     *
     * @psalm-var Collection<array-key, AddressInterface>
     */
-    protected $addresses;
+    protected Collection $addresses;

    /** @var ShopUserInterface|null */


Suggested change

/** @var ShopUserInterface|null */

vvasiloi · 2021-08-27T06:28:50Z

src/Sylius/Component/Core/Model/Customer.php


    /**
     * @var Collection|AddressInterface[]
     *
     * @psalm-var Collection<array-key, AddressInterface>
     */
-    protected $addresses;
+    protected Collection $addresses;

    /** @var ShopUserInterface|null */
    protected $user;


Suggested change

protected $user;

protected ?ShopUserInterface $user = null;

vvasiloi · 2021-08-27T06:29:25Z

src/Sylius/Component/Core/Model/Image.php

@@ -15,24 +15,17 @@

 abstract class Image implements ImageInterface
 {
-    /** @var mixed */
-    protected $id;
+    /** @var mixed|null */


Suggested change

/** @var mixed|null */

/** @var mixed*/

vvasiloi · 2021-08-27T06:29:57Z

src/Sylius/Component/Core/Model/Image.php

-    /** @var mixed */
-    protected $id;
+    /** @var mixed|null */
+    protected $id = null;


Is null here necessary if there's no type-hint?

vvasiloi · 2021-08-27T06:31:25Z

src/Sylius/Component/Core/Model/Order.php

@@ -35,56 +35,47 @@ class Order extends BaseOrder implements OrderInterface
    /** @var ChannelInterface|null */
    protected $channel;


Suggested change

protected $channel;

protected ?ChannelInterface $channel = null;

It also missed the $customer above.
Looks like it ignores the cases when the class/interface is not explicitly imported because it's in the same namespace.

vvasiloi · 2021-08-27T06:34:20Z

src/Sylius/Component/Core/Model/Order.php


-    /** @var string|null */
-    protected $localeCode;
+    protected ?string $localeCode = null;

    /** @var BaseCouponInterface|null */
    protected $promotionCoupon;


Suggested change

protected $promotionCoupon;

protected ?BaseCouponInterface $promotionCoupon = null;

vvasiloi · 2021-08-27T06:34:29Z

src/Sylius/Component/Core/Model/Order.php


-    /** @var string|null */
-    protected $localeCode;
+    protected ?string $localeCode = null;

    /** @var BaseCouponInterface|null */


Suggested change

/** @var BaseCouponInterface|null */

vvasiloi · 2021-08-27T06:38:05Z

src/Sylius/Component/Core/Model/Product.php


-    /** @var BaseTaxonInterface */
-    protected $mainTaxon;
+    protected ?\Sylius\Component\Core\Model\TaxonInterface $mainTaxon = null;


Suggested change

protected ?\Sylius\Component\Core\Model\TaxonInterface $mainTaxon = null;

protected ?TaxonInterface $mainTaxon = null;

vvasiloi · 2021-08-27T06:40:56Z

@Zales0123 is it useful or shall I stop? 😄
For some reason I chose to add single comments instead of starting a review, so you've probably received a ton of notifications... sorry.

arti0090 · 2021-08-27T06:52:53Z

src/Sylius/Component/Core/Model/AdminUser.php

-    /** @var string */
-    protected $firstName;
+    protected ?string $firstName = null;

-    /** @var string */
-    protected $lastName;
+    protected ?string $lastName = null;

-    /** @var string */
-    protected $localeCode;
+    protected ?string $localeCode = null;

-    /** @var ImageInterface */
-    protected $avatar;
+    protected ?ImageInterface $avatar = null;


There was no null before (in annotation) but now you are adding it. May I ask why?

There was no null before (in annotation) but now you are adding it. May I ask why?

@arti0090 The annotation was wrong. Both the getter and the setter allow null values, it's not initialized in the constructor and there's no default value, therefore it can be null in a lot of scenarios.

The truth is, these fields were nullable due to setters definition

vvasiloi · 2021-08-27T06:43:03Z

src/Sylius/Component/Core/Model/Product.php


    /**
     * @var Collection|ReviewInterface[]
     *
     * @psalm-var Collection<array-key, ReviewInterface>
     */
-    protected $reviews;
+    protected Collection $reviews;

    /** @var float */
    protected $averageRating = 0;


Suggested change

protected $averageRating = 0;

protected float $averageRating = 0;

The setter doesn't allow null values, but the getter has ?float as return type, so I'm not sure if it should be nullable.

vvasiloi · 2021-08-27T06:44:58Z

src/Sylius/Component/Core/Model/ProductVariant.php


-    /** @var int */
-    protected $onHold = 0;
+    protected ?int $onHold = 0;

    /** @var int */
    protected $onHand = 0;


Suggested change

protected $onHand = 0;

protected ?int $onHand = 0;

vvasiloi · 2021-08-27T06:45:06Z

src/Sylius/Component/Core/Model/ProductVariant.php


-    /** @var int */
-    protected $onHold = 0;
+    protected ?int $onHold = 0;

    /** @var int */


Suggested change

/** @var int */

vvasiloi · 2021-08-27T06:46:34Z

src/Sylius/Component/Core/Model/ProductVariant.php


-    /** @var Collection */
-    protected $channelPricings;
+    protected Collection $channelPricings;


This should probably have a @psalm-var Collection<array-key, ChannelPricingInterface> annotation

vvasiloi · 2021-08-27T06:49:05Z

src/Sylius/Component/Core/Model/ShopBillingData.php

@@ -15,26 +15,19 @@

 class ShopBillingData implements ShopBillingDataInterface
 {
-    /** @var int|null */
-    protected $id;
+    protected ?int $id = null;


Revert and change annotation to mixed.

vvasiloi · 2021-08-27T07:15:36Z

src/Sylius/Component/Shipping/Model/ShippingMethod.php


-    /** @var string */
-    protected $calculator;
+    protected ?string $calculator = null;

    /** @var array */


Suggested change

/** @var array */

vvasiloi · 2021-08-27T07:15:45Z

src/Sylius/Component/Shipping/Model/ShippingMethod.php


-    /** @var string */
-    protected $calculator;
+    protected ?string $calculator = null;

    /** @var array */
    protected $configuration = [];


Suggested change

protected $configuration = [];

protected array $configuration = [];

vvasiloi · 2021-08-27T07:16:04Z

src/Sylius/Component/Shipping/Model/ShippingMethodRule.php

@@ -21,14 +21,12 @@ class ShippingMethodRule implements ShippingMethodRuleInterface
    /** @var mixed */
    protected $id;

-    /** @var string */
-    protected $type;
+    protected ?string $type = null;

    /** @var array */


Suggested change

/** @var array */

vvasiloi · 2021-08-27T07:16:12Z

src/Sylius/Component/Shipping/Model/ShippingMethodRule.php

@@ -21,14 +21,12 @@ class ShippingMethodRule implements ShippingMethodRuleInterface
    /** @var mixed */
    protected $id;

-    /** @var string */
-    protected $type;
+    protected ?string $type = null;

    /** @var array */
    protected $configuration = [];


Suggested change

protected $configuration = [];

protected array $configuration = [];

vvasiloi · 2021-08-27T07:19:05Z

src/Sylius/Component/User/Model/User.php


-    /** @var \DateTimeInterface|null */
-    protected $credentialsExpireAt;
+    protected ?\DateTimeInterface $credentialsExpireAt = null;

    /**
     * We need at least one role to be able to authenticate


Missed this one.

vvasiloi · 2021-08-27T07:26:38Z

src/Sylius/Component/Addressing/Model/Country.php

     */
-    protected $code;
+    protected ?string $code = null;

    /**
     * @var Collection|ProvinceInterface[]


Suggested change

* @var Collection|ProvinceInterface[]

lchrusciel · 2021-08-27T07:38:38Z

src/Sylius/Component/Core/Model/AdminUser.php

-    /** @var string */
-    protected $firstName;
+    protected ?string $firstName = null;

-    /** @var string */
-    protected $lastName;
+    protected ?string $lastName = null;

-    /** @var string */
-    protected $localeCode;
+    protected ?string $localeCode = null;

-    /** @var ImageInterface */
-    protected $avatar;
+    protected ?ImageInterface $avatar = null;


The truth is, these fields were nullable due to setters definition

lchrusciel · 2021-08-27T07:47:31Z

src/Sylius/Component/Core/Model/Product.php


    /**
     * @var Collection|ReviewInterface[]
     *
     * @psalm-var Collection<array-key, ReviewInterface>
     */
-    protected $reviews;
+    protected Collection $reviews;

    /** @var float */
    protected $averageRating = 0;


Suggested change

protected $averageRating = 0;

protected float $averageRating = 0;

lchrusciel · 2021-08-27T07:47:55Z

src/Sylius/Component/Core/Model/Product.php


    /**
     * @var Collection|ReviewInterface[]
     *
     * @psalm-var Collection<array-key, ReviewInterface>
     */
-    protected $reviews;
+    protected Collection $reviews;

    /** @var float */


Suggested change

/** @var float */

lchrusciel · 2021-08-27T07:48:58Z

src/Sylius/Component/Core/Model/ProductVariant.php

    /** @var int */
    protected $onHand = 0;


Suggested change

/** @var int */

protected $onHand = 0;

protected ?int $onHand = 0;

lchrusciel · 2021-08-27T07:51:15Z

src/Sylius/Component/Core/Model/ShippingMethod.php

    /**
     * @var Collection|ChannelInterface[]
     *
     * @psalm-var Collection<array-key, ChannelInterface>
     */


Suggested change

/**

* @var Collection|ChannelInterface[]

*

* @psalm-var Collection<array-key, ChannelInterface>

*/

/** @psalm-var Collection<array-key, ChannelInterface> */

lchrusciel · 2021-08-27T08:00:37Z

src/Sylius/Component/Promotion/Model/PromotionRule.php

    /** @var array */
    protected $configuration = [];


Suggested change

/** @var array */

protected $configuration = [];

protected array $configuration = [];

lchrusciel · 2021-08-27T08:01:32Z

src/Sylius/Component/Shipping/Model/Shipment.php

     * @var Collection|ShipmentUnitInterface[]
     *


Suggested change

* @var Collection|ShipmentUnitInterface[]

*

lchrusciel · 2021-08-27T08:01:56Z

src/Sylius/Component/Shipping/Model/ShippingMethod.php

    /** @var array */
    protected $configuration = [];


Suggested change

/** @var array */

protected $configuration = [];

protected array $configuration = [];

lchrusciel · 2021-08-27T08:02:13Z

src/Sylius/Component/Taxation/Model/TaxCategory.php

     * @var Collection|TaxRateInterface[]
     *


Suggested change

* @var Collection|TaxRateInterface[]

*

lchrusciel · 2021-08-27T08:02:25Z

src/Sylius/Component/Taxonomy/Model/Taxon.php

     * @var Collection|TaxonInterface[]
     *


Suggested change

* @var Collection|TaxonInterface[]

*

Zales0123 · 2021-08-31T14:37:57Z

Thank you @vvasiloi for the review! I would never believe someone would be determined enough to take a look at it. I'm impressed 👏 I've fixed (hopefully) most of the comments, I've just left those about collection type hints, as I'm not 100% sure how the IDE would handle these types without them 🤷 . Still, can be improved later 🖖

Thank you! 🎉

GSadee · 2021-09-03T12:06:27Z

Thanks, Mateusz! 🥇

Zales0123 added the Maintenance CI configurations, READMEs, releases, etc. label Aug 26, 2021

Zales0123 requested a review from a team as a code owner August 26, 2021 23:35

Zales0123 mentioned this pull request Aug 26, 2021

PHP 7.4 syntax #12891

Closed

10 tasks

Use PHP 7.4 syntax in Sylius components

79fbdf6

Zales0123 force-pushed the php7.4-syntax-components branch from ee33acb to 79fbdf6 Compare August 27, 2021 05:31

vvasiloi reviewed Aug 27, 2021

View reviewed changes

arti0090 reviewed Aug 27, 2021

View reviewed changes

vvasiloi reviewed Aug 27, 2021

View reviewed changes

lchrusciel reviewed Aug 27, 2021

View reviewed changes

Arminek approved these changes Aug 30, 2021

View reviewed changes

After review fixes

b3776ba

GSadee approved these changes Sep 3, 2021

View reviewed changes

GSadee merged commit e4b7c8a into Sylius:1.10 Sep 3, 2021

Zales0123 deleted the php7.4-syntax-components branch September 3, 2021 13:35

Zales0123 restored the php7.4-syntax-components branch September 30, 2021 13:26

Zales0123 deleted the php7.4-syntax-components branch September 30, 2021 16:50

	protected $configuration = [];
	protected array $configuration = [];

	private $requestToChannelMap;
	private \SplObjectStorage $requestToChannelMap;

	private ?ChannelInterface $channel;
	private ChannelInterface $channel;

	private ?\Sylius\Component\Core\Model\OrderInterface $cart = null;
	private ?OrderInterface $cart = null;

		@@ -35,56 +35,47 @@ class Order extends BaseOrder implements OrderInterface
		/** @var ChannelInterface\|null */
		protected $channel;

	protected $channel;
	protected ?ChannelInterface $channel = null;

	protected $promotionCoupon;
	protected ?BaseCouponInterface $promotionCoupon = null;

	protected ?\Sylius\Component\Core\Model\TaxonInterface $mainTaxon = null;
	protected ?TaxonInterface $mainTaxon = null;

	protected $averageRating = 0;
	protected float $averageRating = 0;

	/** @var int */
	protected $onHand = 0;
	protected ?int $onHand = 0;

	/** @var array */
	protected $configuration = [];
	protected array $configuration = [];

Use PHP 7.4 syntax in Sylius components #13012

Use PHP 7.4 syntax in Sylius components #13012

Conversation

Zales0123 commented Aug 26, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vvasiloi commented Aug 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zales0123 commented Aug 31, 2021

GSadee commented Sep 3, 2021