Skip to content

[Enhancement] Improve LiteralNamespacesSniff #9

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

Closed
lenaorobei opened this issue Jan 14, 2019 · 5 comments
Closed

[Enhancement] Improve LiteralNamespacesSniff #9

lenaorobei opened this issue Jan 14, 2019 · 5 comments
Labels
accepted New rule is accepted enhancement Improvements to existing rules

Comments

@lenaorobei
Copy link
Contributor

Description

LiteralNamespacesSniff should not check class_exists($className) || interface_exists($className).

Expected behavior

Sniff should be rewritten in order to make static check and not rely on loaded Magento classes and interfaces.

Benefits

  1. Sniff will work without Magento instance.
  2. Sniff will work in IDE.
@lenaorobei lenaorobei added enhancement Improvements to existing rules accepted New rule is accepted labels Jan 14, 2019
@lenaorobei lenaorobei changed the title Improve LiteralNamespacesSniff [Enhancement] Improve LiteralNamespacesSniff Jan 29, 2019
@ldusan84
Copy link
Contributor

Hi @lenaorobei

Can we just remove class_exists and interface_exists checks or you meant something else?

@lenaorobei
Copy link
Contributor Author

Hi @ldusan84! We can start with it and run the sniff against Magento codebase. Need to make sure that regex pattern covers all use cases.

@ldusan84
Copy link
Contributor

ldusan84 commented Apr 17, 2019

Hi @lenaorobei

I checked and there is almost no difference:

  1. With class exists check:
FILE: /var/www/html/magento2/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Util/ModuleResolver.php
--------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------
 39 | WARNING | Use ::class notation instead.
--------------------------------------------------------------------------------------------------------------------------------------------------

  1. Without class exists check:
FILE: /var/www/html/magento2/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Util/MetadataGenerator/Swagger/autoload.php
----------------------------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------------------------------------------------------------------------
 11 | WARNING | Use ::class notation instead.
----------------------------------------------------------------------------------------------------------------------------------------------------------------------


FILE: /var/www/html/magento2/vendor/magento/magento2-functional-testing-framework/src/Magento/FunctionalTestingFramework/Util/ModuleResolver.php
--------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------------------------------------------------------------------
 39 | WARNING | Use ::class notation instead.
--------------------------------------------------------------------------------------------------------------------------------------------------

Let me know what you think.

@lenaorobei
Copy link
Contributor Author

Hi @ldusan84! Thanks for your investigation.
I run against whole Magento 2.3-develop codebase and here is summary result:

------------------------------------------------------------------------------------------------------
A TOTAL OF 0 ERRORS AND 109 WARNINGS WERE FOUND IN 37 FILES
------------------------------------------------------------------------------------------------------

There are a lot of odd findings in deprecated methods like:

https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Cms/Model/PageRepository.php#L212

https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Cms/Model/BlockRepository.php#L206

https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Customer/Model/ResourceModel/AddressRepository.php#L278

and a lot of findings in test files.

