サムネイル

はじめに

こんにちは。DMM.com 二次元開発本部二次元コンテンツ事業/同人開発部のwksyです。
デジタル作品を販売・閲覧するオンラインECサービス（以下「本プロダクト」と呼びます）で、フロントエンドエンジニアとして開発に携わっています。
今回は本プロダクトにおける「フロントエンドのコード品質を保つ仕組み」を地道に構築した過程をご紹介します。

コード品質とは

まず「コード品質って具体的にはどんなもの？」と思われるかもしれません。
人や組織によってさまざまな意味で解釈されるかもしれませんが、一例としてAWSのコード品質とは何ですか?では以下のように説明されています。

コード品質は、コードの効率性、読みやすさ、使いやすさを示す指標です。コーディングは本質的に自由形式であり、同じプログラミング言語の同じ問題を複数の方法で解決できます。コード品質はコードの正確性と信頼性を測定しますが、バグがないことと移植性があることがコード品質の唯一の測定指標ではありません。デベロッパーにとってのコードの使いやすさも含まれます。コード品質は、コードを理解および変更し、必要に応じて再利用することがどの程度容易であるかを表します。

本記事における「コード品質」では、上記で示されるようなコードの読みやすさや変更容易性に重点を置いて話を進めたいと思います。

なぜ仕組みが必要だったのか

仕組みが必要だった背景として、私たちの状況を少しお話しします。
本プロダクトでは2018年頃から2022年までの約4年間、フロントエンド専任のエンジニアが不在だったため、ライブラリなどのメンテナンスが十分にできていませんでした。
近年はフロントエンドエンジニアが少しずつ増えてきたこともあり、ReactのバージョンアップやTypeScriptの導入など、既存の技術的負債の解消・改善を徐々に進められています。
一方で、保守しづらいコードが時間とともに蓄積していたり、品質を機械的に担保する環境が整っていなかったりと、まさに「荒野」のような環境は依然として残ったままでした。
中でも特に課題となっていたのは以下の3点です。

コードの複雑性が高く、改修工数の増加や不具合の発生しやすい状態につながっていた。
静的解析がCIに組み込まれておらず、エラーがあってもデプロイ可能になっていた。
ファイル・ディレクトリの命名規則やフォーマッターの適用有無がルールで統一できておらず、可読性の低下につながっていた。

開発効率を向上させ、本プロダクトの改善をより素早く進めていくためには、これらのコード品質に関わる課題を解消する仕組みが必要でした。

コード品質を保つ仕組みの構築

課題を解決するための仕組みとして、以下の3つの施策に取り組みました。

循環的複雑度の検査と違反コードの解消
CIでの静的解析（ESLint）とマージルールの設定
静的解析の強化（Prettier、ls-lint）

1. 循環的複雑度の検査と違反コードの解消

まずは、一定以上の複雑度を持つコードの解消に着手しました。
本プロダクトではすでにサーバーサイドで同様の取り組みを行っていたため、着手に際してはこちらを参考にしました。
※取り組みの詳細について気になった方は以下もご覧ください！
サーバーサイドのレガシーシステムを、ビッグバンではなく堅実にリプレイスした話

ESLintには循環的複雑度を計測するルールとしてcomplexityが用意されています。
こちらを使用して既存コードを検査することとし、違反基準はサーバーサイドでの取り組みや以下を参考に「複雑度10までは許容、11以上はエラー」となるようにしました。

循環的複雑度	複雑さの状態	バグ混入確率
10以下	非常に良い構造	25%
30以上	構造的なリスクあり	40%
50以上	テスト不可能	70%
75以上	いかなる変更も誤修正を生む	98%

参考: 循環的複雑度とは?

ESLintの設定例（追加箇所を抜粋）

{
  "rules": {
    "complexity": ["error", { "max": 10 }]
  }
}

上記ルールを追加して検査したところ、52箇所で違反していたことが分かりました。
そのため、「無理に対応すべきでないもの」以外はすべて解消することにしました。
「無理に対応すべきでないもの」は以下の基準で判断し、これらは一時的にeslint-disable-next-line等で対処しています。

処理がシンプルで、コードの可読性が大きく損なわれていないもの（switch文でcaseが多い場合など）
処理が複雑で、リグレッションテストに多大な時間がかかるもの

理想はすべて解消したい…！のですが、100点の状態にじっくり仕上げるよりも、まずは80点でもよいので素早く仕上げることを優先したかったため、このような対応としました。
結果として、一部は複雑度の高いコードが残ったものの、全体の大半を基準値以下に抑えられました。

2. CIでの静的解析（ESLint）とマージルールの設定

静的解析でエラーが出ていてもデプロイできてしまう状態を解決するため、CIに静的解析を組み込みました。
対応したことは以下の2点です。

