Symfony2を例として使用したPHPコードの静的分析

注釈

大規模プロジェクトでの静的解析の必要性は、すでに何度も記述されており、主に、厳密に型指定された言語、たとえばhereやhereに焦点を当てています 。

PHPの場合、状況はより複雑です。PHPコードの静的分析についてはすでに記述されていますが、一般的にツールはここでははるかに貧弱であり、言語の動的な性質により開発テストプロセスはより困難になります。 比較のために、同じJavaでプロジェクトをコンパイルすること自体がエラーの発見に役立ち、PHPでは型付けが弱いため、テストでもエラーをスキップできます。

当社では、開発および検証プロセスに静的解析と厳密な型付けを組み込むことにより、この問題を部分的に解決しました。 その後、他のプロジェクトで何が起こっているのかを見たかった。

ツール

よく知られているユーティリティ（ PHP Mess Detector 、 PHP Copy / Paste Detector 、 PHP CodeSniffer 、 PHP Analyzer 、 Pfff ）をスキップします-特定の段階で使用しましたが、結果は努力する価値がありませんでしたので、スマートツールに移りましょうほとんどすぐに利益が得られます。

理論的には、 SensioLabs Insightを使用できますが、企業の規則では、サードパーティにコードを提供することはできません。 したがって、基本的な要件は、静的解析のIDEへの古典的な統合に戻りました。 私たちの場合、これはPhpStormです（プロジェクトを確認したい場合は、30日間試用版を置くことができます）。 デフォルトでは、静的分析のための適切なルールのセットが利用可能ですが、これは率直に言って、エンタープライズレベルの製品には十分ではないため、拡張Php Inspections（EA Extended）を追加します。 この拡張機能は分析のメインツールであり、以降のすべての例は彼向けです。

symfony2とは

symfonyは、PHP5で記述されたオープンソースフレームワークです（ アーキテクチャの詳細 ）。 レビューのために、バージョン2.7が選択されました。

Symfony2コード分析

コードを分析するために、src / Symfony / Componentフォルダーにあるカーネルコンポーネントのみを取得します。

私たちの分析では、重大度とカテゴリが異なる6924の問題が見つかりました（拡張機能には約40の検査が含まれていますが、すべてが問題を検出したわけではありません）。
比較のため、バージョン2.3では5727の問題（20％減少）がありましたが、これらは新しいコンポーネントと新しいテストの両方である可能性があります。

デブリーフィング

すべてのケースを考慮することは無意味であり、誤検知が発生するため、（問題のタイプに関連して）少数の興味深いコードのみを検討します。

問題の種類-アーキテクチャ：クラスはスーパークラスのインターフェイスを再実装します

namespace Symfony\Component\Translation\Loader; //   class MoFileLoader extends ArrayLoader implements LoaderInterface { ... } //   class ArrayLoader implements LoaderInterface { ... } 

明確ではありません。インターフェースが後で親クラスに導入されたか、開発者が単にクラスの継承を見落としていましたが、子クラスでインターフェースを再継承しても意味がありません。

