-
Notifications
You must be signed in to change notification settings - Fork 576
WIP: Support named arguments #750
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.
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
Changes from all commits
970ad6e
008b659
5db0592
c10a75f
4272b15
9863492
0bcb104
c78a381
07c56f6
659c55d
7e3a86e
80ccc0f
c0e77ec
169d096
48d3907
d62809f
0dca6f5
82c4932
566d355
e84696c
bb9753b
c3f5ddf
faa19fa
aed44ca
da52130
c664c2d
8a83aeb
01e4e7e
e60b57e
766c682
8619d48
4caa639
6033152
03a2baf
12cd852
0ceb2b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||
| { | ||
|
|
||
| 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; | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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'); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| } | ||
|
|
||
| /** | ||
|
|
||
| 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 { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. initial round of unit tests for the new 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()); | ||
| } | ||
| } | ||
| 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)); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
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.