Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
970ad6e
Added failling json test
clxmstaab Nov 1, 2021
008b659
cover encode
clxmstaab Nov 1, 2021
5db0592
fix
clxmstaab Nov 1, 2021
c10a75f
fix
clxmstaab Nov 1, 2021
4272b15
fix
clxmstaab Nov 1, 2021
9863492
fix CS
clxmstaab Nov 1, 2021
0bcb104
fix
clxmstaab Nov 1, 2021
c78a381
More testcoverage
staabm Nov 1, 2021
07c56f6
Update bug-5866.php
staabm Nov 1, 2021
659c55d
Update bug-5866.php
staabm Nov 1, 2021
7e3a86e
Update JsonThrowTypeExtension.php
staabm Nov 1, 2021
80ccc0f
fix readability
clxmstaab Nov 4, 2021
c0e77ec
re-order named arguments
clxmstaab Nov 5, 2021
169d096
Update JsonThrowTypeExtension.php
clxmstaab Nov 5, 2021
48d3907
Update JsonThrowTypeExtension.php
clxmstaab Nov 5, 2021
d62809f
append unknown args to the end
clxmstaab Nov 5, 2021
0dca6f5
fix
clxmstaab Nov 5, 2021
82c4932
cs
clxmstaab Nov 5, 2021
566d355
moved reorderNamedArguments() into NodeScopeResolver
clxmstaab Nov 5, 2021
e84696c
cs
clxmstaab Nov 5, 2021
bb9753b
adjusted tests so all 3 cases fail at master
clxmstaab Nov 5, 2021
c3f5ddf
fix
clxmstaab Nov 5, 2021
faa19fa
Update JsonThrowTypeExtension.php
staabm Nov 5, 2021
aed44ca
cs
clxmstaab Nov 5, 2021
da52130
extract NamedArgumentsHelper
clxmstaab Nov 5, 2021
c664c2d
support named arguments in DynamicFunctionReturnTypeExtension
clxmstaab Nov 5, 2021
8a83aeb
fix cs
clxmstaab Nov 5, 2021
01e4e7e
refactor NamedArgumentsHelper
clxmstaab Nov 5, 2021
e60b57e
implement DynamicMethodThrowTypeExtension, DynamicStaticMethodThrowTy…
clxmstaab Nov 5, 2021
766c682
fix
clxmstaab Nov 5, 2021
8619d48
fix min php version
clxmstaab Nov 5, 2021
4caa639
dynamicMethodReturnTypeExtension and dynamicStaticMethodReturnTypeExt…
clxmstaab Nov 5, 2021
6033152
cs
clxmstaab Nov 5, 2021
03a2baf
cs
clxmstaab Nov 5, 2021
12cd852
added tests and default-value resolving
clxmstaab Nov 7, 2021
0ceb2b2
assert existing default value for optional parameters
clxmstaab Nov 13, 2021
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
13 changes: 8 additions & 5 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -2296,6 +2296,13 @@ private function resolveType(Expr $node): Type
}

$functionReflection = $this->reflectionProvider->getFunction($node->name, $this);
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
$this,
$node->getArgs(),
$functionReflection->getVariants()
);
$node = NamedArgumentsHelper::reorderFuncArguments($parametersAcceptor, $node);

foreach ($this->dynamicReturnTypeExtensionRegistry->getDynamicFunctionReturnTypeExtensions() as $dynamicFunctionReturnTypeExtension) {
if (!$dynamicFunctionReturnTypeExtension->isFunctionSupported($functionReflection)) {
continue;
Expand All @@ -2304,11 +2311,7 @@ private function resolveType(Expr $node): Type
return $dynamicFunctionReturnTypeExtension->getTypeFromFunctionCall($functionReflection, $node, $this);
}

return ParametersAcceptorSelector::selectFromArgs(
$this,
$node->getArgs(),
$functionReflection->getVariants()
)->getReturnType();
return $parametersAcceptor->getReturnType();
}

return new MixedType();
Expand Down
94 changes: 94 additions & 0 deletions src/Analyser/NamedArgumentsHelper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PhpParser\Node\Arg;
use PhpParser\Node\Expr\CallLike;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Reflection\ParametersAcceptor;
use PHPStan\ShouldNotHappenException;

final class NamedArgumentsHelper

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Really nice! I'd like this class to have separate unit tests so that we can see what happens to those gaps between arguments if an optional one is skipped. To me it looks like this code still doesn't handle that.

