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

元気ですか!リファクタしてますか!

概要

A file should declare new symbols (classes, functions, constants, etc.) and cause no other side effects, or it should execute logic with side effects, but should not do both. The first symbol is defined on line 5 and the first side effect is on line 3.
Code language: plaintext (plaintext)

「クラス、関数、定数の定義」と「副作用のある実行コード」を、一つのファイルに書かないほうがいいよ、という警告です。

PHPはクラスや関数の外側にも実行コードを書くことができます。

クラスや関数の外側に実行コードを書いてしまうと、そのクラスを利用するためにファイルを読み込んだ瞬間に、副作用が起きてしまい、再利用しづらいんですね。

namespace設定していないレガシープロジェクトでは、このSideEffects警告と「classにnamespace設定していないよ警告」が大量にでます。

エントリーポイント

ブラウザ側から見るとエンドポイントURL、php側から見るとエントリーポイント、のphpファイルです。

<?php class FooAction { public function main() { } } $foo = new FooAction(); $foo->main();
Code language: PHP (php)

何がいけないのかな?せっかくリファクタして、FooActionクラスにまとめたのに。

リファクタ前はSideEffects警告はなく、リファクタしてクラスにするとSideEffects警告がでるようになります。

この場合のFooActionクラスは他から参照されないので、警告を無視してもかまいません。

が、せっかくリファクタしてFooActionクラスにしたのなら、FooActionクラスを別ファイルにしましょう。

別ファイルにすることのメリットは、FooActionクラスのユニットテストを書けるようになることです。

<?php require_once __DIR__ . '/FooAction.php'; $foo = new FooAction(); $foo->main()
Code language: PHP (php)
<?php class FooAction { public function main() { } }
Code language: PHP (php)

namespaceを設定していないクラスファイル

クラスから別クラスをrequire_onceしている場合です。

<?php require_once __DIR__ . '/Foo.php'; class Bar { public function barbar() { $foo = new Foo(); } }
Code language: PHP (php)

クラスファイルを require_once しても、副作用はないよ?

確かに、クラスファイルを require_once しても、副作用はありません。

この場合は、SideEffects警告を無視していいと思います。

一見、実行コードがないように見えますが、なぜ警告されるのでしょうか?

なぜなら、require_onceも実行コードとみなされるからです。

require_onceは、require_once "index.tpl" のようにHTMLを表示するときにも使います。この使い方では、HTML出力という副作用があります。

そのため、phpcsは、require_onceを副作用のある実行コードとして扱います。

インクルード定義 inc.php

各エントリーポイントのphpから、inc.php だけを require_onceしている場合です。

<?php define('MAX_XXX', 100); define('MAX_YYY', 100); ini_set('xxx', 'yyy'); session_start(); require_once __DIR__ . '/libs/aaa.php'; require_once __DIR__ . '/libs/bbb.php'; function my_foo() { }
Code language: PHP (php)

いかにもレガシーだね

この inc.php では定数定義、ini_set、クラスや関数のrequire_once、ファイルにするほどではない小さい関数などが定義されています。

まず、定数定義を別ファイル にして、require_onceします。

次に、関数も別ファイルにして、require_onceします。

<?php ini_set('xxx', 'yyy'); session_start(); require_once __DIR__ . '/constant.php'; require_once __DIR__ . '/libs/aaa.php'; require_once __DIR__ . '/libs/bbb.php'; require_once __DIR__ . '/my_foo.php';
Code language: PHP (php)

ここままで充分でしょう。

ifの条件式は実行コードとみなされない

ifの条件式は実行コードとみなされないようです。

次のように書いても、SideEffects警告されませんでした。

目視確認も必要ですね。

<?php if (require_once __DIR__ . '/Foo.php') { } if (file_put_contents('xxx', 'yyy')) { } class Bar { } if (!function_exists('foo')) { function foo() { } } if (!class_exists('Bar')) { class Bar { } }
Code language: PHP (php)

最後に

このSideEffects警告と「classにnamespaceを設定していないよ警告」に対処するには、最終的には、namespace設定が必要です。

それなりに面倒な作業です。手間をかけて、namespace設定をしたからといって、Webアプリの性能が向上するということはなく、見た目も動きも変わりません。

管理職みたいなことを言うのね?

まず、E2Eテストやユニットテストを書いてから、namespace設定をしても遅くはありません。

と思ったら、それはそのとおりね

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