Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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));
}
}
33 changes: 33 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-8048.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?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));
};
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);
}
}
31 changes: 31 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-8048.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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);
};
Loading