SendCloud / sendcloud

A Magento 2 module for Sendcloud
9 stars 21 forks source link

Added some fixes for PHP 8.2 and other smaller cleanup. #161

Closed hostep closed 1 year ago

hostep commented 1 year ago

Hi there

Added some fixes:

All these (except the composer.json one) were found by using the phpstan tool and scanning on level 0, these are the things it reported and are now fixed (it finds more problems, but those are more complicated to solve and since I don't know this module very well, I decided not to try to tackle those left overs):

$ /path/to/phpstan analyse --level=0 .

 ------ --------------------------------------------------------------------------------------------------------
  Line   Controller/Adminhtml/AutoConnect/AutoGenerateApiUser.php
 ------ --------------------------------------------------------------------------------------------------------
  98     Instantiated class Magento\Setup\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
  145    Instantiated class Magento\Setup\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
  189    Instantiated class Magento\Setup\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ --------------------------------------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   Controller/Adminhtml/Configuration/Save.php
 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  42     Method SendCloud\SendCloud\Controller\Adminhtml\Configuration\Save::execute() should return Magento\Framework\App\ResponseInterface|Magento\Framework\Controller\ResultInterface but return statement is missing.
 ------ -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

  ------ ---------------------------------------------------------------------
  Line   Model/CheckoutConfiguration.php
 ------ ---------------------------------------------------------------------
  106    Caught class SendCloud\SendCloud\Model\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
  145    Caught class SendCloud\SendCloud\Model\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
  174    Caught class SendCloud\SendCloud\Model\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ ---------------------------------------------------------------------

  ------ ----------------------------------------------------------------------------------------------------------------------------
  Line   Model/ResourceModel/SendcloudDeliveryMethod/Collection.php
 ------ ----------------------------------------------------------------------------------------------------------------------------
  48     Access to an undefined property SendCloud\SendCloud\Model\ResourceModel\SendcloudDeliveryMethod\Collection::$aggregations.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
  59     Access to an undefined property SendCloud\SendCloud\Model\ResourceModel\SendcloudDeliveryMethod\Collection::$aggregations.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
 ------ ----------------------------------------------------------------------------------------------------------------------------

  ------ --------------------------------------------------------------------------------------------------------------------------
  Line   Model/ResourceModel/SendcloudDeliveryZone/Collection.php
 ------ --------------------------------------------------------------------------------------------------------------------------
  39     Access to an undefined property SendCloud\SendCloud\Model\ResourceModel\SendcloudDeliveryZone\Collection::$aggregations.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
  50     Access to an undefined property SendCloud\SendCloud\Model\ResourceModel\SendcloudDeliveryZone\Collection::$aggregations.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
 ------ --------------------------------------------------------------------------------------------------------------------------

  ------ ----------------------------------------------------------------------------------------------------
  Line   Plugin/Order/CheckoutOrderRepository.php
 ------ ----------------------------------------------------------------------------------------------------
  74     Access to an undefined property SendCloud\SendCloud\Plugin\Order\CheckoutOrderRepository::$logger.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
 ------ ----------------------------------------------------------------------------------------------------

 ------ --------------------------------------------------------------------------------------------
  Line   Plugin/Order/OrderRepository.php
 ------ --------------------------------------------------------------------------------------------
  81     Access to an undefined property SendCloud\SendCloud\Plugin\Order\OrderRepository::$logger.
         💡 Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property
 ------ --------------------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   Setup/Patch/Data/UpdateInternationalShipping.php
 ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  101    Method SendCloud\SendCloud\Setup\Patch\Data\UpdateInternationalShipping::apply() should return $this(SendCloud\SendCloud\Setup\Patch\Data\UpdateInternationalShipping) but return statement is missing.
 ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

 ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  Line   Setup/Patch/Data/UpdateServicePoint.php
 ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  105    Method SendCloud\SendCloud\Setup\Patch\Data\UpdateServicePoint::apply() should return $this(SendCloud\SendCloud\Setup\Patch\Data\UpdateServicePoint) but return statement is missing.
 ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Be sure to double check all changes, I haven't tested anything, only looked at the output of phpstan.

Thanks!

hostep commented 1 year ago

Added an additional change: dropping support for PHP 7.0 from the composer.json constraints, since you guys use nullable types syntax here and there, which was only introduced in PHP 7.1

Examples:

$ find . -type f ! -path '*/vendor/*' -iname '*.php' -exec php70 -l {} \;

PHP Parse error:  syntax error, unexpected '?', expecting variable (T_VARIABLE) in ./Block/Adminhtml/AbstractOrder.php on line 33

Parse error: syntax error, unexpected '?', expecting variable (T_VARIABLE) in ./Block/Adminhtml/AbstractOrder.php on line 33

Errors parsing ./Block/Adminhtml/AbstractOrder.php
PHP Parse error:  syntax error, unexpected '?', expecting variable (T_VARIABLE) in ./Block/Adminhtml/Form.php on line 34

Parse error: syntax error, unexpected '?', expecting variable (T_VARIABLE) in ./Block/Adminhtml/Form.php on line 34

Errors parsing ./Block/Adminhtml/Form.php
PHP Parse error:  syntax error, unexpected '?', expecting variable (T_VARIABLE) in ./Helper/Checkout.php on line 51

Parse error: syntax error, unexpected '?', expecting variable (T_VARIABLE) in ./Helper/Checkout.php on line 51

Errors parsing ./Helper/Checkout.php
tanjalog commented 1 year ago

Dear @hostep

Thanks for reaching out to us.

We will investigate your request and get back to you as soon as possible.

Best regards

logeecom commented 1 year ago

[to client on git]

Hi @hostep

Thank you for your request, please note that we review it and accepted and regarding this, a new version of the plugin is released on the marketplace.

Best regards

hostep commented 1 year ago

Thanks @logeecom!

Can you please make sure the github repo is up2date with the latest version? It's easier to review the changes between versions this way. See #162