pressbooks / coding-standards

Pressbooks Coding Standards
GNU General Public License v3.0
2 stars 2 forks source link

feat: support PHP 7.4 (resolves #8) #9

Closed greatislander closed 3 years ago

greatislander commented 3 years ago

This PR updates relevant dependencies so that PHP_CodeSniffer can be run on PHP 7.4.

richard015ar commented 3 years ago

@greatislander should it run on PHP 7.3 as well? I tried it on pressbooks/pressbooks repo and this happens:

$.app/plugins/pressbooks $ vendor/bin/phpcs --standard=phpcs.ruleset.xml *.php inc/ bin/
ERROR: Referenced sniff "WordPress.VIP.SessionVariableUsage" does not exist

Run "phpcs --help" for usage information

The error seems to be here: https://github.com/pressbooks/coding-standards/blob/c4b819bd86c90d825a8816837e311e2fcf6ea358/Pressbooks/ruleset.xml#L21

Maybe should check https://github.com/Automattic/VIP-Coding-Standards/releases to see what's changed?

SteelWagstaff commented 3 years ago

@richard015ar it looks like WordPress coding standards deprecated several sniffs at version 2, including:

The following sniffs which were previously deprecated, have been removed: ... WordPress.VIP.SessionVariableUsage without replacement.

Source: https://github.com/WordPress/WordPress-Coding-Standards/blob/07abb483d9cc85a8c33e83eb55414e68e12d4be6/CHANGELOG.md#removed-2

Humanmade's coding standards updated to WPCS v2.2.1 as of their 1.0 release: https://github.com/humanmade/coding-standards/pull/151

greatislander commented 3 years ago

@richard015ar @SteelWagstaff I believe this is resolved in my most recent commit. I'll test it with Pressbooks before you review again, though.

richard015ar commented 3 years ago

@greatislander thank you for make those changes. I tried on pressbooks/pressbooks using php 7.3 and I still see some problems. As we discussed during our last product meeting, we could adapt the rules from each repo later. But I noticed that it could takes some time that we need to take into account in our sprints. I will create a task for that so we can include it in some future sprint.

Those are error I got on pressbooks/pressbooks:


Fatal error: Uncaught Error: Class 'WordPress\Sniffs\Security\NonceVerificationSniff' not found in /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/pressbooks/coding-standards/Pressbooks/Sniffs/Security/NonceVerificationSniff.php on line 5

Error: Class 'WordPress\Sniffs\Security\NonceVerificationSniff' not found in /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/pressbooks/coding-standards/Pressbooks/Sniffs/Security/NonceVerificationSniff.php on line 5

Call Stack:
    0.0003     418288   1. {main}() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/bin/phpcs:0
    0.0078    1741720   2. PHP_CodeSniffer\Runner->runPHPCS() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/bin/phpcs:18
    0.0159    2212488   3. PHP_CodeSniffer\Runner->init() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Runner.php:70
    0.0208    2451296   4. PHP_CodeSniffer\Ruleset->__construct() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Runner.php:332
    0.0827    3162920   5. PHP_CodeSniffer\Ruleset->registerSniffs() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Ruleset.php:217
    0.0827    3162920   6. PHP_CodeSniffer\Autoload::loadFile() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Ruleset.php:1159
    0.0829    3211184   7. include('/shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/pressbooks/coding-standards/Pressbooks/Sniffs/Security/NonceVerificationSniff.php') /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/autoload.php:16

In order to check this I commented this class: https://github.com/pressbooks/coding-standards/blob/78c095ec951af51358deed17901b4ed6bd05c590/Pressbooks/Sniffs/CSRF/NonceVerificationSniff.php and then I got this:

$ vendor/bin/phpcs --standard=phpcs.ruleset.xml *.php inc/ bin/

Fatal error: Uncaught ReflectionException: Class  does not exist in /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Ruleset.php on line 1179

ReflectionException: Class  does not exist in /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Ruleset.php on line 1179

Call Stack:
    0.0004     418288   1. {main}() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/bin/phpcs:0
    0.0084    1741720   2. PHP_CodeSniffer\Runner->runPHPCS() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/bin/phpcs:18
    0.0152    2212488   3. PHP_CodeSniffer\Runner->init() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Runner.php:70
    0.0201    2451296   4. PHP_CodeSniffer\Ruleset->__construct() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Runner.php:332
    0.0827    3162920   5. PHP_CodeSniffer\Ruleset->registerSniffs() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Ruleset.php:217
    0.0829    3163608   6. ReflectionClass->__construct() /shared/httpd/pressbooks/site/web/app/plugins/pressbooks/vendor/squizlabs/php_codesniffer/src/Ruleset.php:1179

If this is something you can check and solve in this PR I would appreciate it. Additionally I will create a task for compatibility issues on our current repos that are using this one.

richard015ar commented 3 years ago

Issue created: https://github.com/pressbooks/private/issues/406

greatislander commented 3 years ago

@richard015ar The deprecated rules issues have been addressed and I can now run these coding standards against Pressbooks. However, there's a lot of stricter rules in the updated version of the Human Made coding standards that need to be evaluated to decide if you want to apply or disable them. Here's the output I get running against Pressbooks:

https://gist.github.com/greatislander/2ce2cf1c93d3b15d70266a29e1ea6b00