Skip to content

Normalize arguments before calling into TypeSpecifyingExtensions #4034

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 30, 2025
Merged

Normalize arguments before calling into TypeSpecifyingExtensions #4034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ec8af7
Normalize arguments before calling into TypeSpecifyingExtensions
staabm May 30, 2025
bcd8a54
Create bug-13088.php
staabm May 30, 2025
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
20 changes: 20 additions & 0 deletions src/Analyser/ArgumentsNormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ public static function reorderFuncArguments(
return null;
}

// return identical object if not reordered, as TypeSpecifier relies on object identity
Copy link
Contributor Author

Choose a reason for hiding this comment

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

related to this comment is this line:

phpstan-src/src/Rules/Comparison/ImpossibleCheckTypeHelper.php

Lines 317 to 319 in dc8c831

if ($expr === $node) {
return true;
}

without this fix the actual fix in TypeSpecifier would cause a regression:

➜  phpstan-src git:(2.1.x) ✗ vendor/bin/phpunit tests/PHPStan/Analyser/AnalyserIntegrationTest.php  --filter testBug7500
PHPUnit 9.6.23 by Sebastian Bergmann and contributors.

Random Seed:   1748588377
Warning:       XDEBUG_MODE=coverage (environment variable) or xdebug.mode=coverage (PHP configuration setting) has to be set

F                                                                   1 / 1 (100%)

Time: 00:46.162, Memory: 90.00 MB

There was 1 failure:

1) PHPStan\Analyser\AnalyserIntegrationTest::testBug7500
Failed asserting that actual size 1 matches expected size 0.

Emitted errors:
- Call to function ksort() with array<T of Bug7500\PositionEntityInterface&Bug7500\TgEntityInterface> will always evaluate to true
  in /Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/data/bug-7500.php on line 36

/Users/staabm/workspace/phpstan-src/src/Testing/PHPStanTestCase.php:228
/Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/AnalyserIntegrationTest.php:921

FAILURES!
Tests: 1, Assertions: 3, Failures: 1.

if ($reorderedArgs === $functionCall->getArgs()) {
return $functionCall;
}

