Skip to content

refactor: autoload の関数登録を src/_functions.php に集約する - #89

Open
Javakky wants to merge 1 commit into
masterfrom
javakky/autoload-single-entry
Open

refactor: autoload の関数登録を src/_functions.php に集約する#89
Javakky wants to merge 1 commit into
masterfrom
javakky/autoload-single-entry

Conversation

@Javakky

@Javakky Javakky commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

概要(What / Why)

関数の autoload 登録を src/_functions.php に集約し、composer.json 側を 1 エントリに固定します。
autoload.files に 1 ファイルずつ列挙していると、関数を追加する PR がすべて配列の末尾で衝突します

問題

現在 open な 11 本の関数追加 PR は、全部が composer.json の同じ行を触っています。
実際に #75 (chunk_by) が master にマージされた時点で、残り 11 本すべてが衝突しました
(git merge-tree で確認)。1 本マージするごとに残りを rebase する運用になります。

$ git merge-tree --write-tree origin/master origin/javakky/sort-by
CONFLICT (content): Merge conflict in composer.json

変更点

  • src/_functions.php を追加 — src/ 配下の関数定義ファイルを require_once で読み込む。ファイル名の昇順に固定
  • composer.jsonautoload.files["src/_functions.php"] の 1 エントリにする

以降、関数を追加するときは composer.json ではなく src/_functions.php昇順の正しい位置へ 1 行足します。
追加位置が名前で分散するため、同時に走る PR どうしが同じ行を触りにくくなります。

動作確認

  • 手動:
    • composer dump-autoload / composer dump-autoload -o (optimized) の両方で全関数が見えることを確認
  • 自動:
    • vendor/bin/php-cs-fixer fix --dry-run --diff / vendor/bin/phpstan analyse -c phpstan.neon / vendor/bin/phpunit tests
    • 結果:OK (104 tests, 109 assertions) / PHPStan level 10 [OK] No errors / CS Fixer Fixed 0 of 32 files

補足(任意)

glob() 案は計測して落としました

src/_functions.phpglob(__DIR__ . '/*.php') を走査すれば追加時にどのファイルも触らずに済む (衝突が構造的にゼロ) のですが、autoload のロード時間を実測したら悪化しました。

方式 ロード時間 (CLI 20 回平均)
現在の master (明示列挙) 3.06 ms
本 PR (単一エントリ + 明示 require) 3.13 ms
glob() 走査 4.04 ms

glob() のディレクトリ走査は opcache でキャッシュされないので、ライブラリ利用者が毎リクエスト払うことになります。+1ms を払って衝突を完全にゼロにするより、コストなしで衝突を分散させる方を採りました。

昇順でも衝突しうるケース

名前が隣接する関数が別々の PR で同時に追加されると、まだ衝突します (例: dropdrop_while)。ただし解決は「両方の行を残す」で機械的に済み、衝突する PR の本数自体が大きく減ります。

マージ順のお願い

この PR を先にマージしていただけると、残り 11 本の PR から composer.json の変更を落として src/_functions.php への 1 行追加に置き換えられます (こちらで rebase します)。後にすると 11 本ぶんの衝突解決が発生します。

🤖 Generated with Claude Code

composer.json の autoload.files に 1 ファイルずつ列挙していたため、 関数を追加する PR が
すべて配列の末尾で衝突していた。 読み込みを src/_functions.php へ集約し、 composer.json 側は
1 エントリで固定する。

_functions.php の require はファイル名の昇順に固定した。 追加位置が名前で分散するため、
同時に走る PR どうしが同じ行を触りにくくなる。

