Skip to content

Commit 7d50b73

Browse files
rcalicdanrcalicdan
committed
Refactor error handling in GeneratorChecker to return ErrorMessage instead of throwing TypeError; enhance DocblockNormalizer to auto-complete return types for callables; add NestedAggregateService for testing; improve tests for GeneratorChecker and LazyIterators.
1 parent 0a0efdc commit 7d50b73

11 files changed

Lines changed: 236 additions & 120 deletions

File tree

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@
55
docs/.vitepress/cache
66
docs/.vitepress/dist
77
/manual-tests
8-
composer.lock
8+
composer.lock
9+
index.php

‎src/Internal/Checker/GeneratorChecker.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public static function checkSend(string $function, mixed $sendValue, TypeValidat
2929
if ($sendTypeNode !== null) {
3030
$err = $registry->validate($sendValue, $sendTypeNode, "$function(): Generator sent value (TSend)");
3131
if ($err !== null) {
32-
throw new \TypePHP\Exception\TypeError($err->getMessage());
32+
return $err;
3333
}
3434
}
3535
}
@@ -64,14 +64,14 @@ public static function checkYield(string $function, mixed $key, mixed $value, Ty
6464
if ($key !== null && $keyTypeNode !== null) {
6565
$err = $registry->validate($key, $keyTypeNode, "$function(): Return iterator key");
6666
if ($err !== null) {
67-
throw new \TypePHP\Exception\TypeError($err->getMessage());
67+
return $err;
6868
}
6969
}
7070

7171
if ($itemTypeNode !== null) {
7272
$err = $registry->validate($value, $itemTypeNode, "$function(): Return iterator value");
7373
if ($err !== null) {
74-
throw new \TypePHP\Exception\TypeError($err->getMessage());
74+
return $err;
7575
}
7676
}
7777

‎src/Internal/DocblockNormalizer.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ final class DocblockNormalizer
2929
public static function normalize(string $doc): string
3030
{
3131
$doc = preg_replace('/(@(?:phpstan|psalm)-type\s+[a-zA-Z0-9_\x80-\xff]+)\s*=\s*/', '$1 ', $doc) ?? $doc;
32-
32+
$doc = preg_replace('/(callable|Closure)\s*\(([^)]*)\)(?!\s*:)/', '$1($2): mixed', $doc) ?? $doc;
3333
$doc = preg_replace('/(\\\\?[a-zA-Z_\x80-\xff][\\\\a-zA-Z0-9_\x80-\xff]*::[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\s*(\??:)/', '"$1"$2', $doc) ?? $doc;
3434

3535
if (! str_contains($doc, '{')) {
@@ -52,4 +52,4 @@ function (array $matches): string {
5252
$doc
5353
) ?? $doc;
5454
}
55-
}
55+
}

‎src/Internal/ErrorFactory.php‎

Lines changed: 38 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
namespace TypePHP\Internal;
66

7+
use ReflectionClass;
8+
use Throwable;
9+
use TypeError;
10+
711
/**
812
* @internal Factory creating ErrorMessage value objects and preparing TypeError instances with exact caller traces.
913
*/
@@ -23,22 +27,21 @@ public static function createError(string $message): ErrorMessage
2327

