Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Type/ConditionalType.php
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ public function describe(VerbosityLevel $level): string

public function isResolvable(): bool
{
return !TypeUtils::containsTemplateType($this->subject) && !TypeUtils::containsTemplateType($this->target);
if (!TypeUtils::containsTemplateType($this->subject) && !TypeUtils::containsTemplateType($this->target)) {
return true;
}

$isSuperType = $this->target->isSuperTypeOf($this->subject);

return $isSuperType->yes() || $isSuperType->no();
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.

mutation testing suggests we miss tests for this line when $isSuperType is maybe

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pushed. Here's what I did to address staabm's review comment:

  1. Added NSRT test for maybe path (tests/PHPStan/Analyser/nsrt/bug-11894.php): A multi-template class method with @return (TArray is non-empty-array ? non-empty-list<TKey> : list<TKey>) called with a non-empty-array<int, int> argument. This verifies the correct type non-empty-list<int> is inferred. If isResolvable() incorrectly returned true for the maybe case, the conditional would resolve prematurely to the union of both branches (list<int>) — this test catches that mutation.

  2. Added rule test data for maybe cases: Added testMaybeRelation (functions) and testMaybeMethod/testMaybeStaticMethod (methods) that call the conditional return functions with T of string|int. These verify no false "unresolvable type" errors are reported for the maybe path, since TypeCombinator::remove(T of string|int, string) produces int (not never).

}

protected function getResult(): Type
Expand Down
70 changes: 70 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-11894.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
<?php // lint >= 8.0

declare(strict_types = 1);

namespace Bug11894Nsrt;

use function PHPStan\Testing\assertType;

/**
* @template T
* @param T $a
* @return (T is string ? string : T)
*/
function conditionalReturn(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}
return trim($a);
}

/**
* @template T of string|null
* @param T $a
*/
function testNarrowedToString(mixed $a): void
{
if (!is_string($a)) {
return;
}
assertType('string', conditionalReturn($a));
}

/**
* @template T of int|null
* @param T $a
*/
function testNarrowedToNonMatchingType(mixed $a): void
{
if (!is_int($a)) {
return;
}
assertType('T of int (function Bug11894Nsrt\testNarrowedToNonMatchingType(), argument)', conditionalReturn($a));
}

/**
* @template T of string|int
* @param T $a
*/
function testNotFullyNarrowable(mixed $a): void
{
assertType('string|T of int (function Bug11894Nsrt\testNotFullyNarrowable(), argument)', conditionalReturn($a));
}

abstract class ConditionalArrayKeys
{
/**
* @template TKey of array-key
* @template TArray of array<TKey, mixed>
* @param TArray $array
* @return (TArray is non-empty-array ? non-empty-list<TKey> : list<TKey>)
*/
abstract public function arrayKeys(array $array): array;

/** @param non-empty-array<int, int> $nonEmpty */
public function testMaybeStaysUnresolved(array $nonEmpty): void
{
assertType('non-empty-list<int>', $this->arrayKeys($nonEmpty));
}
}
35 changes: 35 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-8048.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php declare(strict_types=1);

namespace Bug8048Nsrt;

use function PHPStan\Testing\assertType;

interface CustomResponseInterface {}

class CustomResponse implements CustomResponseInterface {}

class ApiService
{
/**
* @template T of CustomResponseInterface
*
* @param class-string<T>|null $responseType
*
* @return ($responseType is class-string<T> ? T : null)
*/
public function request(?string $responseType = null): ?CustomResponseInterface
{
if ($responseType === null) {
return null;
}

return new CustomResponse();
}
}

function (): void {
assertType('null', (new ApiService())->request(null));
assertType('Bug8048Nsrt\CustomResponse', (new ApiService())->request(CustomResponse::class));
$x = rand(0, 1) ? CustomResponse::class : null;
assertType('Bug8048Nsrt\CustomResponse|null', (new ApiService())->request($x));
};
Original file line number Diff line number Diff line change
Expand Up @@ -2871,4 +2871,9 @@ public function testBug3842(): void
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-3842.php'], []);
}

public function testBug11894(): void
{
$this->analyse([__DIR__ . '/data/bug-11894.php'], []);
}

}
79 changes: 79 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-11894.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php declare(strict_types = 1);

namespace Bug11894;

/**
* @template T of string|null
* @param T $a
*/
function test(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}

return conditionalReturn($a);
}

/**
* @template T
* @param T $a
* @return (T is string ? string : T)
*/
function conditionalReturn(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}

return trim($a);
}

/**
* @template T of string|null
* @param T $a
*/
function testNegated(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}

return conditionalReturnNegated($a);
}

/**
* @template T
* @param T $a
* @return (T is not string ? T : string)
*/
function conditionalReturnNegated(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}

return trim($a);
}

/**
* @template T of int|null
* @param T $a
*/
function testNoRelation(mixed $a): mixed
{
if (!is_int($a)) {
return $a;
}

return conditionalReturn($a);
}

/**
* @template T of string|int
* @param T $a
*/
function testMaybeRelation(mixed $a): mixed
{
return conditionalReturn($a);
}
16 changes: 16 additions & 0 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4062,4 +4062,20 @@ public function testBug14549(): void
]);
}

public function testBug11894(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/bug-11894.php'], []);
}

public function testBug8048(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/bug-8048.php'], []);
}

}
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1015,4 +1015,10 @@ public function testPipeOperator(): void
]);
}

public function testBug11894(): void
{
$this->checkThisOnly = false;
$this->analyse([__DIR__ . '/data/bug-11894.php'], []);
}

}
81 changes: 81 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-11894.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php declare(strict_types = 1);

namespace Bug11894Methods;

class Converter
{
/**
* @template T
* @param T $a
* @return (T is string ? string : T)
*/
public function conditionalReturn(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}
return trim($a);
}

/**
* @template T
* @param T $a
* @return (T is string ? string : T)
*/
public static function conditionalReturnStatic(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}
return trim($a);
}
}

class Consumer
{
/**
* @template T of string|null
* @param T $a
*/
public function testMethod(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}

$c = new Converter();
return $c->conditionalReturn($a);
}

/**
* @template T of string|null
* @param T $a
*/
public function testStaticMethod(mixed $a): mixed
{
if (!is_string($a)) {
return $a;
}

return Converter::conditionalReturnStatic($a);
}

/**
* @template T of string|int
* @param T $a
*/
public function testMaybeMethod(mixed $a): mixed
{
$c = new Converter();
return $c->conditionalReturn($a);
}

/**
* @template T of string|int
* @param T $a
*/
public function testMaybeStaticMethod(mixed $a): mixed
{
return Converter::conditionalReturnStatic($a);
}
}
33 changes: 33 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-8048.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php declare(strict_types=1);

namespace Bug8048;

interface CustomResponseInterface {}

class CustomResponse implements CustomResponseInterface {}

class ApiService
{
/**
* @template T of CustomResponseInterface
*
* @param class-string<T>|null $responseType
*
* @return ($responseType is class-string<T> ? T : null)
*/
public function request(?string $responseType = null): ?CustomResponseInterface
{
if ($responseType === null) {
return null;
}

return new CustomResponse();
}
}

function (): void {
(new ApiService())->request(null);
(new ApiService())->request(CustomResponse::class);
$x = rand(0, 1) ? CustomResponse::class : null;
(new ApiService())->request($x);
};
Loading