feat: add optional rule to report properties that should be promoted

Author: calebdw

conf/config.neon

@@ -69,6 +69,7 @@ parameters:
 	reportPossiblyNonexistentGeneralArrayOffset: false
 	reportPossiblyNonexistentConstantArrayOffset: false
 	checkMissingOverrideMethodAttribute: false
+	reportPropertiesThatShouldBePromoted: false
 	mixinExcludeClasses: []
 	scanFiles: []
 	scanDirectories: []

conf/parametersSchema.neon

@@ -79,6 +79,7 @@ parametersSchema:
 	reportPossiblyNonexistentGeneralArrayOffset: bool()
 	reportPossiblyNonexistentConstantArrayOffset: bool()
 	checkMissingOverrideMethodAttribute: bool()
+	reportPropertiesThatShouldBePromoted: bool()
 	parallel: structure([
 		jobSize: int(),
 		processTimeout: float(),

src/Rules/Properties/ReportPropertiesThatShouldBePromotedRule.php

@@ -0,0 +1,107 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Properties;
+
+use Override;
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\Error;
+use PhpParser\Node\Expr\PropertyFetch;
+use PhpParser\Node\Expr\Variable;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PHPStan\Analyser\Scope;
+use PHPStan\DependencyInjection\AutowiredParameter;
+use PHPStan\DependencyInjection\RegisteredRule;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use function sprintf;
+
+/**
+ * @implements Rule<ClassMethod>
+ */
+#[RegisteredRule(level: 0)]
+final class ReportPropertiesThatShouldBePromotedRule implements Rule
+{
+
+	public function __construct(
+		#[AutowiredParameter]
+		private bool $reportPropertiesThatShouldBePromoted,
+	)
+	{
+	}
+
+	public function getNodeType(): string
+	{
+		return ClassMethod::class;
+	}
+
+	#[Override]
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (
+			! $this->reportPropertiesThatShouldBePromoted
+				|| $node->name->toLowerString() !== '__construct'
+				|| $node->params === []
+		) {
+			return [];
+		}
+		$errors = [];
+		foreach ($node->params as $param) {
+			if (
+				$param->isPromoted()
+					|| $param->var instanceof Error
+					|| $param->var->name instanceof Expr
+					|| ! $this->assignsUnmodifiedVariableToProperty($param->var->name, $node)
+			) {
+				continue;
+			}
+			$errors[] = RuleErrorBuilder::message(sprintf('Property [%s] should be promoted.', $param->var->name))
+				->identifier('property.shouldBePromoted')
+				->line($param->var->getStartLine())
+				->build();
+		}
+		return $errors;
+	}
+
+	private function assignsUnmodifiedVariableToProperty(string $variable, ClassMethod $node): bool
+	{
+		foreach ($node->getStmts() as $stmt) {
+			if (! $stmt instanceof Expression || ! $stmt->expr instanceof Assign) {
+				continue;
+			}
+			$var = $stmt->expr->var;
+			$expr = $stmt->expr->expr;
+			if (! $var instanceof Variable && ! $var instanceof PropertyFetch) {
+				continue;
+			}
+			if ($var instanceof Variable) {
+				// The variable has been modified, so can't promote it.
+				if ($var->name === $variable) {
+					return false;
+				}
+				continue;
+			}
+			if (
+				! $var->var instanceof Variable
+					|| $var->var->name !== 'this'
+					|| ! $var->name instanceof Identifier
+					|| $var->name->toString() !== $variable
+			) {
+				continue;
+			}
+			if (! $expr instanceof Variable) {
+				continue;
+			}
+			// The variable is being assigned to a property
+			// of the same name, safe to promote it.
+			if ($expr->name === $variable) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+}

tests/PHPStan/Rules/Properties/ReportPropertiesThatShouldBePromotedRuleTest.php

@@ -0,0 +1,30 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Properties;
+
+use PHPStan\Rules\Properties\ReportPropertiesThatShouldBePromoted;
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use function sprintf;
+
+/**
+ * @extends RuleTestCase<ReportPropertiesThatShouldBePromoted>
+ */
+class ReportPropertiesThatShouldBePromotedRuleTest extends RuleTestCase
+{
+
+	protected function getRule(): Rule
+	{
+		return new ReportPropertiesThatShouldBePromoted(true);
+	}
+
+	public function testOverridingFinalMethod(): void
+	{
+		$error = static fn (string $property) => sprintf('Property [%s] should be promoted.', $property);
+
+		$this->analyse([__DIR__ . '/data/properties-that-should-be-promoted.php'], [
+			[$error('name'), 20],
+		]);
+	}
+
+}

tests/PHPStan/Rules/Properties/data/properties-that-should-be-promoted.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace PropertiesThatShouldBePromoted;
+
+class PromotedProperties
+{
+    public string $name;
+
+    /** @var list<string> */
+    public array $tags;
+
+    /** @var array<string, mixed> */
+    public array $options;
+
+    public int $count;
+
+    public int $foo;
+
+    public function __construct(
+        int $count,
+        ?string $name,
+        public ?string $email,
+        /** @var array<int, string> */
+        array $tags,
+        /** @var array<string, mixed> */
+        array $options,
+        int $bar,
+    ) {
+        $this->count = $count;
+
+        $tags          = array_values($tags);
+        $this->tags    = $tags;
+        $this->options = array_filter($options);
+        $this->email ??= '[email protected]';
+        $this->name = $name ?? 'Default Name';
+        $this->foo  = $bar;
+    }
+}