Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement require-extends rules #2859

Merged
merged 26 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7c9fbe3
Implement IncompatibleRequireExtendsTypeRule
staabm Jan 7, 2024
ff66719
Implement RequireExtendsRule
staabm Jan 7, 2024
65dde9f
report final classes
staabm Jan 7, 2024
c6f272f
php <8.1 compat
staabm Jan 7, 2024
b535c9b
use ClassLike instead of InClassNode for phpdoc validation
staabm Jan 8, 2024
06d3343
implement require-implements rules
staabm Jan 8, 2024
a901c62
Update config.level2.neon
staabm Jan 8, 2024
fee7d1a
fix php 7.2 compat
staabm Jan 8, 2024
c44d540
fix name collision
staabm Jan 8, 2024
e6bcbe7
Rules that just have the tag should be in the rules section above.
staabm Jan 9, 2024
2295b93
Utilize ClassReflection->implementsInterface/isSubclassOf
staabm Jan 9, 2024
8d3d9d3
Use instanceof ObjectType
staabm Jan 9, 2024
619790d
cover anonymous classes in tests
staabm Jan 9, 2024
682c4a0
Work on InClassNode
staabm Jan 9, 2024
bb3ee3e
Separate InClassNode and InTraitNode rules
staabm Jan 9, 2024
7f7334e
Utilize ObjectType->getClassReflection()
staabm Jan 9, 2024
1374549
fix php 7.x expectations
staabm Jan 9, 2024
a1ed5d8
error on trait phpdocs only once per declaration
staabm Jan 9, 2024
131c5ca
Rename rule classes
ondrejmirtes Jan 10, 2024
df9cf28
Cosmetic change
ondrejmirtes Jan 10, 2024
fc125fe
Use Trait_
ondrejmirtes Jan 10, 2024
fdab25b
Improve error messages
ondrejmirtes Jan 10, 2024
ec3fc94
Support psalm prefix
staabm Jan 10, 2024
ea57e39
Refactoring: extract RequireExtendsCheck
staabm Jan 10, 2024
f7bda5c
use getDisplayName()
staabm Jan 10, 2024
e31c497
require-extends can only be used once
staabm Jan 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions conf/config.level2.neon
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ rules:
- PHPStan\Rules\PhpDoc\IncompatiblePropertyPhpDocTypeRule
- PHPStan\Rules\PhpDoc\InvalidThrowsPhpDocValueRule
- PHPStan\Rules\Properties\AccessPrivatePropertyThroughStaticRule
- PHPStan\Rules\Classes\RequireImplementsRule
- PHPStan\Rules\Classes\RequireExtendsRule
- PHPStan\Rules\PhpDoc\RequireImplementsDefinitionClassRule
- PHPStan\Rules\PhpDoc\RequireExtendsDefinitionClassRule
- PHPStan\Rules\PhpDoc\RequireExtendsDefinitionTraitRule

conditionalTags:
PHPStan\Rules\Functions\IncompatibleArrowFunctionDefaultParameterTypeRule:
Expand All @@ -64,6 +69,19 @@ services:
checkClassCaseSensitivity: %checkClassCaseSensitivity%
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\PhpDoc\RequireExtendsCheck
arguments:
checkClassCaseSensitivity: %checkClassCaseSensitivity%

-
class: PHPStan\Rules\PhpDoc\RequireImplementsDefinitionTraitRule
arguments:
checkClassCaseSensitivity: %checkClassCaseSensitivity%
tags:
- phpstan.rules.rule

