Code Deodorant

I'm Tom J Nowell

How do people check how well something is built?

Terrible?

Working?

Instant Code Smells

Syntax Errors

Poor Indentation 1. Poorly indented code is hard to read 2. Hard to read code hides logic bugs 3. Logic bugs break programs 4. ∴ poor indentation is bad In a world of auto-indenting editors, there is no excuse.

Goto

query_posts Never call query_posts

exit; and wp_die Code using these functions are impossible to test, a poor mans excuse for not handling errors.

Eval & create_function ● Insecure ● No valid reason for their usage outside of esoterica ● base_64_decode presence may indicate malicious use of these functions

PHP 4-isms ● Using classname() instead of __construct() ● array( &$this, 'callback')

extract() ● Spews variables out into the local ● Requires foreknowledge of what the array contained ● Doesn't mix well with automated tools and auto-completion ● Harder to test

global variables ● Pollutes the global namespace ● Encourages bad design ( or complete lack of it ) ● Clashes can occur

anonymous objects class Davina_Plugin { function __construct() { add_action('the_content', array( $this, 'swear')); } function swear( $content ) { return "don't say sh*t f*ck or b*gger"; } } // try and remove the swear filter // without changing this line: new Davina_Plugin();

Singletons & God Classes There is no valid use for Singletons in PHP, instead use: ● Dependency Injection ● The decorator object pattern Putting everything inside a big class is not OOP

Computer Science to the Rescue!

Cyclomatic & NPath Complexity

Coupling

Inheritance Depth

QA Tools to the Rescue!

Version Control/VCS Git SVN Mercurial and others

VCS: Automated Deployment ● Auto-deploy code on commit/push using hooks ● Push changes to development to the development server automatically ● Deploy changes to production by merging branches ● Never have to use ssh/sftp/ftp/cpanel again

VCS: Automated Testing # pre-commit.sh git stash -q --keep-index ./run_tests.sh RESULT=$? git stash pop -q [ $RESULT -ne 0 ] && exit 1 exit 0

VCS: Automated Testing #!/bin/sh if git-rev-parse --verify HEAD >/dev/null 2>&1; then against=HEAD else against=4b825dc642cb6eb9a060e54bf8d69288fbee4904 fi for FILE in `git diff-index --check --name-status $against -- | cut -c3-` ; do # Check if the file contains 'debugger' if [ "grep 'query_posts' $FILE" ] then echo $FILE ' contains query_posts!' exit 1 fi done exit

PHP Code Sniffer phpcs Detects Coding standards violations, such as: ● Incorrect indentation ● Misnamed classes and functions ● Uneven braces

PHP Mess Detector phpmd text Finds bad code, and mistakes ● Short variable names ● Unused functions and variables ● Cyclomatic Complexity ● NPath complexity ● Suggests refactoring of long methods, complex classes ● xml, text and html output

PHPLOC phploc Provides statistics about a codebase such as: ● Lines of code ● Defined classes, and abstract vs concrete classes ● Lines of code per class ● Average cyclomatic complexity per line of code ● Other high level general stats

PHPLOC phploc 1.6.4 by Sebastian Bergmann. Directories: 3 Files: 33 Lines of Code (LOC): 2358 Cyclomatic Complexity / Lines of Code: 0.08 Comment Lines of Code (CLOC): 903 Non-Comment Lines of Code (NCLOC): 1455 Namespaces: 0 Interfaces: 3 Classes: 28 Abstract: 1 (3.57%) Concrete: 27 (96.43%) Average Class Length (NCLOC): 49 Methods: 149 Scope: Non-Static: 128 (85.91%) Static: 21 (14.09%) Visibility: Public: 103 (69.13%) Non-Public: 46 (30.87%) Average Method Length (NCLOC): 9 Cyclomatic Complexity / Number of Methods: 1.69 Anonymous Functions: 0 Functions: 3 Constants: 9 Global constants: 0 Class constants: 9

PHP_Depend Analyses a codebase and generates a set of graphs demonstrating coupling, inheritance, and abstractness

PHPDP: Abstraction vs Instability

Phantm ./phantm Performs checks on variable type consistency, and various expectations and assumptions, as well as some additional checks. $dict = array(); $dict['en'] = array(); $dict['en']['fr'] = 'Bonjour'; $dict['en']['de'] = 'Hallo'; $dist['en']['it'] = 'Ciao'; $dict['en']['sp'] = 'Hola'; :7 Notice: Type mismatch: expected: Array[...], found: Top $dist['en']['it'] = 'Ciao';

Vulcan Logic Dumper PHP OpCode dumper extension, allows for analysis of code at the bytecode level php -dvld.active=1 program.php A powerful but rarely needed tool

Scheck General check utility from Facebooks pfff toolset, generates a database from a codebase and points out inferred mistakes.e.g. function add_hash( $color ) { return '#'.$colour; } addhash(); // missing '_' The generated pfff database can be plugged into a viewer app with Google Map style browsing and search,

Pfff Visual

PHPDocs run: phpdoc -d . -t docs

PHPUnit

Code Hygiene

PHP Linting php -l file.php Check for PHP syntax errors before deployment/testing. Also available in IDEs & editors built in or via packages

Indentation Most good editors will do this automatically Matching Brace highlighting also helps