{

public static function reorderFuncArguments(
ParametersAcceptor $parametersAcceptor,
FuncCall $functionCall
): FuncCall
{
return new FuncCall(
$functionCall->name,
self::reorderArgs($parametersAcceptor, $functionCall),
$functionCall->getAttributes()
);
}

public static function reorderMethodArguments(
ParametersAcceptor $parametersAcceptor,
MethodCall $methodCall
): MethodCall
{
return new MethodCall(
$methodCall->var,
$methodCall->name,
self::reorderArgs($parametersAcceptor, $methodCall),
$methodCall->getAttributes()
);
}

public static function reorderStaticCallArguments(
ParametersAcceptor $parametersAcceptor,
StaticCall $staticCall
): StaticCall
{
return new StaticCall(
$staticCall->class,
$staticCall->name,
self::reorderArgs($parametersAcceptor, $staticCall),
$staticCall->getAttributes()
);
}

/**
* @return array<int, Arg>
*/
private static function reorderArgs(ParametersAcceptor $parametersAcceptor, CallLike $callLike): array
{
$signatureParameters = $parametersAcceptor->getParameters();
$callArgs = $callLike->getArgs();

$reorderedArgs = [];
$argumentPositions = [];
foreach ($signatureParameters as $i => $parameter) {
$argumentPositions[$parameter->getName()] = $i;

if ($parameter->isOptional()) {
$defaultValue = $parameter->getDefaultValue();
if ($defaultValue === null) {
// a possible "null"-default value is indicated by NullType
throw new ShouldNotHappenException('A optional parameter must have a default value');
}
$reorderedArgs[$i] = new Arg(
$defaultValue
);
}
}

foreach ($callArgs as $i => $arg) {
if ($arg->name === null) {
// add regular args as is
$reorderedArgs[$i] = $arg;
} elseif (array_key_exists($arg->name->toString(), $argumentPositions)) {
// order named args into the position the signature expects them
$reorderedArgs[$argumentPositions[$arg->name->toString()]] = $arg;
}
}

ksort($reorderedArgs);

return $reorderedArgs;
}

}
8 changes: 8 additions & 0 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -2001,6 +2001,8 @@ function (MutatingScope $scope) use ($expr, $nodeCallback, $context): Expression
$expr->getArgs(),
$methodReflection->getVariants()
);

