Go to Qiita Advent Calendar 2024 Top
LoginSignup
81
64

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

@rana_kualu

phpmdのルール一覧

Last updated at Posted at 2017-03-07

phpmdとは

phpmdは静的コード解析ツールです。
phpmd hoge.php text cleancodeとか1行コマンド入れるだけでさくっとチェックしてくれます。
コマンドの文法はここらへんを参照。

ソフトウェアテストのような厳重なチェックは行いませんが、ソースコードの大まかな問題点をざっと確認してくれます。
いちいち確認せずとも問題外のコードを門前払いしてくれるというだけでだいぶ手間が省けるので、とりあえずpre-receiveあたりに突っ込んでおくと捗るでしょう。

なお、実際に動かすと100%死ぬコードでもphpmdチェックは問題なく通ることもあるので、phpmdを導入したからといってテストを省いたりしてはいけません。

ルール一覧

以下は具体的にどんな指標で弾かれるかのルール一覧です。
確認したバージョンは2.6.0です。
ルールセットはPMDから取り入れたようです。

なお、関数と書いてあった場合は基本的に関数/メソッド両方に適用されます。
逆にメソッドと書いてあれば関数には適用されません。

Clean Code Rules

整形ルール。
ただしPHPとしてはちょっと困るようなルールもあります。

BooleanArgumentFlag

function foo(bool $bool = true){}

The method foo has a boolean flag argument $bool, which is a certain sign of a Single Responsibility Principle violation.
関数の引数デフォルト値にboolean値が設定されている。

単一責任の原則に反するかららしいですが、ちょっとはっきりした理由はよくわかりませんでした。

ElseExpression

function foo(bool $bool){
    if($bool){
        hoge();
        $ret = true;
    }else{
        fuga();
        $ret = false;
    }
    return $ret;
}

The method foo uses an else expression. Else is never necessary and you can simplify the code to work without else.
elseは不要。

ということになってるんだけど、elseがないと困るような状況でも出てきます。
上記は作為的な例ですが、これをelse無しで書いても逆にわかりにくくなるだけでしょう。
このルールは外したほうがいいでしょう。

StaticAccess

class HOGE{
    public static function fuga(){}
}
class Foo{
    public function bar(){
        HOGE::fuga();
    }
}

Avoid using static access to class 'HOGE' in method 'bar'.
メソッド内で別クラスのメソッドを静的呼び出ししている。

直接別クラス名が書かれてしまっているため、密結合になってしまっています。
ただ、これを完全に守るとstaticメソッド一切使用不能となって色々困難なので、ルールを外してもいいと思います。

Code Size Rules

コードサイズと複雑さについてのチェック。

CyclomaticComplexity