問題の種類-アーキテクチャ：クラスはスーパークラスのフィールドをオーバーライドします

 namespace Symfony\Component\Translation\Dumper; class IcuResFileDumper extends FileDumper { ... //   protected $relativePathTemplate = '%domain%/%locale%.%extension%'; ... } //   abstract class FileDumper implements DumperInterface { protected $relativePathTemplate = '%domain%.%locale%.%extension%'; ... } 

表示されているデフォルト値をオーバーライドする必要がありました。

問題は、最初は親クラスの属性ですが、属性が子領域に複製されることです。 次のように実装する必要があります。

 use Symfony\Component\Translation\MessageCatalogue; class IcuResFileDumper extends FileDumper { ... public function __construct() { $this->relativePathTemplate = '%domain%/%locale%.%extension%'; } ... } 

問題の種類-可能性のあるバグ：親コンストラクター/クローン呼び出しの欠落

 namespace Symfony\Component\HttpFoundation; class FileBag extends ParameterBag { ... //   public function __construct(array $parameters = array()) { $this->replace($parameters); } ... public function replace(array $files = array()) { $this->parameters = array(); $this->add($files); } ... } //   class ParameterBag implements \IteratorAggregate, \Countable { ... public function __construct(array $parameters = array()) { $this->parameters = $parameters; } ... } 

OOPでは、親コンストラクタ/デストラクタを呼び出すのが一般的です。 この場合、開発者は単にコードを見落としているようです。 次のように実装する必要があります。

 class FileBag extends ParameterBag { ... public function __construct(array $parameters = array()) { parent::__construct(); $this->add($files); } ... } 

問題のタイプ-制御フロー：ループしないループ

 namespace Symfony\Component\Templating\Loader; class ChainLoader extends Loader { ... public function isFresh(TemplateReferenceInterface $template, $time) { foreach ($this->loaders as $loader) { //   return $loader->isFresh($template, $time); } return false; } ... } 

これがメソッドの望ましい動作である可能性がありますが、クラス名とループはむしろエラーを示しています-チェックは常に最初の反復でのみ実行されます。

問題タイプ-制御フロー：三項演算子の単純化

 class CrawlerTest extends \PHPUnit_Framework_TestCase { ... public function testReduce() { ... $nodes = $crawler->reduce(function ($node, $i) { //   return $i == 1 ? false : true; }); ... } ... } 

もちろん、これは間違いではありませんが、そのようなコードにも特別な意味はありません。
テストフラグメントでは、戻り値を「return $ i！= 1;」として単純化できます。

問題の種類-パフォーマンス：elvis演算子を使用できます

 namespace Symfony\Component\HttpKernel\DataCollector; class RequestDataCollector extends DataCollector implements EventSubscriberInterface { ... public function collect(Request $request, Response $response, \Exception $exception = null) { ... $this->data = array( ... //   'content_type' => $response->headers->get('Content-Type') ? $response->headers->get('Content-Type') : 'text/html', ... } ... } 

これも間違いではなく、コードの簡素化であり、「 'content_type' => $ response-> headers-> get（ 'Content-Type'）?: 'Text / html'」と書き換えることができます。
基本的に、新しいSymfony2コードはすでにこの演算子を使用しており、このコードはまだ注目されていません。

AlexPTSは非常に簡単なオプションを提案しました。

  $response->headers->get('Content-Type', 'text/html') 

問題のタイプ-制御フロー：条件が最適ではない

これは、エラーと最適でないコードの別個のクラスです。 多くの問題のバリエーションがあります レガシープロジェクトでの条件付き構造、およびアクティブなリファクタリング後は、すべてのアーキテクトにとって頭痛の種です。

以下に、このアナライザーが検出するいくつかの例を示します。

 namespace Symfony\Component\HttpKernel\Profiler; abstract class BaseMemcacheProfilerStorage implements ProfilerStorageInterface { ... public function find($ip, $url, $limit, $method, $start = null, $end = null) { ... //   if ($ip && false === strpos($itemIp, $ip) || $url && false === strpos($itemUrl, $url) || $method && false === strpos($itemMethod, $method)) { ... } ... } ... } 

全員を一度に混乱させる方法の鮮明な例。 実際、構造は次のとおりです。

  ($ip && false === strpos($itemIp, $ip)) || ($url && false === strpos($itemUrl, $url)) || ($method && false === strpos($itemMethod, $method)) 

メソッドと関数への重複した呼び出し：

 namespace Symfony\Component\Form\Extension\Core\DataMapper; class PropertyPathMapper implements DataMapperInterface { ... public function mapFormsToData($forms, &$data) { ... //   if ($form->getData() instanceof \DateTime && $form->getData() == $this->propertyAccessor->getValue($data, $propertyPath)) { ... } if (!is_object($data) || !$config->getByReference() || $form->getData() !== $this->propertyAccessor->getValue($data, $propertyPath)) { $this->propertyAccessor->setValue($data, $propertyPath, $form->getData()); } ... } } 

「$ form-> getData（）」は数回呼び出されますが、結果をローカル変数に保存する方が論理的です。

結論の代わりに

静的コード分析（PHPコードを含む）は、チームの開発とトレーニングの組織化の観点からは費用がかかりますが、強力で便利なプラクティスです。

しかし、日常の使用では、このプラクティスは非常に効果的に機能します。

• エラーの数を減らします。
• プロジェクトのコンポーネントを客観的に選択するのに役立ちます。
• 新しいプロジェクトを評価し、アプローチする側を理解します。
• チームのレベルを上げます。

オープンソースの例では、これらの点はすべて、企業と同様に関連性があり、発見された問題を一目見ただけです。

PHPの場合、静的分析ツールを使用した状況はここ数年で著しく改善されており、改善し続けるようです。