Code auditing

Exakat 1.8.2 Review

Posted on by dams

Exakat 1.8.2 Review

exakat 1.8.2 review

Exakat 1.8.2 starts a growth cycle with the internal engine. New relations are being built across the code base, to create a shorter path between related pieces of code. In the meantime, we collect three pieces of wisdom last week : infinite foreach() loops, too much of get_class() and identical inherited methods. There is nothing permanent, except the Exakat 1.8.2 review.

Exakat engine extension

Two important experiments are being run with the core of the Exakat engine. Default values for all variables, and property definitions.

default values for all variables

Properties have a chance to get default values. At definition time, it is possible to specify a literal value, or even an expression. This will save the same assignation later, in the constructor. The same is true for static variables (not properties). Rather strangely, not for global variables.

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

class x {  
  private $property = 1;
}

// works with static variables 
static $s = 2;

// won't works with static variables 
//global $g = 3;
//syntax error, unexpected '=', expecting ',' or ';'

// works with local variables 
$v = 4;

?>
 </pre>

Local variables don’t get a definition, so we have to rely on a simple assignation to do so. This way, it actually looks like a local variable definition is another piece of code. In fact, it is a definition, but without any keyword. 

Also, properties cannot get assigned a default value when they have to store a dynamic resource, such as a resource, a callable or an object. Since those structures are only available at execution time, the default value is often set at constructor’s call, or later, when other prerequisites are met. For example, the path for the log. 

Since static analysis is not the same as executing PHP, Exakat is now building a direct link between the property definition and the first assignations that appear in the code. For example, in the following code, the real default value of $property is, in fact, the stdClass object.

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

class x {  
  private $property = null;

  function __construct() {   
    $this->property = new stdClass();
  }
}
?>
 </pre>

With this link, we have a better knowledge of the actual usage of the property, and may even add a typehint for the property, just like in the upcoming PHP 7.4. 

As usual, such description may not apply to some of the code, including the classic struggle with dynamic code, and multi-assignation for a single property. We’ll have to skip a few of them, until we find a better way to handle them. In fact, this may end up as a recommendation for writing code. 

Where are the properties defined

Here is a classic situation where a property is defined in a parent class. $property is set in x, and then, used in y1 and y2. 

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

class x {  
  protected $property;
}

class y1 extends x {  
  function foo() {   $this->property = 1;  } 
}

class y2 extends x {  
  function foo() {   $this->property = 'a'; } 
}

?>
 </pre>

Currently, the definition of the property is held in x, and the usage is defined as a link between the property definition and Member, throughout the code. Since $property is defined in the parent class, it is conceptually shared between the two classes. 

Yet, the usage of this property is strictly segregated. The $property in y1 has no way to leak into y2, and vice-versa. When instantiated, those properties will all have distinct memory spaces, and processes. 

As an upgrade from the current mechanism, the property definitions are now replicated down the class hierarchy, and set, virtually, where are actually used. This helps separate property spread between the child classes, even if there is only one definition. Later, we’ll avoid false positive, such as ‘inconsistent property usage’ : here, with two distinct definitions for property, we should not report that it is used with incompatible functions. In fact, $property in y1 and $propertyin y2 are totally different properties. 

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

class x {  
  protected $property;
}

class y1 extends x {  
  // virtual property, copied from above  
  protected $property;

  function foo() {   
    $this->property = 1;
  } 
}

class y2 extends x {  
  // virtual property, copied from above  
  protected $property;

  function foo() {   
    $this->property = 'a';
  }
}
?>
 </pre>

Working hard on the next version of Exakat

Some progress is already available, and more refactoring will go on in the next versions. We’ll keep you updated with the coming versions. 

Report identical inherited methods

Sometimes, in class hierarchies, the same method is defined both in a parent class and a child class. In this case, the method is not simply overwritten by a child class : it is a plain copy from the parent class.

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

class x {  
  public function foo() {     
    return rand(0, 10);
  } 
}

class y extends x {  
  public function foo() {     
    return rand(0, 10);
  } 
}

?>
 </pre>

There are multiple fixes for this situation. The parent method may be removed, leaving the child method. When other siblings also use the same method, they should then get their definition for the method. 

The child method may be removed, and this is the simplest modification. In fact, the child is often created from the parent, and this is a dead code removal.

The method may be moved to a separate class or trait, for reuse. 

Identical methods is also part of the Class review ruleset. This rule set checks for weird structure in class structures. 

At the moment, abstract methods are also reported. This is fixed next week, in version 1.8.3.

Avoid infinite loop in foreach()

Code inspirator Frederic Bouchery tested our knowledge with this question : ‘What will this code return?’.

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php $a = &#x5B;1];
foreach($a as $v) {    
  $a&#x5B;] = $v + 1; 
}
print_r($a);
?>
 </pre>

Foreach loops through the $a array, and each time, adds a new value to the source array. The trick is that foreach() doesn’t work the same way with a reference or without a reference. 

Without a reference, as shown here, foreach() works on a copy of the original array. Thus, adding new elements, or modifying those elements, during the loop has no effect on the execution. In this case, it ends with two elements in $a.

With a reference, not as shown here, foreach() works on the original array. Thus, adding a new element during the loop, with append, $a[], also includes the element in the loop. So, adding one element means that $a is longer, so it loops again, and adds one element, and is longer, and loops, and adds… This is an infinite loop.

A simple configuration on the blind variable may have tremendous impact on the execution. 

No need for get_class()

We ran into this code : it dynamically access a static variable. The manipulated class is provided as the class of an object, so the code relies on get_class() to find that name. 

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

// This is too much code 
get_class($a->b)::$c

?>
 </pre>

In fact, PHP is able to find the class of an object. It is often possible to use an object to represent a class, and PHP will do the interpretation on its own. Here are other examples: 

<pre class="wp-block-syntaxhighlighter-code"> &lt;?php

// Accessing a static property from an object 
$a->b::$property

// Accessing a static constant from an object 
$a->b::CONSTANT

// Accessing a static method from an object 
$callback = array($a->b, 'method')

// Calling a static method from an object 
// May yield a warning
$a->b->static_method()

?>
 </pre>

The Weekly Audits: 2019, Week #22

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-23

  • Multiply By One : Multiplying by 1 is a fancy type cast.
  • No Hardcoded Port : When connecting to a remove server, port is an important information.
  • Var Keyword : Var was used in PHP 4 to mark properties as public.
  • Can’t Extend Final : It is not possible to extend final classes.
  • Use const : The const keyword may be used to define constant, just like the define() function.

Happy PHP Code Reviews 

All the 359 analyzers are presented in the docs, including the outrageous :  Used Once Variables: variables should be used at least twice! One for writing, one for reading. Recycle your variable, if you want a metaphor! This is a plague in source code (87%). 

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.