add: Rule to inform user to follow silverstripe config doc practices (#8)

Author: mleutenegger

phpstan.neon

@@ -88,6 +88,15 @@ services:
         class: Syntro\SilverstripePHPStan\Type\HasMethodTypeSpecifyingExtension
         tags:
             - phpstan.typeSpecifier.methodTypeSpecifyingExtension
+    # Rule informing the user to add the @config docblock
+    -
+        class: Syntro\SilverstripePHPStan\Rule\ReadWriteConfigPropertiesRule
+        arguments:
+            alwaysWrittenTags: %propertyAlwaysWrittenTags%
+            alwaysReadTags: %propertyAlwaysReadTags%
+            checkUninitializedProperties: %checkUninitializedProperties%
+        tags:
+            - phpstan.rules.rule
     # Special handling for configuration properties
     -
         class: Syntro\SilverstripePHPStan\Reflection\ReadWritePropertiesExtension

src/Rule/ReadWriteConfigPropertiesRule.php

@@ -0,0 +1,54 @@
+<?php declare(strict_types = 1);
+
+namespace Syntro\SilverstripePHPStan\Rule;
+
+use PhpParser\Node;
+use PHPStan\ShouldNotHappenException;
+use PHPStan\Analyser\Scope;
+use PHPStan\Node\ClassPropertiesNode;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Rules\DeadCode\UnusedPrivatePropertyRule;
+use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
+use PHPStan\Type\MixedType;
+use PHPStan\Type\ObjectType;
+use PHPStan\Type\TypeUtils;
+use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Extension;
+
+/**
+ * Adds a rule to add a hint to never read, never written or unused properties,
+ * instructing the user to add an "@config" docblock if necessary
+ */
+class ReadWriteConfigPropertiesRule extends UnusedPrivatePropertyRule /* @phpstan-ignore-line */
+{
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $classReflection = $scope->getClassReflection();
+        if ($classReflection->hasTraitUse(Configurable::class) || $classReflection->isSubclassOf(Extension::class)) {
+            $errors = parent::processNode($node, $scope); /* @phpstan-ignore-line */
+            $hints = [];
+            /** @var \PHPStan\Rules\RuleErrors\RuleError11 $error */
+            foreach ($errors as $error) {
+                $message = $error->getMessage(); /* @phpstan-ignore-line */
+                if (strpos($message, 'Static') !== 0 || strpos($message, 'never written, only read') !== false) {
+                    // NOTE(mleutenegger): 2022-04-30
+                    // In this case, the property wasn't static or the property
+                    // was never written, but only read via $this->property.
+                    // This should not actually happen with config properties.
+                    // In this case, we assume that the error is genuine and
+                    // dont print a hint. We might need to handle this case in
+                    // the future, when adding config rules.
+                    continue;
+                }
+                $nameFound = preg_match("/(\S+)::(\S+)/", $message, $names);
+                if ($nameFound) {
+                    $tip = sprintf('See: %s', 'https://docs.silverstripe.org/en/4/developer_guides/configuration/configuration/');
+                    $hints[] = RuleErrorBuilder::message(sprintf('Have you forgotten to add "@config" for the property %s of the configurable class %s?', $names[2], $names[1]))->line($error->line)->tip($tip)->build();
+                }
+            }
+            return $hints;
+        }
+        return [];
+    }
+}

tests/Rule/Data/ReadWritePropertiesConfig.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace Syntro\SilverstripePHPStan\Tests\Rule\Data;
+
+use SilverStripe\Core\Config\Configurable;
+
+class Foo
+{
+    use Configurable;
+
+    private static $this_should_be_config = 'a value';
+
+    /**
+     * @config
+     */
+    private static $this_is_config = 'another value';
+}

tests/Rule/Data/ReadWritePropertiesNoConfig.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Syntro\SilverstripePHPStan\Tests\Rule\Data;
+
+class NoFoo
+{
+    private static $this_should_be_config = 'a value';
+
+    /**
+     * @config
+     */
+    private static $this_is_config = 'another value';
+}

tests/Rule/DirectReadWritePropertiesExtensionProvider.php

@@ -0,0 +1,31 @@
+<?php declare(strict_types = 1);
+
+namespace Syntro\SilverstripePHPStan\Tests\Rule;
+
+use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
+use PHPStan\Rules\Properties\ReadWritePropertiesExtension;
+
+class DirectReadWritePropertiesExtensionProvider implements ReadWritePropertiesExtensionProvider /* @phpstan-ignore-line */
+{
+
+    /**
+     * @var ReadWritePropertiesExtension[]
+     */
+    private $extensions;
+
+    /**
+     * @param ReadWritePropertiesExtension[] $extensions
+     */
+    public function __construct($extensions)
+    {
+        $this->extensions = $extensions;
+    }
+
+    /**
+     * @return ReadWritePropertiesExtension[]
+     */
+    public function getExtensions(): array
+    {
+        return $this->extensions;
+    }
+}

tests/Rule/ReadWriteConfigPropertiesRuleTest.php

@@ -0,0 +1,38 @@
+<?php declare(strict_types = 1);
+
+namespace Syntro\SilverstripePHPStan\Tests\Rule;
+
+use Syntro\SilverstripePHPStan\Rule\ReadWriteConfigPropertiesRule;
+use Syntro\SilverstripePHPStan\Reflection\ReadWritePropertiesExtension;
+use Syntro\SilverstripePHPStan\Tests\Rule\DirectReadWritePropertiesExtensionProvider;
+use PHPStan\Rules\Rule;
+
+class ReadWriteConfigPropertiesRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+
+    protected function getRule(): Rule
+    {
+        return new ReadWriteConfigPropertiesRule(
+            new DirectReadWritePropertiesExtensionProvider([new ReadWritePropertiesExtension()]),
+            [],
+            [],
+            true
+        );
+    }
+
+    public function testAddsHintsForConfigurable(): void
+    {
+        $this->analyse([__DIR__ . '/Data/ReadWritePropertiesConfig.php'], [
+            [
+                'Have you forgotten to add "@config" for the property $this_should_be_config of the configurable class Syntro\SilverstripePHPStan\Tests\Rule\Data\Foo?',
+                11,
+                'See: https://docs.silverstripe.org/en/4/developer_guides/configuration/configuration/',
+            ],
+        ]);
+    }
+
+    public function testAddsNoHintsForNoConfigurable(): void
+    {
+        $this->analyse([__DIR__ . '/Data/ReadWritePropertiesNoConfig.php'], []);
+    }
+}