最近の興味

コードレビューや設計レビューなど、
未だに人力で頑張っているプロジェクトが多い印象があるので、
なんかおしゃれに自動化出来ないかなと思って考えていて、
とりあえずなんか作ってみようかなとなって作ってみました。

perlcritic選定理由

perlでの静的解析ツールで調べると、
perlcritic推しが多い印象だったのでとりあえず試してみようかという感じ

perlcriticのインストール

最近はcpanmではなくcartonが普通っぽいが、
現環境ではcpanmを使ってるのでここではそれを使用する。

cpanm Perl::Critic;

これで基本的にモジュールは入るが、
perlcriticというスクリプトのパスが通ってなかったので、.bashrcを書き換えてパスを通す。

# ここは設定済みであれば対応不要
# cpanmで落としてくるディレクトリのパスが通ってればオーケー。
export PATH=$HOME/extlib/bin:$PATH

これでperlcriticを使う準備は出来た。

使ってみる

とりあえず既存タイトルのあるモジュールをperlcriticにかけてみました

> perlcritic -5 --verbose 8 Base.pm
[Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 127, column 2.  (Severity: 5)
[Subroutines::ProhibitReturnSort] "return" statement followed by "sort" at line 160, column 2.  (Severity: 5)
[Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 395, column 2.  (Severity: 5)
[Variables::ProhibitConditionalDeclarations] Variable declared in conditional statement at line 445, column 2.  (Severity: 5)
[Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 643, column 2.  (Severity: 5)
[Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 650, column 2.  (Severity: 5)
[Variables::ProhibitConditionalDeclarations] Variable declared in conditional statement at line 665, column 2.  (Severity: 5)
[Variables::ProhibitConditionalDeclarations] Variable declared in conditional statement at line 697, column 2.  (Severity: 5)
[Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 1154, column 2.  (Severity: 5)
[Subroutines::ProhibitExplicitReturnUndef] "return" statement with explicit "undef" at line 1155, column 2.  (Severity: 5)

結構出る。

5 オプションについてどこまで厳格にPerlベストプラクティスに従っているのかの指定。ちなみに5は最も優しいオプション。

他の厳しいオプションで検知されるものとしては、文字列の中に変数展開が含まれてないのに
ダブルクォーテーションを使っているのはNGなどがあるが、今回はそこまでしない。

verbose オプションについてなぜその書き方だと駄目なのかという情報をどれだけしっかり表示するのかの指定。

これぐらいゆるく静的解析をするのはありかもしれない。
githubのhookとかに打ち込めばある程度の品質の担保は行けそう。

ということで実際にhookに仕込んで見る

.git/hooks/pre-push


remote="$1"
url="$2"

z40=0000000000000000000000000000000000000000

while read local_ref local_sha remote_ref remote_sha
do
    if [ "$local_sha" = $z40 ]
    then
        # Handle delete
        :
    else
        if [ "$remote_sha" = $z40 ]
        then
            # New branch, examine all commits
            range="$local_sha"
        else
            # Update to existing branch, examine new commits
            range="$remote_sha..$local_sha"
        fi

        commit=`git diff --name-only "$range"`
        if [ -n "$commit" ]
            for file in $commit; do
                case "${file##*.}" in
                    pl | pm | t)
                        echo $file
                        #perlcritic -5 --verbose 8 $file || exit 1
                        ;;
                    *)
                        ;;
                esac
            done
            exit 0
        then
            exit 1
        fi
    fi
done

exit 0

pushされる前に、
pushしようとしているコミットと最新のコミットの間にある変更差分から、
Perlモジュール、Perlスクリプト、テストのみに対してperlcriticをかけるという処理

作ってみた感想

Perlベストプラクティスに従うならこれでいいかなと言う印象。
あと、実際に大量のコードが存在しているプロジェクトに投入すると、
変更する毎に自分が触った部分ではない処理の解析が大量にされるので、
覚悟を持ってやる必要あるなとは思いました。

でも、push前に指摘するよりPR時に指摘とかの方が良いなと思うので、
そういうのを組み合わせた環境もちょっと今度作ってみたい

参考

Perl::Criticの使用
 git-commit前にsyntaxチェックする
 Perl::Critic