Skip to content

feat: chunk と windowed を追加する - #79

Open
Javakky wants to merge 4 commits into
masterfrom
javakky/chunk-windowed
Open

feat: chunk と windowed を追加する#79
Javakky wants to merge 4 commits into
masterfrom
javakky/chunk-windowed

Conversation

@Javakky

@Javakky Javakky commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

概要(What / Why)

固定サイズで分割する chunk と、スライディング窓を作る windowed を追加します。

変更点

  • src/chunk.phparray_chunk 相当 + mode 対応。size <= 0InvalidArgumentException
  • src/windowed.php — 開始位置を step 刻みで進める。partial で末尾の欠けた窓を含めるか選ぶ
  • tests/ChunkTest.php (11 ケース) / tests/WindowedTest.php (16 ケース)
  • composer.jsonautoload.files に 2 件追加

動作確認

  • 手動:
    • windowed([1,2,3,4,5], 3)[[1,2,3], [2,3,4], [3,4,5]]
    • windowed([1,2,3,4,5], 3, 2, true)[[1,2,3], [3,4,5], [5]]
    • chunk([1,2,3,4,5], 2)[[1,2], [3,4], [5]]
  • 自動:
    • vendor/bin/php-cs-fixer fix --dry-run --diff / vendor/bin/phpstan analyse -c phpstan.neon / vendor/bin/phpunit tests
    • 結果:OK (125 tests, 137 assertions) / PHPStan level 10 [OK] No errors / CS Fixer Fixed 0 of 33 files

補足(任意)

windowed の窓生成規則

Kotlin の windowed(size, step, partialWindows) に合わせました。開始位置は 0, step, 2*step, ... と進み、partial = true のときは開始位置が件数未満であれば残りが size 未満でも窓を作ります (空の窓は作りません)。

windowed([1, 2], 3, 1, true)  // [[1, 2], [2]]  ← 開始位置 1 の窓も作るので 2 個

直感に反しやすいので phpdoc に例を置き、テストでも固定しています。

array_slice の落とし穴

array_slice()$preserve_keys = false でも文字列キーを保持します (再付番されるのは数値キーだけ)。そのため windowedarray_slice(..., true) で切り出してから MODE_LIST のときだけ array_values() する実装にしています。chunk が使う array_chunk() にはこの問題がないので委譲のままです。

なお既存の src/slice.php はこの対処がなく、slice(['a'=>1,'b'=>2], 0, 2, Mode::MODE_LIST)['a'=>1,'b'=>2] を返して宣言している list<V> に反します。既存関数の変更はこの PR のスコープ外なので触っていませんが、別 issue に切り出す価値があります。

その他

  • 例外を投げる関数はこの 2 つがリポジトリで初なので、メッセージ文言 ($size は 1 以上である必要があります。) が先例になります。文体を揃えたい場合は指摘してください
  • chunk_by (隣接要素をキーの変化で切る関数) は feat: chunk_by を追加 #75 で別途追加しています。用途が異なります
  • @phpstan-param の条件型は付けていません (「assoc 入力 + MODE_LIST」が型エラーになるため。slice.php / map.php / unique.php と同じ形)

Closes #70
Closes #71

🤖 Generated with Claude Code


追記: 整数レンジ型を入れました (2 コミット追加)

オーナーの方針 (positive-int / non-negative-int を積極的に使う。特に $size を比較するような場合) を反映しました。

ci: PHPStan の treatPhpDocTypesAsCertain を無効にする

@param positive-int $size と実行時の if ($size <= 0) throwlevel 10 の既定では共存できません

Comparison operation "<=" between int<1, max> and 0 is always false.
🪪 smallerOrEqual.alwaysFalse
💡 ... you can turn off this check by setting treatPhpDocTypesAsCertain: false

phpdoc の型は実行時に強制されないので、ライブラリとしては型宣言と実行時の防御を両立させたいところです。特に windowed$step <= 0 を消すと $offset が進まず無限ループします ($size <= 0 の方は空の窓が並ぶ静かな失敗)。

treatPhpDocTypesAsCertain: false を入れた副作用は実測しました。既存の @phpstan-ignore 2 箇所が無効になるだけで (HeadOptionTest / LastOptionTestmethod.alreadyNarrowedType)、同じコミットで削除しています。native type 由来の常真/常偽検出は残るので、src/reduce.phptests/AnyTest.php の ignore は有効なまま残しました。設定変更の前後で PHPStan のエラーは 0 → 0 です。

refactor: chunk と windowed に整数レンジ型を入れる

  • chunk: @param positive-int $size
  • windowed: @param positive-int $size / @param positive-int $step
  • 戻り値の list 側non-empty-list<V> に絞る (空のチャンク・窓は作らないため)
  • assoc 側は non-empty を付けていませんnon-empty-array<never, never> が unresolvable になり、chunk([], 2) のような空配列リテラルの呼び出しを壊すためです (実測で確認し、回帰テストを追加しました)
  • 実行時の例外チェックと日本語メッセージの固定は維持