return new FuncCall(
$functionCall->name,
$reorderedArgs,
Expand All @@ -116,6 +121,11 @@ public static function reorderMethodArguments(
return null;
}

// return identical object if not reordered, as TypeSpecifier relies on object identity
if ($reorderedArgs === $methodCall->getArgs()) {
return $methodCall;
}

return new MethodCall(
$methodCall->var,
$methodCall->name,
Expand All @@ -135,6 +145,11 @@ public static function reorderStaticCallArguments(
return null;
}

// return identical object if not reordered, as TypeSpecifier relies on object identity
if ($reorderedArgs === $staticCall->getArgs()) {
return $staticCall;
}

return new StaticCall(
$staticCall->class,
$staticCall->name,
Expand All @@ -154,6 +169,11 @@ public static function reorderNewArguments(
return null;
}

// return identical object if not reordered, as TypeSpecifier relies on object identity
if ($reorderedArgs === $new->getArgs()) {
return $new;
}

return new New_(
$new->class,
$reorderedArgs,
Expand Down
42 changes: 33 additions & 9 deletions src/Analyser/TypeSpecifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,15 @@ public function specifyTypesInCondition(

} elseif ($expr instanceof FuncCall && $expr->name instanceof Name) {
if ($this->reflectionProvider->hasFunction($expr->name, $scope)) {
// lazy create parametersAcceptor, as creation can be expensive
$parametersAcceptor = null;

$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
if (count($expr->getArgs()) > 0) {
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $functionReflection->getVariants(), $functionReflection->getNamedArgumentsVariants());
$expr = ArgumentsNormalizer::reorderFuncArguments($parametersAcceptor, $expr) ?? $expr;
}

foreach ($this->getFunctionTypeSpecifyingExtensions() as $extension) {
if (!$extension->isFunctionSupported($functionReflection, $expr, $context)) {
continue;
Expand All @@ -485,10 +493,10 @@ public function specifyTypesInCondition(
return $extension->specifyTypes($functionReflection, $expr, $scope, $context);
}

// lazy create parametersAcceptor, as creation can be expensive
$parametersAcceptor = null;
if (count($expr->getArgs()) > 0) {
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $functionReflection->getVariants(), $functionReflection->getNamedArgumentsVariants());
if ($parametersAcceptor === null) {
throw new ShouldNotHappenException();
}

$specifiedTypes = $this->specifyTypesFromConditionalReturnType($context, $expr, $parametersAcceptor, $scope);
if ($specifiedTypes !== null) {
Expand Down Expand Up @@ -518,6 +526,14 @@ public function specifyTypesInCondition(
$methodCalledOnType = $scope->getType($expr->var);
$methodReflection = $scope->getMethodReflection($methodCalledOnType, $expr->name->name);
if ($methodReflection !== null) {
// lazy create parametersAcceptor, as creation can be expensive
$parametersAcceptor = null;

if (count($expr->getArgs()) > 0) {
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $methodReflection->getVariants(), $methodReflection->getNamedArgumentsVariants());
$expr = ArgumentsNormalizer::reorderMethodArguments($parametersAcceptor, $expr) ?? $expr;
}

$referencedClasses = $methodCalledOnType->getObjectClassNames();
if (
count($referencedClasses) === 1
Expand All @@ -533,10 +549,10 @@ public function specifyTypesInCondition(
}
}

// lazy create parametersAcceptor, as creation can be expensive
$parametersAcceptor = null;
if (count($expr->getArgs()) > 0) {
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $methodReflection->getVariants(), $methodReflection->getNamedArgumentsVariants());
if ($parametersAcceptor === null) {
throw new ShouldNotHappenException();
}

$specifiedTypes = $this->specifyTypesFromConditionalReturnType($context, $expr, $parametersAcceptor, $scope);
if ($specifiedTypes !== null) {
Expand Down Expand Up @@ -571,6 +587,14 @@ public function specifyTypesInCondition(

$staticMethodReflection = $scope->getMethodReflection($calleeType, $expr->name->name);
if ($staticMethodReflection !== null) {
// lazy create parametersAcceptor, as creation can be expensive
$parametersAcceptor = null;

if (count($expr->getArgs()) > 0) {
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $staticMethodReflection->getVariants(), $staticMethodReflection->getNamedArgumentsVariants());
$expr = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $expr) ?? $expr;
}

$referencedClasses = $calleeType->getObjectClassNames();
if (
count($referencedClasses) === 1
Expand All @@ -586,10 +610,10 @@ public function specifyTypesInCondition(
}
}

// lazy create parametersAcceptor, as creation can be expensive
$parametersAcceptor = null;
if (count($expr->getArgs()) > 0) {
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs($scope, $expr->getArgs(), $staticMethodReflection->getVariants(), $staticMethodReflection->getNamedArgumentsVariants());
if ($parametersAcceptor === null) {
throw new ShouldNotHappenException();
}

$specifiedTypes = $this->specifyTypesFromConditionalReturnType($context, $expr, $parametersAcceptor, $scope);
if ($specifiedTypes !== null) {
Expand Down
23 changes: 23 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-13088.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php // lint >= 8.0

namespace Bug13088;

use function PHPStan\dumpType;
use function PHPStan\Testing\assertType;

class HelloWorld
{
public function sayHello(string $s, int $offset): void
{
if (preg_match('~msgstr "(.*)"\n~', $s, $matches, 0, $offset) === 1) {
assertType('array{non-falsy-string, string}', $matches);
}
}

public function sayHello2(string $s, int $offset): void
{
if (preg_match('~msgstr "(.*)"\n~', $s, $matches, offset: $offset) === 1) {
assertType('array{non-falsy-string, string}', $matches);
}
}
}
Loading