GetNonVirtualPropertyHookReadRule - level 3

Author: ondrejmirtes

conf/config.level3.neon

@@ -16,6 +16,7 @@ rules:
 	- PHPStan\Rules\Generators\YieldTypeRule
 	- PHPStan\Rules\Methods\ReturnTypeRule
 	- PHPStan\Rules\Properties\DefaultValueTypesAssignedToPropertiesRule
+	- PHPStan\Rules\Properties\GetNonVirtualPropertyHookReadRule
 	- PHPStan\Rules\Properties\ReadOnlyPropertyAssignRule
 	- PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyAssignRule
 	- PHPStan\Rules\Properties\ReadOnlyPropertyAssignRefRule

src/Rules/Properties/GetNonVirtualPropertyHookReadRule.php

@@ -0,0 +1,118 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Properties;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\ClassPropertiesNode;
+use PHPStan\Node\ClassPropertyNode;
+use PHPStan\Node\Property\PropertyRead;
+use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use function array_key_exists;
+use function sprintf;
+
+/**
+ * @implements Rule<ClassPropertiesNode>
+ */
+final class GetNonVirtualPropertyHookReadRule implements Rule
+{
+
+	public function getNodeType(): string
+	{
+		return ClassPropertiesNode::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		$reads = [];
+		$classReflection = $node->getClassReflection();
+		foreach ($node->getPropertyUsages() as $propertyUsage) {
+			if (!$propertyUsage instanceof PropertyRead) {
+				continue;
+			}
+
+			$fetch = $propertyUsage->getFetch();
+			if (!$fetch instanceof Node\Expr\PropertyFetch) {
+				continue;
+			}
+
+			if (!$fetch->name instanceof Node\Identifier) {
+				continue;
+			}
+
+			$propertyName = $fetch->name->toString();
+			if (!$fetch->var instanceof Node\Expr\Variable || $fetch->var->name !== 'this') {
+				continue;
+			}
+
+			$usageScope = $propertyUsage->getScope();
+			$inFunction = $usageScope->getFunction();
+			if (!$inFunction instanceof PhpMethodFromParserNodeReflection) {
+				continue;
+			}
+
+			if (!$inFunction->isPropertyHook()) {
+				continue;
+			}
+
+			if ($inFunction->getPropertyHookName() !== 'get') {
+				continue;
+			}
+
+			if ($propertyName !== $inFunction->getHookedPropertyName()) {
+				continue;
+			}
+
+			$reads[$propertyName] = true;
+		}
+
+		$errors = [];
+		foreach ($node->getProperties() as $propertyNode) {
+			if (!$propertyNode->hasHooks()) {
+				continue;
+			}
+
+			if (array_key_exists($propertyNode->getName(), $reads)) {
+				continue;
+			}
+
+			$propertyReflection = $classReflection->getNativeProperty($propertyNode->getName());
+			if ($propertyReflection->isVirtual()->yes()) {
+				continue;
+			}
+
+			$errors[] = RuleErrorBuilder::message(sprintf(
+				'Get hook for non-virtual property %s::$%s does not read its value.',
+				$classReflection->getDisplayName(),
+				$propertyNode->getName(),
+			))
+				->line($this->getGetHookLine($propertyNode))
+				->identifier('propertyGetHook.noRead')
+				->build();
+		}
+
+		return $errors;
+	}
+
+	private function getGetHookLine(ClassPropertyNode $propertyNode): int
+	{
+		$getHook = null;
+		foreach ($propertyNode->getHooks() as $hook) {
+			if ($hook->name->toLowerString() !== 'get') {
+				continue;
+			}
+
+			$getHook = $hook;
+			break;
+		}
+
+		if ($getHook === null) {
+			return $propertyNode->getStartLine();
+		}
+
+		return $getHook->getStartLine();
+	}
+
+}

tests/PHPStan/Rules/Properties/GetNonVirtualPropertyHookReadRuleTest.php

@@ -0,0 +1,38 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Properties;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use const PHP_VERSION_ID;
+
+/**
+ * @extends RuleTestCase<GetNonVirtualPropertyHookReadRule>
+ */
+class GetNonVirtualPropertyHookReadRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new GetNonVirtualPropertyHookReadRule();
+	}
+
+	public function testRule(): void
+	{
+		if (PHP_VERSION_ID < 80400) {
+			$this->markTestSkipped('Test requires PHP 8.4.');
+		}
+
+		$this->analyse([__DIR__ . '/data/get-non-virtual-property-hook-read.php'], [
+			[
+				'Get hook for non-virtual property GetNonVirtualPropertyHookRead\Foo::$k does not read its value.',
+				24,
+			],
+			[
+				'Get hook for non-virtual property GetNonVirtualPropertyHookRead\Foo::$l does not read its value.',
+				30,
+			],
+		]);
+	}
+
+}

tests/PHPStan/Rules/Properties/data/get-non-virtual-property-hook-read.php

@@ -0,0 +1,48 @@
+<?php // lint >= 8.4
+
+namespace GetNonVirtualPropertyHookRead;
+
+class Foo
+{
+
+	public int $i {
+		// backed, read and written
+		get => $this->i + 1;
+		set => $this->i + $value;
+	}
+
+	public int $j {
+		// virtual
+		get => 1;
+		set {
+			$this->a = $value;
+		}
+	}
+
+	public int $k {
+		// backed, not read
+		get => 1;
+		set => $value + 1;
+	}
+
+	public int $l {
+		// backed, not read, long get
+		get {
+			return 1;
+		}
+		set => $value + 1;
+	}
+
+	public int $m {
+		// it is okay to only read it sometimes
+		get {
+			if (rand(0, 1)) {
+				return 1;
+			}
+
+			return $this->m;
+		}
+		set => $value + 1;
+	}
+
+}