Skip to content

chore(husky): pre-commit で rector / phpstan / cs-fixer を dry-run 実行 - #6761

Draft
dotani1111 wants to merge 1 commit into
EC-CUBE:4.4from
dotani1111:dev/husky-precommit-enhancement
Draft

chore(husky): pre-commit で rector / phpstan / cs-fixer を dry-run 実行#6761
dotani1111 wants to merge 1 commit into
EC-CUBE:4.4from
dotani1111:dev/husky-precommit-enhancement

Conversation

@dotani1111

Copy link
Copy Markdown
Contributor

概要

.husky/pre-commit を強化し、stage に .php を含む commit 前に rector → phpstan → php-cs-fixer の順で実行 するように変更する。

  • rector / phpstan: dry-run で検出、違反があればコミット中止
  • php-cs-fixer: lint-staged 経由で staged file のみ auto-fix (既存実装)

前提環境

  • composer installvendor/bin/{rector,phpstan,php-cs-fixer} が揃っていること
  • 初回のみ bin/console cache:warmup --env=devvar/cache/dev/Eccube_KernelDevDebugContainer.xml を生成 (rector が参照するため)

改善案

  • stage に .php が含まれる場合のみ実行 (それ以外は exit 0)
  • 実行順序:
    1. vendor/bin/rector process --dry-run --ansi --config=rector.php
    2. vendor/bin/phpstan analyze src/
    3. npx lint-staged (php-cs-fixer auto-fix)
  • rector / phpstan が非 0 → 修正手順をエラーメッセージで案内して exit 1
  • 全 pass → commit 続行

動作ロジック

旧 hook では lint-staged (php-cs-fixer auto-fix) だけが走っていた。
これは style 修正には十分だが、ロジック品質・静的解析違反は CI まで検出されなかった。
本 PR で rector と phpstan を追加し、CI と同じ判定をローカル commit 段階で受けられるようにする。
cs-fixer は logic を変えない style 修正のみなので 従来通り lint-staged で auto-fix のまま 残す。

加えて、旧 hook の || node -e '' (lint-staged 失敗を握り潰す部分) を撤去した。
lint-staged 自体が失敗する時点で何かおかしい状態なので、コミットを止める方針とする。

Test plan

  • staged に .php を含まない commit はスキップされる
  • staged に .php を含む commit:
    • 3 ステップ全 pass → commit 成功
    • rector が修正点を検出 → commit 中止 + 修正コマンドの案内が出る
    • phpstan がエラーを検出 → commit 中止
    • cs-fixer の style 違反は auto-fix されて commit に取り込まれる
  • HUSKY=0 git commit ... で hook をスキップできる

スキップ

HUSKY=0 git commit ... で hook をスキップ可能 (husky 9 公式機能)。

stage に *.php がある場合, rector → phpstan → php-cs-fixer の順で実行.
rector / phpstan が修正点・エラーを検出したらコミット中止.
cs-fixer は logic を変えない style 修正のみのため最後に lint-staged で
auto-fix. project-wide な rector / phpstan は dry-run で開発者に明示的
修正を促す方針.
コマンドは .github/workflows/{rector,phpstan,php-cs-fixer}.yml に準拠.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1f947be-6163-410d-a0ba-0358d32e8545

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (4.4@27a19d8). Learn more about missing BASE report.

Additional details and impacted files
@@          Coverage Diff           @@
##             4.4    #6761   +/-   ##
======================================
  Coverage       ?   76.97%           
  Complexity     ?     6751           
======================================
  Files          ?      483           
  Lines          ?    26277           
  Branches       ?        0           
======================================
  Hits           ?    20228           
  Misses         ?     6049           
  Partials       ?        0           
Flag Coverage Δ
Unit 76.97% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dotani1111

Copy link
Copy Markdown
Contributor Author

commit毎は時間がかかるので、pre-pushで良い

ttokoro20240902 added a commit that referenced this pull request Jun 9, 2026
PR #6817 のレビュー(#issuecomment-4655303640)に対応。

1. スキーマ運用の誤りを修正(要修正)
   - 「カラム追加=マイグレーション」を撤回し、schema:update --force →
     migrations:migrate の2段構えを明記。単純なカラム追加にALTERは不要。
   - マイグレーションの用途をマスタ/初期データのINSERTと、型変更・リネーム等
     schema:updateで扱えない構造変更に限定。
   - 生成コマンドを migrations:diff から migrations:generate(空雛形を手書き)へ。
   - 手本コードをALTER ADDからmtb_sale_typeへの冪等なINSERT例に差し替え。