-
class: PHPStan\Rules\Functions\IncompatibleArrowFunctionDefaultParameterTypeRule
-
Expand Down
20 changes: 20 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,16 @@ parameters:
count: 2
path: src/Rules/Classes/ImpossibleInstanceOfRule.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\ObjectType is error\\-prone and deprecated\\. Use Type\\:\\:isObject\\(\\) or Type\\:\\:getObjectClassNames\\(\\) instead\\.$#"
count: 2
path: src/Rules/Classes/RequireExtendsRule.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\ObjectType is error\\-prone and deprecated\\. Use Type\\:\\:isObject\\(\\) or Type\\:\\:getObjectClassNames\\(\\) instead\\.$#"
count: 1
path: src/Rules/Classes/RequireImplementsRule.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\Constant\\\\ConstantBooleanType is error\\-prone and deprecated\\. Use Type\\:\\:isTrue\\(\\) or Type\\:\\:isFalse\\(\\) instead\\.$#"
count: 6
Expand Down Expand Up @@ -631,6 +641,16 @@ parameters:
count: 1
path: src/Rules/Methods/StaticMethodCallCheck.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\ObjectType is error\\-prone and deprecated\\. Use Type\\:\\:isObject\\(\\) or Type\\:\\:getObjectClassNames\\(\\) instead\\.$#"
count: 1
path: src/Rules/PhpDoc/RequireExtendsCheck.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\ObjectType is error\\-prone and deprecated\\. Use Type\\:\\:isObject\\(\\) or Type\\:\\:getObjectClassNames\\(\\) instead\\.$#"
count: 1
path: src/Rules/PhpDoc/RequireImplementsDefinitionTraitRule.php

-
message: "#^Doing instanceof PHPStan\\\\Type\\\\Generic\\\\GenericObjectType is error\\-prone and deprecated\\.$#"
count: 1
Expand Down
30 changes: 22 additions & 8 deletions src/PhpDoc/PhpDocNodeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprNullNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\MixinTagValueNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\RequireExtendsTagValueNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\RequireImplementsTagValueNode;
use PHPStan\PhpDocParser\Ast\PhpDoc\TemplateTagValueNode;
use PHPStan\Reflection\PassedByReference;
use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper;
Expand Down Expand Up @@ -418,19 +416,35 @@ public function resolveMixinTags(PhpDocNode $phpDocNode, NameScope $nameScope):
*/
public function resolveRequireExtendsTags(PhpDocNode $phpDocNode, NameScope $nameScope): array
{
return array_map(fn (RequireExtendsTagValueNode $requireExtendsTagValueNode): RequireExtendsTag => new RequireExtendsTag(
$this->typeNodeResolver->resolve($requireExtendsTagValueNode->type, $nameScope),
), $phpDocNode->getRequireExtendsTagValues());
$resolved = [];

foreach (['@psalm-require-extends', '@phpstan-require-extends'] as $tagName) {
foreach ($phpDocNode->getRequireExtendsTagValues($tagName) as $tagValue) {
$resolved[] = new RequireExtendsTag(
$this->typeNodeResolver->resolve($tagValue->type, $nameScope),
);
}
}

return $resolved;
}

/**
* @return array<RequireImplementsTag>
*/
public function resolveRequireImplementsTags(PhpDocNode $phpDocNode, NameScope $nameScope): array
{
return array_map(fn (RequireImplementsTagValueNode $requireImplementsTagValueNode): RequireImplementsTag => new RequireImplementsTag(
$this->typeNodeResolver->resolve($requireImplementsTagValueNode->type, $nameScope),
), $phpDocNode->getRequireImplementsTagValues());
$resolved = [];

foreach (['@psalm-require-implements', '@phpstan-require-implements'] as $tagName) {
foreach ($phpDocNode->getRequireImplementsTagValues($tagName) as $tagValue) {
$resolved[] = new RequireImplementsTag(
$this->typeNodeResolver->resolve($tagValue->type, $nameScope),
);
}
}

return $resolved;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/Reflection/ClassReflection.php
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ public function getTraits(bool $recursive = false): array
}

