lulzneko

TSLint の prefer-function-over-method ルールについて悩む

コーディングをしていく上で良いお作法でできるようにしたり、チーム開発でスタイルを合わせたりするために Lint 系のツールを使います。今回 TypeScript で開発をしていて、TSLint のルールを厳しくしたところチェックに引っかかったところが出たのですが、ルールの意図を汲み取れず悩み中の備忘録。

環境
本記事の開発環境は以下となります。

Windows 10 64bit + WSL Ubuntu 18.04.1 LTS
Visual Studio Code
Node.js 8.10.0
Yarn 1.13.0
TypeScript 3.3.4000
TSLint 5.14.0

TSLint 概要

TSLint はTypeScriptコードの読みやすさや保守性を維持するための静的分析ツールです。
不具合を起こしやすいコードを指摘したり、コーディングスタイルをそろえるためのチェックなどをしてくれます。

TSLintはTypeScriptに特化してチェックしてくれるのですが、最近ではTypeScript 開発チームのロードマップでESLint、JavaScript/ECMAScript 用の Lint をサポートしていくことが発表され、ESLintからもTypeScriptのESLint対応への歓迎とチームメンバーが開始したtypescript-eslint プロジェクトについての記事が公開されます。

今後はESLintを使っていくことになりますが、安定するまではTSLintをも使って行くことになります。
TSLintプロジェクトでも、移行に関するイッシューRoadmap: TSLint -> ESLint · Issue #4534 · palantir/tslintが上がっており、一時的にはオーバーラップする期間が発生して、一時的には両方のツールを使うことが推奨されています。

新しいプロジェクトのtypescript-eslintを使う方法については、@typescript-eslint ことはじめ - teppeis blogさんが詳しいです。

prefer-function-over-method ルールと発生した事象

今回悩みの種となったのが、TSLintのprefer-function-over-methodルールになります。

TypeScript でクラスメソッドを作った時にthisを使わなかった場合にClass method does not use 'this'. Use a function instead.という指摘をしてくれます。

以下のコード例ではokメソッドはthis.templateでthisを使っているので問題ありません。一方でngメソッドはthisを使っていないのでprefer-function-over-methodルール違反となります。

export class Action {

  private readonly template = 'Hello %s !!';

  public ok(name: string): void {
    console.debug(this.template, name);
  }

  public ng(message: string): void {
    console.debug(message);
  }
}

まぁ、確かにngメソッドのほうは内部状態を触っていないので static なりにしたほうがよさそうです。

悩み事

ここで気になって悩んでいるのが “static にすべき” ではなく、”function にすべき” とメッセージが出ている点です。

つまり、下記のような修正ではなく (必要箇所のみ抜粋)

export class Action {
  public static ng(message: string): void {
    console.debug(message);
  }
}

このようにする (必要箇所のみ抜粋)

export class Action {
  public ng = (message: string): void => {
    console.debug(message);
  }
}

ルールの名前も prefer function over method だから、メソッドより関数を優先して使いましょうということなんですよね。。。

クラスの中に関数を書いても文法上問題はないわけですが、どうして static ではなく関数なのかなと。せっかくなので理解しておきたいのですが、prefer-function-over-methodは Rationale、論拠の項目が書かれてないのです。

たとえばonly-arrow-functionsルール「JavaScript の伝統的な関数定義ではなく、アロー関数を使いましょう」ですが、以下のように書かれています。

Rationale
Traditional functions don’t bind lexical scope, which can lead to unexpected behavior when accessing ‘this’.

thisへアクセスしたときに予期しない動作を引き起こす可能性があるから、といった説明がされています。
この文章だけだとわかりにくいかもしれませんが、これをもとに調べることもできます。

しかしながら、prefer-function-over-methodは Rationale が書かれていない以上、どのように扱うのが望ましいのかゼロから調べるしかないわけです。

ルールに従い関数に修正する
ルールの修正方針とは異なるが static にする (これでも指摘はなくなる)
ルールがプロジェクトにマッチしないので無効化する

