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 str_contains FunctionTypeSpecifyingExtension #1068

Merged
merged 2 commits into from
Apr 28, 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
5 changes: 5 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1268,6 +1268,11 @@ services:
tags:
- phpstan.dynamicStaticMethodThrowTypeExtension

-
class: PHPStan\Type\Php\StrContainingTypeSpecifyingExtension
tags:
- phpstan.typeSpecifier.functionTypeSpecifyingExtension

-
class: PHPStan\Type\Php\SimpleXMLElementClassPropertyReflectionExtension
tags:
Expand Down
101 changes: 101 additions & 0 deletions src/Type/Php/StrContainingTypeSpecifyingExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
<?php declare(strict_types = 1);

namespace PHPStan\Type\Php;

use PhpParser\Node\Arg;
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Name;
use PhpParser\Node\Scalar\String_;
use PHPStan\Analyser\Scope;
use PHPStan\Analyser\SpecifiedTypes;
use PHPStan\Analyser\TypeSpecifier;
use PHPStan\Analyser\TypeSpecifierAwareExtension;
use PHPStan\Analyser\TypeSpecifierContext;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Type\Accessory\AccessoryLiteralStringType;
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
use PHPStan\Type\Accessory\AccessoryNumericStringType;
use PHPStan\Type\FunctionTypeSpecifyingExtension;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\StringType;
use function array_key_exists;
use function count;
use function strtolower;

final class StrContainingTypeSpecifyingExtension implements FunctionTypeSpecifyingExtension, TypeSpecifierAwareExtension
{

/** @var array<string, array{0: int, 1: int}> */
private array $strContainingFunctions = [
'fnmatch' => [1, 0],
'str_contains' => [0, 1],
'str_starts_with' => [0, 1],
'str_ends_with' => [0, 1],
'strpos' => [0, 1],
'strrpos' => [0, 1],
'stripos' => [0, 1],
'strripos' => [0, 1],
'strstr' => [0, 1],
];

private TypeSpecifier $typeSpecifier;

public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void
{
$this->typeSpecifier = $typeSpecifier;
}

public function isFunctionSupported(FunctionReflection $functionReflection, FuncCall $node, TypeSpecifierContext $context): bool
{
return array_key_exists(strtolower($functionReflection->getName()), $this->strContainingFunctions)
&& $context->truthy();
}

public function specifyTypes(FunctionReflection $functionReflection, FuncCall $node, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes
{
$args = $node->getArgs();

if (count($args) >= 2) {
[$hackstackArg, $needleArg] = $this->strContainingFunctions[strtolower($functionReflection->getName())];

$haystackType = $scope->getType($args[$hackstackArg]->value);
$needleType = $scope->getType($args[$needleArg]->value);

if ($needleType->isNonEmptyString()->yes() && $haystackType->isString()->yes()) {
$accessories = [
new StringType(),
new AccessoryNonEmptyStringType(),
];

if ($haystackType->isLiteralString()->yes()) {
$accessories[] = new AccessoryLiteralStringType();
}
if ($haystackType->isNumericString()->yes()) {
$accessories[] = new AccessoryNumericStringType();
}

return $this->typeSpecifier->create(
$args[$hackstackArg]->value,
new IntersectionType($accessories),
$context,
false,
$scope,
new BooleanAnd(
new NotIdentical(
$args[$needleArg]->value,
new String_(''),
),
new FuncCall(new Name('FAUX_FUNCTION'), [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx for getting this over the finishing line <3.

could you elaborate a bit, what this FAUX_FUNCTION thing is about?
is this a kind-of workaround, which should be encapsulated in a helper somehow, so the code gets readable for non-phpstan-hard-core users :-)?

Copy link
Contributor

@herndlm herndlm Apr 28, 2022

Choose a reason for hiding this comment

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

it's kind of the soft force flag xD but I agree, it is hard to understand and workaround-ish, maybe it should end up in some kind of helper or smth like that hmm

Copy link
Contributor

@herndlm herndlm Apr 28, 2022

Choose a reason for hiding this comment

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

on the other hand this is veeery special and the expression is different for each case maybe

Copy link
Member

Choose a reason for hiding this comment

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

The thing that's problematic for str_contains() type-specifying extensions and similar is that PHPStan thinks the function is is_string_non_empty() because it narrows down the type to non-empty-string. But there are other properties of the narrowed string than that - in this case that it contains another string which can't be expressed by the PHPStan typesystem.

So when you call str_contains($nonEmptyString) PHPStan thinks it's is_string_non_empty($nonEmptyString) and that it has to be always true. But in fact it should be is_string_non_empty($nonEmptyString) && string_contains_string(...). So that's what I'm simulating here.

I don't think we need a helper here.

new Arg($args[$needleArg]->value),
]),
),
);
}
}

return new SpecifiedTypes();
}

}
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7068.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7115.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/constant-array-type-identical.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/non-empty-string-str-containing-fns.php');
}

