From cadfcaaca1e6531079e982e32dbb71661a12ff75 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Wed, 7 Oct 2020 12:58:52 -0400 Subject: [PATCH] Codesniffer: Add a package to hold our coding standard This will make it a bit cleaner in the future to manage tests and dependencies for the coding standard. And, if some other repo wants to use our coding standards, they can more easily do so. The new coding standard package imports the PHPCompatibility and WordPress standards, so those are removed from `.phpcs.xml.dist`. The phpcs configuration specific to Jetpack remains in `.phpcs.xml.dist`. --- .phpcs.xml.dist | 16 +-- bin/PHPCSStandards/Jetpack/ruleset.xml | 4 - bin/phpcs-requirelist.js | 1 - composer.json | 6 +- composer.lock | 42 +++++- packages/codesniffer/.gitattributes | 4 + packages/codesniffer/.gitignore | 2 + .../Constants/MasterUserConstantSniff.php | 2 +- packages/codesniffer/Jetpack/ruleset.xml | 22 +++ packages/codesniffer/README.md | 41 ++++++ packages/codesniffer/composer.json | 28 ++++ packages/codesniffer/phpunit.xml.dist | 16 +++ packages/codesniffer/tests/php/bootstrap.php | 18 +++ .../files/master-user-constant.php.report | 2 + .../files/master-user-constant.php.tolint | 10 ++ .../tests/php/tests/test-jetpackstandard.php | 125 ++++++++++++++++++ 16 files changed, 312 insertions(+), 27 deletions(-) delete mode 100644 bin/PHPCSStandards/Jetpack/ruleset.xml create mode 100644 packages/codesniffer/.gitattributes create mode 100644 packages/codesniffer/.gitignore rename {bin/PHPCSStandards => packages/codesniffer}/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php (95%) create mode 100644 packages/codesniffer/Jetpack/ruleset.xml create mode 100644 packages/codesniffer/README.md create mode 100644 packages/codesniffer/composer.json create mode 100644 packages/codesniffer/phpunit.xml.dist create mode 100644 packages/codesniffer/tests/php/bootstrap.php create mode 100644 packages/codesniffer/tests/php/tests/files/master-user-constant.php.report create mode 100644 packages/codesniffer/tests/php/tests/files/master-user-constant.php.tolint create mode 100644 packages/codesniffer/tests/php/tests/test-jetpackstandard.php diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 5a1048e4bcea5..c1edb9e04ffc3 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -3,23 +3,9 @@ - - - - - - - - - - error - + - - - - /extensions/blocks/podcast-player/templates/* diff --git a/bin/PHPCSStandards/Jetpack/ruleset.xml b/bin/PHPCSStandards/Jetpack/ruleset.xml deleted file mode 100644 index e2fc11150a211..0000000000000 --- a/bin/PHPCSStandards/Jetpack/ruleset.xml +++ /dev/null @@ -1,4 +0,0 @@ - - - Jetpack Specific lint rules. - diff --git a/bin/phpcs-requirelist.js b/bin/phpcs-requirelist.js index 14390c40e6c9c..c554c2c367665 100644 --- a/bin/phpcs-requirelist.js +++ b/bin/phpcs-requirelist.js @@ -87,5 +87,4 @@ module.exports = [ 'tests/php/general/test-class.jetpack-network.php', 'tests/php/test_class.jetpack_photon.php', 'views/admin/deactivation-dialog.php', - 'bin/PHPCSStandards', ]; diff --git a/composer.json b/composer.json index 11d81e317f714..0da55eaae1e48 100644 --- a/composer.json +++ b/composer.json @@ -39,11 +39,9 @@ "nojimage/twitter-text-php": "3.1.1" }, "require-dev": { + "automattic/jetpack-codesniffer": "@dev", "dealerdirect/phpcodesniffer-composer-installer": "0.7.0", - "phpcompatibility/phpcompatibility-wp": "2.1.0", - "sirbrillig/phpcs-changed": "2.5.1", - "sirbrillig/phpcs-variable-analysis": "2.9.0", - "wp-coding-standards/wpcs": "2.3.0" + "sirbrillig/phpcs-changed": "2.5.1" }, "scripts": { "php:compatibility": "vendor/bin/phpcs -p -s --runtime-set testVersion '5.6-' --standard=PHPCompatibilityWP --ignore=docker,tools,tests,node_modules,vendor,packages/*/wordpress --extensions=php", diff --git a/composer.lock b/composer.lock index 470a2b88a8d77..4c6a88e15556c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1fb44b62143c40a6ce29ea8cedd246ed", + "content-hash": "efff4548a1ae7ce5242533ce98017806", "packages": [ { "name": "automattic/jetpack-a8c-mc-stats", @@ -927,6 +927,43 @@ } ], "packages-dev": [ + { + "name": "automattic/jetpack-codesniffer", + "version": "dev-monorepo", + "dist": { + "type": "path", + "url": "./packages/codesniffer", + "reference": "32f787cf2b42f5b1b4ef00244557f289bc5f5756" + }, + "require": { + "dealerdirect/phpcodesniffer-composer-installer": "0.*", + "phpcompatibility/phpcompatibility-wp": "2.1.0", + "sirbrillig/phpcs-variable-analysis": "2.9.0", + "wp-coding-standards/wpcs": "2.3.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.7 || ^6.5 || ^7.5" + }, + "type": "phpcodesniffer-standard", + "autoload": { + "psr-4": { + "Automattic\\Jetpack\\Sniffs\\": "Jetpack/Sniffs" + } + }, + "scripts": { + "phpunit": [ + "@composer install", + "./vendor/phpunit/phpunit/phpunit --colors=always" + ] + }, + "license": [ + "GPL-2.0-or-later" + ], + "description": "Jetpack Coding Standards. Based on the WordPress Coding Standards, with some additions.", + "transport-options": { + "relative": true + } + }, { "name": "dealerdirect/phpcodesniffer-composer-installer", "version": "v0.7.0", @@ -1380,7 +1417,8 @@ "automattic/jetpack-status": 20, "automattic/jetpack-sync": 20, "automattic/jetpack-terms-of-service": 20, - "automattic/jetpack-tracking": 20 + "automattic/jetpack-tracking": 20, + "automattic/jetpack-codesniffer": 20 }, "prefer-stable": true, "prefer-lowest": false, diff --git a/packages/codesniffer/.gitattributes b/packages/codesniffer/.gitattributes new file mode 100644 index 0000000000000..a6921e6098673 --- /dev/null +++ b/packages/codesniffer/.gitattributes @@ -0,0 +1,4 @@ +# Files not needed to be distributed in the package. +.gitattributes export-ignore +phpunit.xml.dist export-ignore +tests/ export-ignore diff --git a/packages/codesniffer/.gitignore b/packages/codesniffer/.gitignore new file mode 100644 index 0000000000000..7579f74311d35 --- /dev/null +++ b/packages/codesniffer/.gitignore @@ -0,0 +1,2 @@ +vendor +composer.lock diff --git a/bin/PHPCSStandards/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php b/packages/codesniffer/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php similarity index 95% rename from bin/PHPCSStandards/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php rename to packages/codesniffer/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php index 58500a3ed04e5..4f52821faf84a 100644 --- a/bin/PHPCSStandards/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php +++ b/packages/codesniffer/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php @@ -1,6 +1,6 @@ + + Jetpack coding standards. Based on the WordPress coding standards, with some additions. + + + + + + + + + + error + + + + + + + + + diff --git a/packages/codesniffer/README.md b/packages/codesniffer/README.md new file mode 100644 index 0000000000000..7e1b20cda393b --- /dev/null +++ b/packages/codesniffer/README.md @@ -0,0 +1,41 @@ +Jetpack Coding Standard +======================= + +This is a package implementing phpcs sniffs for the Jetpack Coding Standard. + +This standard is generally that of WordPress, with a few additions. + +Usage +----- + +In your project's `composer.json`, add the following lines: + +```json +{ + "require-dev": { + "dealerdirect/phpcodesniffer-composer-installer": "*", + "automattic/jetpack-codesniffer": "^1" + } +} +``` + +Your project must use the default composer vendor directory, `vendor`. + +You should then include the Jetpack rules in your `.phpcs.xml.dist`, like +```xml + +``` +You will also likely want to set some configuration for other included rulesets: +```xml + + +``` + +Included Standards +------------------ + +The Jetpack standard includes the following other standards: + +* [PHPCompatibilityWP](https://packagist.org/packages/phpcompatibility/phpcompatibility-wp) +* [WordPress-Core, WordPress-Docs, and WordPress-Extra](https://packagist.org/packages/wp-coding-standards/wpcs) +* [VariableAnalysis](https://packagist.org/packages/sirbrillig/phpcs-variable-analysis) diff --git a/packages/codesniffer/composer.json b/packages/codesniffer/composer.json new file mode 100644 index 0000000000000..cb3136d8a8a8a --- /dev/null +++ b/packages/codesniffer/composer.json @@ -0,0 +1,28 @@ +{ + "name": "automattic/jetpack-codesniffer", + "description": "Jetpack Coding Standards. Based on the WordPress Coding Standards, with some additions.", + "type": "phpcodesniffer-standard", + "license": "GPL-2.0-or-later", + "require": { + "dealerdirect/phpcodesniffer-composer-installer": "0.*", + "phpcompatibility/phpcompatibility-wp": "2.1.0", + "sirbrillig/phpcs-variable-analysis": "2.9.0", + "wp-coding-standards/wpcs": "2.3.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.7 || ^6.5 || ^7.5" + }, + "autoload": { + "psr-4": { + "Automattic\\Jetpack\\Sniffs\\": "Jetpack/Sniffs" + } + }, + "scripts": { + "phpunit": [ + "@composer install", + "./vendor/phpunit/phpunit/phpunit --colors=always" + ] + }, + "minimum-stability": "dev", + "prefer-stable": true +} diff --git a/packages/codesniffer/phpunit.xml.dist b/packages/codesniffer/phpunit.xml.dist new file mode 100644 index 0000000000000..98ca0f0599aee --- /dev/null +++ b/packages/codesniffer/phpunit.xml.dist @@ -0,0 +1,16 @@ + + + + tests/php/tests + + + + + . + + tests + vendor + + + + diff --git a/packages/codesniffer/tests/php/bootstrap.php b/packages/codesniffer/tests/php/bootstrap.php new file mode 100644 index 0000000000000..b938003f446ec --- /dev/null +++ b/packages/codesniffer/tests/php/bootstrap.php @@ -0,0 +1,18 @@ + $details ) { + PHP_CodeSniffer\Autoload::addSearchPath( $details['path'], $details['namespace'] ); +} diff --git a/packages/codesniffer/tests/php/tests/files/master-user-constant.php.report b/packages/codesniffer/tests/php/tests/files/master-user-constant.php.report new file mode 100644 index 0000000000000..0f4b4dfe0660f --- /dev/null +++ b/packages/codesniffer/tests/php/tests/files/master-user-constant.php.report @@ -0,0 +1,2 @@ + 9 | WARNING | JETPACK_MASTER_USER constant should not be used. Use the blog token to make requests instead, or use the current user token when needed. + | | (Jetpack.Constants.MasterUserConstant.ShouldNotBeUsed) diff --git a/packages/codesniffer/tests/php/tests/files/master-user-constant.php.tolint b/packages/codesniffer/tests/php/tests/files/master-user-constant.php.tolint new file mode 100644 index 0000000000000..7830c27e86bc2 --- /dev/null +++ b/packages/codesniffer/tests/php/tests/files/master-user-constant.php.tolint @@ -0,0 +1,10 @@ +assertInternalType( 'string', $contents ); + + $config = new Config(); + + $config->standards = array( __DIR__ . '/../../../Jetpack/ruleset.xml' ); + $config->files = array( $file ); + $config->encoding = 'utf-8'; + $config->reports = array( 'full' => null ); + $config->colors = false; + $config->reportWidth = PHP_INT_MAX; // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + $config->showSources = true; // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + $config->tabWidth = 4; // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + + $ruleset = new Ruleset( $config ); + $dummy = new DummyFile( $contents, $ruleset, $config ); + $dummy->process(); + + if ( ! $fix ) { + $reporter = new Reporter( $config ); + $reporter->cacheFileReport( $dummy ); + ob_start(); + $reporter->printReport( 'full' ); + $result = ob_get_clean(); + + // Clean up output. + $lines = preg_split( '/[\r\n]+/', $result, -1, PREG_SPLIT_NO_EMPTY ); + $lines = preg_grep( '/^-*$|^(?:Time:|FILE:|FOUND|PHPCBF) /', $lines, PREG_GREP_INVERT ); + return implode( "\n", $lines ) . "\n"; + } elseif ( $dummy->getFixableCount() ) { + $dummy->fixer->fixFile(); + return $dummy->fixer->getContents(); + } else { + return $contents; + } + } + + /** + * Test the sniffs by running phpcs or phpcbf against a file. + * + * @dataProvider provide_files + * @param string $file Base filename, without the ".tolint", ".report", or ".fixed" extension. + * @param bool $fix Run as phpcbf rather than phpcs. + */ + public function test_phpcs( $file, $fix ) { + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + $expect = file_get_contents( $fix ? "$file.fixed" : "$file.report" ); + $this->assertInternalType( 'string', $expect ); + $this->assertEquals( $expect, $this->run_phpcs( $file, $fix ) ); + } + + /** + * Provide arguments for `test_phpcs()`. + * + * @return array + */ + public function provide_files() { + $dir_iterator = new RecursiveDirectoryIterator( __DIR__ . '/files', RecursiveDirectoryIterator::CURRENT_AS_PATHNAME ); + $iterator = new RegexIterator( + new RecursiveIteratorIterator( $dir_iterator ), + '/\.(?:tolint|report|fixed)$/' + ); + $files = iterator_to_array( $iterator ); + + $ret = array(); + foreach ( $files as $file => $dummy ) { + $i = strrpos( $file, '.' ); + $ext = substr( $file, $i ); + $file = substr( $file, 0, $i ); + + switch ( $ext ) { + case '.tolint': + if ( ! isset( $files[ "$file.report" ] ) && ! isset( $files[ "$file.fixed" ] ) ) { + fprintf( STDERR, "%s: %s.tolint exists, but both %s.report and %s.fixed are missing.\n", __METHOD__, $file, $file, $file ); + } + break; + + case '.report': + case '.fixed': + if ( isset( $files[ "$file.tolint" ] ) ) { + $ret[ "$file$ext" ] = array( $file, '.fixed' === $ext ); + } else { + fprintf( STDERR, "%s: %s exists, but %s.tolint is missing.\n", __METHOD__, $file . $ext, $file ); + } + break; + } + } + + return $ret; + } + +}