Skip to content

Commit

Permalink
Do not invalidate types passed by value
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Oct 5, 2023
1 parent 52eb6f8 commit 0b8dca7
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -3617,7 +3617,7 @@ private function shouldInvalidateExpression(string $exprStringToInvalidate, Expr
}

// Variables will not contain traversable expressions. skip the NodeFinder overhead
if ($expr instanceof Variable && is_string($expr->name)) {
if ($expr instanceof Variable && is_string($expr->name) && !$requireMoreCharacters) {
return $exprStringToInvalidate === $this->getNodeKey($expr);
}

Expand Down
15 changes: 14 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@
use PHPStan\Type\NeverType;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\ResourceType;
use PHPStan\Type\StaticType;
use PHPStan\Type\StaticTypeFactory;
use PHPStan\Type\StringType;
Expand Down Expand Up @@ -2010,6 +2011,15 @@ static function (): void {
->invalidateExpression(new FuncCall(new Name\FullyQualified('json_last_error_msg'), []));
}

if (
$functionReflection !== null
&& $functionReflection->getName() === 'file_put_contents'
&& count($expr->getArgs()) > 0
) {
$scope = $scope->invalidateExpression(new FuncCall(new Name('file_get_contents'), [$expr->getArgs()[0]]))

This comment has been minimized.

Copy link
@staabm

staabm Oct 5, 2023

Contributor

is this invalidation tested somewhere?

This comment has been minimized.

Copy link
@ondrejmirtes

ondrejmirtes Oct 5, 2023

Author Member

You don’t need to ask - you can comment it and see what test fails.

->invalidateExpression(new FuncCall(new Name\FullyQualified('file_get_contents'), [$expr->getArgs()[0]]));
}

if (
$functionReflection !== null
&& in_array($functionReflection->getName(), ['array_pop', 'array_shift'], true)
Expand Down Expand Up @@ -3600,7 +3610,10 @@ private function processArgs(
$scope = $scope->invalidateExpression($argValue);
}
} elseif ($calleeReflection !== null && $calleeReflection->hasSideEffects()->yes()) {
$scope = $scope->invalidateExpression($arg->value, true);
$argType = $scope->getType($arg->value);
if (!$argType->isObject()->no() || !(new ResourceType())->isSuperTypeOf($argType)->no()) {
$scope = $scope->invalidateExpression($arg->value, true);
}
}
}

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/strtotime-return-type-extensions.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/closure-return-type-extensions.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/array-key.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/discussion-9972.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/intersection-static.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/static-properties.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/static-methods.php');
Expand Down
26 changes: 26 additions & 0 deletions tests/PHPStan/Analyser/data/discussion-9972.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

namespace Discussion9972;

use PHPStan\TrinaryLogic;
use function PHPStan\Testing\assertVariableCertainty;

class HelloWorld
{
public function f1(bool $myBool): void
{
if ($myBool) {
$myObject = new DateTime();
}

$this->helper($myBool);

if ($myBool) {
assertVariableCertainty(TrinaryLogic::createYes(), $myObject);
}
}

protected function helper(bool $input): void
{
}
}

0 comments on commit 0b8dca7

Please sign in to comment.