ESLint ルール追加したいお気持ち

既存の Nuxt プロジェクトに ESLint のルールを追加した時に調べたことを整理しました。

Lint の設定ファイルを秘伝のタレにはしたくないけれども、もう少しだけカスタマイズしたい、特にコードの複雑さを軽減しメンテナンス性を保ちたい、というモチベーションでルールの追加を検討した時の話です。

今のルール

npx create-nuxt-app <project-name> でプロジェクトを作成して ESLint と Prettier を選択すると以下のような .eslintrc.js が生成されます (Nuxt v2.11.0)。

自分たちのプロジェクトも概ねこのような感じです。

module.exports = {
  // ...
  extends: [
    '@nuxtjs',
    'prettier',
    'prettier/vue',
    'plugin:prettier/recommended',
    'plugin:nuxt/recommended'
  ],
  // ...
}

extends している @nuxtjs、実体は @nuxtjs/eslint-config ですが、これがさらに eslint-config-standard を extends しています。

eslint-config-standardeslint:recommendedeslint-config-airbnb-base みたいなあれです。

参考:

追加検討したルール

complexity ってのを見つけたのでこれでおkなのか?と心踊りましたがどうやらそんなに簡単ではないようです。

Why complexity is off? · Issue #1758 · airbnb/javascript · GitHub

For me, more better metric and more quality code you will get if you combine:

max-params, max-statements, max-statements-per-line, max-nested-callbacks and max-depth.

Don't know how complexity is actually implemented, but above rules are more fine grained and leads you to great code.

ESLint の List of available rules から探してみると、complexity の代わりに以下の 8 つが利用できそうです。

その中から max-lenmax-statements-per-line は今回の対象から外しました。この辺りは Prettier に任せます。

ついでに max-lines も外すことにしました。Vue を単一ファイルコンポーネントで作成すると テンプレートや CSS が含まれるためどうしても一般的な JS ファイルより行数が増えてしまうからです。
(.vue ファイルを Lint の対象から外してもいいんですが、まあいいかなあという気持ち。)

残ったのが以下 5 ルールです。

次に、試しにこれらのルールを手元のコードに適用させてみました。 (max-params だけ設定をデフォルト値から変更しました。3 から 4 に変更しています。Vuex の getter など、引数が 4 つある関数がライブラリベースで存在していたためです。)

1年近く運用している開発者がばらばらな 5 つのプロダクトで試してみましたが、レポートされたのは max-lines-per-functionmax-statements がそこそこ、max-params が 1 つだけ、max-depthmax-nested-callbacks は 0 でした。

どんなプロダクトでどれほどのコード量なのかわからなければあまり意味のない数字ですが、一応このような結果でした。

レビュー

ほぼ違反がなかった max-params, max-depth, max-nested-callbacks は利用しないことにしました。(嘘です。max-depth だけは利用することにしました。) この期間開発したコードで違反がでないなら、別に入れなくてもいいかなと。
未来の負債を産まないために適用させておくという考えもあるかもしれませんが、できるだけルールを (設定ファイルを) シンプルにしたい気持ちの方が強いので。

というわけで残りの max-lines-per-functionmax-statements をみていきます。

max-lines-per-function

これは function 内の行数を制限するルールです。デフォルトでは 50 行。空行やコメントをカウントさせたくない場合は skipBlankLines, skipComments を有効にすることで対応できます。

長すぎる function は全体像を把握しにくく、変数の影響もわかりにくく、テストも書きづらいなどデメリットが多いのです。違反している既存コードもそのようなケースがいくつか見られたため、個人的には有効にしたいルールです。

一方、プロパティ数が多いオブジェクトを利用する場合など、どうしても行数が増えてしまう状況もあり (Prettier が改行してくれるし)、常に有効にしたほうがいいともいえないかもしれません。
(単純なオブジェクトなら function の外に定義すれば解決しますが、そうではない場合を想定。)

ルールは有効にしつつ、特殊な function の場合は eslint-disable-next-line などで対応するのも手だと思います。

max-statements

先ほどの max-lines-per-function に多少似たルールに思えますが、これは function 内のステートメントの数を制限するルールです。デフォルト値は 10。

ステートメントの数は、雑に説明すると ;if の数です。
Statements and declarations - JavaScript | MDN

最初はこれも便利に思えたのですが、実際の違反しているコードを確認して考えが変わりました。

例えば、単純に冗長な手続きを書かざるを得ない場合や必要とする変数が多いケースなどは設定値 10 をすぐに超えてしまっていました。

それ以外にも、ステートメントを減らすために「変数を使い回す」といったようなアンチパターンを利用したり (そんな人はいないはずですが)、全体の可読性よりもステートメントを減らすことに意識が向くなど、状況によってはコードが悪化する可能性も感じました。

設定値を変える、そもそも「単純に冗長な手続きを書かざるを得ない場合や必要とする変数が多いケース」この考えが誤っている可能性もありますが、一旦見送りにしました。

max-depth

max-depth はコードの入れ子レベルを制限するルールです。

「ほぼ違反がなかった max-params, max-depth, max-nested-callbacks は利用しないことにしました。」と途中で書きましたが、これは嘘です。デフォルト値は 4 でしたが 3 に変更して、このルールを採用しました。

この設定の場合以下のコードの Nested 4 deep が違反することになります。

/*eslint max-depth: ["error", 3]*/
function foo() {
    for (;;) { // Nested 1 deep
        while (true) { // Nested 2 deep
            if (true) { // Nested 3 deep
                if (true) { // Nested 4 deep
                }
            }
        }
    }
}

設定を変更して再度 Lint を実行したところ、いくつかのコードが違反していました。違反箇所は総じて読みづらく、またリファクタリングも容易だったため、このルールも採用することにしました。

追加ルール no-else-return

max-depth入れ子を確認している時にいくつか見つけたのが、if の中で return しているがその後に if elseelse を続けているコードです。

コードのネストが深いと読み手は『精神的スタック』に条件をプッシュしなければならない

リーダブルコード

といわれているように、不要なネストをつくりたくありません。no-else-return は ESLint の --fix オプションで自動修正が可能なので、このルールも適用させることにしました。

結論

最終的に採用したルールはあまり多くありませんでした。no-else-returnmax-depth はオススメですが、max-lines-per-function はどちらでもいいかなという考えです。

module.exports = {
  // ...
  rules: {
    // ...
    // https://eslint.org/docs/rules/no-else-return
    'no-else-return': [
      2,
      {
        allowElseIf: false,
      },
    ],
    // 利用したくない人は、このルールをオフってね
    // https://eslint.org/docs/rules/max-lines-per-function
    'max-lines-per-function': [
      1,
      {
        skipBlankLines: true,
        skipComments: true,
      },
    ],
    // https://eslint.org/docs/rules/max-depth
    'max-depth': [1, 3],
  },
  overrides: [
    {
      files: ['*.test.{ts,js}'],
      rules: {
        'max-lines-per-function': 0
      }
    }
  ]
}

max-xxx ルールを自身のコードに適用して、有効そうなルールをピックアップするという作業はなかなか面白いと思います。

その他