Editor Integration Packages and plugins available for PHPStorm, Sublime Text, VIM, and others ● Integrate CodeSniffer, Mess Detector, scheck, syntax checking, and others

assertions Used to ensure impossible things never happen. // the following generates an error in a scenario that should never happen assert($user instanceof WP_User, "expected WP_User");

Type Hinting function do_user_things( WP_User $user ) { Further check your assumptions, and prevent potential errors

Dependency Injection Dependency Injection is where components are given their dependencies through their constructors, methods, or directly into fields

Unit and Integration Tests Put them in a script and email the output, upload to a website, use a framework etc Run on: ● File saves ● Commits ● Cron jobs ● Deployment

So How Does WordPress Fair?

WP Core: Our Survey Said

WPCore: PHP Syntax check ● Stock PHP 5.4 install ● Standard WP 3.5.2 svn checkout ● No syntax error messages filtered out ● find . -name \*.php -exec php -l "{}" \; > results.txt

WPCore: PHP Syntax Check Results Strict standards: Redefining already defined constructor for class ftp in ./wp-admin/includes/class-ftp-pure.php on line 34 Strict standards: Redefining already defined constructor for class ftp in ./wp-admin/includes/class-ftp-sockets.php on line 34 Strict standards: Redefining already defined constructor for class ftp_base in ./wp-admin/includes/class-ftp.php on line 128 Strict standards: Declaration of Bulk_Plugin_Upgrader_Skin::before() should be compatible with WP_Upgrader_Skin::before() in ./wp-admin/includes/class-wp- upgrader.php on line 1365 Strict standards: Declaration of Bulk_Plugin_Upgrader_Skin::after() should be compatible with WP_Upgrader_Skin::after() in ./wp-admin/includes/class-wp- upgrader.php on line 1365 Strict standards: Declaration of Bulk_Theme_Upgrader_Skin::before() should be compatible with WP_Upgrader_Skin::before() in ./wp-admin/includes/class-wp- upgrader.php on line 1400 Strict standards: Declaration of Bulk_Theme_Upgrader_Skin::after() should be compatible with WP_Upgrader_Skin::after() in ./wp-admin/includes/class-wp-upgrader. php on line 1400 Strict standards: Redefining already defined constructor for class WP_Widget in ./wp-includes/widgets.php on line 93 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/native.php on line 107 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/native.php on line 122 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/native.php on line 124 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/native.php on line 126 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 102 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 110 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 124 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 126 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 192 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 206 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 213 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 220 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 232 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/string.php on line 239 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/xdiff.php on line 48 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/xdiff.php on line 52 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Engine/xdiff.php on line 56 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Renderer.php on line 101 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff/Renderer.php on line 121 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff.php on line 380 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff.php on line 402 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff.php on line 424 Deprecated: Assigning the return value of new by reference is deprecated in ./wp-includes/Text/Diff.php on line 446 31 violations 8 Strict Standards Violations 23 Deprecations

WPCore PHPLOC Directories: 37 Files: 447 Lines of Code (LOC): 192533 Cyclomatic Complexity / Lines of Code: 0.19 Comment Lines of Code (CLOC): 62271 Non-Comment Lines of Code (NCLOC): 130262 Namespaces: 0 Interfaces: 1 Traits: 0 Classes: 216 Abstract: 2 (0.93%) Concrete: 214 (99.07%) Average Class Length (NCLOC): 269 Methods: 2292 Scope: Non-Static: 2186 (95.38%) Static: 106 (4.62%) Visibility: Public: 2151 (93.85%) Non-Public: 141 (6.15%) Average Method Length (NCLOC): 25 Cyclomatic Complexity / Number of Methods: 5.57 Anonymous Functions: 0 Functions: 2529 Constants: 368 Global constants: 363 Class constants: 5

WPCore: PHPMessDetector ● PHP 5.4, WP 3.5.2 ● Running with standard rules, minus the naming ruleset ● phpmd . text codesize,unusedcode,controversial,design

WPCore: PHPMessDetector Results ● 15,668 items logged ● 1,489 super global accesses ● 677 Cyclomatic complexity notices (threshold of 10) ● 527 high NPath complexity notices (threshold of 200) ● wp-includes/user.php:1282 The function wp_insert_user() has an NPath complexity of 891813888. The configured NPath complexity threshold is 200.

WPCore: PHPMessDetector Results wp-includes/taxonomy.php:1187 The function get_terms() has an NPath complexity of 15603175784448000. The configured NPath complexity threshold is 200. 15,603,175,784,448,000

WPCore: PHPMessDetector Results wp-includes/query.php:1919 The method get_posts() has an NPath complexity of 14357339413974227091249406251885003 71668992000000. The configured NPath complexity threshold is 200. 1,435,733,941,397,422,709,124,940,62 5,188,500,371,668,992,000,000

PDepend ● PHP 5.4, WP 3.5.2 ● pdepend --summary-xml=352summary.xml -- jdepend-chart=352jdepend.svg --overview- pyramid=352pyramid.svg .

PDepend JDepend Graph WordPress/Customiser, WordPress/User, and others SimplePie cache Global namespace WordPress SimplePie & WP_HTTP APIs

PDepend JDepend Graph

Summary

Questions Tom J Nowell @tarendai www.tomjn.com ?