Skip to content
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.
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);
}
21 changes: 21 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-14534.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?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;
}

function test2(\SplTempFileObject $spl): bool
{
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