2014年6月3日火曜日

Smarty 3系では「$cache_dir/$compile_dir/$config_dir/$plugins_dir/$template_dir」のクラス変数に値を直接代入してはいけない!?

君はこのことを知っていたかい?


Risolutoの開発でPhpStormを使っているのだけれど、「Inspect Code」って便利な機能が手放せない。これは静的コード解析をしてくれて、問題がありそうな所をピックアップして教えてくれるって代物。もちろん完璧じゃないけども、「動くけど好ましくない」コードとかが洗い出せるので、スゴイオススメ。これだけでPhpStormを使う価値ある(*1)。

さて、そんな便利機能で出てた指摘をチェックしていたらスゴイ不可解な……というか、なんで出るのかしばらく分からなかったものがあったのでメモっておこうと思う。

それが下記のコード(*2)。何が問題かすぐに分からない人はボクのナカーマ。

$this->smarty = new Smarty();

$this->smarty->cache_dir      = '/path/to/dir/';
$this->smarty->compile_dir   = '/path/to/dir/';
$this->smarty->config_dir     = '/path/to/dir/';
$this->smarty->plugins_dir    = '/path/to/dir/';
$this->smarty->template_dir = '/path/to/dir/';

あ、「require/includeしてない!」とか「インデントがおかしい!」とか「今時Smarty使うなんて時代遅れ!」とかそういうことじゃないよ?「$cache_dir/$compile_dir/$config_dir/$plugins_dir/$template_dir」の各クラス変数に値をセットするところに問題がある。

このコードでは、PHPのテンプレートエンジンであるSmartyのインスタンスを生成して、各種処理で使うディレクトリを指定するって感じの処理をしている。結構ネット上にあるサンプルコードとかを見ているとこのような記述をよく見かけるし、実際にこんな感じでいつも書いてるよ!……って人もいると思う。

特に昔からSmartyを使ってる人は、たぶんこのコードの問題が分からないと思う。ボクもずっとこの書き方に慣れ親しんでいたから、PhpStormでWarningが出てても理由が分からなかった。それに、特にエラーになったりもせずちゃんと動くんだ。

最後のヒント。Warningの内容は「Member has private access, but class has magic method __set()」ってヤツだよ!

コードを読んでみますか


分からないならコードを読めばいいじゃない!……ってことで、Smartyのコードを読んでみることにした。対象になるのは「Smarty.class.php」だ。SmartyはGoogle Codeでホスティングしているようなので、例えば下記からチェックすることができる(*3)。


とりあえず、5つあるうちのひとつだけを取り上げる。ここでは「$cache_dir」を例に取ろう。

まず「$cache_dir」自体がどうやって定義されているのかを見てみよう。前述のメッセージによるとPrivateになっているはずなのだが……。

/**
     * cache directory
     * @var string
     */
    private $cache_dir = null;

おお、確かにPrivateで宣言されている(*4)。……けど、じゃあ、どうして直接代入してもエラーなく動くのかが気になる。「__set()」ってことだから、Setterがどっかにあるはずだ。

/**
     * <<magic>> Generic setter.
     *
     * Calls the appropriate setter function.
     * Issues an E_USER_NOTICE if no valid setter is found.
     *
     * @param string $name  property name
     * @param mixed  $value parameter passed to setter
     */
    public function __set($name, $value)
    {
        $allowed = array(
        'template_dir' => 'setTemplateDir',
        'config_dir' => 'setConfigDir',
        'plugins_dir' => 'setPluginsDir',
        'compile_dir' => 'setCompileDir',
        'cache_dir' => 'setCacheDir',
        );

        if (isset($allowed[$name])) {
            $this->{$allowed[$name]}($value);
        } else {
            trigger_error('Undefined property: ' . get_class($this) . '::$' . $name, E_USER_NOTICE);
        }
    }

おお、たしかに「__set()」がある。しかもPhpStormがWarning出してたクラス変数全部に関係ありそうなアレになっている!どうやら「setCacheDir()」というメソッドがAutomagicallyにコールされるように作られているようだ。

ちなみにこの「__set()」、マジックメソッドのひとつで「オーバーロード機能」なんて言われてたりする。他の言語なんかで使われている一般的な「オーバーロード」ととは意味合いが違うんだけど紛らわしいね。


オンラインマニュアルによると、このマジックメソッドは「宣言されていないプロパティやメソッドを操作しようとしたとき」や「現在のスコープからは アクセス不能な プロパティやメソッドを操作しようとしたとき」に起動するとある。なるほど、これのおかげでPrivateなはずのクラス変数に値がセットできたわけだ。

PhpStormはこんなものを検出して教えてくれたというわけだ。うーんと、「マジックメソッドを使ってアクセスできるけどPublicなクラス変数のつもりしてない?」的な親切心だろうか。

ではどう修正するのか


Warningとして検出された理由と問題なく動いている理由については理解できた。次の疑問は「じゃあどうやって修正するといいの?」ってことだろう。……まあ、直さなくてもいいといえばいいんだろうけど、一応できる限りのWarningは消しておきたいしね。

まあ、先の「__set()」を見ていれば分かるとおり、Setterが用意されているのでそれを使ってやればいい。最初の方に書いたサンプルコードを例に修正すると、こんな感じになる。

$this->smarty = new Smarty();

$this->smarty->setCacheDir('/path/to/dir/');
$this->smarty->setCompileDir('/path/to/dir/');
$this->smarty->setConfigDir('/path/to/dir/');
$this->smarty->setPluginsDir('/path/to/dir/');
$this->smarty->setTemplateDir('/path/to/dir/');

これでPhpStormのWarningは消える。めでたしめでたし。

Smarty 3系を使う機会が今後増えてくるだろうと思うけど、基本的にはこのエントリの知識はなくても何とかなる。なぜならSmarty側が後方互換性を担保してくれているから。ただ、内部でこのように動いているってことを知っておくと、何かの時に役に立つかもしれないよね。

ちなみにRisolutoではこの「SetCacheDir()」等を使うコードに修正することを選んだ。よりよいコードになるかな?


*1:ってPHP勉強会のLTでも誰か言ってた気がする
*2:問題を分かりやすくするためにミニマムな記述にしているよ!
*3:もちろんソースが手元にあればそれを見てもいい
*4:ちなみにSmarty 2系の時は「var $cache_dir = 'cache';」のように定義されていた

0 件のコメント:

コメントを投稿