The only reasonable fix to me is to remove classExists + add exclude for test files.

        <exclude-pattern>*/_files/*</exclude-pattern>
        <exclude-pattern>*/Fixtures/*</exclude-pattern>
        <exclude-pattern>*/Test/*</exclude-pattern>
        <exclude-pattern>*Test.php</exclude-pattern>

@lenaorobei
Copy link
Contributor Author

Resolved in #89

fredden added a commit to fredden/magento-coding-standard that referenced this issue May 24, 2021
    Fatal error: Uncaught TypeError: vsprintf(): Argument magento#2 ($values) must be of type array, int given in /srv/www/vendor/squizlabs/php_codesniffer/src/Files/File.php:1056
    Stack trace:
    #0 /srv/www/vendor/squizlabs/php_codesniffer/src/Files/File.php(1056): vsprintf('Direct throw of...', 531)
    magento#1 /srv/www/vendor/squizlabs/php_codesniffer/src/Files/File.php(706): PHP_CodeSniffer\Files\File->addMessage(false, 'Direct throw of...', 67, 4, 'FoundDirectThro...', 531, 8, false)
    magento#2 /srv/www/vendor/magento/magento-coding-standard/Magento2/Sniffs/Exceptions/DirectThrowSniff.php(48): PHP_CodeSniffer\Files\File->addWarning('Direct throw of...', 526, 'FoundDirectThro...', 531)
    magento#3 /srv/www/vendor/squizlabs/php_codesniffer/src/Files/File.php(498): Magento2\Sniffs\Exceptions\DirectThrowSniff->process(Object(PHP_CodeSniffer\Files\LocalFile), 526)
    magento#4 /srv/www/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(92): PHP_CodeSniffer\Files\File->process()
    #5 /srv/www/vendor/squizlabs/php_codesniffer/src/Runner.php(630): PHP_CodeSniffer\Files\LocalFile->process()
    magento#6 /srv/www/vendor/squizlabs/php_codesniffer/src/Runner.php(434): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile))
    magento#7 /srv/www/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
    magento#8 /srv/www/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
    magento#9 {main}
      thrown in /srv/www/vendor/squizlabs/php_codesniffer/src/Files/File.php on line 1056
magento-devops-reposync-svc pushed a commit that referenced this issue Jul 26, 2021
…coding-standard-206

[Imported] Updated webonyx/graphql-php to ^14.9
fredden added a commit to fredden/magento-coding-standard that referenced this issue Sep 2, 2021
  PHP Fatal error:  Uncaught TypeError: strpos(): Argument magento#2 ($needle) must be of type string, null given in /srv/www/vendor/magento/magento-coding-standard/Magento2/Sniffs/Annotation/MethodArgumentsSniff.php:629
  Stack trace:
  #0 /srv/www/vendor/magento/magento-coding-standard/Magento2/Sniffs/Annotation/MethodArgumentsSniff.php(629): strpos('\\VendorName\\Mod...', NULL)
  magento#1 /srv/www/vendor/magento/magento-coding-standard/Magento2/Sniffs/Annotation/MethodArgumentsSniff.php(515): Magento2\Sniffs\Annotation\MethodArgumentsSniff->validateFormattingConsistency(Array, Array, Object(PHP_CodeSniffer\Files\LocalFile), Array)
  magento#2 /srv/www/vendor/magento/magento-coding-standard/Magento2/Sniffs/Annotation/MethodArgumentsSniff.php(600): Magento2\Sniffs\Annotation\MethodArgumentsSniff->validateMethodParameterAnnotations(385, Array, Array, Object(PHP_CodeSniffer\Files\LocalFile), Array, 356, 380)
  magento#3 /srv/www/vendor/squizlabs/php_codesniffer/src/Files/File.php(498): Magento2\Sniffs\Annotation\MethodArgumentsSniff->process(Object(PHP_CodeSniffer\Files\LocalFile), 385)
  magento#4 /srv/www/vendor/squizlabs/php_codesniffer/src/Files/LocalFile.php(92): PHP_CodeSniffer\Files\File->process()
  #5 /srv/www/vendor/squizlabs/php_codesniffer/src/Runner.php(630): PHP_CodeSniffer\Files\LocalFile->process()
  magento#6 /srv/www/vendor/squizlabs/php_codesniffer/src/Runner.php(434): PHP_CodeSniffer\Runner->processFile(Object(PHP_CodeSniffer\Files\LocalFile))
  magento#7 /srv/www/vendor/squizlabs/php_codesniffer/src/Runner.php(114): PHP_CodeSniffer\Runner->run()
  magento#8 /srv/www/vendor/squizlabs/php_codesniffer/bin/phpcs(18): PHP_CodeSniffer\Runner->runPHPCS()
  magento#9 {main}
    thrown in /srv/www/vendor/magento/magento-coding-standard/Magento2/Sniffs/Annotation/MethodArgumentsSniff.php on line 629
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted New rule is accepted enhancement Improvements to existing rules
Projects
None yet
Development

No branches or pull requests

2 participants