From 7ef01b2c0ded20ea053c35f6fb700e8e4f147f1a Mon Sep 17 00:00:00 2001 From: Bernhard Reiter Date: Tue, 31 Mar 2015 18:01:28 +0200 Subject: [PATCH] Add CONTRIBUTING.md with instructions on how to add a new test --- CONTRIBUTING.md | 102 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..c6ea9f9 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,102 @@ +Filing an Issue +--------------- + +If you've found something in a WordPress theme or plugin that should be reported by VIP-scanner, please file an issue with some sample code. + +Adding a New Check +------------------ + +Adding a new VIP-scanner check can be considered a testboook example of [Test-driven development](https://en.wikipedia.org/wiki/Test-driven_development) (TDD). Mind the difference between a test (as in TDD) and a check, which is a sort of code sniff specific to VIP-scanner, used for scanning a theme or plugin for bad code. We'll a start by a test that will ensure the functionality of our check. + +Here's a recipe: + +1. Think of a suitable name for your check. In this example, which is a simplified version of a check that actually ships with vip-scanner, we're going to add a check that scans files for ill-formed escaping, i.e. `esc_attr` being wrapped around `print` or `printf` statements instead of the other way round. We'll name our check class `EscapingCheck`. +2. Create a file to hold some erroneous code in vip-scanner/tests/data. The file should be named like the check, only with `Check` replaced by `Test`; also, use `inc` instead of `php` for the file extension -- so in our example, EscapingTest.inc. +3. Put the following code into that file: + +```php +assertEqual()` to make assertions for which PHPUnit checks. + +6. Each test method contains of three parts: + + 1. An array of expected errors + 2. The actual check, which returns an array of actual errors found in your 'inc' file. + 3. An assertion comparing those two arrays. + +7. At this point, you should put some thought into finding a slug and description for the error you're diagnosing, and what error level to assign to it -- the latter can be blocker, warning, and note. Use that data in the `$expected_errors` array like so: + +```php + 'functions-file', + 'level' => BaseScanner::LEVEL_BLOCKER, + 'description' => sprintf( + __( 'The function %1$s is being passed as the first parameter of %2$s. This is problematic because %1$s echoes a string which will not be escaped by %2$s.', 'vip-scanner' ), + 'printf()', + 'esc_attr()' + ), + 'file' => 'EscapingTest.inc', + 'lines' => 5, + ), + array( 'slug' => 'functions-file', + 'level' => BaseScanner::LEVEL_BLOCKER, + 'description' => sprintf( + __( '%1$s is being passed as the first parameter of %2$s.', 'vip-scanner' ), + 'print', + 'esc_attr()' + ), + 'file' => 'EscapingTest.inc', + 'lines' => 6, + ), + ); + $actual_errors = $this->checkFile( 'EscapingTest.inc' ); + $this->assertEqualErrors( $expected_errors, $actual_errors ); + } +} +``` + +The 'file' and 'lines' values state where the check is supposed to find the erroneous code -- compare that to the EscapingTest.inc sample code above. + +7. From the top-level vip-scanner directory, run `phpunit`. Your test should fail because no class named `EscapingCheck` exists yet and thus can be found by `CodeCheckTestBase` -- not because of some syntax errors in your test file! + +8. Create your check file in vip-scanner/vip-scanner/checks/. In our case, that's `EscapingCheck.php`. Start with an explanatory commentary and the following stub: + +```php +