From 7eec79d6f60db0b72ce1321aa1ba354f3dc22e0f 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 | 0 packages/codesniffer/Jetpack/ruleset.xml | 22 +++ packages/codesniffer/README.md | 27 ++++ 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, 297 insertions(+), 26 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 (100%) 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 00bcdae05a58b..507478eda07b2 100644 --- a/bin/phpcs-requirelist.js +++ b/bin/phpcs-requirelist.js @@ -86,5 +86,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 a9cd7d1717136..f90bfd88276fc 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.8.3", - "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 1b20731d7b6ed..3707b1c5f06a7 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": "31c2399fec4166dba523747a45bc3e63", + "content-hash": "efff4548a1ae7ce5242533ce98017806", "packages": [ { "name": "automattic/jetpack-a8c-mc-stats", @@ -927,6 +927,43 @@ } ], "packages-dev": [ + { + "name": "automattic/jetpack-codesniffer", + "version": "dev-add/codesniffer-package", + "dist": { + "type": "path", + "url": "./packages/codesniffer", + "reference": "d74b803f4fe94393e83a65207cbbe9b0532da356" + }, + "require": { + "dealerdirect/phpcodesniffer-composer-installer": "0.*", + "phpcompatibility/phpcompatibility-wp": "2.1.0", + "sirbrillig/phpcs-variable-analysis": "2.8.3", + "wp-coding-standards/wpcs": "2.3.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.7 || ^6.5 || ^7.5" + }, + "type": "phpcodesniffer-standard", + "autoload": { + "psr-4": { + "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 100% rename from bin/PHPCSStandards/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php rename to packages/codesniffer/Jetpack/Sniffs/Constants/MasterUserConstantSniff.php diff --git a/packages/codesniffer/Jetpack/ruleset.xml b/packages/codesniffer/Jetpack/ruleset.xml new file mode 100644 index 0000000000000..16b0495ded62e --- /dev/null +++ b/packages/codesniffer/Jetpack/ruleset.xml @@ -0,0 +1,22 @@ + + + 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..8593855673d6f --- /dev/null +++ b/packages/codesniffer/README.md @@ -0,0 +1,27 @@ +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 + +``` diff --git a/packages/codesniffer/composer.json b/packages/codesniffer/composer.json new file mode 100644 index 0000000000000..3487821d714cd --- /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.8.3", + "wp-coding-standards/wpcs": "2.3.0" + }, + "require-dev": { + "phpunit/phpunit": "^5.7 || ^6.5 || ^7.5" + }, + "autoload": { + "psr-4": { + "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; + } + +}