Exakat 1.9.8 Review

This week, Exakat upgrades the presentation of the inventories. It refactored the method definition detection, and added new rule for hiding parameters and wrong case. Several modernisations were applied, in particular to the ‘Insufficient typehint’ rule.

Coding is inherently risky, and it helps to readd the Exakat 1.9.7 review.

Upgraded Inventories

Code inventories represents the code syntaxes of the same kind, gathered in one place. By syntaxes, we mean the same type. That may be if/then, while, foreach, strings, but also incoming variables, dynamic code, regex, MIME types, URL or email. 

Consider one moment the regexes : they are usually sprinkled a bit everywhere in the code, and everytime the code suggests using a regex, a new one is written, literally. At the end of the day, there is a lot of reusability that is lost. 

With an inventory, all the regex are collected across the code, and are presented at the same place. This makes it easy to spot the four different versions used to extract email addressess from strings, and choose the one that is important. 

Exakat collects several inventories, such as : URL, email, regular expressions, MIME types, pack() format, date() format, printf() formats, exceptions and paths. Variables are also presented with the ‘Confusing variables’ section : there, variables whose name a similar are presented together. Although they may be far away in the code, having to switch from $xmlconfig to $configxmlwill add extra burden to any code reviewer.

Inventories are available in a dedicated section of the Ambassador report, and as a separate report, called Inventories.

Hiding Parameters

Hiding paramter is a new rule with Exakat 1.9.8. It spots arguments which are stored in a local variables, and never used. For example, like this : 

<?php

function foo($a) {
    $b = $a;

    return $b + 1;
}

?>

Although the example is quite short, it illustrate the situation. The incoming argument is never used, leading to decreased readability. 

This is also a case of code moving, or copy/paste : the original code worked, but used othere names for the variables. When copy/pasted to a new location, the easiest is to plug the new argument names in the old one. 

It is recommended to simplify such code, and drop the local variable, or rename the argument. In any case, it is worth a short code review.

Insufficient Typehint

Typehinting helps understanding what kind of argument a method needs. Yet, past the argument filtering, there are no check anymore on the usage of the variables. With code aging, it happens that the typehint stays, but the rest of the code changes.

<?php

interface i {
    function fooi() ;
}

class x implements i {
    function fooi() {}
    function foox() {}
}

function bar(i $i) {
    // fooi() is enforced thanks to the i interface
    echo $i->fooi();

    // foox() is not checked by the i interface, but belongs to the class x
    echo $i->foox();
}

?>

In the example here, an interface is used as typehint, but some supplementary methods are used, which are brought by the class. The typehint is insufficient, and should be changed : either by upgrading the interface with the foox() method, or by replacing the i interface with the x class as typehint. 

Insufficient typehint also appears when the typehinted object is used to access its properties. Since interfaces don’t enforce properties, the typehint doesn’t ensure that the following syntax will be valid with arguments. Exakat now also report such shortcoming, and suggests using a class (and even better, an abstract class) as typehint, instead of typehint. 

Too large typehint ?

Too large a typehint is also considered as a rule of analysis : could we find a better suited interface for the current method ? The best would be to check the currently available interfaces, and suggest a smaller interface. This may definitely a job for exakat, so give us a ping if you’d like this feature implemented (on twitter or github).

Wrong Case, anyone?

Functions with wrong case

Wrong case is a code analysis which reports any application whose name is spelled differently than it was defined. For example, 

<?php

function foo() {}

foo();
FOO(); // Wrong case

?>

This code is legit, since PHP is case-insenstivite when it comes to methods and functions. It won’t affect your code, whatever the case it use. Yet, it may be important to coding conventions : there is a specific exakat rulesets which reports such violations.

Methods with wrong case

The case insentivity also applies to methods. With Exakat 1.9.8, this analysis was upgraded to include methods, both static and normal. 

<?php

class x {
    function foo() {}
}

$y = new x;

$y->foo();
$y->FOO(); // Wrong case

?>

Class constants and properties are processed not covered with this analysis, since they are case-sensitve : code right, or get a NULL!

The Weekly Audits: 2019, Week #41

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

• Use Basename Suffix : basename() will remove extension when it is provided.
• Strange Name For Constants : Those constants looks like a typo from other names.
• Useless Parenthesis : Situations where parenthesis are not necessary, and may be removed.
• Property Could Be Local : A property only used in one method may be turned into a local variable.
• Queries In Loops : Avoid querying databases in a loop.

Happy PHP Code Reviews 

All the 385 analyzers are presented in the docs, including the mobile : Avoid Parenthesis: Avoid Parenthesis for language construct.

This is an unusual bug, with more than 10% of chance to appear. 

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.