Skip to content

Commit

Permalink
Codesniffer: Add a package to hold our coding standard
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
anomiex committed Oct 7, 2020
1 parent 8a84a23 commit 7eec79d
Show file tree
Hide file tree
Showing 16 changed files with 297 additions and 26 deletions.
16 changes: 1 addition & 15 deletions .phpcs.xml.dist
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,9 @@
<config name="minimum_supported_wp_version" value="5.4" />
<config name="testVersion" value="5.6-"/>

<rule ref="PHPCompatibilityWP"/>
<rule ref="WordPress-Core" />
<rule ref="WordPress-Docs" />
<rule ref="WordPress-Extra" />
<rule ref="VariableAnalysis" />
<rule ref="./bin/PHPCSStandards/Jetpack" />

<!-- Elevate undefined variables to an Error instead of a Warning. -->
<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable">
<type>error</type>
</rule>
<rule ref="Jetpack" />

<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis">
<properties>
<property name="allowWordPressPassByRefFunctions" value="true" />
</properties>

<!-- Template files use extracted variables. -->
<exclude-pattern>/extensions/blocks/podcast-player/templates/*</exclude-pattern>
</rule>
Expand Down
4 changes: 0 additions & 4 deletions bin/PHPCSStandards/Jetpack/ruleset.xml

This file was deleted.

1 change: 0 additions & 1 deletion bin/phpcs-requirelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
];
6 changes: 2 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
42 changes: 40 additions & 2 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions packages/codesniffer/.gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Files not needed to be distributed in the package.
.gitattributes export-ignore
phpunit.xml.dist export-ignore
tests/ export-ignore
2 changes: 2 additions & 0 deletions packages/codesniffer/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
vendor
composer.lock
22 changes: 22 additions & 0 deletions packages/codesniffer/Jetpack/ruleset.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0"?>
<ruleset name="Jetpack">
<description>Jetpack coding standards. Based on the WordPress coding standards, with some additions.</description>

<rule ref="PHPCompatibilityWP" />
<rule ref="WordPress-Core" />
<rule ref="WordPress-Docs" />
<rule ref="WordPress-Extra" />
<rule ref="VariableAnalysis" />

<!-- Elevate undefined variables to an Error instead of a Warning. -->
<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable">
<type>error</type>
</rule>

<rule ref="VariableAnalysis.CodeAnalysis.VariableAnalysis">
<properties>
<property name="allowWordPressPassByRefFunctions" value="true" />
</properties>
</rule>

</ruleset>
27 changes: 27 additions & 0 deletions packages/codesniffer/README.md
Original file line number Diff line number Diff line change
@@ -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
<rule ref="Jetpack" />
```
28 changes: 28 additions & 0 deletions packages/codesniffer/composer.json
Original file line number Diff line number Diff line change
@@ -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
}
16 changes: 16 additions & 0 deletions packages/codesniffer/phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<phpunit bootstrap="tests/php/bootstrap.php" backupGlobals="true" colors="true">
<testsuites>
<testsuite name="main">
<directory prefix="test" suffix=".php">tests/php/tests</directory>
</testsuite>
</testsuites>
<filter>
<whitelist processUncoveredFilesFromWhitelist="false">
<directory suffix=".php">.</directory>
<exclude>
<directory suffix=".php">tests</directory>
<directory suffix=".php">vendor</directory>
</exclude>
</whitelist>
</filter>
</phpunit>
18 changes: 18 additions & 0 deletions packages/codesniffer/tests/php/bootstrap.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php
/**
* Bootstrap file for the codesniffer test suite.
*
* @package automattic/jetpack-codesniffer
*/

// Include the Composer autoloader.
require_once __DIR__ . '/../../vendor/autoload.php';

// Phpcs needs some bootstrapping of its own for tests to work.
require_once __DIR__ . '/../../vendor/squizlabs/php_codesniffer/tests/bootstrap.php';

// Register all the phpcs installed standards.
$installed_standards = PHP_CodeSniffer\Util\Standards::getInstalledStandardDetails();
foreach ( $installed_standards as $name => $details ) {
PHP_CodeSniffer\Autoload::addSearchPath( $details['path'], $details['namespace'] );
}
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php
/**
* A dummy file to test the Jetpack.Constants.MasterUserConstant sniff.
*
* @package automattic/jetpack-codesniffer
*/

if ( defined( 'JETPACK_MASTER_USER' ) ) {
$const = JETPACK_MASTER_USER;
}
125 changes: 125 additions & 0 deletions packages/codesniffer/tests/php/tests/test-jetpackstandard.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
<?php // phpcs:ignore WordPress.Files.FileName.InvalidClassFileName
/**
* Tests for the Jetpack phpcs standard.
*
* @package automattic/jetpack-codesniffer
*/

namespace Jetpack\Sniffs\Tests;

use PHP_CodeSniffer\Config;
use PHP_CodeSniffer\Files\DummyFile;
use PHP_CodeSniffer\Reporter;
use PHP_CodeSniffer\Ruleset;
use PHPUnit\Framework\TestCase;
use RecursiveDirectoryIterator;
use RecursiveIteratorIterator;
use RegexIterator;

/**
* Tests for the Jetpack phpcs standard.
*/
class JetpackStandardTest extends TestCase {

/**
* Run phpcs on a file.
*
* @param string $file File to process. Actual data will be read from "$file.tolint".
* @param bool $fix Run in fix mode, returning the fixed file.
* @return string If `$fix` is false, the phpcs report. If `$fix` is true,
* the fixed file.
*/
private function run_phpcs( $file, $fix ) {
// phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
$contents = file_get_contents( "{$file}.tolint" );
$this->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;
}

}

0 comments on commit 7eec79d

Please sign in to comment.