function hoge(){
    if ($a===1) {
        if($b===2){
            if($c===3){
                if($d===4){
                    if($e===5){
                        if($f===6){
                            if($g===7){
                                if($h===8){
                                    if($g===9){
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
}

The function hoge() has a Cyclomatic Complexity of 10.
関数が複雑すぎる。デフォルト上限値10。

ネストが深すぎる場合にももちろん出ますが、実際は制御構造の累計をカウントしているだけなので、フラットな関数でも発生することがあります。
具体的にはcaseを10個並べるだけで出ます。
解決策としては、もっと分割するか、そもそも設計を考え直しましょう。

NPathComplexity

function hoge(array $conds) {
    if($conds[0]){}else{}
    if($conds[1]){}else{}
    if($conds[2]){}else{}
    if($conds[3]){}else{}
    if($conds[4]){}else{}
    if($conds[5]){}else{}
    if($conds[6]){}else{}
    if($conds[7]){}else{}
}

The function hoge() has an NPath complexity of 256. The configured NPath complexity threshold is 200.
経路数が多すぎる。デフォルト上限値200。

NPathは何かというと、経路網羅の経路数です。
上記hoge()であれば$condsによって256ルートが存在するため、経路網羅のテストを作るのは現実的ではありません。
まあ元々経路網羅そのものが現実的ではないですが。

ExcessiveMethodLength

function hoge(){
    /* 以下100行以上の関数 */
}

The function hoge() has 100 lines of code. Current threshold is set to 100. Avoid really long methods.
関数が長すぎる。デフォルト上限値100行。

これはもう、分割しろとしか言えませんね。

ExcessiveClassLength

class HOGE{
    /* 以下1000行以上のクラス */
}

The class HOGE has 1000 lines of code. Current threshold is 1000. Avoid really long classes.
1クラスが長すぎる。デフォルト上限値1000行。

長すぎるクラスというのは、何もかもを詰め込んだ万能クラスであることが多いので、設計の見直しを検討しましょう。
ただ、分割しようがない状態で1000行超えるクラスも時には出てきてしまうことがあるので、その場合は@SuppressWarningsを検討してもいいかもしれません。

ExcessiveParameterList

function hoge($p1, $p2, $p3, $p4, $p5, $p6, $p7, $p8, $p9, $p10){}

The function hoge has 10 parameters. Consider reducing the number of parameters to less than 10.
関数の引数が多すぎる。デフォルト上限値10。

10どころか5でも多すぎって判断していいんじゃないか。

ExcessivePublicCount

The class HOGE has 45 public methods and attributes. Consider reducing the number of public items to less than 45.
publicが多すぎる。デフォルト上限値45。

1クラス内にpublicなプロパティとメソッドが合わせて45個以上あると警告されます。
protected/privateはカウントされないので、見せる必要の無いものは隠しましょう。
むしろ機能が多すぎるので分けましょう。

TooManyFields

The class HOGE has 15 fields. Consider redesigning HOGE to keep the number of fields under 15.
プロパティが多すぎる。デフォルト上限値15個。

プロパティが多すぎるクラスに警告されます。
これはpublic/protected/private合わせてカウントされます。
あとvarも。varの存在自体には警告が出ません。

TooManyMethods

The class HOGE has 25 non-getter- and setter-methods. Consider refactoring HOGE to keep number of methods under 25.
メソッドが多すぎる。デフォルト上限値25個。

メソッドが25個以上あるクラスに警告されます。
これはpublic/protected/private合わせてカウントされます。
あとアクセス修飾子未設定メソッドも。未設定メソッド自体には警告が出ません。

なお、setter/getterメソッドについては、このカウントの対象外となります。
setter/getterは必然的にpublicになりますが。

TooManyPublicMethods

The class HOGE has 10 public methods. Consider refactoring HOGE to keep number of public methods under 10.
publicメソッドが多すぎる。デフォルト上限値10個。

publicメソッドが10個以上あるクラスに警告されます。
setter/getterメソッドについては、このカウントの対象外となります。

他の多過ぎ警告はともかく、publicメソッド10個というのはわりと起こりやすい気がしますね。

ExcessiveClassComplexity

The class Foo has an overall complexity of 50 which is very high. The configured complexity threshold is 50.
クラスが複雑すぎる。

1クラス内の複雑さが50を超えると発生します。
複雑さの計算方法はCyclomaticComplexityと同じみたいなので、場合によっては200行足らずのクラスでも簡単に起こってしまいます。
これに関しては制限を緩めていいと思います。
なにしろphpmd自体が違反してますからな、これ。

Controversial Rules

賛否両論あるルール。
なので、ここのルールは要件によって検証対象から外してもかまわないでしょう。

Superglobals

function getA(){
    return $_SESSION['a'];
}

getA accesses the super-global variable $_SESSION.
スーパーグローバル変数にアクセスしている。

セッションやリクエストの管理クラスを作りましょう。
管理クラス自体は最終的に結局スーパーグローバル変数を触ることになるので、そこだけはSuppressWarningsするといいでしょう。
というか既存のFWを使え。

CamelCaseClassName

CamelCasePropertyName

CamelCaseMethodName

CamelCaseParameterName

CamelCaseVariableName

class foo{
    private $hoge_fuga;
    public function set_bar($bar_value){
        $this->hoge_fuga = $bar_value;
    }
}

The class foo is not named in CamelCase.
The property $hoge_fuga is not named in camelCase.
The method set_bar is not named in camelCase.
The parameter $bar_value is not named in camelCase.
The variable $bar_value is not named in camelCase.
クラス名、プロパティ名、メソッド名、引数名、変数名がキャメルケースになっていない。

クラス名だけCamelCaseと先頭が大文字で、他はcamelCaseです。

Design Rules

ソフトウェアデザインというか、コーディングルールに近い問題の検出。

ExitExpression

function hoge(){
    exit;
}

The function hoge() contains an exit expression.
関数内でexitしている。

通常のコード内でexitされると外部からのテストができなくなるため、できるだけ避けるべきです。
問題があれば例外を出すなりして呼び出し側で制御できるようにしましょう。

EvalExpression

function hoge(){
    eval('echo "hoge"');
}

The function hoge() contains an eval expression.
evalが使われている。

余程のことでもないかぎりevalを使わなければならない場面などないので、単純に使用しないようにしましょう。

GotoStatement

function hoge(){
    goto A;
}

The function hoge() utilizes a goto statement.
gotoが使われている。

PHPでgotoを使う機会なんて無いので、別の構造に書き換えましょう。

NumberOfChildren

class A{}
class B1 extends A{} class B2 extends A{} class B3 extends A{} class B4 extends A{} class B5 extends A{}
class B6 extends A{} class B7 extends A{} class B8 extends A{} class B9 extends A{} class B10 extends A{}
class B11 extends A{} class B12 extends A{} class B13 extends A{} class B14 extends A{} class B15 extends A{}

The class A has 15 children. Consider to rebalance this class hierarchy to keep number of children under 15.
直接的な子クラスが多すぎる。デフォルト上限15。

直接の子クラスが一定数を超えると警告されます。
java.lang.Objectに掛けてみたらどうなるんだろう。

DepthOfInheritance

class A{}
class B extends A{}
class C extends B{}
class D extends C{}
class E extends D{}
class F extends E{}
class G extends F{}

The class G has 6 parents. Consider to reduce the depth of this class hierarchy to under 6.
親クラスが多すぎる。デフォルト上限6。

辿れる親クラスが一定数を超えると警告されます。
継承しすぎで大変なのでどうにかしましょう。

CouplingBetweenObjects

class Hoge{
    public function hoge(){
        throw new \BadFunctionCallException();
        throw new \BadMethodCallException();
        throw new \DomainException();
        throw new \InvalidArgumentException();
        throw new \LengthException();
        throw new \LogicException();
        throw new \OutOfBoundsException();
        throw new \OutOfRangeException();
        throw new \OverflowException();
        throw new \RangeException();
        throw new \RuntimeException();
        throw new \UnderflowException();
        throw new \RuntimeException();
        throw new \UnexpectedValueException();
    }
}

The class Hoge has a coupling between objects value of 13. Consider to reduce the number of dependencies under 13.
クラスの依存関係が多すぎる。

クラス内で出てくる他クラス数が多すぎると警告されます。デフォルト13。
他のクラスが多いということは設計がうまくいってないということなので何か手を考えましょう。
でもExceptionをきっちり使うとそれだけでだいぶ増えそうな気がする。

DevelopmentCodeFragment

function dump($var){
    var_dump($var);
}

The function dump() calls the typical debug function var_dump() which is mostly only used during development.
デバッグコードがある。

コメントアウトされていないデバッグコードが残っていると警告されます。
デフォルトではvar_dump,print_r,debug_zval_dump,debug_print_backtraceが対象。
もし意図的に出している場合は@SuppressWarningsしましょう。

Naming Rules

命名のチェック。

ShortVariable

LongVariable

class HOGE{
    function foo($a){}
    function bar($abcdefghijklmnopqrstu){}
}

Avoid variables with short names like $a. Configured minimum length is 3.
Avoid excessively long variable names like $abcdefghijklmnopqrstu. Keep variable name length under 20.
変数名が短すぎる、長すぎる。

文字通りで、デフォルトでは3文字以下、20文字以上で警告が出ます。
引数だけではなく使い捨てローカル変数にも出るので微妙にだるい。
for()内のループ変数定義だけは出ません。

ShortMethodName

function a(){}

Avoid using short method names like ::a(). The configured minimum method name length is 3.
関数名が短すぎる。

何故か逆の関数名が長すぎる警告はありません。

ConstructorWithNameAsEnclosingClass

class Hoge{
    public function Hoge(){}
}

Classes should not have a constructor method with the same name as the class
クラス名コンストラクタ滅ぶべし。

慈悲はない。

ConstantNamingConventions

class Hoge{
    const foo=1;
}

Constant foo should be defined in uppercase
定数が大文字ではない。

クラス内定数に小文字を使っている場合に警告されます。
何故かグローバルな定数定義には警告されませんでした。

BooleanGetMethodName

class Foo {
    /**
     * @return boolean
     */
    public function getFoo(){
        return true;
    }
}

The 'getFoo()' method which returns a boolean should be named 'is...()' or 'has...()'
booleanを返すときはisFoo()かhasFoo()にすること。

このエラー、コメント@returnを書かないかぎり出ません。
きちんとコメントを書きつつBooleanGetMethodNameが出るより、コメントも書かずにエラーも出ないって方がありがちな気がする。

Unused Code Rules

未使用コードのチェック

UnusedPrivateField

UnusedLocalVariable

UnusedPrivateMethod

UnusedFormalParameter

class Hoge{
    private $foo;
    public function getHoge($var){
        $return = 'hoge';
        return 'hoge';
    }
    private function getFuga(){
        return 'fuga';
    }
}

Avoid unused private fields such as '$foo'.
Avoid unused parameters such as '$var'.
Avoid unused local variables such as '$return'.
Avoid unused private methods such as 'getFuga'.
privateなフィールドとメソッドが使われていない、引数が使われていない、ローカル変数が使われていない。

意味するところは明らかなので、解説するまでもないでしょう。

その他

ルールは任意で追加できるので、好きにやってみましょう。
とりあえずプロパティアクセス権varの検出は必要だと思った。

感想

クラスまでは導入してるけど、それ以降の作り方が適当なPHP5以降向けのツールであり、
PHP4の時代から歴々と受け継がれてきた古のPHPにたいしては、あまりにも無力だった。

あと表面的な分類と内部構造が合ってないのが気になった。
ルールセットcodesizeのルールは内部的には何故かdesignと同じところに置いてあって、かと思えばルールセットunusedcodeのルールは親ディレクトリに置いてあるという謎配置。
まあ使う側が内部構造を気にする必要は別にないんだけど。

81
64
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
81
64

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?