チームで培われたベストプラクティスをlintとして周知する

こんにちは。AI・機械学習チームの氏家（@mowmow1259）です。エムスリー福岡オフィスの一人目のエンジニアとして福岡で働いています。マクドナルドの月見バーガーが好きで、今年も発売開始当日に食べに行きました。

私が所属するAI・機械学習チームでは基本的に2週間から1ヶ月程度で新規プロダクトをリリースするなど、高速にプロダクトを開発しています。その過程で、「この書き方は落とし穴があるから使わない方がいい」といった開発に際したベストプラクティスが溜まっていきます。そういったベストプラクティスはレビューでの指摘や技術共有会*1でチームに浸透してきますが、レビュー負荷や新メンバーへの周知などに課題がありました。

この記事では、それを解決するためにベストプラクティスをLinterの独自ruleとして規定し、CIで自動検知することでチーム全体に周知する取り組みについて紹介します。

ベストプラクティスの蓄積
ベストプラクティスの浸透の取り組み
ベストプラクティスの浸透の難しさ
Linterの独自ルールとして自動チェックする
各プロダクトへの導入
導入の効果
まとめ
We are hiring !!

ベストプラクティスの蓄積

AI・機械学習チームでは「打席にたくさん立ちホームランを打ち続ける」というチームバリューを掲げており、数多くのプロダクトが日々生み出されています。ほとんどのプロダクトはチームメンバー1人が主担当として開発、運用までを担当しており（もちろん他メンバーの助力もたくさんもらえます）、これがスピード感の源になっています。

この高速なプロダクト開発のサイクルをチームとして進めていくにつれ、「この書き方は落とし穴が多いので使わないほうがいい」といった開発・運用上のベストプラクティスが徐々に溜まっていきます。

例えば、AI・機械学習チームではgokartという機械学習パイプラインライブラリをよく使うのですが、その中でもバグを生みやすい機能は使用を避けたりしています。こういった知見は、開発・運用上の障害を未然に防ぐものでもあるため、できるだけチームに浸透させたいですよね。

ベストプラクティスの浸透の取り組み

ただ、前述の通りAI・機械学習チームでは各プロダクトを基本的に1人で開発していきます。そのため、あるプロダクトで発見された知見はそのプロダクトの担当者、またそのレビュアに閉じてしまいがちです。

これをチーム全体に還元、浸透させるため、AI・機械学習チームでは技術共有会という会を週に一度開催しています。これは、技術的に面白い話題や開発上生じた知見、ポストモーテムなどをチームメンバーに紹介する会です。ベストプラクティスはこの会で適宜共有され、チームメンバー全員に周知されます。

少し本筋と外れますが、この技術共有会はベストプラクティスの他にも直接業務に関係のないギークな話題も多々あり、例えばdocker imageの軽量化から最新の機械学習周りの話題、はたまたエンジニアが知っておくべき会計/税務の話など、本当に色々な話題が毎週話されています。忙しい中でも技術への好奇心を忘れないチーム文化をよく表していて、とても好きな取り組みの1つです。

ベストプラクティスの浸透の難しさ

ベストプラクティスは技術共有会で共有されるものの、その浸透には依然として課題もあります。基本的に人間は忘れる生き物なため、一度共有された知見を開発時に忘れず徹底することは思った以上に難しい点です。技術共有会時のスライド等はストックされているものの、頻繁に見返すものではないですし、手なりで書くコードに対して都度意識を払うことは難しいものです。第一、人間の注意力に頼った規約はエンジニア的ではありませんよね。

また、新メンバーへの知見の周知も課題です。当然ながら新メンバーは過去の技術共有会に参加していないわけなので、知見の存在も知りません。多くの場合レビューで指摘されますが、レビューが漏れることもありますし、そもそも個々人のレビューに頼る文化も好ましくありません。

Linterの独自ルールとして自動チェックする

周知・徹底を人間に頼っているのがダメなのであって、じゃあLintみたいに自動でチェックすればいいじゃない、ということで、現在はチーム内のベストプラクティスに従っているかを検知するLinterの独自ルールを社内向けに作成し、CIに組み込んでいます。こうすることで、ベストプラクティスに従わないコードはレビュー前の時点でLintが落ち、人間を介さずベストプラクティスが徹底されます。また、Lintのエラーメッセージもできるだけ丁寧にしており、新メンバーがそれを見るだけで知見をインストールできるようになっています。

ここからは、Pythonで実際にLinterの独自ルールを作成した事例を紹介していきます。

実装するベストプラクティス

ここでは、前述のgokartに対して導入したruleを例として取り上げます。

gokartはParameterというクラス変数を定義することでワークフロー中の設定を設定ファイル等から読み込むことができます。このParameterの一つとしてListParameterというクラスがあるのですが、ListParameterはlistを受け付ける一方で、内部的にはtupleを扱っており、その値はtupleとして返却されます。 ListParameterという命名からlistが返却されると勘違いし、しばしばlistとtupleの違いによるバグを生んでいました*2。

実装したルールによりlintされている様子。ListParameterを使用して怒られている。

ここからは、上図のようにListParameterを使用した場合に、混乱の少ないTupleParameterに書き換えてもらうruleを作成していきます。

Linterの選定

チームではruffをPythonのLinterとして採用していますが、残念ながらruffには独自ルールを記述したり、自作pluginを読み込ませたりする機能はリリースされていません*3。そのため、独自ルールをかけるLinterを新たに選定する必要があります。いくつか候補がありますが、今回はstar数が多く機能もミニマルなPylintを採用しました*4。

