Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ parameters:
universalObjectCratesClasses:
- stdClass
stubFiles:
- ../stubs/Memcached.stub
- ../stubs/Redis.stub
- ../stubs/ReflectionAttribute.stub
- ../stubs/ReflectionClassConstant.stub
Expand Down
21 changes: 16 additions & 5 deletions src/Reflection/Php/PhpClassReflectionExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,12 @@ private function createMethod(
$acceptsNamedArguments = true;
$selfOutType = null;
$phpDocComment = null;

$isPure = null;
if ($this->signatureMapProvider->hasMethodMetadata($declaringClassName, $methodReflection->getName())) {
$isPure = !$this->signatureMapProvider->getMethodMetadata($declaringClassName, $methodReflection->getName())['hasSideEffects'];
}

$methodSignaturesResult = $this->signatureMapProvider->getMethodSignatures($declaringClassName, $methodReflection->getName(), $methodReflection);
foreach ($methodSignaturesResult as $signatureType => $methodSignatures) {
if ($methodSignatures === null) {
Expand Down Expand Up @@ -693,6 +699,7 @@ private function createMethod(

$asserts = Assertions::createFromResolvedPhpDocBlock($currentResolvedPhpDoc);
$acceptsNamedArguments = $currentResolvedPhpDoc->acceptsNamedArguments();
$isPure ??= $currentResolvedPhpDoc->isPure();

$selfOutTypeTag = $currentResolvedPhpDoc->getSelfOutTag();
if ($selfOutTypeTag !== null) {
Expand Down Expand Up @@ -727,19 +734,23 @@ private function createMethod(
}
}

if ($this->signatureMapProvider->hasMethodMetadata($declaringClassName, $methodReflection->getName())) {
$hasSideEffects = TrinaryLogic::createFromBoolean($this->signatureMapProvider->getMethodMetadata($declaringClassName, $methodReflection->getName())['hasSideEffects']);
} else {
$hasSideEffects = TrinaryLogic::createMaybe();
if ($isPure === null) {
$classResolvedPhpDoc = $declaringClass->getResolvedPhpDoc();
if ($classResolvedPhpDoc !== null && $classResolvedPhpDoc->areAllMethodsPure()) {
$isPure = true;
} elseif ($classResolvedPhpDoc !== null && $classResolvedPhpDoc->areAllMethodsImpure()) {
$isPure = false;
}
}

return new NativeMethodReflection(
$this->reflectionProviderProvider->getReflectionProvider(),
$declaringClass,
$methodReflection,
$currentResolvedPhpDoc ?? null,
$variantsByType['positional'],
$variantsByType['named'] ?? null,
$hasSideEffects,
$isPure !== null ? TrinaryLogic::createFromBoolean(!$isPure) : TrinaryLogic::createMaybe(),
$throwType,
$asserts,
$acceptsNamedArguments,
Expand Down
16 changes: 16 additions & 0 deletions stubs/Memcached.stub
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

/**
* @phpstan-all-methods-impure
*/
class Memcached {}
Comment thread
staabm marked this conversation as resolved.

/**
* @phpstan-all-methods-impure
*/
class Memcache {}

/**
* @phpstan-all-methods-impure
*/
class MemcachePool {}
40 changes: 40 additions & 0 deletions tests/PHPStan/Rules/Comparison/Bug14534Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PHPStan\Analyser\RicherScopeGetTypeHelper;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function array_merge;

/**
* @extends RuleTestCase<StrictComparisonOfDifferentTypesRule>
*/
class Bug14534Test extends RuleTestCase
{

protected function getRule(): Rule
{
return new StrictComparisonOfDifferentTypesRule(
self::getContainer()->getByType(RicherScopeGetTypeHelper::class),
new PossiblyImpureTipHelper(true),
true,
true,
true,
);
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/bug-14534.php'], []);
}

public static function getAdditionalConfigFiles(): array
{
return array_merge(
parent::getAdditionalConfigFiles(),
[__DIR__ . '/bug-14534.neon'],
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,11 @@ public function testBug14446(): void
$this->analyse([__DIR__ . '/../../Analyser/data/bug-14446.php'], []);
}

public function testBug13444(): void
{
$this->analyse([__DIR__ . '/data/bug-13444.php'], []);
}

public function testBug14473(): void
{
$this->analyse([__DIR__ . '/data/bug-14519.php'], []);
Expand Down
3 changes: 3 additions & 0 deletions tests/PHPStan/Rules/Comparison/bug-14534.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
parameters:
stubFiles:
- data/bug-14534.stub
28 changes: 28 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-13444.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php declare(strict_types = 1);

namespace Bug13444;

function sayStoreCas(string $key): void
{
$memcached = new \Memcached();

do {
$extendedReturn = $memcached->get($key, null, \Memcached::GET_EXTENDED);

if ($memcached->getResultCode() !== \Memcached::RES_SUCCESS) {
return;
}

if (!is_array($extendedReturn) || !isset($extendedReturn['value']) || !isset($extendedReturn['cas'])) {
return;
}

$data = $extendedReturn['value'];
$cas = $extendedReturn['cas'];
\assert(is_float($cas));

// Do some work on the data..
$memcached->cas($cas, $key, $data);

} while ($memcached->getResultCode() !== \Memcached::RES_SUCCESS);
}
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-14534.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

namespace Bug14534;

function test(\SplFileObject $spl): bool
Comment thread
staabm marked this conversation as resolved.
{
if ($spl->key() === 1) {
return $spl->key() === 1;
}

return false;
}
6 changes: 6 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-14534.stub
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<?php

/**
* @phpstan-all-methods-impure
*/
class SplFileObject {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should/can SplFileObject::* be removed now from resources/functionMetadata.php?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a test file, it's just to prove that pure annotation is correctly handled for native classes as #5539 (comment)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand yet, why we will not ship @phpstan-all-methods-impure for SplFileObject, but do/can for Memcache.

other that that this LGTM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't know SplFileObject was impure, I never use this class.
I just focus on the open issue.

Feel free to refactor SplFileObject to use @phpstan-all-methods-impure if you want to

There is maybe more class which can be refactored the same way

Loading