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

Passing a variable by reference to a function and method has side effects #1990

Merged
merged 7 commits into from Jan 8, 2023
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
8 changes: 8 additions & 0 deletions src/Reflection/Annotations/AnnotationMethodReflection.php
Expand Up @@ -113,6 +113,14 @@ public function hasSideEffects(): TrinaryLogic
return TrinaryLogic::createYes();
}

foreach ($this->getVariants() as $variant) {
foreach ($variant->getParameters() as $parameter) {
if ($parameter->passedByReference()->yes()) {
return TrinaryLogic::createYes();
}
}
}

return TrinaryLogic::createMaybe();
}

Expand Down
7 changes: 7 additions & 0 deletions src/Reflection/Native/NativeFunctionReflection.php
Expand Up @@ -81,6 +81,13 @@ public function hasSideEffects(): TrinaryLogic
if ($this->isVoid()) {
return TrinaryLogic::createYes();
}
foreach ($this->variants as $variant) {
foreach ($variant->getParameters() as $parameter) {
if ($parameter->passedByReference()->yes()) {
return TrinaryLogic::createYes();
}
}
}

return $this->hasSideEffects;
}
Expand Down
7 changes: 7 additions & 0 deletions src/Reflection/Native/NativeMethodReflection.php
Expand Up @@ -136,6 +136,13 @@ public function hasSideEffects(): TrinaryLogic
) {
return TrinaryLogic::createYes();
}
foreach ($this->variants as $variant) {
foreach ($variant->getParameters() as $parameter) {
if ($parameter->passedByReference()->yes()) {
return TrinaryLogic::createYes();
}
}
}

return $this->hasSideEffects;
}
Expand Down
7 changes: 7 additions & 0 deletions src/Reflection/Php/PhpFunctionFromParserNodeReflection.php
Expand Up @@ -199,6 +199,13 @@ public function hasSideEffects(): TrinaryLogic
if ($this->isPure !== null) {
return TrinaryLogic::createFromBoolean(!$this->isPure);
}
foreach ($this->getVariants() as $variant) {
foreach ($variant->getParameters() as $parameter) {
if ($parameter->passedByReference()->yes()) {
return TrinaryLogic::createYes();
}
}
}

return TrinaryLogic::createMaybe();
}
Expand Down
7 changes: 7 additions & 0 deletions src/Reflection/Php/PhpFunctionReflection.php
Expand Up @@ -245,6 +245,13 @@ public function hasSideEffects(): TrinaryLogic
if ($this->isPure !== null) {
return TrinaryLogic::createFromBoolean(!$this->isPure);
}
foreach ($this->getVariants() as $variant) {
foreach ($variant->getParameters() as $parameter) {
if ($parameter->passedByReference()->yes()) {
return TrinaryLogic::createYes();
}
}
}

return TrinaryLogic::createMaybe();
}
Expand Down
7 changes: 7 additions & 0 deletions src/Reflection/Php/PhpMethodReflection.php
Expand Up @@ -416,6 +416,13 @@ public function hasSideEffects(): TrinaryLogic
if ($this->isPure !== null) {
return TrinaryLogic::createFromBoolean(!$this->isPure);
}
foreach ($this->getVariants() as $variant) {
foreach ($variant->getParameters() as $parameter) {
if ($parameter->passedByReference()->yes()) {
return TrinaryLogic::createYes();
}
}
}

return TrinaryLogic::createMaybe();
}
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -1170,6 +1170,7 @@ public function dataFileAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8635.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8625.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8621.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-8084.php');
}

/**
Expand Down
20 changes: 20 additions & 0 deletions tests/PHPStan/Analyser/data/bug-8084.php
@@ -0,0 +1,20 @@
<?php

namespace Bug8084;

use Exception;
use function array_shift;
use function PHPStan\Testing\assertType;

class Bug8084
{
/**
* @param array{a: 0, b?: 1} $arr
* @throws Exception
*/
public function run(array $arr): void
{
assertType('0', array_shift($arr) ?? throw new Exception());
assertType('1|null', array_shift($arr));
}
}
8 changes: 8 additions & 0 deletions tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php
Expand Up @@ -360,4 +360,12 @@ public function testBug7968(): void
$this->analyse([__DIR__ . '/data/bug-7968.php'], []);
}

public function testBug8084(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->strictUnnecessaryNullsafePropertyFetch = true;

$this->analyse([__DIR__ . '/data/bug-8084.php'], []);
}

}
19 changes: 19 additions & 0 deletions tests/PHPStan/Rules/Variables/data/bug-8084.php
@@ -0,0 +1,19 @@
<?php // lint >= 8.0

namespace Bug8084;

use Exception;
use function array_shift;
use function PHPStan\Testing\assertType;

class Bug8084
{
/**
* @param array{a?: 0} $arr
* @throws Exception
*/
public function run(array $arr): void
{
assertType('0|null', array_shift($arr) ?? throw new Exception());
}
}