8

リファクタリングの後、クラスの 1 つに次のようなものがありました。

class FooBar
{
    // $foo was $bla before
    private $foo;

    public function setBlubbOnArrayOnlyOnce($value)
    {
        // $this->bla was forgotten during refactoring. Must be $this->foo
        if(!isset($this->bla['blubb'])) {
             $this->foo['blubb'] = $value;
        }
    }
}

したがって、最終的に $this->foo['blubb'] は一度だけではなく、常に設定されていました。これは、PHP のマジック メソッドが原因で発生します。フィールドに動的にアクセスできるようにしたくないので、codesniffer ルールを追加するだけでよいと考えました。しかし、何も見つからず、理由を尋ねました。

PHPStorm は動的に宣言されたフィールドを示していますが、デプロイ サイクル中にコードスニッファー (または同様のもの) で自動的に失敗するようにしたいと考えています。

誰かこれについて考えがありますか？良いルールはありますか？自分で書くべきですか？または、それを無効にするのは悪い習慣でしょうか?

免責事項: テストを使用していますが、見逃すこともあります...まずこれを防止することをお勧めします。また、魔法のメソッドを上書きしないでください。すべてのクラスに特性/抽象を持ちたくありません。

4

3 に答える 3

2

リファクタリング後

まずはこれを防ぐのがいいでしょう。

この種のリファクタリング エラーは、各リファクタリング ステップの後にテストを実行することによってのみ検出できます。foo['blubb']は特定の値に設定されており、セッター ロジックのテストだけでなく、別のテストでも望ましくない影響が生じるため、このエラーも発生します。

私たちはテストを使用しますが、見逃すこともあります...

はい、カバレッジが十分に高くないことは非常に一般的です。そのため、すべてのリファクタリングの出発点は、適切なテスト カバレッジを持つことです。

次の 2 行は、カバレッジ レポートで「緑色」ではありませんでした。

   if(!isset($this->bla['blubb'])) {
       $this->foo['blubb'] = $value;

また、魔法のメソッドを上書きしないでください。すべてのクラスに特性/抽象を持ちたくありません。

あなたはそれを除外しましたが、それはプロパティをキャッチする1つの方法です: 魔法の関数__set()(アクセスできない変数の場合)property_exists()を使用するか、Reflection*クラスを使用して検索します。

もう手遅れなので、別のツールでエラーをキャッチする必要があります。

このツールは、PHP ファイルとその親を (変数スコープのため) 解析し$this->bla、事前にpublic|private|protected変数 (クラス プロパティ) を宣言せずに検索する必要があります。これはエラーの正確なタイプを示すものではなく、「bla」が宣言なしでアクセスされたことを示しています。

これを CodeSniffer ルールとして実装することは可能です。

http://phpmd.org/またはhttps://scrutinizer-ci.com/を試すこともできます。また、PHP7 を使用している場合: https://github.com/etsy/phan

tl;tr

基礎となるコードを実行、評価、分析せずに、正確なエラーとそのコンテキストを判断するのは複雑です。「動的変数名」について考えるだけで、その理由がわかります。ソースコードを見ても、プロパティの名前さえわかりません。これは、プログラムフロー中に動的に構築されるためです。静的アナライザーはそれをキャッチしません。

動的アナライザーはすべてのものを追跡する必要があります。ここで$this->は、アクセスしてコンテキストを考慮します: !isset(x)。コンテキスト評価は、多くの一般的なコーディングの間違いを見つけることができます。最後に、レポートを作成できます: $this->bla は 1 回だけアクセスされ、それは次のいずれかを示します。

  • 動的に宣言されたプロパティが導入されましたが、再利用されることはありませんでした。削除するか、クラス プロパティとして宣言することをお勧めします。
  • OR 追加されたコンテキスト評価: isset() 内からこの変数にアクセスしたとき - 宣言されていないプロパティの存在しないキーが、以前の set() なしでアクセスされたなど。
于 2016-01-13T11:37:00.180 に答える