Detect public static $modules when it should be protected static $modules in tests (#597)

* First stab.

* strict types

* adjust message

---------

Co-authored-by: Matt Glaman <[email protected]>

Author: Boegie

rules.neon

@@ -1,20 +1,21 @@
-rules:
-	- mglaman\PHPStanDrupal\Rules\Drupal\Coder\DiscouragedFunctionsRule
-	- mglaman\PHPStanDrupal\Rules\Drupal\GlobalDrupalDependencyInjectionRule
-	- mglaman\PHPStanDrupal\Rules\Drupal\PluginManager\PluginManagerSetsCacheBackendRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\AccessDeprecatedConstant
-	- mglaman\PHPStanDrupal\Rules\Classes\ClassExtendsInternalClassRule
-	- mglaman\PHPStanDrupal\Rules\Classes\PluginManagerInspectionRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\ConditionManagerCreateInstanceContextConfigurationRule
-	- mglaman\PHPStanDrupal\Rules\Drupal\RenderCallbackRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\StaticServiceDeprecatedServiceRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\GetDeprecatedServiceRule
-	- mglaman\PHPStanDrupal\Rules\Drupal\Tests\BrowserTestBaseDefaultThemeRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\ConfigEntityConfigExportRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\PluginAnnotationContextDefinitionsRule
-	- mglaman\PHPStanDrupal\Rules\Drupal\ModuleLoadInclude
-	- mglaman\PHPStanDrupal\Rules\Drupal\LoadIncludes
-	- mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery\EntityQueryHasAccessCheckRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\SymfonyCmfRouteObjectInterfaceConstantsRule
-	- mglaman\PHPStanDrupal\Rules\Deprecations\SymfonyCmfRoutingInClassMethodSignatureRule
-	- mglaman\PHPStanDrupal\Rules\Drupal\RequestStackGetMainRequestRule
+rules:
+	- mglaman\PHPStanDrupal\Rules\Drupal\Coder\DiscouragedFunctionsRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\GlobalDrupalDependencyInjectionRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\PluginManager\PluginManagerSetsCacheBackendRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\AccessDeprecatedConstant
+	- mglaman\PHPStanDrupal\Rules\Classes\ClassExtendsInternalClassRule
+	- mglaman\PHPStanDrupal\Rules\Classes\PluginManagerInspectionRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\ConditionManagerCreateInstanceContextConfigurationRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\RenderCallbackRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\StaticServiceDeprecatedServiceRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\GetDeprecatedServiceRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\Tests\BrowserTestBaseDefaultThemeRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\ConfigEntityConfigExportRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\PluginAnnotationContextDefinitionsRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\ModuleLoadInclude
+	- mglaman\PHPStanDrupal\Rules\Drupal\LoadIncludes
+	- mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery\EntityQueryHasAccessCheckRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\SymfonyCmfRouteObjectInterfaceConstantsRule
+	- mglaman\PHPStanDrupal\Rules\Deprecations\SymfonyCmfRoutingInClassMethodSignatureRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\RequestStackGetMainRequestRule
+	- mglaman\PHPStanDrupal\Rules\Drupal\TestClassesProtectedPropertyModulesRule

src/Rules/Drupal/TestClassesProtectedPropertyModulesRule.php

@@ -0,0 +1,57 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Rules\Drupal;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\ClassPropertyNode;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\ShouldNotHappenException;
+use PHPUnit\Framework\TestCase;
+
+class TestClassesProtectedPropertyModulesRule implements Rule
+{
+
+    public function getNodeType(): string
+    {
+        return ClassPropertyNode::class;
+    }
+
+    /**
+     * @throws \PHPStan\ShouldNotHappenException
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        assert($node instanceof ClassPropertyNode);
+
+        if (!$scope->isInClass()) {
+            throw new ShouldNotHappenException();
+        }
+
+        if ($node->getName() !== 'modules') {
+            return [];
+        }
+
+        $scopeClassReflection = $scope->getClassReflection();
+        if ($scopeClassReflection->isAnonymous()) {
+            return [];
+        }
+
+        if (!in_array(TestCase::class, $scopeClassReflection->getParentClassesNames(), true)) {
+            return [];
+        }
+
+        if ($node->isPublic()) {
+            return [
+                RuleErrorBuilder::message(
+                    sprintf('Property %s::$modules property must be protected.', $scopeClassReflection->getDisplayName())
+                )->tip('Change record: https://www.drupal.org/node/2909426')->build(),
+            ];
+        }
+
+        return [];
+    }
+}

tests/src/Rules/TestClassesProtectedPropertyModulesRuleTest.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Tests\Rules;
+
+use mglaman\PHPStanDrupal\Rules\Drupal\TestClassesProtectedPropertyModulesRule;
+use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
+use PHPStan\Rules\Rule;
+
+class TestClassesProtectedPropertyModulesRuleTest extends DrupalRuleTestCase
+{
+
+    protected function getRule(): Rule
+    {
+        return new TestClassesProtectedPropertyModulesRule();
+    }
+
+    /**
+     * @dataProvider cases
+     */
+    public function test(array $files, array $errors): void
+    {
+        $this->analyse($files, $errors);
+    }
+
+    public function cases(): \Generator
+    {
+        yield [
+            [__DIR__ . '/data/test-cases-test-classes-protected-property-modules-rule.php'],
+            [
+                [
+                    'Property TestCasesTestClassesProtectedPropertyModulesRule\PublicStaticPropertyModulesClass::$modules property must be protected.',
+                    25,
+                    'Change record: https://www.drupal.org/node/2909426',
+                ],
+            ],
+        ];
+    }
+
+}

tests/src/Rules/data/test-cases-test-classes-protected-property-modules-rule.php

@@ -0,0 +1,86 @@
+<?php
+
+namespace TestCasesTestClassesProtectedPropertyModulesRule;
+
+use Drupal\node\Entity\Node;
+use Drupal\Tests\node\Functional\NodeAccessFieldTest;
+
+class ProtectedStaticPropertyModulesClass extends NodeAccessFieldTest {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    protected static $modules = ['action', 'user'];
+}
+
+class PublicStaticPropertyModulesClass extends NodeAccessFieldTest {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    public static $modules = ['action', 'user'];
+}
+
+class ProtectedStaticPropertyNotModulesClass extends NodeAccessFieldTest {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    protected static $not_modules = ['action', 'user'];
+}
+
+class PublicStaticPropertyNotModulesClass extends NodeAccessFieldTest {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    public static $not_modules = ['action', 'user'];
+}
+
+class NonExtendingProtectedStaticPropertyModulesClass {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    protected static $modules = ['action', 'user'];
+}
+
+class NonExtendingPublicStaticPropertyModulesClass {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    public static $modules = ['action', 'user'];
+}
+
+class NonExtendingPHPUnitFrameworkTestCaseProtectedStaticPropertyModulesClass extends Node {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    protected static $modules = ['action', 'user'];
+}
+
+class NonExtendingPHPUnitFrameworkTestCasePublicStaticPropertyModulesClass extends Node {
+
+    /**
+     * Modules to install.
+     *
+     * @var array
+     */
+    public static $modules = ['action', 'user'];
+}