2428
/**
2529
* Prepares a TypeError exception before throwing.
26-
* For parameter and callback argument errors, it filters out internal library frames
30+
* For parameter, callback, iterator, and generator errors, it filters out internal library frames
2731
* and sets the file and line to accurately blame the caller site.
2832
*/
29-
public static function prepareException(\TypeError $e, ?int $line = null): \TypeError
33+
public static function prepareException(TypeError $e, ?int $line = null): TypeError
3034
{
31-
$ref = new \ReflectionObject($e);
32-
33-
if ($line !== null && $ref->hasProperty('line')) {
34-
$propLine = $ref->getProperty('line');
35-
$propLine->setValue($e, $line);
36-
}
35+
$targetFile = null;
36+
$targetLine = $line;
3737

3838
$message = $e->getMessage();
3939
$isCallSiteError = str_contains($message, 'Argument $')
4040
|| str_contains($message, 'argument #')
41-
|| str_contains($message, 'Callback argument');
41+
|| str_contains($message, 'Callback ')
42+
|| str_contains($message, 'Iterator $')
43+
|| str_contains($message, 'Return iterator')
44+
|| str_contains($message, 'Generator sent value');
4245

4346
if ($isCallSiteError) {
4447
$trace = $e->getTrace();
@@ -47,15 +50,16 @@ public static function prepareException(\TypeError $e, ?int $line = null): \Type
4750
if (isset($frame['file'], $frame['line'])) {
4851
$file = str_replace('\\', '/', $frame['file']);
4952

50-
if (! str_contains($file, 'Internal/ErrorFactory.php') && ! str_contains($file, 'Wrapper/CallableWrapper.php')) {
51-
if ($ref->hasProperty('file')) {
52-
$propFile = $ref->getProperty('file');
53-
$propFile->setValue($e, $frame['file']);
54-
}
53+
$isInternal = str_contains($file, 'src/Internal/')
54+
|| str_contains($file, 'src/Wrapper/')
55+
|| str_contains($file, 'src/Validator/')
56+
|| str_contains($file, 'src/Resolver/')
57+
|| str_contains($file, 'src/Contract/');
5558

56-
if ($line === null && $ref->hasProperty('line')) {
57-
$propLine = $ref->getProperty('line');
58-
$propLine->setValue($e, $frame['line']);
59+
if (! $isInternal) {
60+
$targetFile = $frame['file'];
61+
if ($targetLine === null) {
62+
$targetLine = $frame['line'];
5963
}
6064

6165
break;
@@ -64,6 +68,22 @@ public static function prepareException(\TypeError $e, ?int $line = null): \Type
6468
}
6569
}
6670

71+
try {
72+
$ref = new ReflectionClass(\Error::class);
73+
74+
if ($targetFile !== null) {
75+
$propFile = $ref->getProperty('file');
76+
$propFile->setValue($e, $targetFile);
77+
}
78+
79+
if ($targetLine !== null) {
80+
$propLine = $ref->getProperty('line');
81+
$propLine->setValue($e, $targetLine);
82+
}
83+
} catch (Throwable $err) {
84+
// Silently fallback if reflection mutation fails
85+
}
86+
6787
return $e;
6888
}
69-
}
89+
}

‎src/Internal/Visitor/FunctionContractInjector.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ public static function inject(Node\Stmt\Function_|Node\Stmt\ClassMethod $node):
4949
$isNativeVoid = $node->returnType instanceof Node\Identifier && strtolower($node->returnType->name) === 'void';
5050
$hasThis = $isClassMethod && ! $node->isStatic();
5151

52-
// Pass $this for instance methods, static::class for static methods, or null for global functions
5352
$thisArg = $hasThis
5453
? new Node\Expr\Variable('this')
5554
: ($isClassMethod ? new Node\Expr\ClassConstFetch(new Node\Name('static'), 'class') : new Node\Expr\ConstFetch(new Node\Name('null')));
@@ -259,6 +258,7 @@ public function enterNode(Node $n): int|Node|null
259258
]
260259
)
261260
),
261+
new Node\Arg(new Node\Scalar\LNumber($n->getStartLine())),
262262
]
263263
)
264264
),

‎src/Wrapper/IterableWrapper.php‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace TypePHP\Wrapper;
66

