Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some reported PHPstan issues #262

Open
hostep opened this issue Aug 10, 2023 · 4 comments
Open

Some reported PHPstan issues #262

hostep opened this issue Aug 10, 2023 · 4 comments

Comments

@hostep
Copy link

hostep commented Aug 10, 2023

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
Copy link
Contributor

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
Copy link
Author

hostep commented Feb 5, 2024

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
Copy link

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

@hostep
Copy link
Author

hostep commented Jun 7, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants