feat: add expect() and expectErr() throwing UnwrapException#97
Conversation
expect(string $message) returns the success value or throws LogicException with the caller-supplied message; expectErr() is the mirror for the error side, matching Rust's Result::expect/expect_err. Currently RED: the methods do not exist yet. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
Rust's Result::expect/expect_err equivalents: return the value or throw LogicException with a caller-supplied message that explains why the value was expected to be present. Conditional return types match unwrap()/unwrapErr() (never on the impossible side). Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
Stack this PR on #94 per review, resolving overlapping test and README hunks. expect()/expectErr() switch to UnwrapException in the following commits. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
Review feedback: expect()/expectErr() throwing bare LogicException while unwrap()/unwrapErr() throw UnwrapException leaves an inconsistent contract — catch (UnwrapException) would miss expect() failures — and drops the value context Rust's expect includes. Currently RED. Claude-Session: https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
WalkthroughResult型のOk/ErrクラスおよびResultインターフェイスに Changesexpect/expectErr API追加
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant OkOrErr as Ok/Err
participant UnwrapException
Caller->>OkOrErr: expect(message) / expectErr(message)
alt 期待側の値を保持
OkOrErr-->>Caller: 値を返す
else 期待側でない
OkOrErr->>UnwrapException: withMessage(message, value)
UnwrapException-->>Caller: 例外throw
end
Related Issues: 特に明記なし Poem(レビューの一句):
レビューコメント(建設的指摘):
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 54 59 +5
===========================================
Files 3 3
Lines 107 117 +10
===========================================
+ Hits 107 117 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
There was a problem hiding this comment.
Code Review
This pull request introduces the expect and expectErr methods to the Result interface, along with their implementations in the Ok and Err classes. These methods allow extracting values or throwing an UnwrapException with a custom message and a summary of the contained value. Corresponding unit tests and static analysis type assertions have been added to verify their behavior and type resolution. There are no review comments to address, and I have no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
Rust の Result::expect / expect_err 相当として、Result::expect(string $message) と Result::expectErr(string $message) を追加し、失敗時は unwrap() 系と同じ UnwrapException(呼び出し側メッセージ + 値要約)を投げるようにする PR です。
Changes:
Resultインターフェースにexpect()/expectErr()を追加(条件付き戻り値型 +@throws UnwrapException)Ok/Errにexpect()/expectErr()実装を追加し、UnwrapException::withMessage()で統一した例外生成を導入- テスト/型テスト/README を更新して新 API を反映
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Types/result.php | expect() / expectErr() の条件付き戻り値型が unwrap() 系と同様に解決されることを型テストで固定 |
| tests/OkTest.php | Ok::expect() の正常系と Ok::expectErr() の例外系を追加 |
| tests/ErrTest.php | Err::expect() の例外系と Err::expectErr() の正常系を追加 |
| src/UnwrapException.php | expect() / expectErr() 用の例外生成ファクトリ withMessage() を追加 |
| src/Result.php | expect() / expectErr() をインターフェースに追加(phpdoc で型/throws を定義) |
| src/Ok.php | expect() / expectErr() を実装(expectErr() は UnwrapException::withMessage()) |
| src/Err.php | expect() / expectErr() を実装(expect() は UnwrapException::withMessage()) |
| README.md | Value Extraction に expect() / expectErr() を追記 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[Test] | ||
| public function expectErr_throws_withGivenMessage(): void | ||
| { | ||
| $ok = new Ok(42); | ||
| $this->expectException(\LogicException::class); | ||
| $this->expectExceptionMessage('should have an error'); | ||
| $ok->expectErr('should have an error'); | ||
| } |
| #[Test] | ||
| public function expect_throws_withGivenMessage(): void | ||
| { | ||
| $err = new Err('error'); | ||
| $this->expectException(\LogicException::class); | ||
| $this->expectExceptionMessage('config file should be readable'); | ||
| $err->expect('config file should be readable'); | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/ErrTest.php`:
- Around line 62-77: The ErrTest coverage is missing combined scenarios for
expect/expectErr with Result combinators and nested Result values. Add tests
around Err::expect and Err::expectErr that exercise map, andThen, and orElse
interactions, plus nested Ok/Err cases, so the behavior is verified beyond the
current standalone success and exception tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6b591625-dfa4-4933-9cf0-0feaead15a0e
📒 Files selected for processing (8)
README.mdsrc/Err.phpsrc/Ok.phpsrc/Result.phpsrc/UnwrapException.phptests/ErrTest.phptests/OkTest.phptests/Types/result.php
| #[Test] | ||
| public function expect_throws_withGivenMessage(): void | ||
| { | ||
| $err = new Err('error'); | ||
| $this->expectException(\LogicException::class); | ||
| $this->expectExceptionMessage('config file should be readable'); | ||
| $err->expect('config file should be readable'); | ||
| } | ||
|
|
||
| #[Test] | ||
| public function expectErr_returns_error_value(): void | ||
| { | ||
| $err = new Err('error'); | ||
| $this->assertSame('error', $err->expectErr('should have an error')); | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
コンビネータ・ネストとの組み合わせテストの追加を検討
expect/expectErr 単体の成功・例外系は網羅されていますが、map/andThen/orElse などのコンビネータと組み合わせた場合や、ネストした Result に対する expect/expectErr の挙動を検証するテストが見当たりません。既存の unwrap/unwrapErr にも同種のテストは無いため必須ではありませんが、Rust の Result API に倣うなら組み合わせ利用が想定されるため、余裕があれば追加を検討してください。
As per path instructions, テストは「エッジケース(Ok/Err 双方、ネスト、map/and_then などのコンビネータ)が網羅されているかを見る」必要があります。
Also applies to: 165-173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/ErrTest.php` around lines 62 - 77, The ErrTest coverage is missing
combined scenarios for expect/expectErr with Result combinators and nested
Result values. Add tests around Err::expect and Err::expectErr that exercise
map, andThen, and orElse interactions, plus nested Ok/Err cases, so the behavior
is verified beyond the current standalone success and exception tests.
Source: Path instructions
概要
#95 の後継 PR です(#94 のブランチ削除に伴い #95 が自動クローズされ、クローズ済み PR は base 変更・再オープンともに不可のため作り直し)。内容・ブランチ・コミット履歴は #95 と同一で、#94 が main に入ったため差分は expect 分のみです。
Rust の
Result::expect/expect_err相当を追加します。レビュー指摘を反映済みで、unwrap()系と同じUnwrapException(呼び出し側メッセージ + 値の要約)を投げ、catch (UnwrapException)が unwrap 系 4 メソッドすべてをカバーします。変更内容
Result::expect(string $message)/Result::expectErr(string $message)(インターフェース + Ok / Err 実装、@throws UnwrapException付き)UnwrapException::withMessage(string, mixed)ファクトリを追加unwrap()系と同一(型テストでピン留め)TDD
テスト先行(RED 確認 → コミット → 実装)。85 tests / PHPStan max / cs-fixer すべてローカルでパス済み。
https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK
Summary by CodeRabbit
Resultに、成功時/失敗時の値を指定メッセージ付きで確認できるexpect()/expectErr()が追加されました。Ok/Errで期待値の取得と例外発生の挙動が統一されました。