$expr = NamedArgumentsHelper::reorderMethodArguments($parametersAcceptor, $expr);
$methodThrowPoint = $this->getMethodThrowPoint($methodReflection, $parametersAcceptor, $expr, $scope);
if ($methodThrowPoint !== null) {
$throwPoints[] = $methodThrowPoint;
Expand Down Expand Up @@ -2085,6 +2087,8 @@ static function () use ($scope, $expr): MutatingScope {
$expr->getArgs(),
$methodReflection->getVariants()
);

$expr = NamedArgumentsHelper::reorderStaticCallArguments($parametersAcceptor, $expr);
$methodThrowPoint = $this->getStaticMethodThrowPoint($methodReflection, $expr, $scope);
if ($methodThrowPoint !== null) {
$throwPoints[] = $methodThrowPoint;
Expand Down Expand Up @@ -2661,6 +2665,10 @@ private function getFunctionThrowPoint(
MutatingScope $scope
): ?ThrowPoint
{
if ($parametersAcceptor !== null) {
$funcCall = NamedArgumentsHelper::reorderFuncArguments($parametersAcceptor, $funcCall);
}

foreach ($this->dynamicThrowTypeExtensionProvider->getDynamicFunctionThrowTypeExtensions() as $extension) {
if (!$extension->isFunctionSupported($functionReflection)) {
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ class DynamicMethodThrowTypeExtensionTest extends TypeInferenceTestCase
public function dataFileAsserts(): iterable
{
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-throw-type-extension.php');

if (PHP_VERSION_ID < 80000) {
return;
}

yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-throw-type-extension-named-args.php');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

even though there is a non-php8 early return in the line before, this test-case seems to run on php 7.3/7.4

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ class DynamicReturnTypeExtensionTypeInferenceTest extends TypeInferenceTestCase
public function dataAsserts(): iterable
{
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-types.php');

if (PHP_VERSION_ID >= 80000) {
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-types-named-args.php');
}

yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-compound-types.php');
}

Expand Down
94 changes: 94 additions & 0 deletions tests/PHPStan/Analyser/NamedArgumentsHelperTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<?php declare(strict_types = 1);

namespace PHPStan\Analyser;

use PhpParser\Node\Arg;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Scalar\String_;
use PHPStan\Reflection\ParametersAcceptor;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\SignatureMap\NativeFunctionReflectionProvider;

final class NamedArgumentsHelperTest extends \PHPStan\Testing\PHPStanTestCase {

@staabm staabm Nov 7, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

initial round of unit tests for the new NamedArgumentsHelper.

after the 2nd test - which does not pass atm - is fixed, I will add more test-cases

/**
* all arguments named and given in order
*/
public function testArgumentReorderAllNamed() {
$funcName = new Name('json_encode');
$reflectionProvider = self::getContainer()->getByType(NativeFunctionReflectionProvider::class);
$functionReflection = $reflectionProvider->findFunctionReflection('json_encode');
$parameterAcceptor = ParametersAcceptorSelector::selectSingle($functionReflection->getVariants());

$args = [
new Arg(
new LNumber(0),
false,
false,
[],
new Identifier('flags')
),
new Arg(
new String_('my json value'),
false,
false,
[],
new Identifier('value')
)
];
$funcCall = new FuncCall($funcName, $args);

$funcCall = NamedArgumentsHelper::reorderFuncArguments($parameterAcceptor, $funcCall);
$reorderedArgs = $funcCall->getArgs();

$this->assertArrayHasKey(0 , $reorderedArgs);
$this->assertSame('value', $reorderedArgs[0]->name->toString());

$this->assertArrayHasKey(1 , $reorderedArgs);
$this->assertSame('flags', $reorderedArgs[1]->name->toString());
}

/**
* all args named, not in order
*/
public function testArgumentReorderAllNamedWithSkipped() {
$funcName = new Name('json_encode');
$reflectionProvider = self::getContainer()->getByType(NativeFunctionReflectionProvider::class);
$functionReflection = $reflectionProvider->findFunctionReflection('json_encode');
$parameterAcceptor = ParametersAcceptorSelector::selectSingle($functionReflection->getVariants());

$args = [
new Arg(
new LNumber(128),
false,
false,
[],
new Identifier('depth')
),
new Arg(
new String_('my json value'),
false,
false,
[],
new Identifier('value')
)
];
$funcCall = new FuncCall($funcName, $args);

$funcCall = NamedArgumentsHelper::reorderFuncArguments($parameterAcceptor, $funcCall);
$reorderedArgs = $funcCall->getArgs();

$this->assertArrayHasKey(0 , $reorderedArgs);
$this->assertSame('value', $reorderedArgs[0]->name->toString());

$this->assertArrayHasKey(1 , $reorderedArgs);
$this->assertSame('flags', $reorderedArgs[1]->name->toString());
$this->assertInstanceOf(LNumber::class, $reorderedArgs[1]->value);
$this->assertSame(0, $reorderedArgs[1]->value->value);

$this->assertArrayHasKey(2 , $reorderedArgs);
$this->assertSame('depth', $reorderedArgs[2]->name->toString());
}
}
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public function dataFileAsserts(): iterable

require_once __DIR__ . '/data/instanceof.php';

yield from $this->gatherAssertTypes(__DIR__ . '/data/named-arguments.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/date.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/instanceof.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/integer-range-types.php');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php // lint >= 8.0

namespace DynamicMethodReturnTypesNamespace;

use function PHPStan\Testing\assertType;

class FooNamedArgs
{

public function __construct()
{
}

public function doFoo()
{
$em = new EntityManager();
$iem = new InheritedEntityManager();

assertType('DynamicMethodReturnTypesNamespace\Foo', $em->getByPrimary(className: \DynamicMethodReturnTypesNamespace\Foo::class));
assertType('DynamicMethodReturnTypesNamespace\Foo', $iem->getByPrimary(className: \DynamicMethodReturnTypesNamespace\Foo::class));

assertType('DynamicMethodReturnTypesNamespace\Foo', \DynamicMethodReturnTypesNamespace\EntityManager::createManagerForEntity(className: \DynamicMethodReturnTypesNamespace\Foo::class));
assertType('DynamicMethodReturnTypesNamespace\Foo', \DynamicMethodReturnTypesNamespace\InheritedEntityManager::createManagerForEntity(className: \DynamicMethodReturnTypesNamespace\Foo::class));
}

}
Loading