ForbidVariableTypeOverwritingRule (#34)

* ForbidVariableTypeOverwritingRule

* Better test, dont generalize array key+value

* readme array append note

README.md

@@ -224,6 +224,21 @@ function getFullName(?string $firstName, string $lastName): string {
 }
 ```
 
+### ForbidVariableTypeOverwritingRule
+- Restricts variable assignment to those that does not change its type
+  - Array append `$array[] = 1;` not yet supported
+- Null and mixed are not taken into account, advanced phpstan types like non-empty-X are trimmed before comparison
+- Rule allows type generalization and type narrowing (parent <-> child)
+```neon
+rules:
+- ShipMonk\PHPStan\Rule\ForbidVariableTypeOverwritingRule
+```
+```php
+function example(OrderId $id) {
+    $id = $id->getStringValue(); // denied, type changed from object to string
+}
+```
+
 ### ForbidUnsetClassFieldRule
 - Denies calling `unset` over class field as it causes un-initialization, see https://3v4l.org/V8uuP
 - Null assignment should be used instead

src/Rule/ForbidVariableTypeOverwritingRule.php

@@ -0,0 +1,123 @@
+<?php declare(strict_types = 1);
+
+namespace ShipMonk\PHPStan\Rule;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Assign;
+use PhpParser\Node\Expr\Variable;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Type\Accessory\AccessoryType;
+use PHPStan\Type\ConstantType;
+use PHPStan\Type\Enum\EnumCaseObjectType;
+use PHPStan\Type\GeneralizePrecision;
+use PHPStan\Type\IntegerRangeType;
+use PHPStan\Type\IntersectionType;
+use PHPStan\Type\MixedType;
+use PHPStan\Type\NullType;
+use PHPStan\Type\ObjectType;
+use PHPStan\Type\Type;
+use PHPStan\Type\TypeCombinator;
+use PHPStan\Type\UnionType;
+use PHPStan\Type\VerbosityLevel;
+
+/**
+ * @implements Rule<Assign>
+ */
+class ForbidVariableTypeOverwritingRule implements Rule
+{
+
+    public function getNodeType(): string
+    {
+        return Assign::class;
+    }
+
+    /**
+     * @param Assign $node
+     * @return string[]
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (!$node->var instanceof Variable) {
+            return []; // array append not yet supported
+        }
+
+        $variableName = $node->var->name;
+
+        if ($variableName instanceof Expr) {
+            return []; // no support for cases like $$foo
+        }
+
+        if (!$scope->hasVariableType($variableName)->yes()) {
+            return [];
+        }
+
+        $previousVariableType = $this->generalize($scope->getVariableType($variableName));
+        $newVariableType = $this->generalize($scope->getType($node->expr));
+
+        if ($this->isTypeToIgnore($previousVariableType) || $this->isTypeToIgnore($newVariableType)) {
+            return [];
+        }
+
+        if (
+            !$previousVariableType->isSuperTypeOf($newVariableType)->yes() // allow narrowing
+            && !$newVariableType->isSuperTypeOf($previousVariableType)->yes() // allow generalization
+        ) {
+            return ["Overwriting variable \$$variableName while changing its type from {$previousVariableType->describe(VerbosityLevel::precise())} to {$newVariableType->describe(VerbosityLevel::precise())}"];
+        }
+
+        return [];
+    }
+
+    private function generalize(Type $type): Type
+    {
+        if (
+            $type instanceof ConstantType
+            || $type instanceof IntegerRangeType
+            || $type instanceof EnumCaseObjectType
+            || $type instanceof UnionType // e.g. 'foo'|'bar' -> string or int<min, -1>|int<1, max> -> int
+        ) {
+            $type = $type->generalize(GeneralizePrecision::lessSpecific());
+        }
+
+        if ($type instanceof NullType) {
+            return $type;
+        }
+
+        return $this->removeNullAccessoryAndSubtractedTypes($type);
+    }
+
+    private function isTypeToIgnore(Type $type): bool
+    {
+        return $type instanceof NullType || $type instanceof MixedType;
+    }
+
+    private function removeNullAccessoryAndSubtractedTypes(Type $type): Type
+    {
+        if ($type instanceof NullType) {
+            return $type;
+        }
+
+        if ($type instanceof IntersectionType) {
+            $newInnerTypes = [];
+
+            foreach ($type->getTypes() as $innerType) {
+                if ($innerType instanceof AccessoryType) { // @phpstan-ignore-line ignore bc promise
+                    continue;
+                }
+
+                $newInnerTypes[] = $innerType;
+            }
+
+            $type = TypeCombinator::intersect(...$newInnerTypes);
+        }
+
+        if ($type instanceof ObjectType) {
+            $type = $type->changeSubtractedType(null);
+        }
+
+        return TypeCombinator::removeNull($type);
+    }
+
+}

tests/Rule/ForbidVariableTypeOverwritingRuleTest.php

@@ -0,0 +1,24 @@
+<?php declare(strict_types = 1);
+
+namespace ShipMonk\PHPStan\Rule;
+
+use PHPStan\Rules\Rule;
+use ShipMonk\PHPStan\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<ForbidVariableTypeOverwritingRule>
+ */
+class ForbidVariableTypeOverwritingRuleTest extends RuleTestCase
+{
+
+    protected function getRule(): Rule
+    {
+        return new ForbidVariableTypeOverwritingRule();
+    }
+
+    public function testClass(): void
+    {
+        $this->analyseFile(__DIR__ . '/data/ForbidVariableTypeOverwritingRule/code.php');
+    }
+
+}

tests/Rule/data/ForbidVariableTypeOverwritingRule/code.php

@@ -0,0 +1,126 @@
+<?php
+
+namespace ForbidVariableTypeOverwritingRule;
+
+interface SomeInterface {
+
+}
+
+class ParentClass {
+
+}
+
+class ChildClass1 extends ParentClass {
+
+}
+
+class ChildClass2 extends ParentClass {
+
+}
+
+class AnotherClassWithInterface implements SomeInterface {
+
+}
+
+function testGeneralizationAndNarrowing(
+    object $object,
+    SomeInterface $interface,
+    SomeInterface&ParentClass $classWithInterface1,
+    SomeInterface&ParentClass $classWithInterface2,
+    SomeInterface&ParentClass $classWithInterface3,
+    int|string $intOrString1,
+    int|string $intOrString2,
+    ParentClass $parentClass,
+    ChildClass1 $childClass1,
+    ChildClass2 $childClass2,
+) {
+    $childClass1 = new ParentClass();
+    $parentClass = new ChildClass2();
+    $childClass2 = new ChildClass1(); // error: Overwriting variable $childClass2 while changing its type from ForbidVariableTypeOverwritingRule\ChildClass2 to ForbidVariableTypeOverwritingRule\ChildClass1
+
+    $object = new ParentClass();
+    $intOrString1 = 1;
+    $intOrString2 = []; // error: Overwriting variable $intOrString2 while changing its type from int|string to array{}
+    $classWithInterface1 = new ParentClass();
+    $classWithInterface2 = new AnotherClassWithInterface(); // error: Overwriting variable $classWithInterface2 while changing its type from ForbidVariableTypeOverwritingRule\ParentClass&ForbidVariableTypeOverwritingRule\SomeInterface to ForbidVariableTypeOverwritingRule\AnotherClassWithInterface
+    $classWithInterface3 = $interface;
+}
+
+/**
+ * @param array $array
+ * @param list<int> $intList
+ * @param list<ParentClass> $objectList
+ * @param array<string, string> $map
+ */
+function testBasics(
+    array $array,
+    array $objectList,
+    string $string,
+    ParentClass $class,
+    array $map,
+    array $intList = [1],
+): void {
+    $intList = ['string']; // error: Overwriting variable $intList while changing its type from array<int, int> to array<int, string>
+    $array = 1; // error: Overwriting variable $array while changing its type from array to int
+    $string = 1; // error: Overwriting variable $string while changing its type from string to int
+    $objectList = ['foo']; // error: Overwriting variable $objectList while changing its type from array<int, ForbidVariableTypeOverwritingRule\ParentClass> to array<int, string>
+    $class = new \stdClass(); // error: Overwriting variable $class while changing its type from ForbidVariableTypeOverwritingRule\ParentClass to stdClass
+    $map = [1]; // error: Overwriting variable $map while changing its type from array<string, string> to array<int, int>
+}
+
+function testIgnoredTypes(
+    mixed $mixed1,
+    mixed $mixed2,
+    mixed $mixed3,
+    mixed $mixed4,
+    ?ParentClass $parentClass1,
+    ParentClass $parentClass2,
+): void {
+    $null = null;
+    $null = '';
+    $mixed1 = '';
+    $mixed2 = 1;
+    $mixed3 = null;
+    $parentClass1 = null;
+    $parentClass2 = $mixed4;
+}
+
+/**
+ * @param positive-int $positiveInt
+ * @param int-mask-of<1|2|4> $intMask
+ * @param list<int> $intList
+ * @param 'foo'|'bar' $stringUnion
+ * @param non-empty-string $nonEmptyString
+ * @param non-empty-array<mixed> $nonEmptyArray
+ * @param numeric-string $numericString
+ * @param array<'key1'|'key2', class-string> $strictArray
+ */
+function testAdvancedTypesAreIgnored(
+    array $nonEmptyArray,
+    array $intList,
+    mixed $mixed,
+    int $int,
+    int $positiveInt,
+    int $intMask,
+    string $string,
+    string $stringUnion,
+    string $nonEmptyString,
+    string $numericString,
+    array $strictArray,
+): void {
+    $positiveInt = $int;
+    $intMask = $int;
+    $stringUnion = $string;
+    $nonEmptyArray = ['string'];
+    $intList = [1];
+    $mixed = $nonEmptyArray['unknown'];
+    $nonEmptyString = ' ';
+    $numericString = 'not-a-number';
+    $strictArray = ['string' => 'string'];
+}
+
+function testSubtractedTypeNotKept(ParentClass $someClass) {
+    if (!$someClass instanceof ChildClass1) {
+        $someClass = new ChildClass1();
+    }
+}