Code Deodorant

Code Deodorant

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

Eba4cc68bfbc2b59c3c3a3cf789075f0?s=128

Tom J Nowell

July 14, 2013
Tweet

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 ?