windowed\assert($window !== []); を 1 行入れています。array_slice() の戻り値は array<K, V> なので non-empty-list<V> に絞ると return.type で落ちるためで、ループ不変条件 ($offset < $count かつ $size >= 1) の表明も兼ねています。到達しない if 分岐を作るより素直だと判断しましたが、リポジトリの好みに合わなければ言ってください。

異常系テスト ($size = 0 / -1$step = 0 / -1) は positive-int 宣言後もそのままでは通らないので、int を一段挟んで widening する private ヘルパー経由にしました。テストは 1 件も削っていません (理由はヘルパーの docblock に記載)。

追加コミットのフッターは Refs #70, #71 にしてあります (型注釈に対応する issue が無いため、Closes は既存 2 コミットのぶんだけです)。

Javakky added 2 commits July 27, 2026 16:14
配列を固定サイズ単位で分割する chunk を追加した。末尾は端数チャンクになり、各チャンクの内部はモードに従ってキー保持または連番化する。

Closes #70
スライディング窓を生成する windowed を追加した。size / step / partial で窓の切り方を指定でき、窓の内部はモードに従ってキー保持または連番化する。

Closes #71
@Javakky
Javakky force-pushed the javakky/chunk-windowed branch from e4612d0 to 6096f82 Compare July 27, 2026 07:16
Javakky added 2 commits July 27, 2026 16:26
phpdoc の型は実行時には強制されないため、positive-int のような整数レンジ型を宣言しつつ実行時の防御($size <= 0 での例外)も残したい。既定では両者が共存できず「always false」と報告されるため、phpdoc 由来の型を確定として扱う挙動を無効にして型宣言と実行時の防御を両立させる。

native type 由来の常真・常偽検出は残るため、リテラル配列由来の identical.alwaysFalse 等は引き続き検出される。設定変更で無効になる ignore 2 箇所(method.alreadyNarrowedType)を併せて削除する。

Refs #70, #71
$size / $step は 1 以上しか意味を持たないため positive-int を宣言した。実行時の例外チェックは phpdoc の型が実行時に強制されないため残している(特に windowed の $step <= 0 は無限ループになる)。

戻り値は空のチャンク・空の窓を作らないので list 側を non-empty-list に絞った。assoc 側は non-empty-array<never, never> が解決できず空配列リテラルを渡す呼び出しを壊すため array のままにし、理由を phpdoc に残した。その回帰を捕まえるテストとして、空配列リテラルを直接渡す呼び出しを各モードで追加した。

異常系テストは positive-int の宣言を int 経由ですり抜けるヘルパーを介して維持した。

Refs #70, #71
Repository owner deleted a comment from claude Bot Jul 27, 2026
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

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


コードレビュー

  • 差分とコンテキストの収集
  • src/chunk.php / src/windowed.php のレビュー
  • テストのレビュー
  • 設定変更(composer.json / phpstan.neon)の確認
  • 検証状況の確認(CI Check は success)
  • レビュー結果の投稿

総評

ロジックにバグは見つかりませんでした。 windowed の窓生成規則は Kotlin の windowed(size, step, partialWindows) と一致していますし(doc の 5 例をすべて手で追って確認)、chunkarray_chunk 委譲も妥当です。特に array_slice が文字列キーを保持する落とし穴を把握して array_values() を挟んでいる点、そして既存 slice.php の同種の不整合まで見つけてスコープ外と明示している点が良いです。

条件型の健全性も 4 パターン(MODE_LIST/MODE_ASSOC × list/assoc 入力)を追いましたが、実行時挙動と一致しています。

指摘は blocker なし、設計判断の相談 1 件とテスト/PR本文の nit のみです。


1. treatPhpDocTypesAsCertain: false はリポジトリ全体に効くのがもったいない(要相談 / 中)

phpstan.neon:3

treatPhpDocTypesAsCertain: false

@param positive-int $sizeif ($size <= 0) の実行時ガードが両立しないための対処だと理解しています(実行時ガード自体は、phpdoc の型が実行時に強制されないライブラリとして正しい判断です)。

ただしこの設定は src / tests 全体で「phpdoc 型に由来する到達不能条件・デッドコードの検出」を無効化します。level 10 を採っているリポジトリでこの検出を捨てるのは代償が大きく、影響が既に無関係なファイルへ波及しているのが傍証です:

  • tests/HeadOptionTest.php:56 / tests/LastOptionTest.php:56@phpstan-ignore-next-line method.alreadyNarrowedType 削除は、この設定変更に押し出された修正で、chunk / windowed とは無関係です

対象を 2 ファイルに絞る案:

parameters:
    level: 10
    # treatPhpDocTypesAsCertain はデフォルト(true)のまま
    ignoreErrors:
        -
            identifier: smallerOrEqual.alwaysFalse
            paths:
                - src/chunk.php
                - src/windowed.php

