Skip to content

Commit

Permalink
Bleeding edge - check too wide private property type
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Aug 21, 2024
1 parent 9488c63 commit 7453f4f
Show file tree
Hide file tree
Showing 11 changed files with 342 additions and 1 deletion.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,6 @@ parameters:
preciseMissingReturn: true
validatePregQuote: true
noImplicitWildcard: true
tooWidePropertyType: true
stubFiles:
- ../stubs/bleedingEdge/Rule.stub
5 changes: 5 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.pure%
PHPStan\Rules\DeadCode\PossiblyPureStaticCallCollector:
phpstan.collector: %featureToggles.pure%
PHPStan\Rules\TooWideTypehints\TooWidePropertyTypeRule:
phpstan.rules.rule: %featureToggles.tooWidePropertyType%

parameters:
checkAdvancedIsset: true
Expand Down Expand Up @@ -313,3 +315,6 @@ services:

-
class: PHPStan\Rules\TooWideTypehints\TooWideMethodParameterOutTypeRule

-
class: PHPStan\Rules\TooWideTypehints\TooWidePropertyTypeRule
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ parameters:
validatePregQuote: false
noImplicitWildcard: false
narrowPregMatches: true
tooWidePropertyType: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ parametersSchema:
validatePregQuote: bool()
noImplicitWildcard: bool()
narrowPregMatches: bool()
tooWidePropertyType: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -870,7 +870,7 @@ private function processStmtNode(
$this->processAttributeGroups($stmt, $stmt->attrGroups, $classScope, $classStatementsGatherer);

$this->processStmtNodes($stmt, $stmt->stmts, $classScope, $classStatementsGatherer, $context);
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classReflection), $classScope);
$nodeCallback(new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensionProvider, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope);
$nodeCallback(new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection), $classScope);
$nodeCallback(new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope);
$classReflection->evictPrivateSymbols();
Expand Down
11 changes: 11 additions & 0 deletions src/Node/ClassPropertiesNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use PHPStan\Analyser\Scope;
use PHPStan\Node\Expr\PropertyInitializationExpr;
use PHPStan\Node\Method\MethodCall;
use PHPStan\Node\Property\PropertyAssign;
use PHPStan\Node\Property\PropertyRead;
use PHPStan\Node\Property\PropertyWrite;
use PHPStan\Reflection\ClassReflection;
Expand Down Expand Up @@ -40,6 +41,7 @@ class ClassPropertiesNode extends NodeAbstract implements VirtualNode
* @param array<int, PropertyRead|PropertyWrite> $propertyUsages
* @param array<int, MethodCall> $methodCalls
* @param array<string, MethodReturnStatementsNode> $returnStatementNodes
* @param list<PropertyAssign> $propertyAssigns
*/
public function __construct(
private ClassLike $class,
Expand All @@ -48,6 +50,7 @@ public function __construct(
private array $propertyUsages,
private array $methodCalls,
private array $returnStatementNodes,
private array $propertyAssigns,
private ClassReflection $classReflection,
)
{
Expand Down Expand Up @@ -404,4 +407,12 @@ private function getInitializedProperties(Scope $scope, array $initialInitialize
return $initialInitializedProperties;
}

/**
* @return list<PropertyAssign>
*/
public function getPropertyAssigns(): array
{
return $this->propertyAssigns;
}

}
13 changes: 13 additions & 0 deletions src/Node/ClassStatementsGatherer.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use PhpParser\Node\Identifier;
use PHPStan\Analyser\Scope;
use PHPStan\Node\Constant\ClassConstantFetch;
use PHPStan\Node\Property\PropertyAssign;
use PHPStan\Node\Property\PropertyRead;
use PHPStan\Node\Property\PropertyWrite;
use PHPStan\Reflection\ClassReflection;
Expand Down Expand Up @@ -55,6 +56,9 @@ final class ClassStatementsGatherer
/** @var array<string, MethodReturnStatementsNode> */
private array $returnStatementNodes = [];

/** @var list<PropertyAssign> */
private array $propertyAssigns = [];

/**
* @param callable(Node $node, Scope $scope): void $nodeCallback
*/
Expand Down Expand Up @@ -122,6 +126,14 @@ public function getReturnStatementsNodes(): array
return $this->returnStatementNodes;
}

/**
* @return list<PropertyAssign>
*/
public function getPropertyAssigns(): array
{
return $this->propertyAssigns;
}

