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.

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

Jump to bottom

Improve constant string union handling for concat and encapsed string #2057

Merged
merged 1 commit into from Dec 19, 2022
+87 −80
Merged

Improve constant string union handling for concat and encapsed string #2057

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 11 additions & 51 deletions src/Analyser/MutatingScope.php
View file
Open in desktop
Expand Up @@ -66,8 +66,6 @@
use PHPStan\TrinaryLogic;
use PHPStan\Type\Accessory\AccessoryArrayListType;
use PHPStan\Type\Accessory\AccessoryLiteralStringType;
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
use PHPStan\Type\Accessory\HasOffsetValueType;
use PHPStan\Type\Accessory\NonEmptyArrayType;
use PHPStan\Type\ArrayType;
Expand Down Expand Up @@ -1088,63 +1086,25 @@ private function resolveType(string $exprString, Expr $node): Type
} elseif ($node instanceof String_) {
return $this->initializerExprTypeResolver->getType($node, InitializerExprContext::fromScope($this));
} elseif ($node instanceof Node\Scalar\Encapsed) {
$parts = [];
foreach ($node->parts as $part) {
if ($part instanceof EncapsedStringPart) {
$parts[] = new ConstantStringType($part->value);
continue;
}
$resultType = null;

$partStringType = $this->getType($part)->toString();
if ($partStringType instanceof ErrorType) {
return new ErrorType();
}

$parts[] = $partStringType;
}
foreach ($node->parts as $part) {
$partType = $part instanceof EncapsedStringPart
? new ConstantStringType($part->value)
: $this->getType($part);
if ($resultType === null) {
$resultType = $partType;

$constantString = new ConstantStringType('');
foreach ($parts as $part) {
if ($part instanceof ConstantStringType) {
$constantString = $constantString->append($part);
continue;
}

$isNonEmpty = false;
$isNonFalsy = false;
$isLiteralString = true;
foreach ($parts as $partType) {
if ($partType->isNonFalsyString()->yes()) {
$isNonFalsy = true;
}
if ($partType->isNonEmptyString()->yes()) {
$isNonEmpty = true;
}
if ($partType->isLiteralString()->yes()) {
continue;
}
$isLiteralString = false;
}

$accessoryTypes = [];
if ($isNonFalsy === true) {
$accessoryTypes[] = new AccessoryNonFalsyStringType();
} elseif ($isNonEmpty === true) {
$accessoryTypes[] = new AccessoryNonEmptyStringType();
$resultType = $this->initializerExprTypeResolver->resolveConcatType($resultType, $partType);
if (! $resultType instanceof ConstantStringType && ! $resultType instanceof UnionType) {
return $resultType;
}

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

return new StringType();
}

return $constantString;
return $resultType ?? new ConstantStringType('');
} elseif ($node instanceof DNumber) {
return $this->initializerExprTypeResolver->getType($node, InitializerExprContext::fromScope($this));
} elseif ($node instanceof Expr\CallLike && $node->isFirstClassCallable()) {
Expand Down
54 changes: 31 additions & 23 deletions src/Reflection/InitializerExprTypeResolver.php
View file
Open in desktop
Expand Up @@ -61,6 +61,7 @@
use PHPStan\Type\TypeWithClassName;
use PHPStan\Type\UnionType;
use function array_keys;
use function array_merge;
use function count;
use function dirname;
use function in_array;
Expand Down Expand Up @@ -361,8 +362,16 @@ public function getType(Expr $expr, InitializerExprContext $context): Type
*/
public function getConcatType(Expr $left, Expr $right, callable $getTypeCallback): Type
{
$leftStringType = $getTypeCallback($left)->toString();
$rightStringType = $getTypeCallback($right)->toString();
$leftType = $getTypeCallback($left);
$rightType = $getTypeCallback($right);

return $this->resolveConcatType($leftType, $rightType);
}

public function resolveConcatType(Type $left, Type $right): Type
{
$leftStringType = $left->toString();
$rightStringType = $right->toString();
if (TypeCombinator::union(
$leftStringType,
$rightStringType,
Expand All @@ -382,34 +391,33 @@ public function getConcatType(Expr $left, Expr $right, callable $getTypeCallback
return $leftStringType->append($rightStringType);
}

$leftConstantStrings = $leftStringType->getConstantStrings();
$rightConstantStrings = $rightStringType->getConstantStrings();
$combinedConstantStringsCount = count($leftConstantStrings) * count($rightConstantStrings);

// we limit the number of union-types for performance reasons
if ($leftStringType instanceof UnionType && count($leftStringType->getTypes()) <= 16 && $rightStringType instanceof ConstantStringType) {
$constantStrings = $leftStringType->getConstantStrings();
if (count($constantStrings) > 0) {
$strings = [];
foreach ($constantStrings as $constantString) {
if ($constantString->getValue() === '') {
$strings[] = $rightStringType;
if ($combinedConstantStringsCount > 0 && $combinedConstantStringsCount <= 16) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that there is also CALCULATE_SCALARS_LIMIT in this class, but it was not used here, so I kept the previous limit.

$strings = [];

continue;
}
$strings[] = $constantString->append($rightStringType);
foreach ($leftConstantStrings as $leftConstantString) {
if ($leftConstantString->getValue() === '') {
$strings = array_merge($strings, $rightConstantStrings);

continue;
}
return TypeCombinator::union(...$strings);
}
}
if ($rightStringType instanceof UnionType && count($rightStringType->getTypes()) <= 16 && $leftStringType instanceof ConstantStringType) {
$constantStrings = $rightStringType->getConstantStrings();
if (count($constantStrings) > 0) {
$strings = [];
foreach ($constantStrings as $constantString) {
if ($constantString->getValue() === '') {
$strings[] = $leftStringType;

foreach ($rightConstantStrings as $rightConstantString) {
if ($rightConstantString->getValue() === '') {
$strings[] = $leftConstantString;

continue;
}
$strings[] = $leftStringType->append($constantString);

$strings[] = $leftConstantString->append($rightConstantString);
}
}

if (count($strings) > 0) {
return TypeCombinator::union(...$strings);
}
}
Expand Down
6 changes: 3 additions & 3 deletions tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
View file
Open in desktop
Expand Up @@ -2996,15 +2996,15 @@ public function dataBinaryOperations(): array
'$decrementedFooString',
],
[
'literal-string&non-falsy-string',
"'barbar'|'barfoo'|'foobar'|'foofoo'",
'$conditionalString . $conditionalString',
],
[
'literal-string&non-falsy-string',
"'baripsum'|'barlorem'|'fooipsum'|'foolorem'",
'$conditionalString . $anotherConditionalString',
],
[
'literal-string&non-falsy-string',
"'ipsumbar'|'ipsumfoo'|'lorembar'|'loremfoo'",
'$anotherConditionalString . $conditionalString',
],
[
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/NodeScopeResolverTest.php
View file
Open in desktop
Expand Up @@ -810,7 +810,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6584.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/weird-strlen-cases.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6439.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/constant-string-unions.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6748.php');

yield from $this->gatherAssertTypes(__DIR__ . '/data/array-fill-keys.php');
Expand Down
43 changes: 41 additions & 2 deletions tests/PHPStan/Analyser/data/bug-6439.php → .../Analyser/data/constant-string-unions.php
View file
Open in desktop
@@ -1,10 +1,10 @@
<?php

namespace Bug6439;
namespace ConstantStringUnions;

use function PHPStan\Testing\assertType;


// See https://github.com/phpstan/phpstan/issues/6439
class HelloWorld
{
public function unionOnLeft(string $name, ?int $gesperrt = null, ?int $adaid = null):void
Expand Down Expand Up @@ -33,6 +33,28 @@ public function unionOnRight(string $name, ?int $gesperrt = null, ?int $adaid =
assertType("'branch-a general'|'branch-b branch-a general'|'branch-b general'|'general'", $string);
}

public function unionOnBoth():void
{
$left = rand() ? 'a' : 'b';
$right = rand() ? 'x' : 'y';
assertType("'ax'|'ay'|'bx'|'by'", $left . $right);
}

public function encapsedString():void
{
$str = rand() ? 'a' : 'b';
$int = rand() ? 1 : 0;
$float = rand() ? 1.0 : 2.0;
$bool = (bool) rand();
$nullable = rand() ? 'a' : null;
assertType("'.a.'|'.b.'", ".$str.");
assertType("'.0.'|'.1.'", ".$int.");
assertType("'.1.'|'.2.'", ".$float.");
assertType("'..'|'.1.'", ".$bool.");
assertType("'..'|'.a.'", ".$nullable.");
assertType("'.a.0.'|'.a.1.'|'.b.0.'|'.b.1.'", ".$str.$int.");
Copy link
Member

Choose a reason for hiding this comment

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

Can you please also provide an example where the logic goes over the limit and the type is simplified again? Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}

/**
* @param '1'|'2'|'3'|'4'|'5'|'6'|'7'|'8'|'9'|'10'|'11'|'12'|'13'|'14'|'15' $s15
* @param '1'|'2'|'3'|'4'|'5'|'6'|'7'|'8'|'9'|'10'|'11'|'12'|'13'|'14'|'15'|'16' $s16
Expand All @@ -51,6 +73,23 @@ public function testLimit(string $s15, string $s16, string $s17) {
assertType("'1'|'10'|'10a'|'11'|'11a'|'12'|'12a'|'13'|'13a'|'14'|'14a'|'15'|'15a'|'16'|'16a'|'1a'|'2'|'2a'|'3'|'3a'|'4'|'4a'|'5'|'5a'|'6'|'6a'|'7'|'7a'|'8'|'8a'|'9'|'9a'", $s16);
// fallback to the more general form
assertType("literal-string&non-falsy-string", $s17);
$left = rand() ? 'a' : 'b';
$right = rand() ? 'x' : 'y';
$left .= $right;
$left .= $right;
$left .= $right;
assertType("'axxx'|'axxy'|'axyx'|'axyy'|'ayxx'|'ayxy'|'ayyx'|'ayyy'|'bxxx'|'bxxy'|'bxyx'|'bxyy'|'byxx'|'byxy'|'byyx'|'byyy'", $left);
$left .= $right;
assertType("literal-string&non-falsy-string", $left);

$left = rand() ? 'a' : 'b';
$right = rand() ? 'x' : 'y';
$left = "{$left}{$right}";
$left = "{$left}{$right}";
$left = "{$left}{$right}";
assertType("'axxx'|'axxy'|'axyx'|'axyy'|'ayxx'|'ayxy'|'ayyx'|'ayyy'|'bxxx'|'bxxy'|'bxyx'|'bxyy'|'byxx'|'byxy'|'byyx'|'byyy'", $left);
$left = "{$left}{$right}";
assertType("literal-string&non-falsy-string", $left);
}

/**
Expand Down