Skip to content

Commit 25c4e64

Browse files
rcalicdanrcalicdan
authored
Internal improvements 12 (#45)
* Enhance ContractParser to filter out 'mixed' types and improve handling of parameter contracts; update FunctionContractInjector to prevent double wrapping of return checks; add tests for mixed parameters and return validation * Enhance ContractParser to simplify unions/intersections containing 'mixed' types; refactor FunctionContractInjector to improve parameter and return contract checks * Fix PHPstan errors
1 parent ac2a84f commit 25c4e64

6 files changed

Lines changed: 173 additions & 15 deletions

File tree

‎src/Contract/ContractParser.php‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,13 @@ private static function parseFunction(\ReflectionFunction $ref): array
550550
$type = new ArrayTypeNode($type);
551551
}
552552
$substitutedType = self::substituteAliases($type, $aliases);
553-
$types[$paramName] = SpecialTypeResolver::resolve($substitutedType, $ref);
553+
$resolvedType = SpecialTypeResolver::resolve($substitutedType, $ref);
554+
555+
if ($resolvedType instanceof IdentifierTypeNode && strtolower($resolvedType->name) === 'mixed') {
556+
continue;
557+
}
558+
559+
$types[$paramName] = $resolvedType;
554560
}
555561

556562
$returnTag = DocblockExtractor::getReturnTag($phpDocNode);
@@ -696,7 +702,13 @@ private static function parseMethodHierarchyDocs(
696702
$type = new ArrayTypeNode($type);
697703
}
698704
$substitutedType = self::substituteAliases($type, $aliases);
699-
$types[$targetParamName] = SpecialTypeResolver::resolve($substitutedType, $hierRef);
705+
$resolvedType = SpecialTypeResolver::resolve($substitutedType, $hierRef);
706+
707+
if ($resolvedType instanceof IdentifierTypeNode && strtolower($resolvedType->name) === 'mixed') {
708+
continue;
709+
}
710+
711+
$types[$targetParamName] = $resolvedType;
700712
}
701713
}
702714

@@ -790,15 +802,19 @@ private static function applyConstructorPromotionFallback(\ReflectionMethod $ref
790802
) {
791803
$propType = new ArrayTypeNode($propType);
792804
}
793-
$types[$paramName] = self::substituteAliases($propType, []);
805+
$substitutedProp = self::substituteAliases($propType, []);
806+
if ($substitutedProp instanceof IdentifierTypeNode && strtolower($substitutedProp->name) === 'mixed') {
807+
continue;
808+
}
809+
$types[$paramName] = $substitutedProp;
794810
}
795811
}
796812
}
797813
}
798814
}
799815

800816
/**
801-
* Recursively substitutes all type aliases inside a TypeNode AST.
817+
* Recursively substitutes all type aliases inside a TypeNode AST and simplifies unions/intersections containing `mixed`.
802818
*
803819
* @param array<string, TypeNode> $aliases
804820
*/
@@ -864,17 +880,39 @@ public static function substituteAliases(TypeNode $node, array $aliases): TypeNo
864880
}
865881

866882
if ($node instanceof UnionTypeNode) {
867-
return new UnionTypeNode(array_map(
883+
$types = array_map(
868884
fn ($t) => self::substituteAliases($t, $aliases),
869885
$node->types
870-
));
886+
);
887+
888+
foreach ($types as $t) {
889+
if ($t instanceof IdentifierTypeNode && strtolower($t->name) === 'mixed') {
890+
return new IdentifierTypeNode('mixed');
891+
}
892+
}
893+
894+
return new UnionTypeNode($types);
871895
}
872896

873897
if ($node instanceof IntersectionTypeNode) {
874-
return new IntersectionTypeNode(array_map(
898+
$types = array_map(
875899
fn ($t) => self::substituteAliases($t, $aliases),
876900
$node->types
877-
));
901+
);
902+
903+
$filtered = array_values(array_filter($types, function ($t) {
904+
return ! ($t instanceof IdentifierTypeNode && strtolower($t->name) === 'mixed');
905+
}));
906+
907+
if (\count($filtered) === 0) {
908+
return new IdentifierTypeNode('mixed');
909+
}
910+
911+
if (\count($filtered) === 1) {
912+
return $filtered[0];
913+
}
914+
915+
return new IntersectionTypeNode($filtered);
878916
}
879917

880918
if ($node instanceof ArrayShapeNode) {
@@ -901,4 +939,4 @@ public static function substituteAliases(TypeNode $node, array $aliases): TypeNo
901939

902940
return $node;
903941
}
904-
}
942+
}

