株式会社モバイルファクトリーの技術ブログ

全228件

2020/01/17

Perl::Critic の Policy を作ってチーム独自のコーディング規約チェックをする

概要こんにちは、エンジニアの id:mp0liiu です。自分は Perl でチーム開発をしているのですが、最近ある外部モジュールの使い方に関するチーム独自のコーディング規約が追加されました。このコーディング規約に沿ってコードが書けているかどうかは人間の目でチェックする必要があるので、手間がかかりますし、開発時やコードレビュー時に見落としてしまう可能性があります。そこで Perl コードの構文チェックをするツール Perl::Critic の Policy(ルールのような概念)を作り、チーム独自のコーディング規約を機械的にチェックできるようにしました。この記事ではその際したことをまとめ、チーム独自のコーディング規約のチェックのために Perl::Critic の Policy が作れるよう、 Perl::Critic の Policy の作り方を解説します。今回つくった Policy の完全版はこちらのレポジトリにあります。経緯自分が担当しているプロダクトでは Data::Validator を利用して引数の型チェックを行っています。 Data::Validator では通常 validate メソッドで渡した値を名前付き引数としてチェックします。 use Data::Validator; sub add { # 引数の型の指定 state $v = Data::Validator->new( x => 'Int' , y => 'Int' , ); # 引数の型をチェック my $args = $v->validate ( @_ ); return $args->{ x } + $args->{ y } ; } add( x => 2 , y => 3 ); しかし、引数が1つなのに名前付き引数を渡すのは大抵の場合冗長です。なので最近チームで引数が1つの場合は Data::Validator の拡張機能 StrictSequenced を使って普通の引数として渡すようにする、というコーディング規約が追加されました。 use Data::Validator; sub named { state $v = Data::Validator->new( num => 'Int' ); my $args = $v->validate ( @_ ); return $args->{ num } ; } # 通常はこのように引数が1つでも名前付き引数として渡す必要がある named( num => 1 ); sub sequenced { state $v = Data::Validator ->new( num => 'Int' ) # Data::Validator の拡張機能 StrictSequenced を使う ->with( 'StrictSequenced' ); my $args = $v->validate ( @_ ); return $args->{ num } ; } # StrictSequenced を使っている場合は引数だけを渡す sequenced( 1 ); この規約が導入されると、コードレビューなどの際にコードの書き方をチェックする必要がでてくるわけですが、コードの書き方のチェックを人間がやるのは大変です。なので構文チェックツールなどで機械的に書き方のチェックをしたくなり、 Perl::Critic の Policy を作ることにしました。前提知識 Perl::Critic の Policy を実装するにあたって、次のドキュメントを読みました。 PPI のドキュメント Perl::Critic::Util のドキュメント Perl::Critic::Policy のドキュメント Perl::Critic では PPI で静的解析をして構文チェックを行っているため、自分で Policy を実装する場合も PPI のドキュメント構造を走査するようなコードを書くことになります。なので、PPIの使い方や PDOM(Perl Document Object Model)について理解しているとスムーズに実装が行えるでしょう。 Perl::Critic::Util では Policy を実装するにあたって役に立つような PDOM をパースする関数が提供されています。簡潔に処理を書けたり、自前で実装すると大変な処理が利用できることもありますので、どんな関数があるか目を通しておくといいと思います。 Perl::Critic::Policy は、全ての Policy の親クラスとなるクラスで、 Policy の実装やAPIについて記述されています。これらのドキュメントに目を通した上で、既に実装されている Policy のコードを読み、実装の参考にしました。今回、自分は Perl::Critic::Policy::TryTiny::RequireUse と Perl::Critic::Policy::Subroutines::ProhibitManyArgs を参考にしました。コードと解説以上のようなドキュメントに目を通した上で Policy を実装したので、どのようなコードを書いたのかを解説します。 package Perl::Critic::Policy::DataValidator::RequireStrictSequenced ; use strict ; use warnings ; use utf8 ; our $VERSION = '0.01' ; use List::Util qw( any ) ; use Perl::Critic::Utils qw( $SEVERITY_LOWEST parse_arg_list is_class_name is_method_call ) ; # (1) Perl::Critic::Policy を継承して実装 use parent 'Perl::Critic::Policy' ; # (2) サポートする Policy の設定値 sub supported_parameters { return () } # (3) デフォルトのチェックの厳しさ sub default_severity { return $SEVERITY_LOWEST } # (4) デフォルトのtheme sub default_themes { return qw( cosmetic ) } # (5) この Policy が対象とする PPIのクラス名 sub applies_to { return 'PPI::Statement::Variable' } # (6) Policy の構文チェック処理本体 sub violates { my ( $self , $stmt , $doc ) = @_ ; # (7) 'Data::Validator->' とマッチするか my $data_validator = $stmt->find_first ( sub { my ( undef , $child ) = @_ ; return is_class_name( $child ) && $child->content eq 'Data::Validator' ; }); return unless !! $data_validator ; # (8) '->new' とマッチするか my $guess_new = $data_validator->snext_sibling->snext_sibling ; return unless is_method_call( $guess_new ) && $guess_new->content eq 'new' ; # (9) Data::Validator->new には名前付き引数としてハッシュでパラメータをが渡されているはず my @args_passed_to_new = parse_arg_list( $guess_new ); return if @args_passed_to_new % 2 == 1 ; # (10) 引数がない場合はチェックしない my $params_num = int @args_passed_to_new / 2 ; return if $params_num == 0 ; # (11) new の後で '->' とマッチするか my $guess_method_call_operator = $guess_new->snext_sibling->snext_sibling ; return unless defined $guess_method_call_operator ; # (12) '->with' とマッチするか my $guess_with = $guess_method_call_operator->snext_sibling ; return unless is_method_call( $guess_with ) && $guess_with->content eq 'with' ; # (13) with で渡されている引数を見て StrictSequenced が使われているか調べる my @args_passed_to_with = parse_arg_list( $guess_with ); my @extensions_name = map { $_->literal } map { @$_ } @args_passed_to_with ; my $has_strict_sequenced = any { $_ eq 'StrictSequenced' } @extensions_name ; # (14) 引数が1つでかつ StrictSequenced が使われていなければ Policy 違反を報告 if ( $params_num == 1 && ! $has_strict_sequenced ) { return $self->violation ( q{You should use 'StrictSequenced', an extensions of Data::Validator.} , q{Passing named arguments when a subroutine has only one argument is redundant.} , $stmt ); } return ; } 1 ; Policy を実装するには、まず (1) のように Perl::Critic::Policy を継承します。 (2) の supported_parameters メソッドは、 Policy でサポートする設定値のリストを返すメソッドです。今回は特に設定値使うことを考えていなかったので空のリストを返しています。 (3) の default_severity メソッドは、 Policy デフォルトの厳しさ 1 を返すメソッドです。 (4) の default_themes メソッドは Policy がデフォルトで属する Perl::Critic のテーマ 2 のリストを返すメソッドです。適切な設定かどうかの自信はあまりないのですが、今回はコードの健全さに影響するテーマとして maintenance を指定しています。(独自の theme を設定することもできるのかもしれませんが、未検証です。) (5) の applies_to メソッドはこの Policy が対象とするPPIのクラス名を返すメソッドです。 Policy が構文チェックをするときは、構文チェック中のコードからこのメソッドが返したクラスの PDOM を全部取得して、取得したPDOMごとに (6) のチェック処理本体を呼び出すという挙動をします。何もオーバーライドしない場合のデフォルトは PPI::Element なので、すべてのPDOMを対象に Policy のチェック処理が走ることになりますが、より詳細な対象クラスを指定するとチェック対象のPDOMが絞り込まれるのでパフォーマンスが向上します。今回は Data::Validator で引数の型チェックをする場合一度変数に Data::Validator のインスタンスを格納することが多いかと思い、変数宣言文のクラスである PPI::Statement::Variable を指定しています。 (6) の violates メソッドに Policy のチェック処理本体を記述します。第1引数には applies_to で指定されたチェック対象のPPIクラス(今回は変数宣言文のクラスである PPI::Statement::Variable )のPDOMが、第2引数にはチェック中のコード全体のPPIドキュメントが渡されます。 (7) では変数宣言文のPDOM $stmt の子要素に Data::Validator-> という文字列にマッチするトークンがあるかどうかを、 find_first という引数に渡したコールバックが最初に真を返した要素を取得するメソッドで調べています。子トークンが Data::Validator-> とマッチしているかは、クラス名であるかどうかを判定する is_class_name 関数と子トークンの中身が Data::Validator であるかを調べて判定しています。 (8) では Data::Validator とマッチした部分のトークンの隣でコンストラクタが呼ばれているかどうかを調べています。まずコンストラクタとしてのメソッド名 new があると期待される Data::Validator の次の次のトークンを、 snext_sibling という動作に影響しない空白のようなトークンを除いた、次のトークンを参照するメソッドで取得しています。そしてそれがメソッド呼び出しかどうかを is_method_call 関数で判定し、トークンの中身が new であるかを調ることで、 Data::Validator からコンストラクタが呼ばれるコードであるかを調べています。 (9)、(10) では、 parse_arg_list 関数でコンストラクタにどのような引数が渡されたかを解析し、 Data::Validator のコンストラクタに渡されている引数の数、すなわちそのメソッドの仮引数の数を調べています。 (11)、(12) では Data::Validator の拡張機能を使う with メソッドが呼ばれているかを調べ、 (13) で with メソッドに渡されている引数を解析し、 StrictSequenced が使われているかを調べています。そして (14) でいよいよ仮引数の数が1つの場合 StrictSequenced が使われているかどうかを調べ、使っていなければ violation メソッドで Perl::Critic::Violation のインスタンスを作って Policy の違反を報告しています。 Policy のテストテストがあった方が開発もメンテナンスもしやすいので、 Policy のテストを書きながら開発を進めたいところです。 Perl::Critic のインスタンス生成時、引数 --single-policy にテストしたい Policy を指定しその Policy だけで構文チェックをさせるようにした上で、構文チェックするメソッド critcue にスカラリファレンスで文字列を渡しその文字列をコードとして扱わせ構文チェックすると、テストがしやすいのでおすすめです。 use strict ; use warnings ; use utf8 ; use Test2::V0; use Perl::Critic; my $critic = Perl::Critic->new( '-single-policy' => 'DataValidator::RequireStrictSequenced' , ); my @violations = $critic->critique (\ q{ sub one_args { my $v = Data::Validator->new( "num" => 'Int', )->with(qw/ Method StrictSequenced /); } } ); is @violations , 0 ; done_testing; 実装した Policy を使う実装した Policy のモジュールが Perl::Critic::Policy 名前空間に属していて、かつ Perl::Critic を動かしている perl から実装した Policy がロードできる状態になっていれば、 Perl::Critic は Policy のモジュールを自動的にロードしてくれます。なので、後は設定ファイル(通常は .perlcriticrc )で次のように Policy名(Policyのパッケージ名から Perl::Critic::Policy:: を除いたもの ) を追加する設定を記述するなどすれば実装したPolicyが有効になります。 include = DataValidator::RequireStrictSequenced 設定を追加したらCIを回す時にコーディング規約にそったコードになっているかテストしたり、エディタでチェックしたりと活用することができるようになります。おわりにこのように、 PPI や Perl::Critic についての知識があれば、チーム独自のコーディング規約をチェックする Policy を実装できます。 Perlのプロジェクトで人間の目でチーム独自のコーディング規約をチェックしていて、チェックに要する手間や見落としが問題になっているのであれば、 Perl::Critic の Policy を実装して機械的にチェックさせてみてはいかがでしょうか。どれくらい重大な違反なのかを示すパラメータです。詳細はこちら。 ↩ カテゴリのような概念です。詳細はこちら。 ↩

株式会社モバイルファクトリー の技術ブログ

コンテンツ

株式会社モバイルファクトリーの技術ブログ