レガシープロジェクトで phpcs Generic.Files.LineLength をリファクタするには

概要
引数並びが長い
計算式が長い
ifの条件式が長い
HTMLタグが長い
AタグのURLクエリーが長い
SQL文が長い
文字列が長い

概要

Line exceeds 120 characters; contains 192 charactersCode language: plaintext (plaintext)

1行が長すぎる（横に長い）、という警告ですね。

PHPに限らず、かつては1行80文字まででしたが、ディスプレイが大きくなり、1行120文字まで増えました。

しかし、クラス名もメソッド名も変数名も長くなり、プロパティを参照すると $this-> で7文字使います。1行で表現できる内容は昔と変わっていないか、少なくなっているかもしれません。

引数並びが長い

メソッドや関数の引数並びが長い場合は、1行1引数に改行します。

関数やメソッドを定義している側

function foo($aaaaaaaaa1, $aaaaaaaaa2, $aaaaaaaaa3, $aaaaaaaaa4, $aaaaaaaaa5, $aaaaaaaaa6, $aaaaaaaaa7, $aaaaaaaaa8, $aaaaaaaaa9)
{
}
Code language: PHP (php)

1行1引数に改行します。

function foo(
    $aaaaaaaaa1,
    $aaaaaaaaa2,
    $aaaaaaaaa3,
    $aaaaaaaaa4,
    $aaaaaaaaa5,
    $aaaaaaaaa6,
    $aaaaaaaaa7,
    $aaaaaaaaa8,
    $aaaaaaaaa9
) {
}
Code language: PHP (php)

関数やメソッドを呼んでいる側

$str = sprintf("長いフォーマット文字列", $aaaaaaa1, $aaaaaaa2, $aaaaaaa3);Code language: PHP (php)

1行1引数に改行します。

$str = sprintf(
    "長いフォーマット文字列",
    $aaaaaaa1,
    $aaaaaaa2,
    $aaaaaaa3
);Code language: PHP (php)

関数やメソッドに分けてしまうのもいい方法です。

$str = myFormat($aaaaaaa1, $aaaaaaa2, $aaaaaaa3);

function myFormat($a1, $a2, $a3);
    $fmt = "長いフォーマット文字列";
    $str = sprintf($fmt, $a1, $a2, $a3);
    return $str;
}
Code language: PHP (php)

計算式が長い

計算式に、メソッド呼び出しを含んでいて長い場合は、

$x = $y * anotherFunc1($aaaaaaa1, $aaaaaaa2, $aaaaaaa3) + anotherFunc2($bbbbbbb1, $bbbbbbb2, $bbbbbbb3);
Code language: PHP (php)

メソッドの結果を変数に代入してから、計算します。

$a1 = anotherFunc1($aaaaaaa1, $aaaaaaa2, $aaaaaaa3);
$a2 = anotherFunc2($bbbbbbb1, $bbbbbbb2, $bbbbbbb3);
$x = $y * $a1 + $a2;
Code language: PHP (php)

ifの条件式が長い

ifの条件式が長い場合は、

