私が歌川です

@utgwkk が書いている

perlcriticとのつきあい方

技術 アドベントカレンダー

はじめに

こんにちは。perlcriticの話をします。perlcriticに関するまとまった日本語の情報が乏しいなと感じたので、アドベントカレンダーの記事として書くことにします。

これは Perl Advent Calendar 2020 - Qiita 4日目の記事です。3日目は hkoba さんで zsh 上で .pm のメソッド名を補完する話 - hkoba blog でした。

perlcriticとは

perlcriticとは、Perl向けのlinterです。JavaScriptで言うところのESLintのようなものです*1。

TMTOWTDIという有名な言葉がありますが、集団開発では一貫した書き方を求めたい場面も多くあると思います。また、罠は未然に回避したいでしょう。コーディング規約違反や罠への指摘をperlcriticによって機械的に行うことができます。

設定をする

perlcriticのポリシーの多くは、Perlベストプラクティスという本に由来するものです。 Perlベストプラティスの全てが必ずしもベストプラクティスではない*2、という話題もあります。必ずしも全てのポリシーに従わないといけないというものでもないと思います。

perlcriticを入れてみたけど、既存のコードに警告が出まくってノイズになる*3、使いづらそうだから入れるのをやめよう、となるのはもったいないことです。 適宜設定を行うことで、自分に合ったlinterを作っていくことができます。

.perlcriticrc というファイルにperlcriticの設定を行っていきます。以下に .perlcriticrc の例とその解説を示します。

.perlcriticrc 例

severity = 3
program-extensions = .pl .t

[-Subroutines::ProhibitExplicitReturnUndef]

[-ValuesAndExpressions::ProhibitConstantPragma]

[-ValuesAndExpressions::ProhibitImplicitNewlines]

[-RegularExpressions::RequireExtendedFormatting]

[TestingAndDebugging::RequireUseStrict]
equivalent_modules = Test2::V0

[TestingAndDebugging::RequireUseWarnings]
equivalent_modules = Test2::V0

[Subroutines::ProhibitUnusedPrivateSubroutines]
allow_name_regex = \A_build_.+

設定項目

severity

perlcriticに指摘してもらう最小のseverity (重大度) を指定します。1～5の整数を指定します。

個人的には、デフォルトで入ってるポリシーのうち、severity 3以上については大まかに従うのがよいと思っています。逆にseverity 2以下はけっこう重箱の隅であるとか、好みが分かれるポリシーが多いと思います。

program-extensions

モジュールではなくPerlスクリプトとして扱う拡張子を指定します。モジュールとして扱うと、 package 宣言がファイル名と合致しているか、末尾に 1; があるかを検査されることになります。

.pl は言わずもがなです。 .t はテストスクリプトなのでモジュールとして扱うと不都合があるでしょう。

ポリシーごとの設定

ポリシーを無効化したり、ポリシーに対して設定を行うことができます。

Subroutines::ProhibitExplicitReturnUndef (severity = 5)

return undef; と書かずに return; とだけ書きましょう、というポリシーです。無効化しています。

失敗状態のときに return undef; と書くとリストコンテキストで (undef) というリストが返ってしまうのでよくない、と書いてありますが、むしろリストコンテキストで関数を呼んだときにリストの要素がずれてしまう弊害のほうがあるんじゃないかと思います*4。あと Maybe[T] という気持ちで return undef; って書くことはあると思っていて、そっちに引っかかることのほうが多い気がします。

ValuesAndExpressions::ProhibitConstantPragma (severity = 4)

constant プラグマではなく Readonly モジュールなどを使いましょう、というポリシーです。無効化しています。

constant プラグマにはコンパイルフェーズでの最適化が効く、という利点があるので、使うのを禁止することはないのではないでしょうか。fat commaの左辺に書くときなど、確かに罠があるというのは否定できません。

あと Readonly はperlcritic関連のモジュールでしか見たことなかったです。reverse dependendyを見ると他のディストリビューションでも使われているようですが……。

ValuesAndExpressions::ProhibitImplicitNewlines (severity = 3)

文字列リテラル内で改行するのではなく \n やヒアドキュメントを使いましょう、というポリシーです。無効化しています。

