diff --git a/rules.neon b/rules.neon index 687dbf3..3b8f325 100644 --- a/rules.neon +++ b/rules.neon @@ -18,6 +18,8 @@ parameters: enabled: %shipmonkRules.enableAllRules% enforceListReturn: enabled: %shipmonkRules.enableAllRules% + enforceNativePropertyTypehint: + enabled: %shipmonkRules.enableAllRules% enforceNativeReturnTypehint: enabled: %shipmonkRules.enableAllRules% enforceReadonlyPublicProperty: @@ -116,6 +118,9 @@ parametersSchema: enforceListReturn: structure([ enabled: bool() ]) + enforceNativePropertyTypehint: structure([ + enabled: bool() + ]) enforceNativeReturnTypehint: structure([ enabled: bool() ]) @@ -230,6 +235,8 @@ conditionalTags: phpstan.rules.rule: %shipmonkRules.enforceEnumMatch.enabled% ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule: phpstan.rules.rule: %shipmonkRules.enforceIteratorToArrayPreserveKeys.enabled% + ShipMonk\PHPStan\Rule\EnforceNativePropertyTypehintRule: + phpstan.rules.rule: %shipmonkRules.enforceNativePropertyTypehint.enabled% ShipMonk\PHPStan\Rule\EnforceNativeReturnTypehintRule: phpstan.rules.rule: %shipmonkRules.enforceNativeReturnTypehint.enabled% ShipMonk\PHPStan\Rule\EnforceListReturnRule: @@ -319,6 +326,10 @@ services: class: ShipMonk\PHPStan\Rule\EnforceIteratorToArrayPreserveKeysRule - class: ShipMonk\PHPStan\Rule\EnforceListReturnRule + - + class: ShipMonk\PHPStan\Rule\EnforceNativePropertyTypehintRule + arguments: + treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain% - class: ShipMonk\PHPStan\Rule\EnforceNativeReturnTypehintRule arguments: diff --git a/src/Rule/EnforceNativePropertyTypehintRule.php b/src/Rule/EnforceNativePropertyTypehintRule.php new file mode 100644 index 0000000..af2e2f4 --- /dev/null +++ b/src/Rule/EnforceNativePropertyTypehintRule.php @@ -0,0 +1,362 @@ + + */ +class EnforceNativePropertyTypehintRule implements Rule +{ + + private FileTypeMapper $fileTypeMapper; + + private PhpVersion $phpVersion; + + private bool $treatPhpDocTypesAsCertain; + + public function __construct( + FileTypeMapper $fileTypeMapper, + PhpVersion $phpVersion, + bool $treatPhpDocTypesAsCertain + ) + { + $this->fileTypeMapper = $fileTypeMapper; + $this->phpVersion = $phpVersion; + $this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain; + } + + public function getNodeType(): string + { + return Property::class; + } + + /** + * @param Property $node + * @return list + */ + public function processNode( + Node $node, + Scope $scope + ): array + { + if ($this->treatPhpDocTypesAsCertain === false) { + return []; + } + + if ($node->type !== null) { + return []; + } + + if ($scope->isInTrait()) { + return []; // type may easily differ for each usage + } + + if ($this->phpVersion->getVersionId() < 70_400) { + return []; // typed properties not available + } + + $classReflection = $scope->getClassReflection(); + + if ($classReflection === null) { + return []; + } + + $phpDocType = $this->getPhpDocVarType($node, $scope); + + if ($phpDocType === null) { + return []; + } + + $errors = []; + + foreach ($node->props as $prop) { + $propertyName = $prop->name->name; + + if ($this->isPropertyDeclaredInParent($propertyName, $scope)) { + continue; // avoid LSP issues + } + + $typeHint = $this->getTypehintByType($phpDocType, $scope, true); + + if ($typeHint === null) { + continue; + } + + $errors[] = RuleErrorBuilder::message(sprintf('Missing native property typehint %s', $typeHint)) + ->identifier('shipmonk.missingNativePropertyTypehint') + ->line($prop->getStartLine()) + ->build(); + } + + return $errors; + } + + private function getPhpDocVarType(Property $node, Scope $scope): ?Type + { + $docComment = $node->getDocComment(); + + if ($docComment === null) { + return null; + } + + $classReflection = $scope->getClassReflection(); + + if ($classReflection === null) { + return null; + } + + $resolvedPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $classReflection->getName(), + $scope->getTraitReflection() === null ? null : $scope->getTraitReflection()->getName(), + null, + $docComment->getText(), + ); + + $varTags = $resolvedPhpDoc->getVarTags(); + + foreach ($varTags as $varTag) { + return $varTag->getType(); + } + + return null; + } + + private function isPropertyDeclaredInParent(string $propertyName, Scope $scope): bool + { + $classReflection = $scope->getClassReflection(); + + if ($classReflection === null) { + return false; + } + + $parent = $classReflection->getParentClass(); + + while ($parent !== null) { + if ($parent->hasNativeProperty($propertyName)) { + return true; + } + + $parent = $parent->getParentClass(); + } + + return false; + } + + private function getTypehintByType( + Type $type, + Scope $scope, + bool $topLevel + ): ?string + { + if ($type instanceof MixedType || $this->isUnionTypeWithMixed($type)) { + return $this->phpVersion->getVersionId() >= 80_000 ? 'mixed' : null; + } + + if ($type->isNull()->yes()) { + if (!$topLevel || $this->phpVersion->getVersionId() >= 80_200) { + return 'null'; + } + + return null; + } + + // Check union types first (handles nullable unions properly) + if ($type instanceof UnionType) { + return $this->getUnionTypehint($type, $scope); + } + + // Check intersection types before other type checks + if ($type instanceof IntersectionType) { // @phpstan-ignore phpstanApi.instanceofType + return $this->getIntersectionTypehint($type, $scope); + } + + $typeWithoutNull = TypeCombinator::removeNull($type); + $typeHint = null; + + if ((new BooleanType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + if (($typeWithoutNull->isTrue()->yes() || $typeWithoutNull->isFalse()->yes()) && $this->phpVersion->getVersionId() >= 80_200) { + $typeHint = $typeWithoutNull->describe(VerbosityLevel::typeOnly()); + } else { + $typeHint = 'bool'; + } + } elseif ((new IntegerType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + $typeHint = 'int'; + } elseif ((new FloatType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + $typeHint = 'float'; + } elseif ((new ArrayType(new MixedType(), new MixedType()))->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + $typeHint = 'array'; + } elseif ((new StringType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + $typeHint = 'string'; + } elseif ($typeWithoutNull instanceof StaticType) { + $typeHint = 'self'; // properties cannot use static keyword + } elseif (count($typeWithoutNull->getObjectClassNames()) === 1) { + $className = $typeWithoutNull->getObjectClassNames()[0]; + + if ($className === $this->getClassName($scope)) { + $typeHint = 'self'; + } else { + $typeHint = '\\' . $className; + } + } elseif ((new IterableType(new MixedType(), new MixedType()))->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + $typeHint = 'iterable'; + } elseif ((new ObjectWithoutClassType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) { + $typeHint = 'object'; + } + + if ($typeHint !== null && TypeCombinator::containsNull($type)) { + $typeHint = '?' . $typeHint; + } + + return $typeHint; + } + + private function getClassName(Scope $scope): ?string + { + if ($scope->getClassReflection() === null) { + return null; + } + + return $scope->getClassReflection()->getName(); + } + + private function getUnionTypehint( + Type $type, + Scope $scope + ): ?string + { + if (!$type instanceof UnionType) { + return null; + } + + // Handle nullable types specially for PHP < 8.0 + if (!$this->phpVersion->supportsNativeUnionTypes()) { + // Check if this is a simple nullable type (T|null with exactly 2 types) + $types = $type->getTypes(); + + if (count($types) === 2 && TypeCombinator::containsNull($type)) { + $typeWithoutNull = TypeCombinator::removeNull($type); + $innerHint = $this->getTypehintByType($typeWithoutNull, $scope, false); + + if ($innerHint !== null) { + return '?' . $innerHint; + } + } + + return null; + } + + $typehintParts = []; + + foreach ($type->getTypes() as $subtype) { + $wrap = false; + + if ($subtype instanceof IntersectionType) { // @phpstan-ignore phpstanApi.instanceofType + if ($this->phpVersion->getVersionId() < 80_200) { // DNF + return null; + } + + $wrap = true; + } + + $subtypeHint = $this->getTypehintByType($subtype, $scope, false); + + if ($subtypeHint === null) { + return null; + } + + if (in_array($subtypeHint, $typehintParts, true)) { + continue; + } + + $typehintParts[] = $wrap ? "($subtypeHint)" : $subtypeHint; + } + + return implode('|', $typehintParts); + } + + private function getIntersectionTypehint( + Type $type, + Scope $scope + ): ?string + { + if (!$type instanceof IntersectionType) { // @phpstan-ignore phpstanApi.instanceofType + return null; + } + + if (!$this->phpVersion->supportsPureIntersectionTypes()) { + return null; + } + + $typehintParts = []; + + foreach ($type->getTypes() as $subtype) { + $wrap = false; + + if ($subtype instanceof UnionType) { + if ($this->phpVersion->getVersionId() < 80_200) { // DNF + return null; + } + + $wrap = true; + } + + $subtypeHint = $this->getTypehintByType($subtype, $scope, false); + + if ($subtypeHint === null) { + return null; + } + + if (in_array($subtypeHint, $typehintParts, true)) { + continue; + } + + $typehintParts[] = $wrap ? "($subtypeHint)" : $subtypeHint; + } + + return implode('&', $typehintParts); + } + + private function isUnionTypeWithMixed(Type $type): bool + { + if (!$type instanceof UnionType) { + return false; + } + + foreach ($type->getTypes() as $innerType) { + if ($innerType instanceof MixedType) { + return true; + } + } + + return false; + } + +} diff --git a/tests/Rule/EnforceNativePropertyTypehintRuleTest.php b/tests/Rule/EnforceNativePropertyTypehintRuleTest.php new file mode 100644 index 0000000..f63ce93 --- /dev/null +++ b/tests/Rule/EnforceNativePropertyTypehintRuleTest.php @@ -0,0 +1,73 @@ + + */ +class EnforceNativePropertyTypehintRuleTest extends RuleTestCase +{ + + private ?PhpVersion $phpVersion = null; + + protected function getRule(): Rule + { + self::assertNotNull($this->phpVersion); + + return new EnforceNativePropertyTypehintRule( + self::getContainer()->getByType(FileTypeMapper::class), + $this->phpVersion, + true, + ); + } + + public function testPhp74(): void + { + $this->phpVersion = $this->createPhpVersion(70_400); + $this->analyseFile(__DIR__ . '/data/EnforceNativePropertyTypehintRule/code-74.php'); + } + + public function testPhp80(): void + { + $this->phpVersion = $this->createPhpVersion(80_000); + $this->analyseFile(__DIR__ . '/data/EnforceNativePropertyTypehintRule/code-80.php'); + } + + public function testPhp81(): void + { + if (PHP_VERSION_ID < 80_100) { + self::markTestSkipped('Requires PHP 8.1'); + } + + $this->phpVersion = $this->createPhpVersion(80_100); + $this->analyseFile(__DIR__ . '/data/EnforceNativePropertyTypehintRule/code-81.php'); + } + + public function testPhp82(): void + { + if (PHP_VERSION_ID < 80_100) { + self::markTestSkipped('Requires PHP 8.1'); + } + + $this->phpVersion = $this->createPhpVersion(80_200); + $this->analyseFile(__DIR__ . '/data/EnforceNativePropertyTypehintRule/code-82.php'); + } + + public function testParent(): void + { + $this->phpVersion = $this->createPhpVersion(80_000); + $this->analyseFile(__DIR__ . '/data/EnforceNativePropertyTypehintRule/code-parent.php'); + } + + private function createPhpVersion(int $version): PhpVersion + { + return new PhpVersion($version); + } + +} diff --git a/tests/Rule/data/EnforceNativePropertyTypehintRule/code-74.php b/tests/Rule/data/EnforceNativePropertyTypehintRule/code-74.php new file mode 100644 index 0000000..b1ff6f1 --- /dev/null +++ b/tests/Rule/data/EnforceNativePropertyTypehintRule/code-74.php @@ -0,0 +1,91 @@ + */ + public $shouldHaveArray2; // no error - list is not representable as array in native type + + /** @var array */ + public $shouldHaveArray3; // error: Missing native property typehint array + + /** @var array{id: int} */ + public $shouldHaveArray4; // error: Missing native property typehint array + + /** @var object */ + public $shouldHaveObject; // error: Missing native property typehint object + + /** @var iterable */ + public $shouldHaveIterable; // error: Missing native property typehint iterable + + /** @var self */ + public $shouldHaveSelf; // error: Missing native property typehint self + + /** @var A */ + public $shouldHaveClass; // error: Missing native property typehint \EnforceNativePropertyTypehintRule74\A + + /** @var \stdClass */ + public $shouldHaveStdClass; // error: Missing native property typehint \stdClass + + /** @var ?int */ + public $shouldHaveNullableInt; // error: Missing native property typehint ?int + + /** @var int|null */ + public $shouldHaveNullableInt2; // error: Missing native property typehint ?int + + /** @var ?string */ + public $shouldHaveNullableString; // error: Missing native property typehint ?string + + /** @var ?A */ + public $shouldHaveNullableClass; // error: Missing native property typehint ?\EnforceNativePropertyTypehintRule74\A + + /** @var class-string */ + public $shouldHaveString2; // error: Missing native property typehint string + + // Should NOT report - already has type + public int $hasType; + + // Should NOT report - no @var annotation + public $noPhpDoc; + + // Should NOT report - union type not available in 7.4 + /** @var string|int */ + public $unionType; + + // Should NOT report - mixed not available in 7.4 + /** @var mixed */ + public $mixedType; + + // Should NOT report - null standalone not available in 7.4 + /** @var null */ + public $nullType; + + // Should NOT report - callable not allowed for properties + /** @var callable */ + public $callableType; + +} + +trait MyTrait { + // Should NOT report - in trait + /** @var int */ + public $traitProperty; +} diff --git a/tests/Rule/data/EnforceNativePropertyTypehintRule/code-80.php b/tests/Rule/data/EnforceNativePropertyTypehintRule/code-80.php new file mode 100644 index 0000000..0597744 --- /dev/null +++ b/tests/Rule/data/EnforceNativePropertyTypehintRule/code-80.php @@ -0,0 +1,57 @@ +