/**
* @return string[]
* @return list<class-string>
*/
public function getParentClassesNames(): array
{
Expand Down
79 changes: 79 additions & 0 deletions src/Rules/Classes/RequireExtendsRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Classes;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\InClassNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;
use PHPStan\Type\VerbosityLevel;
use function sprintf;

/**
* @implements Rule<InClassNode>
*/
class RequireExtendsRule implements Rule
{

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

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

$errors = [];
foreach ($classReflection->getImmediateInterfaces() as $interface) {
$extendsTags = $interface->getRequireExtendsTags();
foreach ($extendsTags as $extendsTag) {
$type = $extendsTag->getType();
if (!$type instanceof ObjectType) {
continue;
}

if ($classReflection->isSubclassOf($type->getClassName())) {
continue;
}

$errors[] = RuleErrorBuilder::message(
sprintf(
'Interface %s requires implementing class to extend %s, but %s does not.',
$interface->getName(),
$type->describe(VerbosityLevel::typeOnly()),
$classReflection->getName(),
),
)->build();
}
}

foreach ($classReflection->getTraits() as $trait) {
$extendsTags = $trait->getRequireExtendsTags();
foreach ($extendsTags as $extendsTag) {
$type = $extendsTag->getType();
if (!$type instanceof ObjectType) {
continue;
}

if ($classReflection->isSubclassOf($type->getClassName())) {
continue;
}

$errors[] = RuleErrorBuilder::message(
sprintf(
'Trait %s requires using class to extend %s, but %s does not.',
$trait->getName(),
$type->describe(VerbosityLevel::typeOnly()),
$classReflection->getName(),
),
)->build();
}
}

return $errors;
}

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

namespace PHPStan\Rules\Classes;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\InClassNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;
use PHPStan\Type\VerbosityLevel;
use function sprintf;

/**
* @implements Rule<InClassNode>
*/
class RequireImplementsRule implements Rule
{

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

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

$errors = [];
foreach ($classReflection->getTraits() as $trait) {
$implementsTags = $trait->getRequireImplementsTags();
foreach ($implementsTags as $implementsTag) {
$type = $implementsTag->getType();
if (!$type instanceof ObjectType) {
continue;
}

if ($classReflection->implementsInterface($type->getClassName())) {
continue;
}

$errors[] = RuleErrorBuilder::message(
sprintf(
'Trait %s requires using class to implement %s, but %s does not.',
$trait->getName(),
$type->describe(VerbosityLevel::typeOnly()),
$classReflection->getName(),
),
)->build();
}
}

return $errors;
}

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

namespace PHPStan\Rules\PhpDoc;

use PhpParser\Node;
use PHPStan\PhpDoc\Tag\RequireExtendsTag;
use PHPStan\Rules\ClassCaseSensitivityCheck;
use PHPStan\Rules\ClassNameNodePair;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\ObjectType;
use PHPStan\Type\VerbosityLevel;
use function array_merge;
use function sprintf;

final class RequireExtendsCheck
{

public function __construct(
private ClassCaseSensitivityCheck $classCaseSensitivityCheck,
private bool $checkClassCaseSensitivity,
)
{
}

/**
* @param array<RequireExtendsTag> $extendsTags
* @return RuleError[]
*/
public function checkExtendsTags(Node $node, array $extendsTags): array
{
$errors = [];
foreach ($extendsTags as $extendsTag) {
$type = $extendsTag->getType();
if (!$type instanceof ObjectType) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @phpstan-require-extends contains non-object type %s.', $type->describe(VerbosityLevel::typeOnly())))->build();
continue;
}

$class = $type->getClassName();
$referencedClassReflection = $type->getClassReflection();

if ($referencedClassReflection === null) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @phpstan-require-extends contains unknown class %s.', $class))->discoveringSymbolsTip()->build();
continue;
}

if (!$referencedClassReflection->isClass()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @phpstan-require-extends cannot contain non-class type %s.', $class))->build();
} elseif ($referencedClassReflection->isFinal()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @phpstan-require-extends cannot contain final class %s.', $class))->build();
} elseif ($this->checkClassCaseSensitivity) {
$errors = array_merge(
$errors,
$this->classCaseSensitivityCheck->checkClassNames([
new ClassNameNodePair($class, $node),
]),
);
}
}

return $errors;
}

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

namespace PHPStan\Rules\PhpDoc;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\InClassNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use function count;

/**
* @implements Rule<InClassNode>
*/
class RequireExtendsDefinitionClassRule implements Rule
{

public function __construct(
private RequireExtendsCheck $requireExtendsCheck,
)
{
}

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

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

if (count($extendsTags) === 0) {
Copy link
Contributor Author

@staabm staabm Jan 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should error when a class defines 2 or more @*-require-extends

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not error, but support and check all of them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, I misunderstood you. Please add a test and fix the rule.

return [];
}

if (!$classReflection->isInterface()) {
return [
RuleErrorBuilder::message('PHPDoc tag @phpstan-require-extends is only valid on trait or interface.')->build(),
];
}

return $this->requireExtendsCheck->checkExtendsTags($node, $extendsTags);
}

}