/**
Expand Down
123 changes: 123 additions & 0 deletions tests/PHPStan/Analyser/data/non-empty-string-str-containing-fns.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
<?php

namespace NonEmptyStringStrContains;

use function PHPStan\Testing\assertType;

class Foo {
/**
* @param non-empty-string $nonES
* @param numeric-string $numS
* @param literal-string $literalS
* @param non-empty-string&numeric-string $nonEAndNumericS
*/
public function strContains(string $s, string $s2, $nonES, $numS, $literalS, $nonEAndNumericS, int $i): void
{
if (str_contains($s, ':')) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (str_contains($s, $s2)) {
assertType('string', $s);
}

if (str_contains($s, $nonES)) {
assertType('non-empty-string', $s);
}
if (str_contains($s, $numS)) {
assertType('non-empty-string', $s);
}
if (str_contains($s, $literalS)) {
assertType('string', $s);
}

if (str_contains($s, $nonEAndNumericS)) {
assertType('non-empty-string', $s);
}
if (str_contains($numS, $nonEAndNumericS)) {
assertType('non-empty-string&numeric-string', $numS);
}

if (str_contains($nonES, $s)) {
assertType('non-empty-string', $nonES);
}
if (str_contains($nonEAndNumericS, $s)) {
assertType('non-empty-string&numeric-string', $nonEAndNumericS);
}

if (str_contains($i, $s2)) {
assertType('int', $i);
}
}

public function variants(string $s) {
if (fnmatch("*gr[ae]y", $s)) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (str_starts_with($s, ':')) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (str_ends_with($s, ':')) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (strpos($s, ':') !== false) {
assertType('non-empty-string', $s);
}
assertType('string', $s);
if (strpos($s, ':') === false) {
assertType('string', $s);
}
assertType('string', $s);

if (strpos($s, ':') === 5) {
assertType('string', $s); // could be non-empty-string
}
assertType('string', $s);
if (strpos($s, ':') !== 5) {
assertType('string', $s);
}
assertType('string', $s);

if (strrpos($s, ':') !== false) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (stripos($s, ':') !== false) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (strripos($s, ':') !== false) {
assertType('non-empty-string', $s);
}
assertType('string', $s);

if (strstr($s, ':') === 'hallo') {
assertType('string', $s); // could be non-empty-string
}
assertType('string', $s);
if (strstr($s, ':', true) === 'hallo') {
assertType('string', $s); // could be non-empty-string
Comment on lines +103 to +108
Copy link
Contributor Author

@staabm staabm Mar 19, 2022

Choose a reason for hiding this comment

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

optimal would be a non-empty-string here.. but the extension as-is cannot deliver it atm.

I have no idea yet, how a type-specifying extension could evaluate a comparision like if (strstr($s, ':', true) === 'hallo') {.
atm I am only verifying a plain string type, to make sure the new extension does not scramble the existing type in this scenario

Copy link
Contributor Author

@staabm staabm Mar 26, 2022

Choose a reason for hiding this comment

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

Comparison based inference can be implemented like df27a12 but === should be preferred over ==

}
assertType('string', $s);
if (strstr($s, ':', true) !== false) {
assertType('non-empty-string', $s);
}
assertType('string', $s);
if (strstr($s, ':', true) === false) {
assertType('string', $s);
} else {
assertType('non-empty-string', $s);
}
assertType('string', $s);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -528,4 +528,11 @@ public function testSlevomatCsInArrayBug(): void
$this->analyse([__DIR__ . '/data/slevomat-cs-in-array.php'], []);
}

public function testNonEmptySpecifiedString(): void
{
$this->checkAlwaysTrueCheckTypeFunctionCall = true;
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/non-empty-string-impossible-type.php'], []);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

namespace NonEmptyStringImpossibleType;

class Foo {
private function isPrefixedInterface(string $shortClassName): bool
{
if (strlen($shortClassName) <= 3) {
return false;
}

if (! \str_starts_with($shortClassName, 'I')) {
return false;
}

if (! ctype_upper($shortClassName[1])) {
return false;
}

return ctype_lower($shortClassName[2]);
}
}