Exakat 1.8.9 Review
Exakat 1.8.9 is out. This week, we are packing new analysis and several graph refactoring that are helping us push the analysis further. The analysis cover disconnected classes, the double usage of blind variables in foreach(), usage of mb_detect_encoding(), and useless type hint checks.
Constant values are propagated even further; foreach now include links to ALL blind values, may it be simple variables or complex list() calls; ! precedence was upgraded to compare it with assignations.
What you leave behind is not what is engraved in stone monuments, but what is check with the Exakat 1.8.9 review.
Double Usage of Blind Variables in foreach
PHP used to accept multiple arguments with the same name, in methods definition. This is now forbidden, since PHP 7.0.
<?php function foo($a, $a, $a) {} ?>
Yet, there are some other command that still allow this kind of behavior : foreach. The source and the blind variables (the variables used by the foreach to describe the elements in the source array) may be the same :
<?php $a = range('a', 'c'); // This produces 012 (keys have precedence over values) foreach($a as $a => $a) { print "$a"; } // This produces abc (only the values) $a = range('a', 'c'); foreach($a as $a) { print "$a"; } ?>
Just like for the arguments situation, this is totally legit PHP code. The source array is initially copied for the foreach, so overwriting it immediately to use it as a key or a value still works.
In fact, when the source array is not used again after the loop, it is rather painless.
It is recommended avoiding using the same name for the source and the various blind variables. This also applies to foreach with the list() syntax, when PHP dereferences immediately a sub-array.
<?php $a = array(range('a', 'c'), range('a', 'c'), range('a', 'c')); foreach($a as [$a, $a, $a]) { print "$a"; } ?>
Disconnected Classes
Disconnected classes are classes that are linked with an extends keyword, but that are not using each other.
The heritage paradigm supposes that the classes of the family are sharing data or methods, or both. In the most extreme situations, the parent is doing all the heavy-lifting while the child only specialize or configure it; or the child is doing all the work, while the parent provides support and interface.
<?php class Foo { protected $pfoo = 2; function foofoo() { return $this->pfoo; } } // Bar extends Foo, but both exist independently class Bar extends Foo { protected $pbar = 2; function bar() { return $this->pfoo; } } ?>
When two classes are in an extends relationship, but both are independent from each other, this is a good start for refactoring. Why did they end up so close, while doing so little together ?
Disconnected Classes are reported in the ClassReview ruleset.
Avoid mb_detect_encoding()
Arnout Boks explains in detail, in his DPC2019: Of representation and interpretation: A unified theory presentation, how mbdetectencoding() is easily fooled by strings of bytes, and that is it important to transmit and store the format of string, when you want to differentiate UTF-8 and ISO-8859-1 and others.
This is now reported with a specific analysis.
And watch the rest of the presentation, it is extremely interesting.
Useless Checks With Typehints
One of the great advantages of typehint is the removal of verbose checks. For example :
<?php function foo(A $a) { if ($a === null) { throw new Exception('my error'); } } ?>
The check on $a
is now useless, since PHP won’t let a null value be passed as argument. In fact, it must be an object of class A
, and nothing else.
Yet, there are two special cases : nullable types and null default value.
The first is when the type is completed with a question mark ?
, which allows the null
value to pass, along with any A objects. The second one is the default value null
, which then makes null values a valid argument. This second situation actually accepts both usage of null : null
as default argument (when omitted), or null
as explicit argument.
<?php function goo(?A $a) { echo __FUNCTION__; } function foo(A $a = null) { echo __FUNCTION__; } goo(null); // Accept null argument, with ? foo(); // Use the default value foo(null); // Accept null as argument ?>
As a result, it is still important to test for the existence of an object when the default value is null, or when the nullable type is used.
New option for Upgrade : -version
When you upgrade Exakat, you may now add the -version
option to mention the target version. By default, when omitting it, Exakat will find the latest version on the https://178.62.231.40/website. Otherwise, it will locate the requested version and install it.
As usual, the default is a dry run, so there is no harm in checking if a new version is available (hint : it is on Monday).
Exakat will keep some version online in case a bug emerges and prevent audits. Do send us a note on the Github repository, so we can take a look.
The Weekly Audits: 2019, Week #30
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-30
- Implement Is For Interface : With class heritage, implements should be used for interfaces, and extends with classes.
- Use Constant As Arguments : Some methods and functions are defined to be used with constants as arguments.
- Pathinfo() Returns May Vary : pathinfo() function returns an array whose content may vary.
- Useless Global : Global are useless in two cases.
- Undefined Class Constants : Class constants that are used, but never defined.
Happy PHP Code Reviews
All the 360 analyzers are presented in the docs, including the narcissic Dont Change The Blind Var: When using a foreach(), the blind variables hold a copy of the original value.
This is a common bug, with more than 72% 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.