"" 内で改行するのは確かにおやっとなるけど、SQLを書くときなど、複数行の文字列を q{} などで囲って書くことはあると思います(GoやJavaScriptなどでも ` で複数行の文字列リテラルを書くことはありますよね？)。そういうのも一括で弾かれるので無効にしています。

RegularExpressions::RequireExtendedFormatting (severity = 3)

正規表現に /x 修飾子を付けましょう、というポリシーです。無効化しています。

常に付けることはないんじゃないかと思います。

TestingAndDebugging::RequireUseStrict, TestingAndDebugging::RequireUseWarnings (severity = 3)

use strict と use warnings を書きましょう、というポリシーです。

このポリシー自体に異論の余地はないと思いますが、世の中にはuseするとimportした側でも use strict; use warnings; 相当の効果が得られるモジュールがあります*5。あるいは t::test みたいなテスト用の便利モジュールを自作してuseすることで実現していることもあると思います。そういった場合は equivalent_modules にそのようなモジュールを指定するとよいでしょう。

Subroutines::ProhibitUnusedPrivateSubroutines (severity = 3)

同一モジュール内から使われていないプライベートメソッドを定義しないようにしましょう、というモジュールです。

Class::Accessor::Lite::Lazyでは _build_ から始まるメソッドで、指定されたフィールドのアクセサの遅延ビルダを書くことができます。似たような機能を使っている場合は allow_name_regex に許容するメソッド名のパターンを指定するべきでしょう。

また、Moose::Roleなどをuseしているときは呼び出してなくてもよいことにする、というのを skip_when_using にモジュール名を列挙することで実現できます。

設定を行うコツ

先ほど「perlcriticを入れてみたけど、既存のコードに警告が出まくってノイズになる」と言いました。既存のプロジェクトに対してperlcriticを試すと、一貫した書き方だけどperlcriticのデフォルトの設定に合っていない場合に警告がずらずら出てくることになります。また、既存のコーディング規約や慣習に明らかに違反するようなポリシーも有効化されているかもしれません。

まずは最初にプロジェクトのPerlコード全体にperlcriticを severity 1 でかけて、出力を記録してみるのがよいでしょう。このとき、以下のようにしてperlcriticを実行すると便利です。

perlcritic --top=1000 -1 --verbose "%f:%l:%c:%m (%P, Severity: %s)\n" lib

ポリシー違反の箇所と、コーディング規約を見比べて、指摘されるべきでない箇所をリストアップしてみましょう。設定を変えることで指摘されないようにできる箇所や、あるいはポリシー自体が合わない箇所もあると思うので、そういった箇所に対して設定を行っていきます。

severityをどうするかですが、先述したように、低いseverityのポリシーには意見が分かれるものも多いと思います。まずはseverity 3あたりで導入して、ちょっとずつseverityの設定を低くしつつポリシーの設定も変える・無効化していく、というのがよいのではないでしょうか。

やや骨の折れる作業ですが、デフォルトの設定で不満を抱えたままperlcriticに従うよりも精神的に良いと思います。集団開発プロジェクトに組み込むときも、ノイズだらけの状態だと導入に合意を得られづらいでしょう。あるいは手前味噌ですが、先述した .perlcriticrc の例をベースに改造していくのはどうでしょうか。

部分的に警告を抑制する

行末に ## no critic と書くことで、perlcriticに当該行への警告を出さないように指示することができます。ESLintでいう // eslint-disable-line みたいなものです。

また、 ## no critic とだけ書いた行を書くことで、perlcriticに以降の行への警告を出さないように指示することができます。 /* eslint-disable */ みたいなものです。 警告を再開するには ## use critic と書けばよいです。これも /* eslint-enable */ みたいなものです。

注意点として、デフォルトでは ## no critic で無効化するポリシーを指定しないと Miscellanea::ProhibitUnrestrictedNoCritic ポリシー違反になります。ポリシー名を調べて ## no critic (TestingAndDebugging::ProhibitNoStrict) のように書くことで警告を回避できます。

サードパーティー製のポリシーを追加する

perlcriticには、組み込みのポリシー以外にも、サードパーティー製のポリシーを有効化してコードを検査する仕組みがあります。ESLintのruleをインストールするみたいな感じです。

普段通り、cpanmなどでインストールしたポリシーが有効になります。以下に、ざっと見た中でおすすめできそうなサードパーティー製のポリシーを列挙します。ここに列挙したポリシーの全てを実際に動かしたわけではありませんが、PODとコードはひと通り読んで挙動を把握しているつもりです。

また、Perl::Critic::Policy - 私が歌川ですScrapbox支店 というページに、perlcriticのポリシー一覧をまとめています。参考にどうぞ。

Perlsecret

Perlの不思議な見た目の演算子を使わないようにしましょう、というポリシーです。演算子についてはperlsecretや Perlの食えない事情 - 演算子編 - アリ を読んでください。

Bang Bang !! などは使うことがあると思うので、適宜 allow_secrets に列挙するとよさそうです。

Bangs::ProhibitDebuggingModules

Data::Dumper などのデバッグ用のモジュールをuseしないようにしましょう、というモジュールです。printデバッグしていたときのコードをうっかりcommitしてしまったときに気づくことができます。

TryTiny::RequireUse

Try::Tinyを使ってtry-catchを書くときは use Try::Tiny しましょう、というポリシーです。間接オブジェクト記法の怪 - 時計を壊せ を読むと理由が分かると思います。

TryTiny::ProhibitExitingSubroutine

Try::Tinyのtry-catch内からreturnしないようにしましょう、というポリシーです。Try::TinyのPODや Syntax::Keyword::TryとPerlのキーワードプラグイン (その1) - Masteries を読むと理由が分かります。

ValuesAndExpressions::PreventSQLInjection

SQLインジェクションの原因になりうる、SQL内でのPerlの変数展開をやめましょう、というポリシーです。SQLっぽい文字列を正規表現で判定しています*6。

Variables::ProhibitLoopOnHash

ハッシュに対してforループを回さないようにしましょう、 keys %h や values %h などに対して回しましょう、というポリシーです。

Pythonだと、辞書に対するforループ for x in dic: ... を回すと、 x には辞書のキーが入ります。Perlの場合はハッシュをリストコンテキストで展開することになるので、キーと値が交互にiterateされることになります。

Variables::ProhibitUnusedVarsStricter

perlcriticの組み込みポリシーに Variables::ProhibitUnusedVariables という、変数宣言のみで使ってない変数を消しましょう、というポリシーがありますが、それをより厳格にしたものです。

Variables::ProhibitUnusedVariables では、変数宣言と同時に値を初期化して、その後変数を参照しなくても警告されることはありませんが、Variables::ProhibitUnusedVarsStricter では警告されます。Scope::Guardなどを使ったguardの場合は allow_if_computed_by で例外に指定できます。

このポリシーを有効にするさいは Variables::ProhibitUnusedVariables ポリシーを無効化するのがよいでしょう。

VSCodeと連携する

2020/12/2 時点で、perlcriticをVSCodeと連携させる拡張機能は3つ出ています*7。

zhiyuan-lin.simple-perl

perltidyによるコード整形とperlcriticによるlintingに対応したVSCode拡張です。

他2つの拡張機能に比べると設定項目が少ないですが、そのぶん .perlcriticrc に書いてない設定は行われないので、振る舞いの予測がしやすいです。現状では一番おすすめできる拡張機能です。

d9705996.perl-toolbox

Perlプログラムの文法チェックと、perlcriticによるlintingに対応したVSCode拡張です。severityごとにVSCode上で警告として表示するかエラーにするか、など柔軟に設定することができます。

ただし、lint時に生成される一時ファイルのパスが実際のディレクトリ構造を反映しない・拡張子が保存されないという問題があります。これによって、Modules::RequireFilenameMatchesPackageポリシーがうまく動かなかったり、 program-extensions の設定が無視されたりすることになります。これも修正PRを出しているのですが、まだ反応をもらえていません。

上述した問題が無視できるのであれば、3つの中で一番おすすめできるVSCode拡張になると思います。

sfodje.perlcritic

そのものずばり、perlcriticという名前のVSCode拡張です。

perlcriticに --top オプションを渡したとき、severityが指定されてなかったら1になる - 私が歌川です でも言及しましたが、拡張機能側で --top オプションを渡している & --top オプションを渡すとseverityが1になる、の合わせ技で、severityを指定しつつコマンドライン引数にも渡す、という形式を取らざるを得ません。 また、インストールすると (Java|Type)Script という語の補完しかできなくなる、という問題があります。PRは出してるけど2週間ぐらい経ってまだ反応をもらえていない……。 Language Serverとして動いているためか、環境によっては重いかもしれません*8。 perlcritic.onSave オプションを有効にしたら保存したときだけperlcriticを走らせるようになって多少マシになります。

CIに組み込む

CIに組み込むことで、ポリシー違反のコードが残るのを防ぐことができます。

テストを書く

Test::Perl::Critic を使うことで、指定されたPerlプログラムがperlcriticの検査に通らなければテストを落とす、ということができます。使い方はPODや Test::Perl::Critic で PBP 準拠なコードを自動テスト: blog.bulknews.net を参照してください。

大規模プロジェクトになると、CIで全ファイルに対してperlcriticを回すとあまりに時間がかかるということもあると思います。CI で変更のあったファイルに対してのみ Perl::Critic を実行する | monolithic kernel では、差分のあるファイルに対してのにperlcriticを実行する方法が解説されています。

GitHub Actionsを使う

shogo82148/actions-setup-perl アクションを使っている場合、Problem Matcherを使ってかんたんにポリシー違反の箇所にアノテーションを付けることができます*9。 /^(.+)\sat\s(.+)\sline\s(\d+)\.$/ という正規表現にマッチするためにperlcriticに --verbose '[%p] %m (Severity: %s) at %f line %l.\n' という引数を渡して、出力のフォーマットがうまくProblem Matcherに引っかかるようにしています。perlcriticがポリシー違反を検出したときはexit status 2で終了するので、コーディング規約に違反していたらCIが通らない、というのも実現できます。

ところで、単にperlcriticをCI内で実行するだけだと、既存のコードでポリシー違反を修正できていない箇所も警告されてCIが落ちることになります。直ちに全ての箇所を修正できるならそれでもよいかもしれませんが一般にそうできるとは限らないでしょう。そういう場合は、ポリシー違反のうちPRの差分に関係ある箇所だけを報告するようなラッパーを書くか、後述するreviewdogを使うのがよいかと思います。

reviewdogを使う

reviewdogは、汎用的な自動コードレビューツールです。ESLintなどの出力をもとにbotがPRにレビューをしたり、CIを落としたりすることでコーディング規約を強制することができます。errorformatという形式に従った入力を与えさえすればなんでもreviewdogにレビューしてもらうことができます。

perlcriticとreviewdogの統合事例については reviewdog x perlcritic x Jenkins で最高の GitHub レビューライフ - Mirrativ tech blog が詳しいので、そちらをご覧ください。

reviewdogをGitHub Actionsで使う用のactionも用意されています*10。このactionと先述した記事の内容を組み合わせることで、かんたんにGitHub Actionsとreviewdogとperlcriticを組み合わせることができます。以下のPRぐらいの差分で、reviewdogにポリシー違反の箇所をレビューで指摘してもらえるようになります。

github.com

おわりに

ポリシーを自作する話も書こうと思ったのですが、Perl::Critic の Policy を作ってチーム独自のコーディング規約チェックをする - Mobile Factory Tech Blog が日本語で詳しく解説しているので、この記事では割愛します。書こうと思っていたことが全部書かれていて安心しました。

明日は @mp0liiu さんで「何か書きます。」です。

*1:ただし --fix はできない

*2:Perlベストプラクティスのベストプラクティスじゃないやつをまとめてみた | おそらくはそれさえも平凡な日々

*3:私もそういう経験があります

*4:リストコンテキストでハッシュの要素をずらしたことがない人だけが石を投げてください

*5:Test2::V0とか

*6:https://metacpan.org/source/AUBERTG/Perl-Critic-Policy-ValuesAndExpressions-PreventSQLInjection-v1.4.0/lib/Perl/Critic/Policy/ValuesAndExpressions/PreventSQLInjection.pm#L573-581

*7:https://marketplace.visualstudio.com/search?term=perlcritic&target=VSCode&category=All%20categories&sortBy=Relevance

*8:手元のちょっと世代が前のMacだと有意にVSCodeが重くなった

*9:実行例

*10:https://github.com/reviewdog/action-setup