📜 ⬆️ ⬇️

Static analysis of PHP code on the example of Symfony2 (part 2)

annotation


The second part of this article was not planned, but the topic found a response, so you can continue.

So, static code analysis in large projects is necessary, and PHP projects are no exception. In essence, the problems and methodology of implementing static analysis tools will be the same as, say, in C ++ .

With everyday use of static analysis tools, you can achieve not only a noticeable reduction in the number of errors, but also an improvement in the quality of the code as a whole - to show this in practice is the goal of this article.
')
That can be found and corrected with the minimum investment of time (and the maximum return) I will tell under a cat.

Instruments


From the tools I use the Php Inspections analyzer (EA Extended) , which is set as an extension to PhpStorm.

In order to automate the routine to fix the source code, you can use the PHP CS Fixer , especially since some of the Php Inspections (EA Extended) will be added to the CS Fixer (for example). It is worth looking at this utility - it automatically corrects the code, frees up a lot of time, and reduces the chance of introducing errors, correcting the code manually.

And, of course, one should not blindly believe analyzers: the semantics of projects is not a trivial thing. Make sure that the code is covered with tests before making changes - working with static analyzers requires some safety precautions.

Defect examples


Reference mismatch and memory problems


The likelihood of finding this type of problem without long profiling is very small, but a static analysis makes it possible to identify such places. The problem is present in large systems and frameworks in which the optimization of memory consumption was carried out.

Ideally, of course, the developer will notice a diagnostic message, and we will not see such a code. I will make a reservation that the inspection will be only in the next release of the plugin.

namespace Symfony\Component\HttpFoundation\Session\Attribute; class NamespacedAttributeBag extends AttributeBag { ... public function get($name, $default = null) { /* * protected function &resolveAttributePath($name, $writeContext = false); * reference mismatch,        */ $attributes = $this->resolveAttributePath($name); ... } ... } 

It should be:
  public function get($name, $default = null) { $attributes = &$this->resolveAttributePath($name); ... } 

Foreach, reference values ​​and non-obvious errors


The foreach feature is that it does not create its own scope, and the variables remain in the parent area. Therefore, declaring the value as a reference, you can add a very unpleasant bug, when the last value of the array changes for no apparent reason.

Static analysis will reveal such places already on the first pass.
 namespace Symfony\Component\Console\Descriptor; class ApplicationDescription { ... private function sortCommands(array $commands) { ... foreach ($namespacedCommands as &$commands) { ksort($commands); } /* *   -   $commands, *    $namespacedCommands   */ return $namespacedCommands; } ... } 

It should be:
  private function sortCommands(array $commands) { ... foreach ($namespacedCommands as &$commands) { ksort($commands); } unset($commands); ... return $namespacedCommands; } 

Micro optimizations


Here I just give examples of code that are not optimal, but perform their task.
According to my observations, such code appears in the systems when the team members change periodically with the inclusion of novice developers in the team.

You can laugh and discuss the competence of developers, but it is better for the architect or team leader to fix such places on his own and without unnecessary criticism - the team will appreciate and focus on the main tasks.

 // if (false !== strpos($lang, '-')) -   if (strstr($lang, '-')) { ... } // $cast = (int) $value -   $cast = intval($value); // return (float) $value -   return floatval($value); // $this->ignore |= static::IGNORE_DOT_FILES -   $this->ignore = $this->ignore | static::IGNORE_DOT_FILES; // if (!in_array(static::$availableOptions[$option], $this->options)) -   if (false === array_search(static::$availableOptions[$option], $this->options)) { ... } // ++$calls[$id] -   $calls[$id] += 1; // if ('root' === get_current_user()) -   if (in_array(get_current_user(), array('root'))) { ... } // if (array_key_exists($id, $container->getAliases())) -   if (in_array($id, array_keys($container->getAliases()))) { ... } // return '' === $relativePath ? './' : $relativePath -   return (strlen($relativePath) === 0) ? './' : $relativePath; 

Dead code


Sometimes you can find refactoring artifacts, unnoticed by simple analyzers.
 namespace Symfony\Component\Security\Acl\Dbal; class MutableAclProvider extends AclProvider implements MutableAclProviderInterface, PropertyChangedListener { ... private function updateNewFieldAceProperty($name, array $changes) { ... //   $currentIds = array(); foreach ($changes[1] as $field => $new) { for ($i = 0, $c = count($new); $i < $c; $i++) { ... if (null === $ace->getId()) { ... } else { //   $currentIds[$ace->getId()] = true; } } } } ... } 

Instead of conclusion


I hope that the examples from the article (and they are all part of the PRs in Symfony2) motivate you to check your projects again and implement the static analysis tools in your daily work.

In large commercial systems, which at the age of 5-10 years require a lot of resources to restore order and eliminate "strange" errors, Php Inspections (EA Extended) and PHP CS Fixer will become your indispensable tool for every day.

For open source, I’ll remind you about Scrutinizer CI and Sensio Labs Insights - very useful tools that can be used along with the previous two.

Source: https://habr.com/ru/post/254949/


All Articles