linter を古いプロジェクトに導入しよう (導入はignore、放置は検知する)

こんにちは、エンジニアをやっている @nukisashineko です。

今回は古いプロジェクトに linter を導入しようとしたときの一番のハードル。
「linter 導入時の大量の差分により、導入が許可されない」
を華麗に回避するための方法を紹介します。

導入

「linter 導入時の大量の差分により、導入が許可されない」を回避する方法って？

草案 1

導入時に既存ファイルは全部 ignore する
- そうすれば linter 導入時に修正差分が発生しない

想定問答:
「 ignore したら意味ない」
「一番 linter で reformat したいのは古いファイル」
「新規ファイルだけの話かぁー。それなら知ってるからブラウザバックするわ」

ああ、待って待ってブラウザバックしないで、
もうちょっと話が続くのでどうか付き合って。

草案 2

……じゃあ、こうすればいいのでは？

導入時に全部 ignore する
ファイルに触ったら ignore ファイルから消去と linter の実行を義務付ければいい

想定問答:
「絶対守られないよねそのルール」
「レビューの時にそれを指摘するの大変」
「develop ブランチなら PR 分けて refomat 時の動作確認できるけど、小さな機能追加では動作確認が増えるのが嫌」

わかる。
わかるので、もうちょっとひねります。

草案 3

導入時に全部 ignore する
ファイルに触ったら ignore ファイルから消去と linter の実行がなかったら機械的に通知する
- "developブランチの変更で"

想定問答:
「それができたら苦労しない」

はい。ほんとにそう。
だから、linter の導入は大体ここで詰まります。

導入がしにくいのは何が原因？

古いプロジェクトへの linter の導入時の最大の敵は、
大きすぎる差分 と 不安になる大きさの影響範囲
( 分割する場合のレビュー負荷の増大 ) です。

レガシープロジェクトへの linter や静的解析の導入は下記のどれかになりがちです。

導入できない
- 大きすぎる差分 と 不安になる大きさの影響範囲 が許容できず
- 分割する場合のレビュー負荷の増大 も許容できなかったケース
動作確認せずに一括適用
- 大きすぎる差分 と 不安になる大きさの影響範囲 を許容したケース
導入されたけど ignore が放置され続ける
- 大きすぎる差分 と 不安になる大きさの影響範囲 が許容できず、
- 分割する場合のレビュー負荷の増大 が上手くコントロールできなかったケース

これらは github action を上手く使うことで、
分割する場合のレビュー負荷の増大 をコントロールできるようになり、
チームメンバーの努力により、中長期的に linter の実現ができるのです。

注意事項

レビュー指摘や紳士ルールはチーム内で策定しても守られなければ意味が有りません。
人間が守れないなら機械にチェックしてもらればいい を地で行く
linter の導入方法となります。

※ あくまでも導入方法であり、作業者はチームメンバー全員で取り掛かることであるため、 ※ 事前に十分に説明と紳士ルールとしてのチーム内の決を取ってから実行してください。
※ これを導入したことで CI が落ちていることが常態化するのは本意では有りません。

stylelint での実践例

今回の肝: `ignore で導入` 、`放置を検知する CI の設置`

実際にチーム内で解決した stylelint を例に上げて説明しますが、
別に php-cs-fix でも php code sniffer でも phpstan でもやることは変わりません。
ignore で導入 、放置されないように CI を設置 です。

stylelint の導入

まずは package.json を更新して、stylelint を導入します。
script には --allow-empty-input を書いておくと実行対象ファイルが無くても
exit(1) にならないので全ファイルがignore されている時に
常に CI の結果が red にならずに済むので、
このような設定をデフォルトで付けておくと導入に便利です。

package.json

{
  "name": "productname",
  "scripts": {
    "lint:css": "stylelint --allow-empty-input",
    "lint:css:fix": "stylelint --allow-empty-input --fix"
  },
  "devDependencies": {
    "stylelint": "11.0.0",
    "stylelint-config-standard": "19.0.0"
  }
}

.stylelintrc

{
  "extends": "stylelint-config-standard",
  "rules": {
    "no-descending-specificity" : null
  }
}

.stylelintignore の設置

※ ここでは css は htdocs/css/ に入っているという前提にします。

次に stylelint を実際に走らせて、
エラーが出るファイルについて全部 ignore に詰め込みましょう。
一番最初に膨大な差分がでてしまうと、導入自体が難しかったりします。
徐々に均すために一旦 ignore するのが大事です。
これを commit ください。

# 既に ignore ファイルが有るなら削除
rm .stylelintignore

