Skip to content

Commit 4b1d5dc

Browse files
ondrejmirtesphpstan-bot
ondrejmirtes
authored and
phpstan-bot
committed
Fix false positive argument.type for invariant generics with nullable types
- Fixed false positive where passing Foo<string|null> to a parameter expecting Foo<string|null> reported an error at rule levels 5-7 - Root cause: RuleLevelHelper::transformAcceptedType strips null from inside generic type arguments of the accepted type, but the accepting type retains its nulls, causing the invariance check to compare string|null vs string - Fix in GenericObjectType::isSuperTypeOfInternal: when in accepts context, if the invariance check fails, verify whether the difference is only due to null stripping by checking if the types are equivalent modulo null - New regression test in tests/PHPStan/Rules/Methods/data/bug-10698.php Fixes phpstan/phpstan#10698
1 parent 9156e51 commit 4b1d5dc

4 files changed

Lines changed: 58 additions & 1 deletion

File tree

‎CLAUDE.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,14 @@ Bugs occur when impure points are missed (e.g. inherited constructors of anonymo
335335
- **Regression tests**: For each bug fix, add a test data file reproducing the issue (e.g. `tests/PHPStan/Rules/*/data/bug-12345.php` or `tests/PHPStan/Analyser/nsrt/bug-12345.php`).
336336
- **Integration tests**: `AnalyserIntegrationTest` runs full analysis on test files and checks error output.
337337

338+
### RuleLevelHelper transformAcceptedType and asymmetric null stripping in generics
339+
340+
`RuleLevelHelper::transformAcceptedType()` uses `TypeTraverser::map` to strip `null` from the accepted type when `checkNullables=false` (levels < 8). Because `GenericObjectType::traverse` recurses into inner type arguments, null gets stripped from inside generic type arguments too (e.g. `Foo<string|null>` → `Foo<string>`). The accepting type is NOT transformed the same way, so for invariant template parameters, `TemplateTypeVariance::isValidVariance()` compares `string|null` (accepting) vs `string` (accepted) using `equals()`, which fails — producing false positives where the error message shows identical types on both sides.
341+
342+
The fix is in `GenericObjectType::isSuperTypeOfInternal()`: when in `$acceptsContext` (the `accepts()` path), if the invariance check fails, also check whether the mismatch is solely due to null stripping by verifying `TypeCombinator::removeNull($this->types[$i])` equals the ancestor type. This approach avoids modifying `RuleLevelHelper` or `TemplateTypeVariance` and keeps the fix localized.
343+
344+
**Key insight**: Fixes to `RuleLevelHelper::transformAcceptedType` null stripping can have unintended effects on callable parameter types (which have contravariant parameters). Changes should be tested against `testCallablesWithoutCheckNullables` and `testBug4801`.
345+
338346
### Adding support for new PHP versions
339347

340348
Recent work on PHP 8.5 support shows the pattern:

‎src/Type/Generic/GenericObjectType.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use PHPStan\Type\IsSuperTypeOfResult;
2121
use PHPStan\Type\ObjectType;
2222
use PHPStan\Type\Type;
23+
use PHPStan\Type\TypeCombinator;
2324
use PHPStan\Type\TypeWithClassName;
2425
use PHPStan\Type\UnionType;
2526
use PHPStan\Type\VerbosityLevel;
@@ -186,7 +187,17 @@ private function isSuperTypeOfInternal(Type $type, bool $acceptsContext): IsSupe
186187
if (!$thisVariance->invariant()) {
187188
$results[] = $thisVariance->isValidVariance($templateType, $this->types[$i], $ancestor->types[$i]);
188189
} else {
189-
$results[] = $templateType->isValidVariance($this->types[$i], $ancestor->types[$i]);
190+
$result = $templateType->isValidVariance($this->types[$i], $ancestor->types[$i]);
191+
if ($acceptsContext && !$result->yes()) {
192+
$thisWithoutNull = TypeCombinator::removeNull($this->types[$i]);
193+
if (
194+
$this->types[$i]->isSuperTypeOf($ancestor->types[$i])->yes()
195+
&& $ancestor->types[$i]->isSuperTypeOf($thisWithoutNull)->yes()
196+
) {
197+
$result = IsSuperTypeOfResult::createYes();
198+
}
199+
}
200+
$results[] = $result;
190201
}
191202

192203
$results[] = IsSuperTypeOfResult::createFromBoolean($thisVariance->validPosition($ancestorVariance));

‎tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3852,4 +3852,12 @@ public function testBug13805(): void
38523852
$this->analyse([__DIR__ . '/data/bug-13805.php'], []);
38533853
}
38543854

3855+
public function testBug10698(): void
3856+
{
3857+
$this->checkThisOnly = false;
3858+
$this->checkNullables = false;
3859+
$this->checkUnionTypes = true;
3860+
$this->analyse([__DIR__ . '/data/bug-10698.php'], []);
3861+
}
3862+
38553863
}

‎tests/PHPStan/Rules/Methods/data/bug-10698.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug10698;
4+
5+
/** @template T */
6+
class Foo {
7+
/** @param T $subject */
8+
public function __construct(private $subject) {}
9+
10+
/** @return T */
11+
public function getSubject() {
12+
return $this->subject;
13+
}
14+
}
15+
16+
class Bar {
17+
/**
18+
* @template T
19+
* @param Foo<T> $foo
20+
* @return T
21+
*/
22+
public function qux(Foo $foo) {
23+
return $foo->getSubject();
24+
}
25+
}
26+
27+
function x(?string $str): void {
28+
$bar = new Bar();
29+
$bar->qux(new Foo($str));
30+
}

0 commit comments

Comments
 (0)