ShortGetPropertyHookReturnTypeRule - level 3

Author: ondrejmirtes

conf/config.level3.neon

@@ -20,6 +20,7 @@ rules:
 	- PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyAssignRule
 	- PHPStan\Rules\Properties\ReadOnlyPropertyAssignRefRule
 	- PHPStan\Rules\Properties\ReadOnlyByPhpDocPropertyAssignRefRule
+	- PHPStan\Rules\Properties\ShortGetPropertyHookReturnTypeRule
 	- PHPStan\Rules\Properties\TypesAssignedToPropertiesRule
 	- PHPStan\Rules\Variables\ParameterOutAssignedTypeRule
 	- PHPStan\Rules\Variables\ParameterOutExecutionEndTypeRule

src/Node/InPropertyHookNode.php

@@ -27,7 +27,7 @@ public function getClassReflection(): ClassReflection
 		return $this->classReflection;
 	}
 
-	public function getMethodReflection(): PhpMethodFromParserNodeReflection
+	public function getHookReflection(): PhpMethodFromParserNodeReflection
 	{
 		return $this->hookReflection;
 	}

src/Rules/Properties/ShortGetPropertyHookReturnTypeRule.php

@@ -0,0 +1,74 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Properties;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\InPropertyHookNode;
+use PHPStan\Rules\FunctionReturnTypeCheck;
+use PHPStan\Rules\Rule;
+
+/**
+ * @implements Rule<InPropertyHookNode>
+ */
+final class ShortGetPropertyHookReturnTypeRule implements Rule
+{
+
+	public function __construct(private FunctionReturnTypeCheck $returnTypeCheck)
+	{
+	}
+
+	public function getNodeType(): string
+	{
+		return InPropertyHookNode::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		// return statements in long property hook bodies are checked by Methods\ReturnTypeRule
+		// short set property hook type is checked by TypesAssignedToPropertiesRule
+		$hookReflection = $node->getHookReflection();
+		if ($hookReflection->getPropertyHookName() !== 'get') {
+			return [];
+		}
+
+		$originalHookNode = $node->getOriginalNode();
+		$hookBody = $originalHookNode->body;
+		if (!$hookBody instanceof Node\Expr) {
+			return [];
+		}
+
+		$methodDescription = sprintf(
+			'Get hook for property %s::$%s',
+			$hookReflection->getDeclaringClass()->getDisplayName(),
+			$hookReflection->getHookedPropertyName(),
+		);
+
+		$returnType = $hookReflection->getReturnType();
+
+		return $this->returnTypeCheck->checkReturnType(
+			$scope,
+			$returnType,
+			$hookBody,
+			$node,
+			sprintf(
+				'%s should return %%s but empty return statement found.',
+				$methodDescription,
+			),
+			sprintf(
+				'%s with return type void returns %%s but should not return anything.',
+				$methodDescription,
+			),
+			sprintf(
+				'%s should return %%s but returns %%s.',
+				$methodDescription,
+			),
+			sprintf(
+				'%s should never return but return statement found.',
+				$methodDescription,
+			),
+			$hookReflection->isGenerator(),
+		);
+	}
+
+}

tests/PHPStan/Rules/Properties/ShortGetPropertyHookReturnTypeRuleTest.php

@@ -0,0 +1,56 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Properties;
+
+use PHPStan\Rules\FunctionReturnTypeCheck;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleLevelHelper;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<ShortGetPropertyHookReturnTypeRule>
+ */
+final class ShortGetPropertyHookReturnTypeRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new ShortGetPropertyHookReturnTypeRule(
+			new FunctionReturnTypeCheck(new RuleLevelHelper($this->createReflectionProvider(), true, false, true, true, false, false))
+		);
+	}
+
+	public function testRule(): void
+	{
+		if (PHP_VERSION_ID < 80400) {
+			$this->markTestSkipped('Test requires PHP 8.4.');
+		}
+
+		$this->analyse([__DIR__ . '/data/short-get-property-hook-return.php'], [
+			[
+				'Get hook for property ShortGetPropertyHookReturn\Foo::$i should return int but returns string.',
+				9,
+			],
+			[
+				'Get hook for property ShortGetPropertyHookReturn\Foo::$s should return non-empty-string but returns \'\'.',
+				18,
+			],
+			[
+				'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$a should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.',
+				36,
+				'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.',
+			],
+			[
+				'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$b should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.',
+				50,
+				'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.',
+			],
+			[
+				'Get hook for property ShortGetPropertyHookReturn\GenericFoo::$c should return T of ShortGetPropertyHookReturn\Foo but returns ShortGetPropertyHookReturn\Foo.',
+				59,
+				'Type ShortGetPropertyHookReturn\Foo is not always the same as T. It breaks the contract for some argument types, typically subtypes.',
+			],
+		]);
+	}
+
+}

tests/PHPStan/Rules/Properties/data/short-get-property-hook-return.php

@@ -0,0 +1,69 @@
+<?php // lint >= 8.4
+
+namespace ShortGetPropertyHookReturn;
+
+class Foo
+{
+
+	public int $i {
+		get => 'foo';
+	}
+
+	public int $i2 {
+		get => 1;
+	}
+
+	/** @var non-empty-string */
+	public string $s {
+		get => '';
+	}
+
+	/** @var non-empty-string */
+	public string $s2 {
+		get => 'foo';
+	}
+
+}
+
+/**
+ * @template T of Foo
+ */
+class GenericFoo
+{
+
+	/** @var T */
+	public Foo $a {
+		get => new Foo();
+	}
+
+	/** @var T */
+	public Foo $a2 {
+		get => $this->a2;
+	}
+
+	/**
+	 * @param T $c
+	 */
+	public function __construct(
+		/** @var T */
+		public Foo $b {
+			get => new Foo();
+		},
+
+		/** @var T */
+		public Foo $b2 {
+			get => $this->b2;
+		},
+
+		public Foo $c {
+			get => new Foo();
+		},
+
+		public Foo $c2 {
+			get => $this->c2;
+		}
+	)
+	{
+	}
+
+}