‎src/Internal/ContractVisitor.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ public function enterNode(Node $node): array|int|null
7878
$effectiveVarName = ($varName !== '') ? $varName : 'return';
7979
$checkCall = NodeBuilder::createVariableCheckCall($node->expr, $typeString, $effectiveVarName);
8080
$node->expr = NodeBuilder::createTernaryThrowExpr($checkCall, $node->getStartLine());
81+
$node->setAttribute('typephp_var_wrapped', true);
8182
}
8283
}
8384
}

‎src/Internal/Visitor/FunctionContractInjector.php‎

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public static function inject(Node\Stmt\Function_|Node\Stmt\ClassMethod $node):
3636
$methodName = $isClassMethod ? strtolower($node->name->toString()) : '';
3737
$isMagicLifecycle = $isClassMethod && \in_array($methodName, ['__construct', '__destruct', '__clone'], true);
3838

39-
$hasParam = $isClassMethod || str_contains($docText, '@param') || str_contains($docText, '@phpstan-param') || str_contains($docText, '@psalm-param');
40-
$hasReturn = ! $isMagicLifecycle && ($isClassMethod || str_contains($docText, '@return') || str_contains($docText, '@phpstan-return') || str_contains($docText, '@psalm-return'));
39+
$hasParam = self::hasParamContracts($docText, $isClassMethod);
40+
$hasReturn = ! $isMagicLifecycle && self::hasReturnContracts($docText, $isClassMethod);
4141

4242
if (! $hasParam && ! $hasReturn) {
4343
return;
@@ -61,6 +61,67 @@ public static function inject(Node\Stmt\Function_|Node\Stmt\ClassMethod $node):
6161
$node->stmts = [...$injectedStmts, ...$node->stmts];
6262
}
6363

64+
private static function hasParamContracts(string $docText, bool $isClassMethod): bool
65+
{
66+
if ($isClassMethod) {
67+
return true;
68+
}
69+
70+
if (! str_contains($docText, '@param') && ! str_contains($docText, '@phpstan-param') && ! str_contains($docText, '@psalm-param') && ! str_contains($docText, '@template')) {
71+
return false;
72+
}
73+
74+
if (str_contains($docText, '@template') || str_contains($docText, '@phpstan-param') || str_contains($docText, '@psalm-param')) {
75+
return true;
76+
}
77+
78+
if ((int) preg_match_all('/@param\s+([^\s$]+)/', $docText, $matches) > 0) {
79+
foreach ($matches[1] as $typeStr) {
80+
$unionParts = explode('|', $typeStr);
81+
$hasMixed = false;
82+
foreach ($unionParts as $part) {
83+
if (strtolower(trim($part)) === 'mixed') {
84+
$hasMixed = true;
85+
break;
86+
}
87+
}
88+
89+
if (! $hasMixed) {
90+
return true;
91+
}
92+
}
93+
94+
return false;
95+
}
96+
97+
return false;
98+
}
99+
100+
private static function hasReturnContracts(string $docText, bool $isClassMethod): bool
101+
{
102+
if ($isClassMethod) {
103+
return true;
104+
}
105+
106+
if (str_contains($docText, '@template') || str_contains($docText, '@phpstan-return') || str_contains($docText, '@psalm-return')) {
107+
return true;
108+
}
109+
110+
if (preg_match('/@return\s+([^\s$]+)/', $docText, $matches) === 1) {
111+
$returnTypeStr = $matches[1];
112+
$unionParts = explode('|', $returnTypeStr);
113+
foreach ($unionParts as $part) {
114+
if (strtolower(trim($part)) === 'mixed') {
115+
return false; // Collapses to mixed
116+
}
117+
}
118+
119+
return true;
120+
}
121+
122+
return false;
123+
}
124+
64125
private static function shouldSkipInjection(string $docText): bool
65126
{
66127
$shouldRespectIgnore = (bool) (Config::get()['respect_ignore_tags'] ?? true);
@@ -556,6 +617,10 @@ public function enterNode(Node $n): int|array|null
556617
}
557618

558619
if ($n instanceof Node\Stmt\Return_) {
620+
if ($n->getAttribute('typephp_var_wrapped') === true) {
621+
return null;
622+
}
623+
559624
$exprToWrap = $n->expr ?? new Node\Expr\ConstFetch(new Node\Name('null'));
560625
$checkCall = FunctionContractInjector::buildReturnCheckCall($exprToWrap, $this->thisArg, $this->needsReturnVars);
561626

@@ -588,4 +653,4 @@ public function enterNode(Node $n): int|array|null
588653

589654
return $newStmts;
590655
}
591-
}
656+
}

