phpmd unusedcode警告の解説

unusedcode

The Unused Code Ruleset contains a collection of rules that find unused code.

(メンバー変数、メソッド、引数、ローカル変数)が未使用かどうか

UnusedPrivateField

メッセージ例

Avoid unused private fields such as 'xx'.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.UnusedPrivateField)
 */Code language: PHP (php)

次の書き方は、抑制されません。

@SuppressWarnings」と「(PHPMD」の間にスペースがある場合

/**
 *
 * @SuppressWarnings (PHPMD.UnusedPrivateField)
 */Code language: PHP (php)

PHPDOCの先頭行の場合

/**
 * @SuppressWarnings(PHPMD.UnusedPrivateField)
 */Code language: PHP (php)

こう書いてしまって、なかなか警告を抑制できなくて、しばらく原因がわからなかったよ

説明

Detects when a private field is declared and/or assigned a value, but not used.

privateメンバー変数が宣言されていたり、値が割り当てられているけど、参照されていないよ。

コード例

class Something
{
    private static $FOO = 2; // Unused
    private $i = 5; // Unused
    private $j = 6;

    public function addOne()
    {
        return $this->j += 1;
    }
}Code language: PHP (php)

解説

未使用なメンバー変数は思い切って削除しましょう。

実は使っているケース

(1)require、include先で参照している

/**
 * $x、$y  require、include先で使っている
 * @SuppressWarnings(PHPMD.UnusedPrivateField)
 */
class Bar
{
    private static $y = 'yyy';
    private $x = 'xxx';

    public function foo()
    {
        require 'foo.php';
    }Code language: PHP (php)
// foo.php
<?php echo self::$x; ?>
<?php echo $this->y; ?>
Code language: PHP (php)

(2)可変プロパティ

プロパティに直接アクセスしていなくても、可変プロパティでアクセスしている場合があります。

class Bar
{
    private $i;

    public function foo()
    {
        $name = 'i';
        return $this->$name;
    }Code language: PHP (php)

@SuppressWarningsで抑制し、「可変プロパティ$this->$nameで使っている」と書いておきましょう。

/**
 * $i  可変プロパティで使っている
 * @SuppressWarnings(PHPMD.UnusedPrivateField)
 */
class Bar
{
    private $i;

    public function foo()
    {
        $name = 'i';
        return $this->$name;
    }Code language: PHP (php)

そもそも可変プロパティを使う必要があるのかな?

主にReflection用途ね。ふつうのアプリケーションでは使わないほうがいいわね

UnusedLocalVariable

メッセージ例

Avoid unused local variables such as 'xx'.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.UnusedLocalVariable)
 */Code language: PHP (php)

説明

Detects when a local variable is declared and/or assigned, but not used.

ローカル変数が宣言されていたり、値が割り当てられているけど、参照されていません。

コード例

class Foo
{
    public function doSomething()
    {
        $i = 5; // Unused
    }
}
Code language: PHP (php)

解説

unusedシリーズのなかでも、ローカル変数はスコープが狭いので、比較的安心して削除できます。

実は使っているケース

(1)requireやincludeしたファイルで参照

    public function doSomething()
    {
        $text = 'foo'; // Unused ?

        require 'my_element.php';
    }Code language: PHP (php)
<p><?php echo $text; ?></p>

Code language: PHP (php)

@SuppressWarningsで抑制し、「requireした先で使っている」と書いておきます。

    /**
     * requireした先で使っている
     * @SuppressWarnings(PHPMD.UnusedLocalVariable)
     */
    public function doSomething()
    {
        $text = 'foo'; // Unused ?

        require 'my_element.php';
    }Code language: PHP (php)

(2)可変変数

可変変数を使う機会はまずありませんが、ねんのため「$$」で検索しましょう。

    public function doSomething()
    {
        $i = 5; // Unused ?

        $name = 'i'; 
        return $$name;
    }Code language: PHP (php)

可変変数でアクセスしていたら、@SuppressWarningsで抑制します。「可変変数 $$name で使っている」と書いておきます。

    /**
     * $iは、可変変数 $$name で使っている
     * @SuppressWarnings(PHPMD.UnusedLocalVariable)
     */
    public function doSomething()
    {
        $i = 5; // Unused ?

        $name = 'i'; 
        return $$name;
    }Code language: PHP (php)

削除したら、テストケースを走らせて、壊していないことを確認します。

何百行もあるメソッドだと、自信をもって削除できないよ

モンスターメソッド!私を巻き込まないで

UnusedPrivateMethod

メッセージ例

Avoid unused private methods such as 'xx'.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
 */Code language: PHP (php)

説明

Unused Private Method detects when a private method is declared but is unused.

privateメソッドが宣言されているけど、使用されていません。

コード例

class Something
{
    private function foo() // unused
    {
    }
}Code language: PHP (php)

解説

実は使っているケース

(1)コールバック関数

array_maparray_walk[$this, 'foo'] と渡しているかもしれません。

class Something
{
    private function bar()
    {
        $arr = array_map([$this, 'foo'], $arr);
    }

    private function foo($item) // unused?
    {
    }
}Code language: PHP (php)

@SuppressWarningsで抑制し、「array_mapのコールバック関数」と書いておきます。

class Something
{
    private function bar()
    {
        $arr = array_map([$this, 'foo'], $arr);
    }

    /**
     * array_mapのコールバック関数
     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
     */
    private function foo($item)
    {
    }
}Code language: PHP (php)

(2)可変メソッド

class Something
{
    private function bar()
    {
        $name = 'foo';
        $this->$name();  // 可変メソッドとして、$this->foo()を呼んでいる
    }

    private function foo() // unused?
    {
    }
}Code language: PHP (php)

@SuppressWarningsで抑制し、「可変メソッド $this->$nameで使っている」と書いておきます。

class Something
{
    private function bar()
    {
        $name = 'foo';
        $this->$name();  // 可変メソッドとして、$this->foo()を呼んでいる
    }

    /**
     * 可変メソッド $this->$name で使っている
     * @SuppressWarnings(PHPMD.UnusedPrivateMethod)
     */
    private function foo()
    {
    }
}Code language: PHP (php)

メソッドやクラスになると、本当に未使用だとしても、誰かがせっかく書いたんだし、ちょっともったいないかなと思って、削除しにくいよね

gitに残っているとはいえ、削除した過去のコードは見つけにくいしね

// 未使用 2020-11-23、半年後2021-05-23以降に削除予定、とコメントに残しておいたらどうかしら

UnusedFormalParameter

メッセージ例

Avoid unused parameters such as 'xx'.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.UnusedFormalParameter)
 */Code language: PHP (php)

説明

Avoid passing parameters to methods or constructors and then not using those parameters.

メソッドまたはコンストラクタの引数に、参照されていない引数があります。

コード例

class Foo
{
    private function bar($howdy)
    {
        // $howdy is not used
    }
}Code language: PHP (php)

解説

まだリリースしていないメソッドなら、引数を削除しましょう。

実は使っているケース

(1)requireやinclude先で使っている場合

class Foo
{
    /**
     * $howdyはrequire先で使っている。
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     */
    public function bar($howdy)
    {
        require 'bar.php';
    }
}Code language: PHP (php)
// bar.php
<?php echo $howdy; ?>
Code language: PHP (php)
使っていないけれども、引数を削除していいか迷うケース

(1)すでにリリース済みの場合

定義している箇所だけでなく、呼び出し元もリファクタする必要があるので、無理にリファクタする必要はありません。

privateメソッドは、呼び出し元を特定しやすいので、リファクタしてもいいでしょう。

protectedメソッド、publicメソッドは、リファクタしないほうがいいと思います。

リファクタする場合は、呼び出し元をIDEのfind usagesで探し、プロジェクト内をメソッド名で全検索し、どこからも使われていないことを確認し、全てのテストケースがグリーンを確認しましょう。

このような手間をかけるよりは、引数を残しておいたままにして、別の機能追加や修正をしたほうがいいかもしれません。

@SuppressWarningsで抑止し、「$howdyは#123の理由で未使用となった。数カ所から呼ばれている。引数を削除する場合は、呼んでいる箇所もリファクタすること」などを書いておきましょう。

class Foo
{
    /**
     * $howdyは#123の理由で未使用となった。数カ所から呼ばれている。引数を削除する場合は、呼んでいる箇所もリファクタすること。
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     */
    public function bar($howdy)
    {
        // $howdy is not used
    }
}Code language: PHP (php)

引数の削除は、セマンティックバージョニングの「1.APIの変更に互換性のない場合はメジャーバージョンを上げます」に相当するんだね

(2)コールバック関数

例えば array_walkのコールバック関数は、$value、$keyの2つ引数が渡されますが、$keyは使わないといった場合です。

class Foo
{
    public function bar()
    {
        array_walk($arr, [$this, 'mycallback']);
    }

    private function mycallback($value, $key)
    {
        // $key is not used
    }
}Code language: PHP (php)

2つめの引数を削除してもいいと思いますが、未使用の引数を残しておくという考えもあります。

PHPも型チェックを厳密にしていく方向のように思います。将来PHPの仕様が変わり、呼ぶ側と定義側の引数の個数が一致しないと警告するようになるかもしれません(そうならないかもしれません)

未使用の引数を残しておく場合は、@SuppressWarningsで抑制し、「array_walkのコールバック関数」と書いておきましょう。

class Foo
{
    public function bar()
    {
        array_walk($arr, [$this, 'mycallback']);
    }

    /**
     * array_walkのコールバック関数
     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
     */
    private function mycallback($value, $key)
    {
        // $key is not used
    }
}Code language: PHP (php)

タイトルとURLをコピーしました