Code Deodorant

Determining code quality though scientific numerical means rather than subjective opinion, and tools to analyse and prevent issues

Transcript

1. Code Deodorant
2. I'm Tom J Nowell
3. How do people check how well something is built?
4. Terrible?
5. Working?
6. Instant Code Smells
7. Syntax Errors
8. 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.
9. Goto
10. query_posts Never call query_posts
11. exit; and wp_die Code using these functions are impossible to test, a poor mans excuse for not handling errors.
12. Eval & create_function • Insecure • No valid reason for their usage outside of esoterica • base_64_decode presence may indicate malicious use of these functions
13. PHP 4-isms • Using classname() instead of __construct() • array( &$this, 'callback')
14. 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
15. global variables • Pollutes the global namespace • Encourages bad design ( or complete lack of it ) • Clashes can occur
16. 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();
17. 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
18. Computer Science to the Rescue!
19. Cyclomatic & NPath Complexity
20. Coupling
21. Inheritance Depth
22. QA Tools to the Rescue!
23. Version Control/VCS Git SVN Mercurial and others
24. 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
25. 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
26. 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
27. PHP Code Sniffer phpcs <folder/filename> Detects Coding standards violations, such as: • Incorrect indentation • Misnamed classes and functions • Uneven braces
28. PHP Mess Detector phpmd <folder/filename> 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
29. PHPLOC phploc <folder or file> 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
30. 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
31. PHP_Depend Analyses a codebase and generates a set of graphs demonstrating coupling, inheritance, and abstractness
32. PHPDP: Abstraction vs Instability
33. Phantm ./phantm <target.php> 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'; <input>:7 Notice: Type mismatch: expected: Array[...], found: Top $dist['en']['it'] = 'Ciao';
34. 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
35. 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,
36. Pfff Visual
37. PHPDocs run: phpdoc -d . -t docs
38. PHPUnit
39. Code Hygiene
40. PHP Linting php -l file.php Check for PHP syntax errors before deployment/testing. Also available in IDEs & editors built in or via packages
41. Indentation Most good editors will do this automatically Matching Brace highlighting also helps
42. Editor Integration Packages and plugins available for PHPStorm, Sublime Text, VIM, and others • Integrate CodeSniffer, Mess Detector, scheck, syntax checking, and others
43. 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");
44. Type Hinting function do_user_things( WP_User $user ) { Further check your assumptions, and prevent potential errors
45. Dependency Injection Dependency Injection is where components are given their dependencies through their constructors, methods, or directly into fields
46. 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
47. So How Does WordPress Fair?
48. WP Core: Our Survey Said
49. 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
50. 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
51. 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
52. WPCore: PHPMessDetector • PHP 5.4, WP 3.5.2 • Running with standard rules, minus the naming ruleset • phpmd . text codesize,unusedcode,controversial,design
53. 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.
54. 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
55. 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
56. PDepend • PHP 5.4, WP 3.5.2 • pdepend --summary-xml=352summary.xml -- jdepend-chart=352jdepend.svg --overview- pyramid=352pyramid.svg .
57. PDepend JDepend Graph WordPress/Customiser, WordPress/User, and others SimplePie cache Global namespace WordPress SimplePie & WP_HTTP APIs
58. PDepend JDepend Graph
59. Summary
60. Questions Tom J Nowell @tarendai www.tomjn.com ?