‎tests/TypeChecking/Boundaries/InlineReturnValidationTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
declare(strict_types=1);
44

5+
use TypePHP\Internal\StreamWrapper;
6+
57
/**
68
* Function with broad return type, but specific inline @var on return statement
79
*/
@@ -64,4 +66,22 @@ function testInlineVarOnReturnInClosure(): array
6466
->toThrow(TypeError::class, 'positive-int')
6567
;
6668
});
69+
70+
test('inline @var on return statement is not double wrapped with checkReturn in AST', function () {
71+
$source = <<<'PHP'
72+
<?php
73+
74+
function sampleSingleWrapReturn(): string
75+
{
76+
/** @var non-empty-string */
77+
return 'hello_world';
78+
}
79+
PHP;
80+
81+
$transformed = StreamWrapper::transformSource($source, 'test_single_wrap.php');
82+
83+
expect($transformed)->toContain('RuntimeTypeChecker::checkVariable')
84+
->and($transformed)->not()->toContain('checkReturn(__METHOD__, ($__typephpVal = \TypePHP\Internal\RuntimeTypeChecker::checkVariable')
85+
;
86+
});
6787
});

‎tests/TypeChecking/Boundaries/ParamContractsTest.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
declare(strict_types=1);
44

5+
use TypePHP\Contract\ContractParser;
56
use TypePHP\Tests\Fixtures\Domain\Car;
67
use TypePHP\Tests\Fixtures\Domain\Dog;
78
use TypePHP\Tests\Fixtures\Services\VariadicPropertyService;
@@ -59,6 +60,17 @@ function testProcessIntKeyGenerator(iterable $items): array
5960
return $out;
6061
}
6162

63+
/**
64+
* Function with purely mixed parameters
65+
*
66+
* @param mixed $data
67+
* @param mixed $meta
68+
*/
69+
function testPureMixedParamFunction(mixed $data, mixed $meta): bool
70+
{
71+
return true;
72+
}
73+
6274
describe('Function & Method Parameter Contracts', function () {
6375
test('inherits variadic constructor parameter contracts from property @var array docblocks without double-wrapping', function () {
6476
$service = new VariadicPropertyService(['tag1', 'tag2'], new Dog(), new Dog());
@@ -100,6 +112,16 @@ function testProcessIntKeyGenerator(iterable $items): array
100112
->toThrow(TypeError::class, 'Argument $strings[3] must be of type string')
101113
;
102114
});
115+
116+
test('filters out pure mixed parameters so hasParamContract is false', function () {
117+
$contract = ContractParser::parse('testPureMixedParamFunction');
118+
119+
expect($contract['types'])->toBeEmpty()
120+
->and($contract['hasParamContract'])->toBeFalse()
121+
;
122+
123+
expect(testPureMixedParamFunction('anything', 12345))->toBeTrue();
124+
});
103125
});
104126

105127
describe('Lazy Wrapped Callable Parameter Contracts', function () {
@@ -155,6 +177,7 @@ function testProcessIntKeyGenerator(iterable $items): array
155177
};
156178

157179
expect(fn () => testProcessIntKeyGenerator($badKeyGenerator()))
158-
->toThrow(TypeError::class, 'Iterator $items key');
180+
->toThrow(TypeError::class, 'Iterator $items key')
181+
;
159182
});
160183
});

‎typephp.php‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*/
4343
'respect_ignore_tags' => true,
4444

45-
/*
45+
/*
4646
|--------------------------------------------------------------------------
4747
| Enable Caching & Cache Directory
4848
|--------------------------------------------------------------------------
@@ -67,7 +67,7 @@
6767
// \Acme\Domain\TypePHPExtension::class,
6868
],
6969

70-
/*
70+
/*
7171
|--------------------------------------------------------------------------
7272
| Array Validation Strategy
7373
|--------------------------------------------------------------------------
@@ -106,6 +106,17 @@
106106
'objects' => true,
107107
],
108108

109+
/*
110+
|--------------------------------------------------------------------------
111+
| Stub Files (DocBlock Overrides for Third-Party & Vendor Packages)
112+
|--------------------------------------------------------------------------
113+
| Path globs or specific file paths containing stub files (.stub, .stub.php, .php)
114+
| that override inaccurate or missing DocBlocks in third-party vendor packages.
115+
*/
116+
'stubs' => [
117+
// 'stubs/**',
118+
],
119+
109120
/*
110121
|--------------------------------------------------------------------------
111122
| Included Paths & Whitelisting

0 commit comments

Comments
 (0)