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

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

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

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

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

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

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

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

1
2
3
4
5
6
7
8
9
10
11
12
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 にすべき” とメッセージが出ている点です。

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

1
2
3
4
5
export class Action {
public static ng(message: string): void {
console.debug(message);
}
}

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

1
2
3
4
5
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-methodclass-methods-use-this にあたるとのこと。

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

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

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

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
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 にしています。

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
"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 にした ngStaticAction.ngStatic に追加されています。

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

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

現在の状況

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

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

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

参考情報


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

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