public function __invoke(Node $node, Scope $scope): void
{
$nodeCallback = $this->nodeCallback;
Expand Down Expand Up @@ -189,6 +201,7 @@ private function gatherNodes(Node $node, Scope $scope): void
}
if ($node instanceof PropertyAssignNode) {
$this->propertyUsages[] = new PropertyWrite($node->getPropertyFetch(), $scope, false);
$this->propertyAssigns[] = new PropertyAssign($node, $scope);
return;
}
if (!$node instanceof Expr) {
Expand Down
31 changes: 31 additions & 0 deletions src/Node/Property/PropertyAssign.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node\Property;

use PHPStan\Analyser\Scope;
use PHPStan\Node\PropertyAssignNode;

/**
* @api
*/
final class PropertyAssign
{

public function __construct(
private PropertyAssignNode $assign,
private Scope $scope,
)
{
}

public function getAssign(): PropertyAssignNode
{
return $this->assign;
}

public function getScope(): Scope
{
return $this->scope;
}

}
132 changes: 132 additions & 0 deletions src/Rules/TooWideTypehints/TooWidePropertyTypeRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassPropertiesNode;
use PHPStan\Reflection\PropertyReflection;
use PHPStan\Rules\Properties\PropertyReflectionFinder;
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\NullType;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use function count;
use function sprintf;

/**
* @implements Rule<ClassPropertiesNode>
*/
final class TooWidePropertyTypeRule implements Rule
{

public function __construct(
private ReadWritePropertiesExtensionProvider $extensionProvider,
private PropertyReflectionFinder $propertyReflectionFinder,
)
{
}

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

public function processNode(Node $node, Scope $scope): array
{
$errors = [];
$classReflection = $node->getClassReflection();

foreach ($node->getProperties() as $property) {
if (!$property->isPrivate()) {
continue;
}
if ($property->isDeclaredInTrait()) {
continue;
}
if ($property->isPromoted()) {
continue;
}
$propertyName = $property->getName();
if (!$classReflection->hasNativeProperty($propertyName)) {
continue;
}

$propertyReflection = $classReflection->getNativeProperty($propertyName);
$propertyType = $propertyReflection->getWritableType();
if (!$propertyType instanceof UnionType) {
continue;
}
foreach ($this->extensionProvider->getExtensions() as $extension) {
if ($extension->isAlwaysWritten($propertyReflection, $propertyName)) {
continue 2;
}
if ($extension->isInitialized($propertyReflection, $propertyName)) {
continue 2;
}
}

$assignedTypes = [];
foreach ($node->getPropertyAssigns() as $assign) {
$assignNode = $assign->getAssign();
$assignPropertyReflections = $this->propertyReflectionFinder->findPropertyReflectionsFromNode($assignNode->getPropertyFetch(), $assign->getScope());
foreach ($assignPropertyReflections as $assignPropertyReflection) {
if ($propertyName !== $assignPropertyReflection->getName()) {
continue;
}
if ($propertyReflection->getDeclaringClass()->getName() !== $assignPropertyReflection->getDeclaringClass()->getName()) {
continue;
}

$assignedTypes[] = $assignPropertyReflection->getScope()->getType($assignNode->getAssignedExpr());
}
}

if ($property->getDefault() !== null) {
$assignedTypes[] = $scope->getType($property->getDefault());
}

if (count($assignedTypes) === 0) {
continue;
}

$assignedType = TypeCombinator::union(...$assignedTypes);
$propertyDescription = $this->describePropertyByName($propertyReflection, $propertyName);
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType, $assignedType);
foreach ($propertyType->getTypes() as $type) {
if (!$type->isSuperTypeOf($assignedType)->no()) {
continue;
}

if ($property->getNativeType() === null && (new NullType())->isSuperTypeOf($type)->yes()) {
continue;
}

$errors[] = RuleErrorBuilder::message(sprintf(
'%s (%s) is never assigned %s so it can be removed from the property type.',
$propertyDescription,
$propertyType->describe($verbosityLevel),
$type->describe($verbosityLevel),
))
->identifier('property.unusedType')
->line($property->getStartLine())
->build();
}

}
return $errors;
}

private function describePropertyByName(PropertyReflection $property, string $propertyName): string
{
if (!$property->isStatic()) {
return sprintf('Property %s::$%s', $property->getDeclaringClass()->getDisplayName(), $propertyName);
}

return sprintf('Static property %s::$%s', $property->getDeclaringClass()->getDisplayName(), $propertyName);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

use PHPStan\Rules\Properties\DirectReadWritePropertiesExtensionProvider;
use PHPStan\Rules\Properties\PropertyReflectionFinder;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<TooWidePropertyTypeRule>
*/
class TooWidePropertyTypeRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new TooWidePropertyTypeRule(
new DirectReadWritePropertiesExtensionProvider([]),
new PropertyReflectionFinder(),
);
}

public function testRule(): void
{
if (PHP_VERSION_ID < 80000) {
self::markTestSkipped('Test requires PHP 8.0.');
}

$this->analyse([__DIR__ . '/data/too-wide-property-type.php'], [
[
'Property TooWidePropertyType\Foo::$foo (int|string) is never assigned string so it can be removed from the property type.',
9,
],
/*[
'Property TooWidePropertyType\Foo::$barr (int|null) is never assigned null so it can be removed from the property type.',
15,
],
[
'Property TooWidePropertyType\Foo::$barrr (int|null) is never assigned null so it can be removed from the property type.',
18,
],*/
[
'Property TooWidePropertyType\Foo::$baz (int|null) is never assigned null so it can be removed from the property type.',
20,
],
[
'Property TooWidePropertyType\Bar::$c (int|null) is never assigned int so it can be removed from the property type.',
45,
],
[
'Property TooWidePropertyType\Bar::$d (int|null) is never assigned null so it can be removed from the property type.',
47,
],
]);
}

}
Loading

0 comments on commit 7453f4f

Please sign in to comment.