glob() で src/*.php を走査する案も計測したが、 autoload のロードが 3.06ms から 4.04ms へ
悪化した (CLI 20 回平均) ため採らなかった。 明示 require なら 3.13ms で従来と同等。
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude encountered an error after 1s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

Claude finished @Javakky's task in 4m 41s —— View job


レビュー結果

  • 差分の確認(composer.json / src/_functions.php
  • src/ 配下のファイル一覧と require の網羅性チェック
  • 読み込み順変更の影響確認
  • CI / ツール設定の確認
  • レビュー結果の投稿

結論: 方針・実装ともに妥当で、そのままマージして問題ないと思います。 以下は改善提案と運用上の注意です(ブロッカーなし)。


検証したこと

項目 結果
require 一覧の網羅性 src/*.php の関数定義ファイル 15 本と過不足なく一致(旧 composer.json の 15 エントリと同一集合)
並び順 anyunique まで昇順、崩れなし
src/Mode.php の扱い ✅ enum なので require せず PSR-4 に委ねている(正しい)
読み込み順の変更影響 ✅ 各関数ファイルは相互に require/include しておらず、全ファイルが自前で declare(strict_types=1) 済み。先頭が filter.phpany.php に変わっても影響なし
CI 対象 .github/workflows/check.ymlpathssrc/** があるため新ファイルもトリガーされる
CI 結果 Check(CS Fixer / PHPStan level 10 / PHPUnit)が success(run 30246422165

注記: 実行環境の都合で composer install とテストのローカル実行はできていません。上記の CI 結果と PR 本文の実測に依拠しています。

glob() 案をロード時間の実測で落としている点(ライブラリ利用者が毎リクエスト払うコスト、opcache でキャッシュされない点)も、判断の根拠として妥当だと思います。


指摘

1. [提案・中] 追記漏れを機械的に検出する手段がない

src/ に新しい関数ファイルを足して _functions.php への 1 行追記を忘れた場合、いま気づけるのは「その関数を呼ぶテストが undefined function で落ちる」ときだけです。テストごと忘れると CI をすり抜けて、リリース後に利用者側で初めて壊れます(composer.json 直列挙のときも同じリスクでしたが、集約したことで「追記漏れ」の窓口が増えたわけではないものの、依然として残っています)。

glob()テスト内だけで使えば、ロード時間を一切増やさずに漏れをゼロにできます。PR で glob() を落とした理由(実行時コスト)とも競合しません。

// tests/FunctionsAutoloadTest.php(イメージ)
public function testAllFunctionFilesAreRequired(): void
{
    $expected = array_values(array_filter(
        array_map('basename', glob(__DIR__ . '/../src/*.php')),
        // _functions.php 自身と、PSR-4 で解決されるクラス/enum(StudlyCaps)を除く
        static fn (string $f) => $f !== '_functions.php' && !ctype_upper($f[0]),
    ));
    sort($expected); // LC_ALL=C 相当のバイト順

    preg_match_all(
        "#^require_once __DIR__ \. '/([^']+)';$#m",
        file_get_contents(__DIR__ . '/../src/_functions.php'),
        $m,
    );

    self::assertSame($expected, $m[1]); // 網羅性と昇順を同時に検証
}

ついでに composer.jsonautoload.files["src/_functions.php"] の 1 エントリのままかも assert しておくと、下の 2. の事故も止められます。.claude/rules/20_quality.md の「変更箇所のテストを追加する」にも沿います。

Fix this →

2. [注意・中] 残り 11 本の rebase 時、composer.json_functions.php二重登録は fatal になります

マージ順のお願いに関連して、運用上ひとつ落とし穴があります。Composer が生成する autoload_real.php の files ローダーは、重複ガード($GLOBALS['__composer_autoload_files'])を通過したあと require_once ではなく require でファイルを読み込みます。ガードのキーはエントリごとに異なるため、

  • composer.jsonfilessrc/foo.php が残っている
  • かつ src/_functions.php にも require_once __DIR__ . '/foo.php'; がある

という状態になると、_functions.php が先に foo.php を読み、その後 composer が require 'src/foo.php' を実行して Cannot redeclare foo() で fatal になります。「衝突したので両方残す」という素直な解決が、no-op ではなく即死につながる形です。

CI は確実に落ちるので見逃しはしませんが、11 本を順次 rebase する際は「composer.json の変更を必ず落とす」ことを PR 本文なり _functions.php のコメントなりに明記しておくと安全だと思います(1. のテストを入れておけば機械的に止まります)。

私の環境で composer install を実行できていないため、生成コードの実物では未確認です。1 本目の rebase のときに実際に二重登録して確認しておくと確実です。

3. [nit] 「昇順」の定義が曖昧

_functions.php のコメントは「ファイル名の昇順」とだけ書かれていますが、アンダースコアの扱いで結果が変わります。

  • バイト順(LC_ALL=C / PHP の sort()): zip.php < zip_with.php < zipped.php_ = 0x5F < p = 0x70)
  • ロケール依存の sort(記号を無視): zip.php < zipped.php < zip_with.php

現状の並びはバイト順です。「LC_ALL=C 相当のバイト昇順(_ も 1 文字として比較)」と明記しておくと、後続 PR が別の順序で挿入して無駄な衝突を生むのを防げます。

Fix this →

4. [nit] composer.json のインデント整形が目的と無関係

"php": ">=8.4" の行と autoload-dev ブロックのインデント(2 → 4 スペース)が、autoload 集約とは独立に変更されています。ファイル内で揃うのは good ですが、.claude/rules/10_workflow.md の「無関係な整形はしない」に照らすと本来は別 PR です。

しかも今回に限っては、この PR の目的(衝突を減らす)と少しだけ逆行します — 依存を触る PR や autoload-dev を触る PR と、余計に衝突しうる行を増やしています。影響は小さいので、そのままでも構わないと思います。

Fix this →


良かった点

  • 「なぜこの設計なのか」が _functions.php の冒頭コメントに書かれていて、次に関数を足す人が composer.json を触らずに済む導線になっている
  • glob() 案を印象ではなく実測(3.06 / 3.13 / 4.04 ms)で棄却し、その数字を PR に残している
  • 「昇順でも隣接名は衝突しうる」という残存リスクを自分から明示している

セキュリティ観点(.claude/rules/20_quality.md)でも、追加されるのは静的な require_once のみで動的パス解決や外部入力の混入はなく、問題ありません。
| ブランチ: javakky/autoload-single-entry

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.

1 participant