CircleCIのワークフローに静的解析のジョブを追加。
CIで静的解析が失敗した場合、developブランチへのPull Requestをマージ拒否するように設定。

本プロダクトではブランチ戦略としてGit-Flowを採用しており、feature → develop → masterという流れで変更を反映しています。
リリースまでの変更内容は必ずdevelopブランチに集約されるため、ここでマージ拒否される変更はリリースできず、プロダクトのコードとしても残らない形になります。

手順としては、まずCIに以下のような静的解析のジョブ（check）を追加し、これをfeatureブランチへのプッシュごとに実行するワークフローに組み込みました。
これにより、変更が加わった時は必ず静的解析が行われるようにしました。

{
  "scripts": {
    "eslint": "eslint 'src/**/*.{ts,tsx}'"
  }
}

jobs:
  check:
    executor: node-executor
    working_directory: ~/build
    steps:
      - checkout
      - run:
          name: Install dependencies
          command: npm ci
      - run:
          name: eslint
          command: npm run eslint
          when: always

次にGitHub上のブランチ保護ルールから「Require status checks to pass before merging」をdevelopブランチに適用し、ステータスチェックの内容として静的解析のジョブ（check）を指定しました。
この設定が有効化されていると、指定したステータスチェックがエラーとなっている際にブランチへのPull Requestがマージ拒否されるため、マージするにはエラー解消を行ってステータスチェックをパスする必要があります。

これらの設定により、developブランチには静的解析（ESLint）をパスした変更だけが取り込まれるようになり、一定の基準を満たすコードを維持できる環境になりました。

3. 静的解析の強化（Prettier、ls-lint）

フォーマットや命名規則は不具合に直結することは少ないため、そもそも違反に気付きにくかったり、気付いたとしても修正が開発者の任意になりがちでした。
この現状を踏まえると、気付きにくい負債の検知は個人の努力に委ねるのではなく、自動化した仕組みで検知する必要がありました。
そこで、これらのコードの可読性を損なう問題を解消するために、静的解析の強化も引き続き行いました。

フォーマッターの適用が任意になっており、コード全体でフォーマットが統一できていない。
- Prettierで検査
ファイル・ディレクトリの命名規則が決まっておらず、camelCase/PascalCase/kebab-case/snake_caseが混在している。
- ls-lintで検査

Prettierはもともと導入されていたため、現状の設定のまま--checkオプションでの検査を新たにCIに組み込みました。
ls-lintに関しては実行速度や設定のシンプルさから新たに採用し、同じくCIに組み込みました。

{
  "scripts": {
    "eslint": "eslint 'src/**/*.{ts,tsx}'",
    "prettier:check": "prettier --check 'src/**/*.{ts,tsx,css}'",
    "ls-lint": "ls-lint --workdir 'src'"
  }
}

jobs:
  check:
    executor: node-executor
    working_directory: ~/build
    steps:
      - checkout
      - run:
          name: Install dependencies
          command: npm ci
      - run:
          name: eslint
          command: npm run eslint
          when: always
      - run:
          name: prettier
          command: npm run prettier:check
          when: always
      - run:
          name: ls-lint
          command: npm run ls-lint
          when: always

これらの設定により、フォーマットや命名規則に沿わない変更はPull Requestのステータスチェックで弾かれるため、可読性の高いコードを維持できるようになりました。

余談ですが、CIで静的解析エラーを検知する仕組みを運用するには、当然ルールに沿わない現行のコードを運用前にすべて修正する必要があります。
修正箇所が多いほど時間がかかり、なおかつ他の開発者とコンフリクトする可能性が高くなるため、変更範囲が広くなりがちなフォーマッターや命名規則の修正は、導入タイミングや周囲との調整を計画的に進めることが重要だと感じました。

まとめと今後の展望

これらの一連の取り組みにより、以下のようなメリットを継続的に得られるようになりました。

循環的複雑度が一定以下のコードを維持することで、読みにくいコードや複雑なコードへの悪化を防止できている。
CI上で静的解析が行われることで、開発者が意識せずとも自動的にコード品質を一定に保てる仕組みが構築できている。
ファイル・ディレクトリの命名規則やフォーマッターの適用を統一することで、可読性を損ねるコードの追加を防止できている。

手探りしながら進める中で、これは仕組みとして適切だろうか？と自問することもありましたが、導入前の状態と比較すると、コード品質を保つ仕組みとして十分な効果が出ていると実感しています。
とはいえ、循環的複雑度の高いコードを潰しきれていなかったり、ESLintで適用するルールを精査し切れていなかったり、CI上にE2Eテストやユニットテストがなかったりと、まだまだ課題は残っています。
今後も本プロダクトではどんな仕組みが適切かをチームで検討しながら、より品質高く、より効率良く開発できる環境を目指して改善を続けていきます。