# 1. `-- --quiet --formatter json` オプションにより stylelint の結果が json として出力されるように仕向けたら、
# 2. jq comamnd を使って error と表示されるファイル path のみを取得
# 3. docker で動作させたため、/app という path prefix が付与されているので、削除
# 4. 上記手順で .styellintignore の再作成が行われた時に、差分が少なくなるように sort もしておく
docker run --rm -it -v "$PWD/app" -w /app node:14.15.0 sh -c "npm run --silent lint:css htdocs/css/ -- --quiet --formatter json" | jq -r '.[] | select(.errored == true)| .source' |sed -e 's/^\/app\///g' | sort >> .stylelintignore

github action で stylelint を実行する

gtihub action を書いて実際に stylelint を走らせましょう。
最初は paths: '**.css' をコメントアウトしたり、
.stylelintignore からいくつか path を削除して commit することで、
red で落ちて確実に stylelint が動いていることを確認するといいですよ。

.github/workflows/stylelint.yml

name: SYTLELINT

on:
  push:
    branches:
      - master
  pull_request:
    types: [synchronize, opened, reopened]
    # PR の差分ファイルに .css 拡張子のファイルが含まれていた場合のみ動作させる
    paths: '**.css'

jobs:
  stylelint-check-diff-files:
    runs-on: ubuntu-latest
    steps:
      # clone repository to be able to merge master
      - name: checkout repository
        uses: actions/checkout@v2
        with:
          ref: ${{ github.event.pull_request.head.ref }}
      - name: setting for git
        # 1.2. setting for git merge command (require user.email and user.name)
        run: |
          git config --global user.email "xxxxxx.ci@users.noreply.github.com" && \
          git config --global user.name "xxxxxx.ci"
      # install eclint only
      - uses: actions/setup-node@v2
        with:
          node-version: '12'
      - run: npm install
      # get diff files (excluded binary) and eclint check files
      - run: |
          npm run --silent lint:css htdocs/css/

develop ブランチで ignore されたファイルが差分にあったら CI で通知する

ignore されたファイルがそのまま ignore に居続けると、
導入された linter の効果が発揮されにくくなります。
develop に関するPRで、ignore されたファイルが差分に含まれていたら、
ignore から外して lint による fix を書けることを CI で検知しましょう。

.github/workflows/require_deleting_notpath_from__stylelintignore__by_new_pr.yml

name: REQUIRE-DELETING-NOTPATH-FROM-STYLELINT-BY-NEW-PR
on:
  pull_request:
    types: [synchronize, opened, reopened]
    # prefix が develop と名のついたブランチでしか動作しないように修正
    branches:
      - 'develop**'
      - 'develop/**'
jobs:
  require_deleting_notpath_from__stylelintignore__by_new_pr:
    runs-on: ubuntu-latest
    steps:
      # clone repository to be able to merge master
      - name: checkout repository
        uses: actions/checkout@v2
        with:
          ref: ${{ github.event.pull_request.head.ref }}
          fetch-depth: 0
      - name: setting for git
        # 1.2. setting for git merge command (require user.email and user.name)
        run: |
          git config --global user.email "xxxxxx.ci@users.noreply.github.com" && \
          git config --global user.name "xxxxxx.ci"
      - name: setting for git
        # 1.2. setting for git merge command (require user.email and user.name)
        run: |
          git config --global user.email "xxxxxx.ci@users.noreply.github.com" && \
          git config --global user.name "xxxxxx.ci"
      # 1行目. 指定されているファイルがPRの差分ファイル一覧から .stylelintignore に 載っているファイルを取得する
      # 2行目. .stylelintignore にファイルが載っているなら exit code 0
      # 3.4.5行目. CIに引っかかったファイル一覧を表示して、新しいPRでCIを修正するように促して exit code 1 で終了する
      - run: |
          export FILES="$((for j in  $( git --no-pager diff --name-only ${{ github.event.pull_request.head.ref }}  origin/${{ github.event.pull_request.base.ref }}  | xargs file | grep ".*: .* text" | sed "s;\(.*\): .* text.*;\1;"  | grep .css );  do echo  "$j: $(grep -c "$j" .stylelintignore)";  done) | grep -v -E ' 0$' | sed "s;\(.*\.css\): .*;\1;")" \
          && [ -z "$FILES" ] \
          || ( echo "discovered ignore targets (.stylelintignore) in diff files:" && (for i in $FILES ; do echo $i ; done) \
                && echo "please create new branch and PR to remove path from .stylelintignore" \
                && ( [ "1" = "" ]  ) )