7+
use Generator;
78
use PHPStan\PhpDocParser\Ast\Type\ArrayTypeNode;
89
use PHPStan\PhpDocParser\Ast\Type\GenericTypeNode;
910
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
@@ -20,16 +21,15 @@ final class IterableWrapper
2021
{
2122
/**
2223
* Wraps Traversable iterators and Generators to lazily validate keys and values during iteration.
23-
*
24-
* Performs the following steps:
25-
* 1. Resolves key and item TypeNodes from contract metadata or aliases.
26-
* 2. Constructs a callback to evaluate key and value type constraints.
27-
* 3. Wraps Traversable objects with IteratorProxy for rewindability and method forwarding.
28-
* 4. Wraps Generators with an interceptor generator to evaluate yielded items lazily.
2924
*/
3025
public static function wrap(string $function, string $paramName, mixed $iterable, TypeValidatorRegistry $registry): mixed
3126
{
32-
if (! is_iterable($iterable) || \is_array($iterable)) {
27+
if (! is_iterable($iterable)) {
28+
return $iterable;
29+
}
30+
31+
// Preserve native PHP arrays for all standard function parameters!
32+
if (\is_array($iterable) && $paramName !== 'return') {
3333
return $iterable;
3434
}
3535

@@ -56,7 +56,14 @@ public static function wrap(string $function, string $paramName, mixed $iterable
5656
$prefix = ($paramName === 'return') ? "$function(): Return iterator" : "$function(): Iterator \$$paramName";
5757
$typeCheckCallback = self::createValidationCallback($registry, $keyTypeNode, $itemTypeNode, $prefix);
5858

59-
if (! ($iterable instanceof \Generator)) {
59+
// Wrap delegated yield from arrays in a lazy generator
60+
if (\is_array($iterable)) {
61+
return self::wrapGenerator((function () use ($iterable) {
62+
yield from $iterable;
63+
})(), $typeCheckCallback);
64+
}
65+
66+
if (! ($iterable instanceof Generator)) {
6067
return new IteratorProxy($iterable, $typeCheckCallback);
6168
}
6269

@@ -128,13 +135,13 @@ private static function createValidationCallback(
128135
* @param iterable<mixed, mixed> $iterable
129136
* @param \Closure(mixed, mixed): void $typeCheckCallback
130137
*
131-
* @return \Generator<mixed, mixed>
138+
* @return Generator<mixed, mixed>
132139
*/
133-
private static function wrapGenerator(iterable $iterable, \Closure $typeCheckCallback): \Generator
140+
private static function wrapGenerator(iterable $iterable, \Closure $typeCheckCallback): Generator
134141
{
135142
foreach ($iterable as $key => $value) {
136143
$typeCheckCallback($key, $value);
137144
yield $key => $value;
138145
}
139146
}
140-
}
147+
}

‎tests/Fixtures/Services/NestedAggregateService.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace TypePHP\Tests\Fixtures\Services;
6+
7+
use ArrayIterator;
8+
use Countable;
9+
use IteratorAggregate;
10+
use Traversable;
11+
12+
class NestedAggregateService implements IteratorAggregate, Countable
13+
{
14+
/**
15+
* @param array<string, int> $items
16+
*/
17+
public function __construct(
18+
private array $items = ['alpha' => 10, 'beta' => 20]
19+
) {
20+
}
21+
22+
public function getIterator(): Traversable
23+
{
24+
return new class ($this->items) implements IteratorAggregate {
25+
public function __construct(private array $data)
26+
{
27+
}
28+
29+
public function getIterator(): Traversable
30+
{
31+
return new ArrayIterator($this->data);
32+
}
33+
};
34+
}
35+
36+
public function count(): int
37+
{
38+
return \count($this->items);
39+
}
40+
41+
public function getCustomMetadata(): string
42+
{
43+
return 'custom_metadata_string';
44+
}
45+
}

‎tests/Internal/DocblockNormalizerTest.php‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,28 @@
1010
expect(DocblockNormalizer::normalize($doc))->toBe($doc);
1111
});
1212

13+
test('auto-completes omitted return types for callable and Closure signatures', function () {
14+
$doc1 = '/** @var callable(int[] $items) $callback */';
15+
$expected1 = '/** @var callable(int[] $items): mixed $callback */';
16+
expect(DocblockNormalizer::normalize($doc1))->toBe($expected1);
17+
18+
$doc2 = '/** @param Closure(string $name) $closure */';
19+
$expected2 = '/** @param Closure(string $name): mixed $closure */';
20+
expect(DocblockNormalizer::normalize($doc2))->toBe($expected2);
21+
22+
$doc3 = '/** @param callable() $emptyCallable */';
23+
$expected3 = '/** @param callable(): mixed $emptyCallable */';
24+
expect(DocblockNormalizer::normalize($doc3))->toBe($expected3);
25+
});
26+
27+
test('preserves existing return types on callable signatures untouched', function () {
28+
$doc1 = '/** @param callable(int): string $cb */';
29+
expect(DocblockNormalizer::normalize($doc1))->toBe($doc1);
30+
31+
$doc2 = '/** @param Closure(int, string): bool $closure */';
32+
expect(DocblockNormalizer::normalize($doc2))->toBe($doc2);
33+
});
34+
1335
test('strips optional equals sign from @phpstan-type and @psalm-type tags', function () {
1436
$doc1 = '/** @phpstan-type MetricTypeValues = "histogram"|"gauge" */';
1537
$expected1 = '/** @phpstan-type MetricTypeValues "histogram"|"gauge" */';
@@ -104,4 +126,4 @@
104126

105127
expect(DocblockNormalizer::normalize($doc))->toBe($expected);
106128
});
107-
});
129+
});

‎tests/RuntimeChecker/GeneratorCheckerTest.php‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
declare(strict_types=1);
44

55
use TypePHP\Internal\Checker\GeneratorChecker;
6+
use TypePHP\Internal\ErrorMessage;
67
use TypePHP\Validator\TypeValidatorRegistry;
78

89
/**
@@ -22,11 +23,13 @@ function sampleGeneratorFixture(): Generator
2223
expect($result)->toBe(10);
2324
});
2425

25-
test('checkYield throws TypeError on invalid yielded value', function () {
26+
test('checkYield returns ErrorMessage on invalid yielded value', function () {
2627
$registry = new TypeValidatorRegistry();
2728

28-
expect(fn () => GeneratorChecker::checkYield('sampleGeneratorFixture', 'a', -50, $registry))
29-
->toThrow(TypeError::class, 'Return iterator value')
29+
$result = GeneratorChecker::checkYield('sampleGeneratorFixture', 'a', -50, $registry);
30+
31+
expect($result)->toBeInstanceOf(ErrorMessage::class)
32+
->and($result->getMessage())->toContain('Return iterator value')
3033
;
3134
});
3235

@@ -38,11 +41,13 @@ function sampleGeneratorFixture(): Generator
3841
expect($result)->toBe(100);
3942
});
4043

41-
test('checkSend throws TypeError on invalid TSend input value', function () {
44+
test('checkSend returns ErrorMessage on invalid TSend input value', function () {
4245
$registry = new TypeValidatorRegistry();
4346

44-
expect(fn () => GeneratorChecker::checkSend('sampleGeneratorFixture', -500, $registry))
45-
->toThrow(TypeError::class, 'Generator sent value (TSend)')
47+
$result = GeneratorChecker::checkSend('sampleGeneratorFixture', -500, $registry);
48+
49+
expect($result)->toBeInstanceOf(ErrorMessage::class)
50+
->and($result->getMessage())->toContain('Generator sent value (TSend)')
4651
;
4752
});
48-
});
53+
});

0 commit comments

Comments
 (0)