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

Running your code through WPCS automatically using CI tools

.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

speakerdeck.com/jrf

Juliette

Reinders Folmer

Stephen

Edgar

https://wptide.org/api/tide/v1/audit/wporg/plugin/wordpress-seo

  1. Manual code review
  2. Elements of a code review
  3. Pros & Cons of an automated system
  4. Tools for JS, CSS and PHP
  5. Integrating the tools in your workflow

Next Steps

  1. Install one of the tools locally
  2. Set up the tool in your editor
  3. Add a config in your project
  4. Run the tool
  5. Analyse and the fix the issues

Ulrich Pogson

@grapplerulrich

Made with Slides.com