Declare Memcached methods as impure (#5536)

Author: VincentLanglet

conf/config.neon

@@ -125,6 +125,7 @@ parameters:
 	universalObjectCratesClasses:
 		- stdClass
 	stubFiles:
+		- ../stubs/Memcached.stub
 		- ../stubs/Redis.stub
 		- ../stubs/ReflectionAttribute.stub
 		- ../stubs/ReflectionClassConstant.stub

src/Reflection/Php/PhpClassReflectionExtension.php

@@ -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) {
@@ -693,6 +699,7 @@ private function createMethod(
 
 						$asserts = Assertions::createFromResolvedPhpDocBlock($currentResolvedPhpDoc);
 						$acceptsNamedArguments = $currentResolvedPhpDoc->acceptsNamedArguments();
+						$isPure ??= $currentResolvedPhpDoc->isPure();
 
 						$selfOutTypeTag = $currentResolvedPhpDoc->getSelfOutTag();
 						if ($selfOutTypeTag !== null) {
@@ -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,

stubs/Memcached.stub

@@ -0,0 +1,16 @@
+<?php
+
+/**
+ * @phpstan-all-methods-impure
+ */
+class Memcached {}
+
+/**
+ * @phpstan-all-methods-impure
+ */
+class Memcache {}
+
+/**
+ * @phpstan-all-methods-impure
+ */
+class MemcachePool {}

tests/PHPStan/Rules/Comparison/Bug14534Test.php

@@ -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'],
+		);
+	}
+
+}

tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

@@ -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'], []);

tests/PHPStan/Rules/Comparison/bug-14534.neon

@@ -0,0 +1,3 @@
+parameters:
+	stubFiles:
+		- data/bug-14534.stub

tests/PHPStan/Rules/Comparison/data/bug-13444.php

@@ -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);
+}

tests/PHPStan/Rules/Comparison/data/bug-14534.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace Bug14534;
+
+function test(\SplFileObject $spl): bool
+{
+	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;
+}

tests/PHPStan/Rules/Comparison/data/bug-14534.stub

@@ -0,0 +1,6 @@
+<?php
+
+/**
+ * @phpstan-all-methods-impure
+ */
+class SplFileObject {}