ESLint では、どうなる？

この疑問をつぶやいたときに、お仕事チームの仲間が ESLint の情報で応えてくれました！（ありがとうございます)

typescript-eslint/ROADMAP.md at master · typescript-eslint/typescript-eslintに TSLint と ESLint の対応表があるとのことで、prefer-function-over-methodはclass-methods-use-thisにあたるとのこと。

では、ESLint のclass-methods-use-thisはどうなっているかというと「static メソッドにすること」。
メソッドと関数が混在するよりも、メソッドに統一して static と分けて実装するとのことで、わかりやすいです。

しかしながら、こちらも論拠は書かれてませんでした。
「関数を使う」という選択肢がなければ、そうだよねってことで static メソッドにするのですが、今回は関数が使えることを知ってしまったので「static と関数のどちらにすべきか」という疑問が生じてしまいました。困った。

TypeScript のコンパイル後を確認する

Lint ルールに明確な論拠がないので、出力された JavaScript/ECMAScript を見て考えます。
TypeScript のソース。

export class Action {

  private readonly template = 'Hello %s !!';

  public ok(name: string): void {
    console.debug(this.template, name);
  }

  public ng(message: string): void {
    console.debug(message);
  }

  public ngFunction = (message: string): void => {
    console.debug(message);
  }

  public static ngStatic(message: string): void {
    console.debug(message);
  }
}

コンパイルされた結果。
※ ターゲットはクラス構文が導入されていない ES5 にしています。

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var Action = (function () {
    function Action() {
        this.template = 'Hello %s !!';
        this.ngFunction = function (message) {
            console.debug(message);
        };
    }
    Action.prototype.ok = function (name) {
        console.debug(this.template, name);
    };
    Action.prototype.ng = function (message) {
        console.debug(message);
    };
    Action.ngStatic = function (message) {
        console.debug(message);
    };
    return Action;
}());
exports.Action = Action;

結果を見ると指摘のあったngメソッドはAction.prototype.ngに関数として追加されています。
TSLint のprefer-function-over-methodにしたがって関数に修正したngFunctionはコンストラクター内でthis.ngFunctionに追加されています。
ESLint のclass-methods-use-thisにしたがって static にしたngStaticはAction.ngStaticに追加されています。

こうしてみると以下の順が良さそうです。

static にする
メソッドのままにする (TSLintprefer-function-over-methodを無効化)
関数にする (TSLintprefer-function-over-methodに従う)

static にすればクラスレベルで追加されるので効率が良いでしょう。
メソッドと関数の違いについてですが、関数にすると各インスタンスに関数が追加されています。それに対してメソッドはprototypeに追加しています。とくに理由がなければprototypeに追加したほうが良い(はずだったと記憶)。
この辺の裏付けができていないのと、クラウス構文が入ってからも変わりがないのかはわからないので、決定打とはならないですが説明の方針としては１つ線が引けそうです。

現在の状況

この記事を書いている時点での対応は以下としました。

static にできる場合は、static メソッドにする
static にできない場合は、ルールを無効化してメソッドにする

今回、継承して実装するメソッドが一部指摘されていました。
簡易フレームワーク的な作りになっていて、親クラスから決められた abstract メソッドを実装するようになっており、その部分が引数だけでthisを使わないで完結できるような処理になったりもします。
この部分は// tslint:disable-next-line: prefer-function-over-methodでルールを無効化してメソッドのままとしました。

参考情報

Lint 系のツールはコードに対して指針を与えるので、やはり論拠が欲しいところですね。
とくにいくつかの選択肢が出てきてしまうと、いろいろと考えてしまいます。

今回は残念ながら TypeScript力というか、JavaScript/ECMAScript力がたりなくて明確な根拠を持って指針化できませんでしたが、考え方は整理できました。
裏付けをとるのは、ちょっと難しいかな。。

Author:lulzneko