Skip to content

Commit

Permalink
Do not remember values of function with side effects
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Mar 26, 2021
1 parent edc8446 commit d4edc59
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 15 deletions.
29 changes: 28 additions & 1 deletion src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -869,13 +869,40 @@ public function create(
Expr $expr,
Type $type,
TypeSpecifierContext $context,
bool $overwrite = false
bool $overwrite = false,
?Scope $scope = null
): SpecifiedTypes
{
if ($expr instanceof New_ || $expr instanceof Instanceof_) {
return new SpecifiedTypes();
}

if (
$expr instanceof FuncCall
&& $expr->name instanceof Name
&& $this->reflectionProvider->hasFunction($expr->name, $scope)
) {
$functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope);
if ($functionReflection->hasSideEffects()->yes()) {
return new SpecifiedTypes();
}
}

if (
$expr instanceof MethodCall
&& $expr->name instanceof Node\Identifier
&& $scope !== null
) {
$methodName = $expr->name->toString();
$calledOnType = $scope->getType($expr->var);
if ($calledOnType->hasMethod($methodName)->yes()) {
$methodReflection = $calledOnType->getMethod($methodName, $scope);
if ($methodReflection->hasSideEffects()->yes()) {
return new SpecifiedTypes();
}
}
}

$sureTypes = [];
$sureNotTypes = [];

Expand Down
8 changes: 8 additions & 0 deletions src/Rules/Comparison/ImpossibleCheckTypeHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ public function findSpecifiedType(
return null;
}

if ($sureType[0] === $node) {
return null;
}

if ($this->treatPhpDocTypesAsCertain) {
$argumentType = $scope->getType($sureType[0]);
} else {
Expand All @@ -202,6 +206,10 @@ public function findSpecifiedType(
return null;
}

if ($sureNotType[0] === $node) {
return null;
}

if ($this->treatPhpDocTypesAsCertain) {
$argumentType = $scope->getType($sureNotType[0]);
} else {
Expand Down
6 changes: 6 additions & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5760,6 +5760,11 @@ public function dataBug560(): array
return $this->gatherAssertTypes(__DIR__ . '/data/bug-560.php');
}

public function dataDoNotRememberImpureFunctions(): array
{
return $this->gatherAssertTypes(__DIR__ . '/data/do-not-remember-impure-functions.php');
}

/**
* @dataProvider dataArrayFunctions
* @param string $description
Expand Down Expand Up @@ -11395,6 +11400,7 @@ private function gatherAssertTypes(string $file): array
* @dataProvider dataBug3190
* @dataProvider dataTernarySpecifiedTypes
* @dataProvider dataBug560
* @dataProvider dataDoNotRememberImpureFunctions
* @param string $assertType
* @param string $file
* @param mixed ...$args
Expand Down
72 changes: 72 additions & 0 deletions tests/PHPStan/Analyser/data/do-not-remember-impure-functions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

namespace DoNotRememberImpureFunctions;

use function PHPStan\Analyser\assertType;

class Foo
{

public function doFoo()
{
function (): void {
if (rand(0, 1)) {
assertType('int', rand(0, 1));
}
};

function (): void {
if (rand(0, 1) === 0) {
assertType('int', rand(0, 1));
}
};
}

public function doBar(): bool
{

}

/** @phpstan-pure */
public function doBaz(): bool
{

}

/** @phpstan-impure */
public function doLorem(): bool
{

}

public function doIpsum()
{
if ($this->doBar() === true) {
assertType('true', $this->doBar());
}

if ($this->doBaz() === true) {
assertType('true', $this->doBaz());
}

if ($this->doLorem() === true) {
assertType('bool', $this->doLorem());
}
}

public function doDolor()
{
if ($this->doBar()) {
assertType('true', $this->doBar());
}

if ($this->doBaz()) {
assertType('true', $this->doBaz());
}

if ($this->doLorem()) {
assertType('bool', $this->doLorem());
}
}

}
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/data/isset.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ public function doFoo(array $integers, string $string, $mixedIsset, $mixedArrayK
'a' => 1,
'b' => 2,
];
} elseif (rand(0, 11) === 0) {
} elseif (rand(0, 10) === 0) {
$array = [
'a' => 2,
];
} elseif (rand(0, 12) === 0) {
} elseif (rand(0, 10) === 0) {
$array = [
'a' => 3,
'b' => 3,
Expand Down
12 changes: 0 additions & 12 deletions tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,6 @@ public function testReturnTypeRule(): void
'Method ReturnTypes\TrickyVoid::returnVoidOrInt() should return int|void but returns string.',
656,
],
[
'Method ReturnTypes\TernaryWithJsonEncode::toJsonOrNull() should return string|null but returns string|false|null.',
671,
],
[
'Method ReturnTypes\TernaryWithJsonEncode::toJson() should return string but returns string|false.',
684,
],
[
'Method ReturnTypes\TernaryWithJsonEncode::toJson() should return string but returns string|false.',
687,
Expand Down Expand Up @@ -235,10 +227,6 @@ public function testReturnTypeRule(): void
'Method ReturnTypes\WrongMagicMethods::__clone() with return type void returns int but should not return anything.',
757,
],
[
'Method ReturnTypes\ReturnSpecifiedMethodCall::doFoo() should return string but returns string|false.',
776,
],
[
'Method ReturnTypes\ArrayFillKeysIssue::getIPs2() should return array<string, array<ReturnTypes\Foo>> but returns array<string, array<int, ReturnTypes\Bar>>.',
815,
Expand Down

0 comments on commit d4edc59

Please sign in to comment.