Improving through automation
@grapplerulrich
slides.com/ulrichpogson/automated-improvement-wcbelfast
Peer Review
Has your code been reviewed before?
Have you reviewed code before?
Reviewing large changes is difficult!
Problem:
Smaller pull requests
Solution:
Partial
Things to check in a review
Code Style
class wp_foo extends wp_bar {
public function sample_method( $a, $b = null ) {
if ( $a === $b ) {
bar();
} elseif ( $a > $b ) {
$foo->bar( $arg1 );
}
}
final public static function bar() {
// method body
}
}
namespace Vendor\Package;
use BarClass as Bar;
class Foo extends Bar
{
public function sampleMethod($a, $b = null)
{
if ($a === $b) {
bar();
} elseif ($a > $b) {
$foo->bar($arg1);
}
}
final public static function bar()
{
// method body
}
}
WordPress
PSR
Things to check in a review
Best Practice
Code Style
function is_custom_page() {
if ( is_page() ) {
if ( is_page_template( 'about.php' ) ) {
return true;
}
} else {
return false;
}
}
function is_custom_page() {
if ( ! is_page() ) {
return false;
}
if ( ! is_page_template( 'about.php' ) ) {
return false;
}
return true;
}
Nested conditionals
Return early
Things to check in a review
Best Practice
Code Style
Code Compatibility
Fatal error: Cannot redeclare enqueue_scripts() (previously declared)
Things to check in a review
Best Practice
Code Style
Code Compatibility
Documentation
Things to check in a review
Best Practice
Code Style
Code Compatibility
Documentation
Code Quality
Alternative?
Automation
Benefits
Scalability
Consistency
Team Moral
Challenges
False positives
Only parses code, does not understand it
stylelint
$ stylelint style.css
style.css
107:15 ⚠ Expected numeric font-weight notation
169:26 ⚠ Unexpected duplicate name monospace
509:9 ⚠ Unexpected named color "royalblue"
$ stylelint '**/*.scss' --syntax scss
--config node_modules/stylelint-config-wordpress/scss.js
sass/elements/_elements.scss
7:11 ✖ Expected empty line before comment
15:11 ✖ Expected newline after ","
42:17 ✖ Unexpected missing end-of-source newline
$ stylelint '**/*.scss' --syntax less
{
"name": "_s",
"version": "1.0.0",
"scripts": {
"css-lint": "stylelint style.css"
},
"devDependencies": {
"stylelint": "^8.1.1",
"stylelint-config-wordpress": "^12.0.0"
},
"stylelint": {
"defaultSeverity": "warning",
"extends": [
"stylelint-config-wordpress"
]
}
}
package.json
ESLint
$ eslint js/**
js/skip-link-focus-fix.js
9:11 error Unexpected space before function parentheses
13:50 error Expected line before comment
{
"name": "_s",
"version": "1.0.0",
"scripts": {
"js-lint": "eslint js/**",
},
"devDependencies": {
"eslint": "^4.3.0",
"eslint-config-wordpress": "^2.0.0"
},
"eslintConfig": {
"extends": [
"wordpress"
]
},
"eslintIgnore": [
"js/**.min.js"
]
}
package.json
PHPCS
$ phpcs inc/template-functions.php
FILE: .../themes/_s/inc/template-functions.php
-----------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 3 LINES
-----------------------------------------------------------------------------
5 | ERROR | [x] Line indented incorrectly; expected 2 tabs, found 3
17 | ERROR | [ ] PHP syntax error: syntax error, unexpected '}'
28 | ERROR | [ ] Expected next thing to be an escaping function (see Codex
| | for 'Data Validation'), not 'get_bloginfo'
-----------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------
Time: 195ms; Memory: 10Mb
<?xml version="1.0"?>
<ruleset name="WordPress Theme Coding Standards">
<description>A custom set of code standard rules.</description>
<rule ref="WordPress.WP.I18n">
<properties>
<property name="text_domain" type="array" value="_s" />
</properties>
</rule>
<rule ref="WordPress.WP.DeprecatedFunctions">
<properties>
<property name="minimum_supported_version" value="4.0" />
</properties>
</rule>
<!-- Include sniffs for PHP cross-version compatibility. -->
<config name="testVersion" value="5.2-99.0"/>
<rule ref="PHPCompatibility"/>
</ruleset>
phpcs.xml.dist
Autofixing
$ stylelint style.css --fix
$ stylefmt style.css
$ eslint js/** --fix
$ phpcs --report=diff /path/to/code
$ phpcbf /path/to/code
Workflow
Editor
GitHub
Travis
.travis.yml
# Use this to prepare your build for testing.
before_script:
- if [[ "$SNIFF" == "1" ]]; then composer require -g squizlabs/php_codesniffer; fi
- if [[ "$SNIFF" == "1" ]]; then composer require -g wp-coding-standards/wpcs; fi
- if [[ "$SNIFF" == "1" ]]; then composer require -g wimg/php-compatibility; fi
- if [[ "$SNIFF" == "1" ]]; then npm install -g eslint; fi
- if [[ "$SNIFF" == "1" ]]; then npm install -g stylelint; fi
# All commands must exit with code 0 on success. Anything else is considered failure.
script:
# Search for PHP syntax errors.
- find -L . -name '*.php' -print0 | xargs -0 -n 1 -P 4 php -l
- if [[ "$SNIFF" == "1" ]]; then eslint js/**; fi
- if [[ "$SNIFF" == "1" ]]; then stylelint style.css; fi
- if [[ "$SNIFF" == "1" ]]; then phpcs; fi
Projects
Juliette
Reinders Folmer
Stephen
Edgar
https://wptide.org/api/tide/v1/audit/wporg/plugin/wordpress-seo
- Manual code review
- Elements of a code review
- Pros & Cons of an automated system
- Tools for JS, CSS and PHP
- Integrating the tools in your workflow
Next Steps
- Install one of the tools locally
- Set up the tool in your editor
- Add a config in your project
- Run the tool
- Analyse and the fix the issues
Ulrich Pogson
@grapplerulrich
Improving code with automation
By Ulrich Pogson
Improving code with automation
- 1,747