// リファクタ前
if ($a1->a1 * $b1->b1 < $c1->c1 * $d1->d1 + $e1->e1() || $a2->a2 * $b2->b2 < $c2->c2 * $e2->e2()) {Code language: PHP (php)

|| や && を区切りに改行したり、

if ($a1->a1 * $b1->b1 < $c1->c1 * $d1->d1 + $e1->e1() ||
    $a2->a2 * $b2->b2 < $c2->c2 * $e2->e2()) {Code language: PHP (php)

条件部の計算や比較をメソッドに分けます。

if ($this->isSomething1($a1->a1, $b1->b1, $c1->c1, $d1->d1, $e1->e1()) ||
    $this->isSomething2($a2->a2, $b2->b2, $c2->c2, $e2->e2())) {
}

function isSomething1($a, $b, $c, $d, $e)
{
    return $a * $b < $c * $d + $e;
}

function isSomething2($a, $b, $c, $e)
{
    return $a * $b < $c * $e;
}
Code language: PHP (php)

さらに、次のように書くとデバッガでトレースしやすいですが、さきほどの例とは意味が違うので、注意が必要です。

$f1 = $this->isSomething1($a1->a1, $b1->b1, $c1->c1, $d1->d1, $e1->e1());
$f2 = $this->isSomething2($a2->a2, $b2->b2, $c2->c2, $e2->e2());
if ($f1 || $f2) {
}
Code language: PHP (php)

何が違うのかな？

$e2->e2()が呼ばれるたびに、$e2内部のカウンタをインクリメントしているとするわね

１つめの例で、isSomething1()がtrueとします。isSomething2()は呼ばれません。$e2内部のカウンタはいくつかしら？

初期値0とするね。呼ばれないんだから、0のままだね

ピンポーン

２つめの例も、isSomething1()がtrueとします。常にisSomething2()は呼ばれます。$e2内部のカウンタはいくつかしら？

1回呼ばれるから、1だね。さっきと違うね！

ピンポーン。&&は左側がfalseなら、右側の判定はしないの。

じゃあメソッドを呼ばず、プロパティだけを使っていれば、1つめの例も2つめの例も同じかな？

通常は同じね。でもgetterのマジックメソッドを実装しているかもしれないから、断言はできないわ。

HTMLタグが長い

HTMLを表示するphpの場合です。

タグを入れ子で改行して、1行1タグにしておくだけでいいと思います。

    <input type="text" id="xxxx" name="yyyy" value="<?php echo $xxxx; ?>" />Code language: PHP (php)

長いタグを途中で改行したり、

    <input type="text" id="xxxx" name="yyyy"
        value="<?php echo $xxxx; ?>" />Code language: PHP (php)

属性ごとに改行したり。ここは好みにおまかせします。

    <input
        type="text"
        id="xxxx"
        name="yyyy"
        value="<?php echo $xxxx; ?>"
        />Code language: PHP (php)

AタグのURLクエリーが長い

出力する直前でurlエンコード、htmlエンコードしたほうがいいのですが、それを守ると、クエリー変数２つ並べると、120文字をオーバーしてしまいます。

<a href="xxxx.php?category=<?php echo htmlspecialchars(urlencode($category));?>&amp;from_date=<?php echo htmlspecialchars(urlencode($from_date));?>">Code language: PHP (php)

方法1：urlエンコード、htmlエンコード済みの変数を用意します。

Aタグは少し短くなります。HTMLタグに混じってPHPコードがあるので、ゴチャゴチャした感じになります。Aタグごとに似たようなエンコード処理を書くことになります。

<?php
  $hu_category  = htmlspecialchars(urlencode($category));
  $hu_from_date = htmlspecialchars(urlencode($from_date));
?>
<a href="xxxx.php?category=<?php echo $hu_category;?>&amp;from_date=<?php echo $hu_from_date;?>">Code language: PHP (php)

方法2：短い名前のヘルパー関数をphp側のグローバルに用意しておきます。

<?php
function ehu($str)
{
  echo htmlspecialchars(urlencode($str));
}
?>
Code language: PHP (php)

短くなりました。名前が短すぎて何をしているか連想しやすいかというと．．．

<a href="xxxx.php?category=<?php ehu($category);?>&amp;from_date=<?php ehu($from_date);?>">Code language: PHP (php)

方法3：build_http_query関数を使って、URLクエリーを作成します。

<?php
$vars = [
    'category'  => $category,
    'from_date' => $from_date,
    'to_date'   => $to_date,
];
$query = build_http_query($vars);
?>
<a href="xxxx.php?<?php echo htmlspecialchars($query);?>">Code language: PHP (php)

外部ファイルに関数定義しておき、

<?php
function xxxx_build_query($category, $from_date, $to_date)
{
    $vars = [
        'category'  => $category,
        'from_date' => $from_date,
        'to_date'   => $to_date,
    ];
    $query = build_http_query($vars);
    return htmlspecialchars($query);
}
Code language: PHP (php)

ファイル先頭でrequire_onceします。

<?php
require_once __DIR__ . '/xxxx_helper.php';
?>

<a href="xxxx.php?<?php echo xxxx_build_query($category, $from_date, $to_date);?>">Code language: PHP (php)

SQL文が長い

生のSQLで、SELECTするカラムを "*" ではなく、一つ一つ指定すると、すぐに1行が長くなり、phpcsのLineLengthに引っかかります。

    public function selectXxx()
    {
        ....
        $sql = "SELECT id, name, category, aaa, bbb, ccc, ddd FROM ... WHERE ... ORDER BY ...;";
       ....   
    }
    Code language: PHP (php)

まず、元々の箇所からコピペして、SQL文だけを返すメソッドに分けて、phpcs:ignoreしておきます。

    public function selectXxx()
    {
        ....
        $sql = $this->sqlToSelectXxx();
        ....   
    }
    
    public function sqlToSelectXxx()
    {
        // phpcs:ignore Generic.Files.LineLength
        $sql = "SELECT id, name, category, aaa, bbb, ccc, ddd FROM ... WHERE ... ORDER BY ...;";
        return $sql;
    }Code language: PHP (php)

元々の場所で、SQL文字列内で変数を参照している場合は、

    public function selectXxx()
    {
        ....
        $sql = "SELECT id, name, category FROM ... WHERE date >= '$date_from' ORDER BY ...;";
       ....   
    }
    Code language: PHP (php)

参照している変数をメソッドの引数で渡します。

    public function selectXxx()
    {
        ....
        $sql = $this->sqlToSelectXxx($date_from);
        ....   
    }
    
    public function sqlToSelectXxx($date_from)
    {
        // phpcs:ignore Generic.Files.LineLength
        $sql = "SELECT id, name, category FROM ... WHERE date >= '$date_from' ORDER BY ...;";
        return $sql;
    }Code language: PHP (php)

phpMyAdminのSQLフォーマット風に改行してあると行数が長くなり、phpmdの1メソッド100行にひっかかります。

元のメソッドを @SupressWarnings(PHPMD.ExssesiveMethodLength)で抑制するよりは、

    /**
     *
     * @SupressWarnings(PHPMD.ExssesiveMethodLength)
     */
    public function selectXxx()
    {
        ....
        $sql = "
SELECT
    id,
    name,
    category
FROM
     ... 
WHERE
    date >= '$date_from'
ORDER BY
    ...;
";
       ....   
    }
Code language: PHP (php)

SQL文だけをメソッドに分けて、分けたメソッドを@SupressWarningsで抑制したほうがいいでしょう。元々のメソッドの本来の行数の長さでphpmdをかけることができます。

    public function selectXxx()
    {
        ....
        $sql = $this->sqlToSelectXxx($date_from);
        ....
    }

    /**
     *
     * @SupressWarnings(PHPMD.ExssesiveMethodLength)
     */
    public function sqlToSelectXxx($date_from)
    {
        $sql = "
SELECT
    id,
    name,
    category
FROM
     ... 
WHERE
    date >= '$date_from'
ORDER BY
    ...;
";
        return $sql;
    }
Code language: PHP (php)

コピペしたSQL文は、リファクタせずに、そのままにしておきます。この手のSQL文を編集する機会はとても少ないので、そのままでも問題ないと思います。

SQL文をリファクタする場合は、テスト用にインポートするデータを用意して、selectXxxのユニットテストを書いてからにします。

SQL文のここは間違いかも？と思っても、修正も慎重になりましょう。

ユニットテストがない状態で、修正やリファクタする場合は、SQL文を表示して、それをコピペして、phpMyAdminやmysqlコマンドで動作確認します。

何回も動作確認するのが面倒になって、ユニットテストを書くようになるのよ

文字列が長い

巨大文字列は、別ファイルにしましょう。

    public function foo()
    {
        ....
        $text = "
とにかく長い $name
とにかく長い $category
";
        ....
    }
Code language: PHP (php)

まず、文字列だけを返すメソッドに切り分けます。

    public function foo()
    {
        ....
        $text = $this->getTextFoo($name, $category);
        ....
    }

    /**
     *
     * @SupressWarnings(PHPMD.ExssesiveMethodLength)
     */
    public function getTextFoo($name, $category)
    {
        $text = "
とにかく長い $name とにかく長い 
とにかく長い $category とにかく長い 
";
        return $text;
    }
Code language: PHP (php)

文字列を別のphpファイル、foo_text.phpにします。

$nameや、$categoryなどの変数を参照している箇所は、<?php echo $name; ?>、<?php echo $category; ?>にします。


とにかく長い <?php echo $name; ?> とにかく長い 
とにかく長い <?php echo $category; ?> とにかく長い 
Code language: PHP (php)

つまり、HTMLを表示するphpファイルと同じ使い方です。

バックスラッシュエスケープは必要ありません。ダブルクォート、シングルクォートも書きたいように書くことができます。

気をつけることは、行末に <?php echo $name; ?>を書くと、その行は改行されません。そこで、改行するための空行を一つ入れます。

とにかく長い <?php echo $name; ?>

とにかく長い <?php echo $category; ?>

元のphpファイルに戻ります。

一時的に新メソッドgetTextFoo_v2を作ります。foo_text.phpを（require_onceではなく）requireします。このままではfoo_text.phpの内容が表示されてしまうので、ob_start()とob_get_clean()を使って変数にキャプチャします。

    public function foo()
    {
        ...
        $text = $this->getTextFoo($name, $category);
        ...
    }

    public function getTextFoo($name, $category)
        ...
    }

    public function getTextFoo_v2($name, $category)
    {
        ob_start();
        require __DIR__ . '/foo_text.php';
        $text = ob_get_clean();
        return $text;
    }
Code language: PHP (php)

簡易的な動作確認として、foo()メソッド内で比較するコードを書いておきます。デバッガのステップ実行で$fの値がtrueかどうか確認したり、$fの値をvar_dumpします。

（最後にユニットテストの例があります）

    public function foo()
    {
        ....
        $text  = $this->getTextFoo($name, $category);
        $text2 = $this->getTextFoo_v2($name, $category);
        $f = $text === $text2;
        var_dump($f);
        ....
    }

    public function getTextFoo($name, $category)
    }

    public function getTextFoo_v2($name, $category)
    {
        ob_start();
        require __DIR__ . '/foo_text.php';
        $text = ob_get_clean();
        return $text;
    }
Code language: PHP (php)

古いメソッドを削除し、getTextFoo_v2メソッドをgetTextFooにリネームして、リファクタ完了です。

    public function foo()
    {
        ....
        $text = $this->getTextFoo($name, $category);
        ....
    }

    public function getTextFoo($name, $category)
    {
        ob_start();
        require __DIR__ . '/foo_text.php';
        $text = ob_get_clean();
        return $text;
    }
Code language: PHP (php)

この例はデータベースを使わないので、ユニットテストを書いて、テスト実行しましょう。

元の巨大文字列をユニットテスト側にコピペしておきます。そのメソッドの結果を$expected、新しいメソッドの結果を$actualとして、$this->assertEqualsで比較します。

    /**
     * @test
     */
    public function getTextFoo()
    {
        $name = "太郎";
        $category = "カテＡ";

        $target = new Bar();
        $actual = $target->getTextFoo($name, $category);

        $expected = $this->getTextFoo_old($name, $category);

        $this->assertSame($expected, $actual);
    }

    private function getTextFoo_old($name, $category)
    {
        $text = "
とにかく長い $name とにかく長い 
とにかく長い $category とにかく長い 
";
        return $text;
    }
Code language: PHP (php)