Technology

Exakat 1.3.5 review

Posted on by dams

Exakat 1.3.5 review

More stability and refinements for Exakat 1.3 series. Exakat closed several bugs and added refinements to analysis during last week. Undefined ::class are now reported, and useless assignations are now reported as issues. Array_key_fill() suggestions has been upgraded and preprocessing recommendations are extended to static concatenations. The Exakat 1.3.5 review packs quite some interesting changes!

Undefined ::class

::class is a special constant. Since PHP 5.5, it provides a fully qualified name at compile time.

 
<?php namespace A;

class X {}

echo x::class; 
// displays \A\x 
?>

This is a very convenient way to access a class name. It handles the file’s use expressions. Yet, it also doesn’t check if the class exists.

 
<?php namespace A;

echo x::class; 
// displays \A\x 
?>

::class is a compile time option : at compile time, the class may not be loaded yet, or be located in another file. As such, PHP builds the class name, and emits no error. So, unusable ::class may be emitted.

Since 1.3.5, Exakat reports undefined ::class results, so you can check the name or the use expression before being hit with a Class 'x' not found or worse, silent dead code.

Class Tree

Exakat maintains a lot of documentation related to the code, and use it when refining analysis. This its way to remove false positives. As a side effect, some interesting documentation is available and provided as such.

The class tree is the hierarchy of classes, based on extends. Classes extends other classes, inheriting their methods, properties and constants.

Note that this tree may be very different from the namespace tree: classes may extends each other in very different namespaces, leading to two different trees. Sometimes, also, namespace tree and classes tree are the same.

The Ambassador report (Exakat default report) includes the class trees, in the Inventories section. It reports classes from their root: it is the first non-extended class that is available in the code. Although, this class may actually extends an included class, like a composer or a PHP extension class.

The Ambassador report doesn’t display non-extended classes, so as to keep the tree simple and cleaner.

Reading the class inventory is a good documentation about the structure of the code. It answers several questions: + does the code has inheritance + how deep are the inheritance tree + which are the wide trees, typical of modules + which are the classes that could be abstract + how many root-class are available?

array_fill_key() with variables

array_fill_keys() is a native PHP function that creates an array from keys. It takes an array of keys, and set each of the keys with the same value. Whenever an array has to be initialized with keys, it is a compact and fast solution.

 
<?php

$array = range('a', 'z');

// Fast way to build the array $b = array<em>fill</em>key($a, 0);

// Slow way to build the array foreach($array as $a) { $b[$a] = 0; }

?>

Introduced in Exakat 1.1.7, this analysis currently reports too many false positive: as soon as an array is filled with the same keys as the source array of the foreach.

This approach is actually useful: the default value may be a variable, dynamically build but repeated for each key.

It may also be an object. When filling an array with array_fill_keys and an object, the same object is filled everywhere. Changing one index in the array changes all of them. This requires a clone of the object.

From the false positive we observed, both usage are common. When the assigned value is a literal, array_fill_keys is a good guess, while when the assigned value is a variable, extra steps must be taken to check if this variable is modified during the loop, or if it is a cloned object.

Both extra checks are too expensive for the number of suggestions that this analysis is providing. So, we decided to keep the false positive, and the speed. We’ll keep reviewing code with it, and upgraded if the situation change, or when we get better tools for the extra checks.

useless assignations

More frequently that one may think, variables are assigned at return time. Something like that:

 
<?php
class Foo { 
   function bar() { 
     // code that set $cache, $loader, $clientId, $lang return 
    $clean = $cache->get($loader, array(), md5(serialize(array($clientId, $lang)))) 
  } 
} 
?>

You don’t need to see the rest of the code to understand that $clean is assigned, then removed: the method has ended. This kind of assignation is useless.

All assignations are not useless: in fact, assigning data at return time, to a local property is valid. Really, the local property will survive. Compare the previous code to this one.

 
<?php
class Foo { 
  function bar() { 
  // code that set $cache, $loader, $clientId, $lang 
   return $this->clean = $cache->get($loader, array(), md5(serialize(array($clientId, $lang)))) 
  } 
} 
?>

Now the clean cache is returned, but it is also stored for later usage. This makes sense, and that usage would cover also static properties.

And yet, there is a whole range of variable that may survive the return of a method, a function or even a closure. They actually are of three types:

  • references, passed as arguments
  • static variables
  • global variables

Any of those variables may be assigned at the last moment, and still provide some useful storage.

All those were taken into account for this new analysis in Exakat 1.3.5.

Preprocessing concatenation

There is already an analysis that report preprocessing opportunities. For example:

 
<?php

// Building an array from a string 
$name = 'PHP'.' '.'7.2';

// Building an array from a string 
$list = explode(',', 'a,b,c,d,e,f');

// Calculating a power $kbytes = 
$bytes / pow(2, 10);

// This will never change 
$name = ucfirst(strtolower('PARIS'));

?>

In a nutshell, some data preparation is done by PHP, at execution time. Those default values could be turned into a static scalar expression, like the first example: simple expression may be solved at compile time. They need to rely on operators and constants (any type), and PHP will turn the whole expression into its final value while compiling.

On the other hand, any expression that calls a function, native PHP or custom, cannot be processed. While some of those expressions may work without any side effect, this is not the case for every function, so functions are not allowed in static scalar expression.

This means that calculating the first letters of the alphabet, as shown in the second example, is easy to write but costs PHP some cycles every time this code is executed. By turning that piece of code into a real array, PHP will cache it and save some

 
<?php

// Building an array from a string 
$html = ''; 
$html .= '<header>'; 
$html .= '<meta charset="utf-8">'; 
$html .= "<title>$title</title>"; 
$html .= '</header><span>'; 
// more code 
?>

This is the case here. Until the variable $title is included in the string, the $html variable is built statically. May be the extra expression are made on purpose for presentation: splitting the tags on several lines tends to make the HTML code more readable. Yet, PHP spends time executing expression that are the same, call after call.

For readability purposes, a switch to HEREDOC or NOWDOC would help and avoid the repetitions.

As usual with micro-optimizations, the impact is low if this code is run once. As soon as this inside a loop, it may suddenly be a lot costly.

Happy PHP code reviews

All the 358 analyzers are presented in the docs, including the uptight Should use array_filter(): turn a foreach() with a condition into a simple array_filter call, clean and readable. array_filter is in the (top 100 of all PHP functions and its usage in loops, it probably yields more than little. Besides it is a common bug: 50% applications are vulnerable.

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.