あるいはガード直上のインライン @phpstan-ignore でも同じ効果になります。こうすると HeadOptionTest / LastOptionTest の差分も不要になり、この PR が「関数 2 つの追加」に閉じます。

なお 81eb228 が独立コミットなので revert / 差し替えは容易です。全体無効化を選ぶ判断もあり得ます(この手の実行時ガードは今後も増えるはずなので)が、その場合は その理由を phpstan.neon にコメントで残しておくことをおすすめします。将来「なぜ level 10 なのに alwaysFalse が出ないのか」を追う人が必ず出るので。

Fix this →


2. windowed の「落とし穴パス」だけ結果アサーションが無い(小)

この PR の技術的な核は「assoc 入力 + MODE_LISTarray_slice の文字列キー保持を潰す」ことですが、そのパスと $partial = true(末尾の欠けた窓)が交差するケースの結果が固定されていません。

  • tests/WindowedTest.php:154 — assoc + MODE_LIST はあるが $partial = false
  • tests/WindowedTest.php:179 — assoc + MODE_LIST + $partial = true を通しているが、アサートしているのは入力の非破壊だけで戻り値を見ていない

末尾の欠けた窓は array_slice の残り要素が 1 個になる経路で、通常窓とは別に踏む価値があります:

public function testWindowedOnAssocInputWithListModeReindexesPartialWindow(): void
{
    $input = ['alice' => 20, 'bob' => 17];

    $result = windowed($input, 3, 1, true, Mode::MODE_LIST);

    $this->assertSame([[20, 17], [17]], $result);
}

Fix this →


3. 例外メッセージの $ はシングルクォート依存(nit)

src/chunk.php:27src/windowed.php:45,49

throw new \InvalidArgumentException('$size は 1 以上である必要があります。');

現状シングルクォートなので正しく動きますが、将来誰かがダブルクォートに変えると $size が値に展開されて 0 は 1 以上である必要があります。 になります。先例になるメッセージなので、'size は 1 以上である必要があります。' のように $ を落とすか、'引数 size は…' とするほうが事故りません。文体自体(「〜である必要があります。」)は素直で良いと思います。


4. PR 本文の「変更点」に漏れ(nit)

.claude/rules/40_output.md 的に、以下が本文に載っていません:

  • phpstan.neontreatPhpDocTypesAsCertain: false)— この PR で一番議論の余地がある変更なので、本文で理由を書いておくとレビューが速くなります
  • tests/HeadOptionTest.php / tests/LastOptionTest.php@phpstan-ignore-next-line 削除
  • テストケース数: 本文は 11 / 16 ですが、実際は ChunkTest 12 / WindowedTest 18 です(62b2005 以降のドリフト)

補足として同意した点

  • slice.php の指摘は正しいです。array_slice()$preserve_keys = false でも文字列キーを保持するので、slice(['a'=>1,'b'=>2], 0, 2, Mode::MODE_LIST)list<V> 宣言に反します。別 issue 化に賛成です。
    • ちなみに tail.php:14 / init.php:14@phpstan-param の条件型で「MODE_LIST のとき入力は list<V>」と縛っているため、この経路を型で塞いでいます。slice.php / map.php / unique.php にはそれが無い、という違いです。
    • 本 PR の chunk / windowed は「@phpstan-param で塞がず、実行時に正しく振り直す」方向を採っており、assoc 入力 + MODE_LIST を許す分こちらのほうが素直です。既存関数を揃えるならこの形が基準になりそうです。
  • windowed([1, 2], 3, 1, true)[[1, 2], [2]] の非直感さは、phpdoc の例(src/windowed.php:19)とテスト(tests/WindowedTest.php:47)の両方で固定されていて十分です。
  • \assert($window !== [])src/windowed.php:67)は array_values() の戻りを non-empty-list に絞るために必要で、コメントで意図も書かれています。zend.assertions=-1 で消える点も含めて問題ありません。
  • native_function_invocation@compiler_optimized 設定により \count / \array_slice\ が付き array_values / array_chunk には付かない、という混在は CS Fixer 由来で既存コードと一致しています。
  • invalidPositiveInt() ヘルパーの 2 ファイル重複は、テストヘルパー置き場がまだ無いので現状これでよいと思います(3 つ目が出たら trait 化)。

検証について

ローカルでの composer install / php 実行は権限で拒否されたため、Linter・PHPStan・PHPUnit を自分では走らせられませんでした。 代わりに CI の実行結果を確認しています。

ワークフロー 結果
Check (62b2005) ✅ success

.claude/rules/20_quality.md の 3 コマンドは CI 側で緑になっているため、PR 本文の実行結果(125 tests / PHPStan level 10 no errors / CS Fixer 0 fixed)は信頼できると判断しました。上記コマンドを自分で回すには --allowedToolsBash(composer install) / Bash(vendor/bin/*) の許可が必要です。

コード変更は行っていません(レビュー依頼のため)。
· ブランチ: javakky/chunk-windowed

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.

feat: windowed を追加する feat: chunk を追加する

1 participant