Skip to content

Commit

Permalink
forbidPhpDocNullabilityMismatchWithNativeTypehint (#64)
Browse files Browse the repository at this point in the history
* forbidPhpDocNullabilityMismatchWithNativeTypehint

* better docs
  • Loading branch information
janedbal committed Jan 2, 2023
1 parent 6c33d02 commit 5e470fb
Show file tree
Hide file tree
Showing 10 changed files with 338 additions and 5 deletions.
14 changes: 14 additions & 0 deletions README.md
Expand Up @@ -50,6 +50,8 @@ parameters:
forbidNullInBinaryOperations:
enabled: true
blacklist: ['===', '!==', '??']
forbidPhpDocNullabilityMismatchWithNativeTypehint:
enabled: true
forbidVariableTypeOverwriting:
enabled: true
forbidUnsetClassField:
Expand Down Expand Up @@ -290,6 +292,18 @@ function getFullName(?string $firstName, string $lastName): string {
}
```

### forbidPhpDocNullabilityMismatchWithNativeTypehint
- Disallows having nullable native typehint while using non-nullable phpdoc
- Checks `@return` and `@param` over methods and `@var` over properties
- PHPStan itself allows using subtype of native type in phpdoc, but [resolves overall type as union of those types](https://phpstan.org/r/6f447c03-d79b-4731-b8c8-125eab3e56fc) making such phpdoc actually invalid

```php
/**
* @param string $param
*/
public function sayHello(?string $param) {} // invalid phpdoc not containing null
```

### forbidVariableTypeOverwriting
- Restricts variable assignment to those that does not change its type
- Array append `$array[] = 1;` not yet supported
Expand Down
9 changes: 9 additions & 0 deletions rules.neon
Expand Up @@ -32,6 +32,8 @@ parameters:
forbidNullInBinaryOperations:
enabled: true
blacklist: ['===', '!==', '??']
forbidPhpDocNullabilityMismatchWithNativeTypehint:
enabled: true
forbidVariableTypeOverwriting:
enabled: true
forbidUnsetClassField:
Expand Down Expand Up @@ -98,6 +100,9 @@ parametersSchema:
enabled: bool()
blacklist: arrayOf(string())
])
forbidPhpDocNullabilityMismatchWithNativeTypehint: structure([
enabled: bool()
])
forbidVariableTypeOverwriting: structure([
enabled: bool()
])
Expand Down Expand Up @@ -154,6 +159,8 @@ conditionalTags:
phpstan.rules.rule: %shipmonkRules.forbidNullInAssignOperations.enabled%
ShipMonk\PHPStan\Rule\ForbidNullInBinaryOperationsRule:
phpstan.rules.rule: %shipmonkRules.forbidNullInBinaryOperations.enabled%
ShipMonk\PHPStan\Rule\ForbidPhpDocNullabilityMismatchWithNativeTypehintRule:
phpstan.rules.rule: %shipmonkRules.forbidPhpDocNullabilityMismatchWithNativeTypehint.enabled%
ShipMonk\PHPStan\Rule\ForbidVariableTypeOverwritingRule:
phpstan.rules.rule: %shipmonkRules.forbidVariableTypeOverwriting.enabled%
ShipMonk\PHPStan\Rule\ForbidUnsetClassFieldRule:
Expand Down Expand Up @@ -225,6 +232,8 @@ services:
class: ShipMonk\PHPStan\Rule\ForbidNullInBinaryOperationsRule
arguments:
blacklist: %shipmonkRules.forbidNullInBinaryOperations.blacklist%
-
class: ShipMonk\PHPStan\Rule\ForbidPhpDocNullabilityMismatchWithNativeTypehintRule
-
class: ShipMonk\PHPStan\Rule\ForbidVariableTypeOverwritingRule
-
Expand Down
213 changes: 213 additions & 0 deletions src/Rule/ForbidPhpDocNullabilityMismatchWithNativeTypehintRule.php
@@ -0,0 +1,213 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\Rule;

use PhpParser\Node;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\FunctionLike;
use PhpParser\Node\Stmt\Property;
use PHPStan\Analyser\Scope;
use PHPStan\PhpDoc\ResolvedPhpDocBlock;
use PHPStan\Rules\Rule;
use PHPStan\Type\FileTypeMapper;
use PHPStan\Type\NullType;
use PHPStan\Type\Type;
use function array_merge;
use function is_string;

/**
* @implements Rule<Node>
*/
class ForbidPhpDocNullabilityMismatchWithNativeTypehintRule implements Rule
{

private FileTypeMapper $fileTypeMapper;

public function __construct(
FileTypeMapper $fileTypeMapper
)
{
$this->fileTypeMapper = $fileTypeMapper;
}

public function getNodeType(): string
{
return Node::class;
}

/**
* @return list<string>
*/
public function processNode(Node $node, Scope $scope): array
{
if ($node instanceof FunctionLike) {
return [
...$this->checkReturnTypes($node, $scope),
...$this->checkParamTypes($node, $scope),
];
}

if ($node instanceof Property) {
return $this->checkPropertyTypes($node, $scope);
}

return [];
}

/**
* @return list<string>
*/
private function checkReturnTypes(FunctionLike $node, Scope $scope): array
{
$phpDocReturnType = $this->getFunctionPhpDocReturnType($node, $scope);
$nativeReturnType = $this->getFunctionNativeReturnType($node, $scope);

return $this->comparePhpDocAndNativeType($phpDocReturnType, $nativeReturnType, $scope, '@return');
}

/**
* @return list<string>
*/
private function checkPropertyTypes(Property $node, Scope $scope): array
{
$phpDocReturnType = $this->getPropertyPhpDocType($node, $scope);
$nativeReturnType = $this->getPropertyNativeType($node, $scope);

return $this->comparePhpDocAndNativeType($phpDocReturnType, $nativeReturnType, $scope, '@var');
}

/**
* @return list<string>
*/
private function checkParamTypes(FunctionLike $node, Scope $scope): array
{
$errors = [];

foreach ($node->getParams() as $param) {
if (!$param->var instanceof Variable || !is_string($param->var->name)) {
continue;
}

$paramName = $param->var->name;

$phpDocParamType = $this->getPhpDocParamType($node, $scope, $paramName);
$nativeParamType = $scope->getFunctionType($param->type, false, false);

$errors = array_merge(
$errors,
$this->comparePhpDocAndNativeType($phpDocParamType, $nativeParamType, $scope, "@param \$$paramName"),
);
}

return $errors;
}

private function getPropertyNativeType(Property $node, Scope $scope): ?Type
{
if ($node->type === null) {
return null;
}

return $scope->getFunctionType($node->type, false, false);
}

private function getFunctionNativeReturnType(FunctionLike $node, Scope $scope): ?Type
{
if ($node->getReturnType() === null) {
return null;
}

return $scope->getFunctionType($node->getReturnType(), false, false);
}

private function getPropertyPhpDocType(Property $node, Scope $scope): ?Type
{
$resolvedPhpDoc = $this->resolvePhpDoc($node, $scope);

if ($resolvedPhpDoc === null) {
return null;
}

$varTags = $resolvedPhpDoc->getVarTags();

foreach ($varTags as $varTag) {
return $varTag->getType();
}

return null;
}

private function getFunctionPhpDocReturnType(FunctionLike $node, Scope $scope): ?Type
{
$resolvedPhpDoc = $this->resolvePhpDoc($node, $scope);

if ($resolvedPhpDoc === null) {
return null;
}

$returnTag = $resolvedPhpDoc->getReturnTag();

if ($returnTag === null) {
return null;
}

return $returnTag->getType();
}

private function resolvePhpDoc(Node $node, Scope $scope): ?ResolvedPhpDocBlock
{
$docComment = $node->getDocComment();

if ($docComment === null) {
return null;
}

return $this->fileTypeMapper->getResolvedPhpDoc(
$scope->getFile(),
$scope->getClassReflection() === null ? null : $scope->getClassReflection()->getName(),
$scope->getTraitReflection() === null ? null : $scope->getTraitReflection()->getName(),
$scope->getFunctionName(),
$docComment->getText(),
);
}

private function getPhpDocParamType(FunctionLike $node, Scope $scope, string $parameterName): ?Type
{
$resolvedPhpDoc = $this->resolvePhpDoc($node, $scope);

if ($resolvedPhpDoc === null) {
return null;
}

$paramTags = $resolvedPhpDoc->getParamTags();

foreach ($paramTags as $paramTagName => $paramTag) {
if ($paramTagName === $parameterName) {
return $paramTag->getType();
}
}

return null;
}

/**
* @return list<string>
*/
private function comparePhpDocAndNativeType(?Type $phpDocReturnType, ?Type $nativeReturnType, Scope $scope, string $phpDocIdentification): array
{
if ($phpDocReturnType === null || $nativeReturnType === null) {
return [];
}

$strictTypes = $scope->isDeclareStrictTypes();
$nullType = new NullType();

// the inverse check is performed by native PHPStan rule checking that phpdoc is subtype of native type
if (!$phpDocReturnType->accepts($nullType, $strictTypes)->yes() && $nativeReturnType->accepts($nullType, $strictTypes)->yes()) {
return ["The $phpDocIdentification phpdoc does not contain null, but native return type does"];
}

return [];
}

}
2 changes: 1 addition & 1 deletion src/Visitor/ClassPropertyAssignmentVisitor.php
Expand Up @@ -22,7 +22,7 @@ class ClassPropertyAssignmentVisitor extends NodeVisitorAbstract

/**
* @param Node[] $nodes
* @return Node[]
* @return Node[]|null
*/
public function beforeTraverse(array $nodes): ?array
{
Expand Down
2 changes: 1 addition & 1 deletion src/Visitor/NamedArgumentSourceVisitor.php
Expand Up @@ -21,7 +21,7 @@ class NamedArgumentSourceVisitor extends NodeVisitorAbstract

/**
* @param Node[] $nodes
* @return Node[]
* @return Node[]|null
*/
public function beforeTraverse(array $nodes): ?array
{
Expand Down
Expand Up @@ -23,7 +23,7 @@ class TopLevelConstructorPropertyFetchMarkingVisitor extends NodeVisitorAbstract

/**
* @param Node[] $nodes
* @return Node[]
* @return Node[]|null
*/
public function beforeTraverse(array $nodes): ?array
{
Expand Down
2 changes: 1 addition & 1 deletion src/Visitor/UnusedExceptionVisitor.php
Expand Up @@ -30,7 +30,7 @@ class UnusedExceptionVisitor extends NodeVisitorAbstract

/**
* @param Node[] $nodes
* @return Node[]
* @return Node[]|null
*/
public function beforeTraverse(array $nodes): ?array
{
Expand Down
2 changes: 1 addition & 1 deletion src/Visitor/UnusedMatchVisitor.php
Expand Up @@ -31,7 +31,7 @@ class UnusedMatchVisitor extends NodeVisitorAbstract

/**
* @param Node[] $nodes
* @return Node[]
* @return Node[]|null
*/
public function beforeTraverse(array $nodes): ?array
{
Expand Down
@@ -0,0 +1,27 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\Rule;

use PHPStan\Rules\Rule;
use PHPStan\Type\FileTypeMapper;
use ShipMonk\PHPStan\RuleTestCase;

/**
* @extends RuleTestCase<ForbidPhpDocNullabilityMismatchWithNativeTypehintRule>
*/
class ForbidPhpDocNullabilityMismatchWithNativeTypehintRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new ForbidPhpDocNullabilityMismatchWithNativeTypehintRule(
self::getContainer()->getByType(FileTypeMapper::class),
);
}

public function testBasic(): void
{
$this->analyseFile(__DIR__ . '/data/ForbidPhpDocNullabilityMismatchWithNativeTypehintRule/code.php');
}

}

0 comments on commit 5e470fb

Please sign in to comment.