2. 機械が扱える内容はツールへ委譲
   - 「約50行/約7依存」の数値メトリクスを散文から削除し質的シグナルへ転換。
     計測は tools/check-architecture.php に一本化。
   - rector/phpstan/php-cs-fixer のローカル実行節を新設(PR #6761 pre-commit を参照)。

3. EC-CUBE固有のドメイン知識を拡充
   - service.md に PurchaseFlow パイプライン節を追加。
   - entity.md に状態ロジック(Entity可)と処理(PurchaseFlow/Service)の線引き、
     getId() が nullable な理由を追記。
   - AGENTS.md に SaleType / OrderItem明細種別 / Shipping のドメイン用語集を追加。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@ttokoro20240902

Copy link
Copy Markdown
Contributor

方向性に賛成です(style だけでなく rector/phpstan の CI 判定を手元に前倒しするのは良い)。その上で、Docker ベースで開発している立場から 2点 提案させてください。

1. 実行環境が host 決め打ち → Docker 開発環境で崩れる

この hook は vendor/bin/rector vendor/bin/phpstan npx lint-stagedhost 側で 実行する前提です。docker-compose で開発し host に PHP / node を持たない構成だと、vendor/bin/*npx が解決できず hook が失敗します(|| node -e '' 系の握り潰しが残っていると、失敗しても無音で素通りし「掛けているつもりで実は掛かっていない」状態にもなりやすい)。

実際に手元の Docker-QA 環境(ec-cube サービス内で phpunit/phpstan/cs-fixer を回す運用)では host 実行前提だと期待どおり動きませんでした。

提案: RUN プレフィックスで実行先を自動判定(host 派・Docker 派どちらの貢献者でも壊れない)

  • ec-cube コンテナ稼働中 → docker compose exec -T ec-cube …
  • コンテナ無し & host に php + vendor/bin → host 実行
  • どちらも無い → 握り潰さず明示スキップcommand -v php まで見て「phar は在るが php 無し」の誤判定も回避)

2. 全部 pre-commit は重い → cs-fixer=pre-commit / rector・phpstan=pre-push に分離

手元で計測すると、この hook 一式は rector + phpstan を project-wide に走査し、docker exec 越しで 1 commit あたり初回約35秒(キャッシュを消さなければ以降は約4.5秒)かかりました。commit のたびにこれが乗ると、テンポを損ねて結局 HUSKY=0 を常用されかねません。

commit は細かく刻む WIP の単位、push は「人に渡す境界」= CI と同じ役割。なので:

フック 中身 コスト(手元実測)
pre-commit php-cs-fixer(staged のみ auto-fix) 約0.5秒
pre-push rector --dry-run + phpstan(src 全体) 初回約35秒 / 以降約4.5秒(push 1回に1度)

cs-fixer だけ commit 段階に残せば style は常にクリーンに保たれ、push/CI が style で落ちることもありません。重い静的解析は「壊れたまま push しない」ゲートとして pre-push に置くのが、テンポと安全性の両立になります。

具体案(pre-commit)

#!/usr/bin/env sh
[ "$HUSKY" = "0" ] && exit 0
STAGED_PHP=$(git diff --cached --name-only --diff-filter=ACMR | grep '\.php$' || true)
[ -z "$STAGED_PHP" ] && exit 0

DC="docker compose"; SVC="ec-cube"
case "${ECCUBE_HOOK_RUNNER:-auto}" in
  docker) RUN="$DC exec -T $SVC" ;;
  host)   RUN="" ;;
  skip)   exit 0 ;;
  auto)
    if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
    elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/php-cs-fixer ]; then RUN=""
    else echo "pre-commit: no container / no host php+vendor; skip"; exit 0
    fi ;;
esac

echo "$STAGED_PHP" | xargs $RUN vendor/bin/php-cs-fixer \
  --config=.php-cs-fixer.dist.php --path-mode=intersection fix || exit 1
echo "$STAGED_PHP" | xargs git add

具体案(pre-push)

#!/usr/bin/env sh
[ "$HUSKY" = "0" ] && exit 0

DC="docker compose"; SVC="ec-cube"
case "${ECCUBE_HOOK_RUNNER:-auto}" in
  docker) RUN="$DC exec -T $SVC" ;;
  host)   RUN="" ;;
  skip)   exit 0 ;;
  auto)
    if $DC exec -T "$SVC" true >/dev/null 2>&1; then RUN="$DC exec -T $SVC"
    elif command -v php >/dev/null 2>&1 && [ -x vendor/bin/phpstan ]; then RUN=""
    else echo "pre-push: no container / no host php+vendor; skip"; exit 0
    fi ;;
esac

$RUN vendor/bin/rector process --dry-run --ansi --config=rector.php || { echo "FAIL: vendor/bin/rector process --config=rector.php で修正"; exit 1; }
$RUN vendor/bin/phpstan analyze src/ || { echo "FAIL: PHPStan"; exit 1; }

補足の変更意図:

  • npx lint-staged を直接 php-cs-fixer --path-mode=intersection + git add に置換:コンテナに node/lint-staged が無くても動くように(lint-staged の部分ステージ退避は失うので要検討)。
  • キャッシュ削除(rm -rf var/rector_cache / /tmp/phpstan)は外す提案:commit/push ごとに消すと毎回コールドで遅い。incremental キャッシュを活かしたい。
  • HUSKY=0 git commit/pushECCUBE_HOOK_RUNNER=docker|host|skip で緊急バイパス・強制切替可。

いずれも手元(Docker)で pre-commit 0.5秒 / pre-push で rector・phpstan 完走を確認済みです。分離までは重い、という判断なら「自動判定だけ入れて全部 pre-commit 据え置き」でも 1 の効果は得られます。

ttokoro20240902 added a commit that referenced this pull request Jul 30, 2026
- 実行環境を自動判定(ec-cube コンテナ稼働中は `docker compose exec`、
  無ければ host の vendor/bin、どちらも無ければ握り潰さず明示スキップ)
- pre-commit: php-cs-fixer を staged のみ auto-fix(lint-staged 非依存=
  コンテナに node が無くても動作。約0.5秒)
- pre-push: rector --dry-run + phpstan を project-wide 実行(重い解析は
  「人に渡す境界」である push へ寄せ、commit のテンポを保つ)
- キャッシュ削除(rm -rf var/rector_cache /tmp/phpstan)を廃止し
  incremental キャッシュを活かす(pre-push 初回~35s→以降~4.5s)
- ECCUBE_HOOK_RUNNER=docker|host|skip で強制切替、HUSKY=0 でバイパス可

Refs #6761

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants