Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Code Deodorant

Code Deodorant

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

Tom J Nowell

July 14, 2013
Tweet

More Decks by Tom J Nowell

Other Decks in Technology

Transcript

  1. Code Deodorant

    View full-size slide

  2. I'm Tom J Nowell

    View full-size slide

  3. How do people check how well
    something is built?

    View full-size slide

  4. Instant Code Smells

    View full-size slide

  5. Syntax Errors

    View full-size slide

  6. 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.

    View full-size slide

  7. query_posts
    Never call query_posts

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  11. 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

    View full-size slide

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

    View full-size slide

  13. 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();

    View full-size slide

  14. 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

    View full-size slide

  15. Computer Science to the Rescue!

    View full-size slide

  16. Cyclomatic & NPath Complexity

    View full-size slide

  17. Inheritance Depth

    View full-size slide

  18. QA Tools to the Rescue!

    View full-size slide

  19. Version Control/VCS
    Git SVN Mercurial and others

    View full-size slide

  20. 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

    View full-size slide

  21. 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

    View full-size slide

  22. 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

    View full-size slide

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

    View full-size slide

  24. 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

    View full-size slide

  25. 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

    View full-size slide

  26. 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

    View full-size slide

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

    View full-size slide

  28. PHPDP: Abstraction vs Instability

    View full-size slide

  29. 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';

    View full-size slide

  30. 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

    View full-size slide

  31. 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,

    View full-size slide

  32. PHPDocs
    run: phpdoc -d . -t docs

    View full-size slide

  33. Code Hygiene

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  37. 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");

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  40. 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

    View full-size slide

  41. So How Does WordPress Fair?

    View full-size slide

  42. WP Core: Our Survey Said

    View full-size slide

  43. 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

    View full-size slide

  44. 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

    View full-size slide

  45. 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

    View full-size slide

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

    View full-size slide

  47. 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.

    View full-size slide

  48. 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

    View full-size slide

  49. 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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  52. PDepend JDepend Graph

    View full-size slide

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

    View full-size slide