search

klaviyo / magento2-klaviyo

38 stars 51 forks source link

Some reported PHPstan issues #262

Open hostep opened 1 year ago

hostep commented 1 year ago

Environment details

Klaviyo extension version: 4.0.12

Steps to reproduce

Run from inside a Magento shop:

$ vendor/bin/phpstan clear-result-cache && bin/magento setup:di:compile && vendor/bin/phpstan analyse --level=0 vendor/klaviyo

Expected result

Ideally we should see 0 problems

Actual result

We find a couple of small issues in the code that should be able to be resolved easily:

 ------ ---------------------------------------------------------------------
  Line   magento2-extension/Observer/KlaviyoOAuthObserver.php
 ------ ---------------------------------------------------------------------
  88     Caught class Klaviyo\Reclaim\Observer\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ ---------------------------------------------------------------------

 ------ ----------------------------------------------------------------------------------------------------------------------------------
  Line   magento2-extension/Test/Fakes/RadioBtnFake.php
 ------ ----------------------------------------------------------------------------------------------------------------------------------
  5      Class Klaviyo\Reclaim\Model\Config\Source\Radiobtn referenced with incorrect case: Klaviyo\Reclaim\Model\Config\Source\RadioBtn.
  7      Class Klaviyo\Reclaim\Model\Config\Source\Radiobtn referenced with incorrect case: Klaviyo\Reclaim\Model\Config\Source\RadioBtn.
 ------ ----------------------------------------------------------------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------------------
  Line   magento2-extension/Test/Unit/Helper/ScopeSettingTest.php
 ------ -------------------------------------------------------------------------------------
  61     Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_USERNAME.
  64     Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_PASSWORD.
  67     Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_EMAIL.
  103    Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_USERNAME.
  104    Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_PASSWORD.
  105    Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_EMAIL.
  119    Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_USERNAME.
  122    Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_PASSWORD.
  125    Access to undefined constant Klaviyo\Reclaim\Helper\ScopeSetting::KLAVIYO_EMAIL.
 ------ -------------------------------------------------------------------------------------

 ------ -----------------------------------------------------------------------------------------------
  Line   magento2-extension/Test/Unit/Observer/UserProfileNewsletterSubscribeObserverTest.php
 ------ -----------------------------------------------------------------------------------------------
  49     Instantiated class Klaviyo\Reclaim\Observer\UserProfileNewsletterSubscribeObserver not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
  58     Class Klaviyo\Reclaim\Observer\UserProfileNewsletterSubscribeObserver not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ -----------------------------------------------------------------------------------------------

Additional information

Maybe consider adding a static analyser check (like phpstan) to your automated test checks so code quality stays high all the time?

Thanks!

siddwarkhedkar commented 1 year ago

Hi @hostep, Thank you for creating this issue and providing this suggestion! Making sure this is correctly recorded and considered to be added to our product roadmap.

hostep commented 9 months ago

Stil not fixed in latest version 4.1.2 ...

And a new problem was even introduced:

 ------ ---------------------------------------------------------------------
  Line   magento2-extension/KlaviyoV3Sdk/KlaviyoV3Api.php
 ------ ---------------------------------------------------------------------
  500    Caught class Klaviyo\Reclaim\KlaviyoV3Sdk\Exception not found.
         💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ ---------------------------------------------------------------------

I think that fixing this one + the first one from the earlier report could fix some edge case problems. The other problems it reported are in Test classes, so are not as important to get fixed (but then I wonder if you ever run these tests ...)

etcarter42 commented 9 months ago

Thank you @hostep - I've added this to our internal ticket tracking this issue.

hostep commented 5 months ago

Still nothing has improved in the latest version: 4.1.4 ...

These aren't hard to fix, any idea why it takes so long?

Would it help if I send in a Pull Request?