Skip to content
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

implemented constant-type inference in ImplodeFunctionReturnTypeExtension #991

Merged
merged 3 commits into from Feb 6, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 33 additions & 14 deletions src/Type/Php/ImplodeFunctionReturnTypeExtension.php
Expand Up @@ -7,11 +7,15 @@
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Type\Accessory\AccessoryLiteralStringType;
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ConstantScalarType;
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use function count;
use function implode;
use function in_array;

class ImplodeFunctionReturnTypeExtension implements DynamicFunctionReturnTypeExtension
Expand All @@ -35,20 +39,7 @@ public function getTypeFromFunctionCall(
if (count($args) === 1) {
$argType = $scope->getType($args[0]->value);
if ($argType->isArray()->yes()) {
$accessoryTypes = [];
if ($argType->isIterableAtLeastOnce()->yes() && $argType->getIterableValueType()->isNonEmptyString()->yes()) {
$accessoryTypes[] = new AccessoryNonEmptyStringType();
}
if ($argType->getIterableValueType()->isLiteralString()->yes()) {
$accessoryTypes[] = new AccessoryLiteralStringType();
}

if (count($accessoryTypes) > 0) {
$accessoryTypes[] = new StringType();
return new IntersectionType($accessoryTypes);
}

return new StringType();
Comment on lines -38 to -51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this code was duplicated in the extension. de-duplicated the existing logic with the first commit with 180b2af

return $this->implode($argType, new ConstantStringType(''));
}
}

Expand All @@ -58,6 +49,19 @@ public function getTypeFromFunctionCall(

$separatorType = $scope->getType($args[0]->value);
$arrayType = $scope->getType($args[1]->value);

return $this->implode($arrayType, $separatorType);
}

private function implode(Type $arrayType, Type $separatorType): Type
{
if ($arrayType instanceof ConstantArrayType && $separatorType instanceof ConstantStringType) {
$constantType = $this->inferConstantType($arrayType, $separatorType);
if ($constantType !== null) {
return $constantType;
}
}

$accessoryTypes = [];
if ($arrayType->isIterableAtLeastOnce()->yes()) {
if ($arrayType->getIterableValueType()->isNonEmptyString()->yes() || $separatorType->isNonEmptyString()->yes()) {
Expand All @@ -77,4 +81,19 @@ public function getTypeFromFunctionCall(
return new StringType();
}

private function inferConstantType(ConstantArrayType $arrayType, ConstantStringType $separatorType): ?Type
{
$valueTypes = $arrayType->getValueTypes();

$arrayValues = [];
foreach ($valueTypes as $valueType) {
if (!$valueType instanceof ConstantScalarType) {
return null;
}
$arrayValues[] = $valueType->getValue();
}

return new ConstantStringType(implode($separatorType->getValue(), $arrayValues));
}

}
3 changes: 3 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -14,6 +14,9 @@ class NodeScopeResolverTest extends TypeInferenceTestCase

public function dataFileAsserts(): iterable
{
require_once __DIR__ . '/data/implode.php';
yield from $this->gatherAssertTypes(__DIR__ . '/data/implode.php');

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

yield from $this->gatherAssertTypes(__DIR__ . '/data/bug2574.php');
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/data/bug-5219.php
Expand Up @@ -7,9 +7,9 @@
class HelloWorld
{

protected function foo(string $message): void
protected function foo(string $message, string $x): void
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adjusted the existing test, so it still leads to the same result as before this PR.

{
$header = sprintf('%s-%s', '', implode('-', ['x']));
$header = sprintf('%s-%s', '', implode('-', [$x]));

assertType('non-empty-string', $header);
assertType('non-empty-array<non-empty-string, string>', [$header => $message]);
Expand Down
24 changes: 24 additions & 0 deletions tests/PHPStan/Analyser/data/implode.php
@@ -0,0 +1,24 @@
<?php declare(strict_types = 1);

namespace ImplodeFunctionReturn;

use function PHPStan\Testing\assertType;

class Foo
{
const X = 'x';
const ONE = 1;

public function constants() {
assertType("'12345'", implode(['12', '345']));

assertType("'12345'", implode('', ['12', '345']));
assertType("'12345'", join('', ['12', '345']));

assertType("'12,345'", implode(',', ['12', '345']));
assertType("'12,345'", join(',', ['12', '345']));

assertType("'x,345'", join(',', [self::X, '345']));
assertType("'1,345'", join(',', [self::ONE, '345']));
}
}
8 changes: 4 additions & 4 deletions tests/PHPStan/Analyser/data/non-empty-string.php
Expand Up @@ -206,10 +206,10 @@ public function doFoo4(string $s, array $nonEmptyArrayWithNonEmptyStrings): void
assertType('non-empty-string', implode($nonEmptyArrayWithNonEmptyStrings));
}

public function sayHello(): void
public function sayHello(int $i): void
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adjusted the existing test, so it still leads to the same result as before this PR.

{
// coming from issue #5291
$s = array(1, 2);
$s = array(1, $i);

assertType('non-empty-string', implode("a", $s));
}
Expand All @@ -227,10 +227,10 @@ public function nonE($glue, array $a)
assertType('non-empty-string', implode($glue, $a));
}

public function sayHello2(): void
public function sayHello2(int $i): void
{
// coming from issue #5291
$s = array(1, 2);
$s = array(1, $i);

assertType('non-empty-string', join("a", $s));
}
Expand Down