Статический анализ PHP кода на примере Symfony2 (часть 2)

Аннотация

Второй части этой статьи не планировалось, но тема нашла отклик, так что можно продолжить.

Итак, статический анализ кода в больших проектах необходим, и проекты на PHP — не исключение. По сути, проблемы и методология внедрения средств статического анализа будут те же, что и, скажем, в С++.

При повседневном использовании средств статического анализа можно добиться не только заметного уменьшения количества ошибок, но и улучшения качества кода в целом — показать это на практике и есть цель данной статьи.

О том, что можно найти и исправить с минимальным вложением времени (и максимальной отдачей) я расскажу под катом.

Инструменты

Из инструментов я использую анализатор Php Inspections (EA Extended), который ставится как расширение к PhpStorm.

Для того чтобы автоматизировать рутину по исправлению исходного кода, можно воспользоваться PHP CS Fixer, тем более, что кое-что из Php Inspections (EA Extended) будет добавлено в CS Fixer (вот, например). Стоит присмотреться к этой утилите — она автоматически исправляет код, освобождая много времени, и уменьшает вероятность внести ошибки, исправляя код вручную.

Ну и, конечно, не стоит слепо верить анализаторам: семантика проектов — вещь нетривиальная. Обязательно убедитесь, что код покрыт тестами до внесения изменений — работа со статическими анализаторами требует определенной техники безопасности.

Примеры дефектов

Reference mismatch и проблемы с памятью

Вероятность найти этот тип проблемы без долгого профилирования очень мала, но статический анализ позволяет выявить такие места. Проблема присутствует в больших системах и фреймворках, в которых проводилась оптимизация расхода памяти.

В идеале, конечно, разработчик заметит диагностическое сообщение, и такой код мы уже не увидим. Оговорюсь, что инспекция будет только в следующем релизе плагина.

namespace SymfonyComponentHttpFoundationSessionAttribute;

class NamespacedAttributeBag extends AttributeBag {
    ...
    public function get($name, $default = null) {
        /*
         * protected function &resolveAttributePath($name, $writeContext = false);
         * reference mismatch, копия возвращаемого массива будет передана в переменную
         */
         $attributes = $this->resolveAttributePath($name);
         ...
    }
    ...
}

Как должно быть:

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

Foreach, значения по ссылке и неочевидные ошибки

Особенность foreach в том, что он не создаёт собственной области видимости, и переменные остаются в родительской области. Поэтому, объявляя значение как ссылку, можно внести очень неприятный баг, когда последнее значение массива меняется без видимой причины.

Статический анализ выявит такие места уже на первом проходе.

namespace SymfonyComponentConsoleDescriptor;

class ApplicationDescription {
    ...
    private function sortCommands(array $commands) {
        ...
        foreach ($namespacedCommands as &$commands) {
            ksort($commands);
        }
        /*
         * Если тут что-то сделать с $commands,
         * то послений элемент $namespacedCommands будет повреждён
         */

        return $namespacedCommands;
    }
    ...
}

Как должно быть:

    private function sortCommands(array $commands)
    {
        ...
        foreach ($namespacedCommands as &$commands) {
            ksort($commands);
        }
        unset($commands);
        ...

        return $namespacedCommands;
    }

Микро-оптимизации

Тут я просто приведу примеры кода, которые неоптимальны, но выполняют свою задачу.
По моим наблюдениям, такой код появляется в системах при периодической смене членов команды с включением в команду начинающих разработчиков.

Можно смеяться и обсуждать компетентность разработчиков, но архитектору или тимлиду лучше исправить такие места самому и без лишней критики — команда оценит и сфокусируется на основных задачах.

//  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;

Мёртвый код

Иногда можно найти артефакты рефакторинга, незамеченные простыми анализаторами.

namespace SymfonyComponentSecurityAclDbal;

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;		
                }
            }
        }
    }

	...
}

Вместо заключения

Надеюсь, что примеры из статьи (а все они часть PRs в Symfony2) мотивируют вас проверить свои проекты еще раз и внедрить инструменты статического анализа в повседневную работу.

В больших коммерческих системах, которые в возрасте 5-10 лет требуют очень много ресурсов на наведение порядка и устранение «странных» ошибок, Php Inspections (EA Extended) и PHP CS Fixer станут для вас незаменимыми иструментами на каждый день.

Для опенсорса я напомню про Scrutinizer CI и Sensio Labs Insights — очень полезные инструменты, которые можно использовать вместе с двумя предыдущими.

Автор: kalessil

Источник