Code auditing, Technology

Exakat 1.8.3 Review

Posted on by dams

Exakat 1.8.3 Review

Exakat 1.8.3 has been refactoring a lot of property and method infrastructure. We are now quite satisfied with the new handling of properties, even if some unit tests are still not passing. In the meantime, we added new tactical checks : how to speed up usage of magic properties, by choosing the best one to cache; one silly trick with auto-appending arrays and even more in the Changelog. Find joy in everything you code, for the rest, check the Exakat 1.8.3 review.

SARB : Static Analysis Results Baseliner

When starting with Exakat, the best report to use is the Ambassador report : it is the most extensive report, and its GUI interface is suited for discovery. It is a great first step.

After working with Exakat for a while, it is important to set up a baseline. A baseline is the concept than the tare : it is the weight of the container, so you can deduce the weight of the substance from the total weight. Applied to static analysis, the baseliner creates a list of results, and remove those results from the current audit, leaving only the new one. This leads to a much smaller result set, and shows only the new detected code smells.

SARB is the Static Analysis Results Baseliner. It creates the baseline, then weed out the old issues in the next audits. The synopsis is straightforward : 

  • Run Exakat, and produce the Sarb report. Keep the ‘exakat.sarb.json’ file handy.
  • Create the SARB baseline : 
    vendor/bin/sarb create-baseline \
    projects/my_project/exakat.sarb.json \
    reports/sarb_baseline.json \
    exakat-json
  • Update your code, and run again exakat, with the ‘SARB’ report.
  • Use the SARB to remove the baseline results
    vendor/bin/sarb remove-baseline-results \
    reports/exakat.sarb.json \
    reports/sarb_baseline.json \
    reports/issues_since_baseline.json

With the baseline, you may now make your code better on a daily basis : new issues are easy to identify, and should be worked upon immediately. This keeps them from coming back, or even entering the code. 

The history, or the baseline, is still here. You may then work on it as time permits, reducing technical debt each time. 

SARB authored by Dave Liddament and is currently in Beta version. You can visit the home repository to use it and help with its development.

Performance : from Magic to Concrete property

Magic properties are the magic properties which doesn’t exist, and are managed by four PHP magic methods : __get, __set, __unset and __isset. 

Magic properties are convenient when the actual list of properties is not known until execution time. For example, rows from a database, or properties in a JSON file. 

<?php

class x {
    function __get($a) {
        return 'a'; // simplistic magic method
    }
}

$x = new x();
echo $x->y.$x->z; // display 'aa'

?>

Magic properties are convenient when the actual list of properties is not known until execution time. For example, rows from a database, or properties in a JSON file. 

Magic properties are also quite slow, compared to their traditional version. Indeed, magic properties turn a simple property access into a functioncall, with context switching and typehint checks. When the property access has a very simple logic, this may slow down the script from 100% to 400%. This may easily get worse as soon as extra checks are added (existence). 

At that point, it is difficult to suggest avoiding magic methods altogether. First, when properties are dynamic, they are quite helpful. Secondly, this is a micro-optimisation : there needs to be several millions of hits on the property to reach one second slow down. 

With magic method being very convenient, they are used. And, as soon as they are part of the developer tool belt, there is an unbalanced usage of properties. Some are seldom used, some are used very often. In fact, if you’re using all of them, in the same way, you’ll probably have made them concrete, not magic. 

Exakat helps you refine your property usage with two interesting cases. Choosing the concrete properties in the class, and memoizing the values. 

Choosing concrete properties

Properties that are commonly used in the code base should be turned into a concrete property. As long as they are used dynamically, there is no way to prepare for them, but once we can spot multiple usage of the same name, and yet can’t find a definition, this should be a property.

<?php

class x {
    function __get($a) {
        return 'a'; // simplistic magic method
    }
}

function foo() {
    $x = new x();
    $a = $x->a;   // get $x->a from the object
    echo $a * $a; // use the local cache

    echo $x->b * $x->b;    // calling the class twice
}

?>

In the example above, y is used in two different context, foo and foo2. This is now a common usage of a property, which also relies on the __get method in class x. On the other hand, a and z are used once only in the code, and may stay totally magic, just like $b, for which we have no information. 

Exakat reports that y should be turned into a concrete property.

Memoizing the property

The other potential cache we can use is in the context itself. Requesting the property is a costly expression, so the first access should be put into a variable, and then reused, instead of requesting again the value from the object.

<?php

class x {
    function __get($a) {
        return 'a'; // simplistic magic method
    }
}

$x = new x();
echo $x->y.$x->z; // display 'aa'

?>

You may parameter this analysis with the magicMemberUsage parameter. By default it is 1, so any double call to the same property from the same context triggers the analysis. Add the following section to your config/exakat.ini or projects/<my_project>/config.ini. 

[Classes/MakeMagicConcrete]
  magicMemberUsage = 3;

Avoid very long recursion with appends

Last week, a small typo was the starting point of a long head-scratching problem. The code is summarized here, and you’ll probably find the flaw immediately : 

 
&lt;?php 

$results = [];
foreach($array as $result) {
    // More code
    $results[] = $results;
}

?&gt;

So, $array is built into $results, via $result, and with some extra logic. The //more code comment doesn’t do justice the piece of code here : it was a lot more distracting. In the end, the resulting array is appended to the current list, though a typo prevented this : the same array was added to itself.

A quick look make you understand that the array will double each loop, and it will probably never reach the 100th iteration without exhausting the memory. With names so close, results and result, and no error except a surprisingly very long to execute script, it took us way too long to spot. 

This is now an Exakat analysis. It may pass unnoticed when the loop is small enough, or if the append is out of any loop, but it will probably bite you some day. So, now, it is under control. 

The Weekly Audits: 2019, Week #24

Exakat includes a ‘weekly’ report: this report is built with a selection of five analyses. This means a short audit report, with few issues to review. This is not a lot to read them, and review them in your code. Everyone in the PHP community can focus on one of the classic coding problems and fix it. Talk about the weekly audit around you: you’ll find programmers facing the same challenges.

To obtain the ‘weekly’ audit, run an audit, and request the ‘Weekly’ report.

# Init the project (skip when it is already done)    
php exakat.phar init -p <yourproject> -R https://github.com/Seldaek/monolog.git -git 

# Run the project (skip when it is already done)    
php exakat.phar project -p <yourproject> 

# Export the weekly project (every monday)    
php exakat.phar report -p <yourproject> -format Weekly 

# Open projects/<yourproject>/weekly/index.html in your browser

Every week, you can find here 5 new analysis to review in your code. In fact, when your code is clean, you can also take a quick look at the upcoming analysis. 

Weekly recommendations for PHP code review : 2019, week 2019-24

  • Useless Brackets : Standalone brackets have no use.
  • Typehint Must Be Returned : When using a typehint for a method, it is compulsory to use a at least one return in the method’s body.
  • Failed Substr Comparison : The extracted string must be of the size of the compared string.
  • Incompilable Files : Files that cannot be compiled, and, as such, be run by PHP.
  • Use Constant : The following functioncall have a constant equivalent, that is faster to use than calling the functions.

Happy PHP Code Reviews 

All the 360 analyzers are presented in the docs, including the hideous :
: Only Variable Returned By Reference: Function can’t return literals by reference. This is a rare bug in source code (3%). 

You can check all of the Exakat reports at the gallery: exakat gallery.

Download Exakat on exakat.io, install it with Docker, upgrade it with ‘exakat.phar upgrade -u’ and like us on github.