Skip to content

feat: add expect() and expectErr()#95

Closed
valbeat wants to merge 5 commits into
feat/unwrap-exceptionfrom
feat/expect-methods
Closed

feat: add expect() and expectErr()#95
valbeat wants to merge 5 commits into
feat/unwrap-exceptionfrom
feat/expect-methods

Conversation

@valbeat

@valbeat valbeat commented Jul 7, 2026

Copy link
Copy Markdown
Owner

概要

コードレビューで提案した Rust API とのギャップ解消の 1 つ目、expect() / expectErr() の追加です。

unwrap() と違い、呼び出し側が「なぜ値があるはずなのか」を説明するメッセージを付けられるため、失敗時の原因調査が容易になります(Rust で常用されるパターン)。

$config = loadConfig()->expect('config.json should exist and be valid');

変更内容

  • Result::expect(string $message) — 成功値を返す。Err なら $messageLogicException を throw
  • Result::expectErr(string $message) — エラー値を返す。Ok なら $messageLogicException を throw
  • 条件付き戻り値型は unwrap()/unwrapErr() と同じ(不可能側は never、具象 Ok<T>/Err<E> レシーバで正確な型)
  • README API Reference に追記

備考

  • Rust の expect はメッセージに加えてエラー値の Debug 表現も含めます。feat: throw UnwrapException with value context from unwrap()/unwrapErr() #94(UnwrapException)がマージされたら、この 2 メソッドも同例外+値要約付きメッセージに揃える follow-up を推奨します(独立してレビューできるよう本 PR では素の LogicException に留めています)。

TDD

  1. 挙動テスト 4 件 + 型テスト(条件付き戻り値型の assertType)を先に追加し RED を確認してコミット
  2. 実装して GREEN: 73 tests / PHPStan max / cs-fixer すべてパス

https://claude.ai/code/session_017XTM7pxbWPVNLV639i5WgK

valbeat added 2 commits July 7, 2026 13:10
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
@valbeat valbeat self-assigned this Jul 7, 2026
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • main

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e94397a6-0a8e-4ebc-9866-802d96b7aa47

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
  • Commit unit tests in branch feat/expect-methods

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.

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (98a2fcb) to head (ed90782).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##                main       #95   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        40        44    +4     
===========================================
  Files              2         2           
  Lines             80        88    +8     
===========================================
+ Hits              80        88    +8     
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

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

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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the expect and expectErr methods to the Result interface and its implementations, Ok and Err. These methods allow users to extract values or throw a LogicException with a custom error message. The changes are well-documented in the README, fully tested with unit tests, and include static analysis type assertions to ensure correct type resolution. There are no review comments, 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.

@valbeat valbeat marked this pull request as ready for review July 7, 2026 04:35
Copilot AI review requested due to automatic review settings July 7, 2026 04:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Rust-like expect() / expectErr() APIs to the Result abstraction to let callers attach contextual failure messages when extracting Ok/Err values.

Changes:

  • Add Result::expect(string $message) and Result::expectErr(string $message) with conditional return types matching unwrap() / unwrapErr().
  • Implement the methods in Ok and Err (return value on the “correct” variant; throw LogicException($message) on the “wrong” variant).
  • Add runtime tests and PHPStan type-level assertions, and update the README API reference.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/Types/result.php Adds PHPStan assertType() coverage for expect() / expectErr() conditional return types on a generic Result<T,E> receiver.
tests/OkTest.php Adds behavior tests for Ok->expect() returning the value and Ok->expectErr() throwing with the provided message.
tests/ErrTest.php Adds behavior tests for Err->expect() throwing with the provided message and Err->expectErr() returning the error value.
src/Result.php Extends the Result interface with expect() / expectErr() and their conditional return types.
src/Ok.php Implements expect() (returns stored value) and expectErr() (throws LogicException($message)).
src/Err.php Implements expect() (throws LogicException($message)) and expectErr() (returns stored error).
README.md Documents expect() / expectErr() in the API Reference “Value Extraction” list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread README.md Outdated
Comment on lines 207 to 209
- `expect(string $message): mixed` - Returns the success value or throws LogicException with the given message
- `expectErr(string $message): mixed` - Returns the error value or throws LogicException with the given message
- `unwrapOr(mixed $default): mixed` - Returns the success value or a default
@valbeat valbeat changed the base branch from main to feat/unwrap-exception July 7, 2026 04:40
valbeat added 3 commits July 7, 2026 13:46
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
@valbeat

valbeat commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

レビュー指摘への対応: この PR を #94 の上にスタックし(base を feat/unwrap-exception に変更、マージコミットで統合)、expect() / expectErr()UnwrapException(メッセージ + 値の要約)を投げるよう統一しました。catch (UnwrapException) が unwrap 系 4 メソッドすべてをカバーし、予告していた follow-up は不要になります。#94 のマージ後にこの PR をマージしてください#94 のブランチ削除時に base は自動で main に付け替わります)。

@valbeat valbeat deleted the branch feat/unwrap-exception July 7, 2026 06:34
@valbeat valbeat closed this Jul 7, 2026
@valbeat

valbeat commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

#94 のブランチ削除に伴い自動クローズされました(クローズ済み PR は base 変更不可)。同一ブランチ・同一内容の後継 PR を main ベースで作成しています → 上記タイムライン参照。

@valbeat valbeat deleted the feat/expect-methods branch July 7, 2026 06:39
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