github.com

独自ルールの記述

PylintにはCustom Pylint Checkerとして独自のlintルールをpluginとして追加できる機能があります。 Pylintの実行時に走査されるASTについて、各ノード訪問時のcallbackとしてlintルールを実装できます。

公式ドキュメントが丁寧なため、それを参照すれば実装はそこまで難しくありません。実際の実装例を以下に紹介します。

import astroid
from pylint.checkers import BaseChecker
from pylint.lint import PyLinter


class ListParameterChecker(BaseChecker):
    """ListParameterを使うとtupleが返るので、TupleParameterを使うように警告する。"""

    # このruleのメタデータ。名前、lint時のメッセージを定義する。
    name = 'list-parameter-use'
    msgs = {
        'W0001': (
            'ListParameter should not be used. Please use TupleParameter instead.',
            'list-parameter-use',
            'ListParameter is confusing because the return value is tuple. Please use TupleParameter instead.',
        ),
    }

    def visit_assign(self, node: 'astroid.Assign') -> None:
        """変数への代入nodeのcallback関数。

        以下コード例における、ListParameterに訪れた際の処理を行う

        class TaskA(TaskOnKart):
            a = ListParameter()
        """
        value = node.value

        # nodeがCall（呼び出し）であり、かつその呼び出しがListParameterという名前のクラスであった場合に、
        # add_messageでルール違反として検知する
        if isinstance(value, astroid.Call) and isinstance(value.func, astroid.Attribute) and value.func.attrname == 'ListParameter':
            self.add_message('list-parameter-use', node=node)

def register(linter: 'PyLinter') -> None:
    """ListParameterCheckerをPylintのruleとして登録する
    """
    linter.register_checker(ListParameterChecker(linter))

これはListParameterというクラスの代入を検知しTupleParameterの使用を推奨するルールです。

BaseCheckerを継承したクラスについて、クラス変数としてルール名name、ルールの設定msgsを設定します。また、visit_assignといったようにASTのノード名を指定したcallbackを設定し、add_messageすることでそのルールが検知されたことになります。ここでは代入式の右辺にListParameterが指定された場合に検知される簡単なルールを作っていますが、本来はListParameterが特定のモジュール（ここではluigi）のものである確認をする必要があります。visit_importなどでimportしたモジュールを状態として持つことで実現できますが、ここでは簡単のため省きます。

また、次のようにテストもかけるため保守性も悪くなさそうです。

    def test_list_parameters_with_assign(self) -> None:
        # @をコメントとして付記することで、その行のnodeを抽出する
        node = astroid.extract_node(
        """
        import gokart
        import luigi

        class TaskA(gokart.TaskOnKart):
            a = luigi.ListParameter() #@
        """
        )
        with self.assertAddsMessages(
            MessageTest(
                msg_id='list-parameter-use',
                node=node,
                confidence=UNDEFINED,
                line=6,
                col_offset=4,
                end_line=6,
                end_col_offset=29,
            )
        ):
            self.checker.visit_assign(node)

あとは作成したcheckerをlinter.register_checkerで登録し、packageにした上でPylintのpluginとして読み込めば使えるようになります。 pluginはpyproject.tomlでは次のように指定できます。基本的なLinterは前述の通りruffを使用したいため、独自ルール以外は無効化しています。

[tool.pylint.MASTER]
load-plugins = ["pylint_gokart"]
disable = "all"
enable = ["list-parameter-use"]

各プロダクトへの導入

AI・機械学習チームではcookiecutterでプロジェクトを初期化、cruftで最新のテンプレートに追従する運用をしています。その仕組みを使い、プロジェクトテンプレートに今回のLinterを導入し、cruftにより各プロダクトに取り込んでいきました。

この辺の運用については詳しくは次の記事で紹介していますので、ぜひそちらもご覧ください！

www.m3tech.blog

導入の効果

導入後しばらく経過していますが、CIでベストプラクティスに沿わないコードが早速検知されています。また、Linterとして修正が半ば強制されることで、古いプロダクトでもベストプラクティスに合わないものが修正されていく効果もあります。レビュー負荷軽減はもちろんのこと、コードリーディングの認知負荷軽減にもつながりますし、踏みがちなバグを未然に防ぐこともできるため、今の所導入して良かったと感じています。

まとめ

AI・機械学習チームのベストプラクティス周知に関する取り組みを紹介しました。レビューやドキュメントに頼らずLinterとして運用していくことで、人間に頼らない周知の仕組みが構築できました。今回はPythonについて紹介しましたが、他言語でも独自ルールを導入できる各種ツールがありますので、みなさんも試してみてはいかがでしょうか。

We are hiring !!

エムスリーではプロダクトを高速に開発していくため、生産性向上のための改善も積極的に行なっています！そんなチームの雰囲気に少しでも興味がある方は、次のURLからカジュアル面談をご応募ください！

*1:AI・機械学習チームで週に一度行われている、面白い、共有したい技術の共有会です

*2:例えばpandasのカラムの指定では、listで指定した場合は複数カラムの指定になりますが、tupleで指定した場合はtupleを値とした1つのカラムを指定することになります。

*3:議論自体は活発にされているようです。https://github.com/astral-sh/ruff/issues/283 参照

*4:他にはflake8、sylver-cliなどが候補に上がりました。ルールの保守性などからPylint、flake8に絞り、star数の多いPyilntに決定しました。

エムスリーテックブログ

エムスリー(m3)のエンジニア・開発メンバーによる技術ブログです