-
Notifications
You must be signed in to change notification settings - Fork 0
feat: throw UnwrapException with value context from unwrap()/unwrapErr() #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
4d2ca42
test: pin UnwrapException with value context for unwrap()/unwrapErr()
valbeat 67d62e8
feat: throw UnwrapException with value context from unwrap()/unwrapErr()
valbeat b9b473c
test: pin describe() edge cases found in code review
valbeat 336e8ae
fix: harden UnwrapException::describe() against review findings
valbeat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Valbeat\Result; | ||
|
|
||
| /** | ||
| * unwrap() / unwrapErr() を反対側の変種に対して呼び出したときに送出される例外です. | ||
| * | ||
| * \LogicException を継承しているため、既存の catch (\LogicException) はそのまま動作します. | ||
| * メッセージには保持している値の要約が含まれます(Rust の panic メッセージに相当). | ||
| * 注意: スカラー値はメッセージにそのまま(切り詰めの上)現れるため、機微な文字列を | ||
| * エラー値に載せる場合はログ出力先に注意してください. | ||
| */ | ||
| final class UnwrapException extends \LogicException | ||
| { | ||
| /** | ||
| * メッセージに埋め込む値要約の最大長(超過分は切り詰め). | ||
| */ | ||
| private const int MAX_SUMMARY_LENGTH = 120; | ||
|
|
||
| /** | ||
| * Err に対して unwrap() が呼ばれた場合の例外を生成します. | ||
| */ | ||
| public static function unwrapOnErr(mixed $error): self | ||
| { | ||
| return new self(\sprintf('called Result::unwrap() on an Err value: %s', self::describe($error))); | ||
| } | ||
|
|
||
| /** | ||
| * Ok に対して unwrapErr() が呼ばれた場合の例外を生成します. | ||
| */ | ||
| public static function unwrapErrOnOk(mixed $value): self | ||
| { | ||
| return new self(\sprintf('called Result::unwrapErr() on an Ok value: %s', self::describe($value))); | ||
| } | ||
|
|
||
| /** | ||
| * 例外メッセージ用に値の要約を生成します. | ||
| * | ||
| * 要約は単一行に正規化し、MAX_SUMMARY_LENGTH を超える部分は切り詰めます. | ||
| */ | ||
| private static function describe(mixed $value): string | ||
| { | ||
| $summary = match (true) { | ||
| $value instanceof \Throwable => \sprintf('%s: %s', self::className($value), $value->getMessage()), | ||
| $value instanceof \UnitEnum => \sprintf('%s::%s', $value::class, $value->name), | ||
| $value instanceof \Stringable => self::describeStringable($value), | ||
| \is_object($value) => self::className($value), | ||
| \is_scalar($value), null === $value => var_export($value, true), | ||
| default => get_debug_type($value), | ||
| }; | ||
|
|
||
| $summary = str_replace(["\r\n", "\r", "\n"], '\n', $summary); | ||
| if (\strlen($summary) > self::MAX_SUMMARY_LENGTH) { | ||
| return substr($summary, 0, self::MAX_SUMMARY_LENGTH) . '... (truncated)'; | ||
| } | ||
|
|
||
| return $summary; | ||
| } | ||
|
|
||
| /** | ||
| * Stringable の要約を生成します。__toString() が例外を投げてもこの例外を | ||
| * 置き換えないよう、失敗時はクラス名のみへフォールバックします. | ||
| */ | ||
| private static function describeStringable(\Stringable $value): string | ||
| { | ||
| try { | ||
| return \sprintf('%s: %s', self::className($value), (string) $value); | ||
| } catch (\Throwable) { | ||
| return self::className($value); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * クラス名を返します。匿名クラスはファイルパス・行番号を除いた | ||
| * 「Foo@anonymous」形式に正規化します. | ||
| */ | ||
| private static function className(object $value): string | ||
| { | ||
| $class = $value::class; | ||
| $pos = strpos($class, '@anonymous'); | ||
| if ($pos === false) { | ||
| return $class; | ||
| } | ||
|
|
||
| return substr($class, 0, $pos + \strlen('@anonymous')); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| use PHPUnit\Framework\TestCase; | ||
| use Valbeat\Result\Err; | ||
| use Valbeat\Result\Ok; | ||
| use Valbeat\Result\UnwrapException; | ||
|
|
||
| class OkTest extends TestCase | ||
| { | ||
|
|
@@ -89,6 +90,30 @@ public function unwrapErr_throws_exception(): void | |
| $ok->unwrapErr(); | ||
| } | ||
|
|
||
| #[Test] | ||
| public function unwrapErr_throwsUnwrapException_withValueInMessage(): void | ||
| { | ||
| $ok = new Ok(42); | ||
| $this->expectException(UnwrapException::class); | ||
| $this->expectExceptionMessage('called Result::unwrapErr() on an Ok value: 42'); | ||
| $ok->unwrapErr(); | ||
| } | ||
|
|
||
| #[Test] | ||
| public function unwrapErr_withStringableValue_includesClassAndString(): void | ||
| { | ||
| $value = new class () implements \Stringable { | ||
| public function __toString(): string | ||
| { | ||
| return 'stringable value'; | ||
| } | ||
| }; | ||
| $ok = new Ok($value); | ||
| $this->expectException(UnwrapException::class); | ||
| $this->expectExceptionMessage('stringable value'); | ||
| $ok->unwrapErr(); | ||
| } | ||
|
Comment on lines
+102
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. __toString() が例外をスローした場合でも、UnwrapException が正しくスローされ、クラス名にフォールバックされることを検証するテストケースを追加することをお勧めします。 #[Test]
public function unwrapErr_withStringableValue_includesClassAndString(): void
{
$value = new class () implements \\Stringable {
public function __toString(): string
{
return 'stringable value';
}
};
$ok = new Ok($value);
$this->expectException(UnwrapException::class);
$this->expectExceptionMessage('stringable value');
$ok->unwrapErr();
}
#[Test]
public function unwrapErr_withStringableValueThatThrows_fallsBackToClassName(): void
{
$value = new class () implements \\Stringable {
public function __toString(): string
{
throw new \\RuntimeException('toString failed');
}
};
$ok = new Ok($value);
$this->expectException(UnwrapException::class);
$this->expectExceptionMessage('called Result::unwrapErr() on an Ok value:');
$ok->unwrapErr();
} |
||
|
|
||
| #[Test] | ||
| public function unwrapOr_returns_value(): void | ||
| { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stringable インターフェースを実装したオブジェクトを文字列キャストする際、__toString() メソッド内で例外(Throwable)がスローされる可能性があります。
もし __toString() が例外をスローした場合、本来スローされるべき UnwrapException がその例外によって上書き(マスク)されてしまい、unwrap 失敗のデバッグが困難になります。
これを防ぐため、Stringable のキャスト処理を try-catch ブロックで囲み、例外が発生した場合はクラス名のみを返すようにフォールバックする設計に改善することをお勧めします。