Skip to content

Commit

Permalink
Refactoring - extract MixinCheck
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Sep 3, 2024
1 parent c47730f commit 57ccd8c
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 120 deletions.
3 changes: 0 additions & 3 deletions conf/config.level2.neon
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ conditionalTags:
services:
-
class: PHPStan\Rules\Classes\MixinRule
arguments:
checkClassCaseSensitivity: %checkClassCaseSensitivity%
absentTypeChecks: %featureToggles.absentTypeChecks%
tags:
- phpstan.rules.rule

Expand Down
6 changes: 6 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,12 @@ services:
arguments:
checkClassCaseSensitivity: %checkClassCaseSensitivity%

-
class: PHPStan\Rules\Classes\MixinCheck
arguments:
checkClassCaseSensitivity: %checkClassCaseSensitivity%
absentTypeChecks: %featureToggles.absentTypeChecks%

-
class: PHPStan\Rules\Classes\PropertyTagCheck
arguments:
Expand Down
4 changes: 3 additions & 1 deletion src/PhpDoc/StubValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use PHPStan\Rules\Classes\MethodTagRule;
use PHPStan\Rules\Classes\MethodTagTraitRule;
use PHPStan\Rules\Classes\MethodTagTraitUseRule;
use PHPStan\Rules\Classes\MixinCheck;
use PHPStan\Rules\Classes\MixinRule;
use PHPStan\Rules\Classes\PropertyTagCheck;
use PHPStan\Rules\Classes\PropertyTagRule;
Expand Down Expand Up @@ -182,6 +183,7 @@ private function getRuleRegistry(Container $container): RuleRegistry
$phpClassReflectionExtension = $container->getByType(PhpClassReflectionExtension::class);
$genericCallableRuleHelper = $container->getByType(GenericCallableRuleHelper::class);
$methodTagTemplateTypeCheck = $container->getByType(MethodTagTemplateTypeCheck::class);
$mixinCheck = $container->getByType(MixinCheck::class);

$rules = [
// level 0
Expand Down Expand Up @@ -256,7 +258,7 @@ private function getRuleRegistry(Container $container): RuleRegistry
$rules[] = new PropertyTagRule($propertyTagCheck);
$rules[] = new PropertyTagTraitRule($propertyTagCheck, $reflectionProvider);
$rules[] = new PropertyTagTraitUseRule($propertyTagCheck);
$rules[] = new MixinRule($reflectionProvider, $classNameCheck, $genericObjectTypeCheck, $missingTypehintCheck, $unresolvableTypeHelper, true, true);
$rules[] = new MixinRule($mixinCheck);
$rules[] = new LocalTypeTraitUseAliasesRule($localTypeAliasesCheck);
$rules[] = new MethodTagTemplateTypeTraitRule($methodTagTemplateTypeCheck, $reflectionProvider);
}
Expand Down
128 changes: 128 additions & 0 deletions src/Rules/Classes/MixinCheck.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Classes;

use PhpParser\Node\Stmt\ClassLike;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\ClassNameCheck;
use PHPStan\Rules\ClassNameNodePair;
use PHPStan\Rules\Generics\GenericObjectTypeCheck;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\MissingTypehintCheck;
use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\VerbosityLevel;
use function array_merge;
use function implode;
use function sprintf;

final class MixinCheck
{

public function __construct(
private ReflectionProvider $reflectionProvider,
private ClassNameCheck $classCheck,
private GenericObjectTypeCheck $genericObjectTypeCheck,
private MissingTypehintCheck $missingTypehintCheck,
private UnresolvableTypeHelper $unresolvableTypeHelper,
private bool $checkClassCaseSensitivity,
private bool $absentTypeChecks,
)
{
}

/**
* @return list<IdentifierRuleError>
*/
public function check(ClassReflection $classReflection, ClassLike $node): array
{
$mixinTags = $classReflection->getMixinTags();
$errors = [];
foreach ($mixinTags as $mixinTag) {
$type = $mixinTag->getType();
if (!$type->canCallMethods()->yes() || !$type->canAccessProperties()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains non-object type %s.', $type->describe(VerbosityLevel::typeOnly())))
->identifier('mixin.nonObject')
->build();
continue;
}

if (
$this->unresolvableTypeHelper->containsUnresolvableType($type)
) {
$errors[] = RuleErrorBuilder::message('PHPDoc tag @mixin contains unresolvable type.')
->identifier('mixin.unresolvableType')
->build();
continue;
}

$errors = array_merge($errors, $this->genericObjectTypeCheck->check(
$type,
'PHPDoc tag @mixin contains generic type %s but %s %s is not generic.',
'Generic type %s in PHPDoc tag @mixin does not specify all template types of %s %s: %s',
'Generic type %s in PHPDoc tag @mixin specifies %d template types, but %s %s supports only %d: %s',
'Type %s in generic type %s in PHPDoc tag @mixin is not subtype of template type %s of %s %s.',
'Call-site variance of %s in generic type %s in PHPDoc tag @mixin is in conflict with %s template type %s of %s %s.',
'Call-site variance of %s in generic type %s in PHPDoc tag @mixin is redundant, template type %s of %s %s has the same variance.',
));

foreach ($this->missingTypehintCheck->getNonGenericObjectTypesWithGenericClass($type) as [$innerName, $genericTypeNames]) {
$errors[] = RuleErrorBuilder::message(sprintf(
'PHPDoc tag @mixin contains generic %s but does not specify its types: %s',
$innerName,
implode(', ', $genericTypeNames),
))
->identifier('missingType.generics')
->build();
}

if ($this->absentTypeChecks) {
foreach ($this->missingTypehintCheck->getIterableTypesWithMissingValueTypehint($type) as $iterableType) {
$iterableTypeDescription = $iterableType->describe(VerbosityLevel::typeOnly());
$errors[] = RuleErrorBuilder::message(sprintf(
'%s %s has PHPDoc tag @mixin with no value type specified in iterable type %s.',
$classReflection->getClassTypeDescription(),
$classReflection->getDisplayName(),
$iterableTypeDescription,
))
->tip(MissingTypehintCheck::MISSING_ITERABLE_VALUE_TYPE_TIP)
->identifier('missingType.iterableValue')
->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingSignature($type) as $callableType) {
$errors[] = RuleErrorBuilder::message(sprintf(
'%s %s has PHPDoc tag @mixin with no signature specified for %s.',
$classReflection->getClassTypeDescription(),
$classReflection->getDisplayName(),
$callableType->describe(VerbosityLevel::typeOnly()),
))->identifier('missingType.callable')->build();
}
}

foreach ($type->getReferencedClasses() as $class) {
if (!$this->reflectionProvider->hasClass($class)) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains unknown class %s.', $class))
->identifier('class.notFound')
->discoveringSymbolsTip()
->build();
} elseif ($this->reflectionProvider->getClass($class)->isTrait()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains invalid type %s.', $class))
->identifier('mixin.trait')
->build();
} else {
$errors = array_merge(
$errors,
$this->classCheck->checkClassNames([
new ClassNameNodePair($class, $node),
], $this->checkClassCaseSensitivity),
);
}
}
}

return $errors;
}

}
109 changes: 2 additions & 107 deletions src/Rules/Classes/MixinRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,15 @@
use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\InClassNode;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\ClassNameCheck;
use PHPStan\Rules\ClassNameNodePair;
use PHPStan\Rules\Generics\GenericObjectTypeCheck;
use PHPStan\Rules\MissingTypehintCheck;
use PHPStan\Rules\PhpDoc\UnresolvableTypeHelper;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\VerbosityLevel;
use function array_merge;
use function implode;
use function sprintf;

/**
* @implements Rule<InClassNode>
*/
final class MixinRule implements Rule
{

public function __construct(
private ReflectionProvider $reflectionProvider,
private ClassNameCheck $classCheck,
private GenericObjectTypeCheck $genericObjectTypeCheck,
private MissingTypehintCheck $missingTypehintCheck,
private UnresolvableTypeHelper $unresolvableTypeHelper,
private bool $checkClassCaseSensitivity,
private bool $absentTypeChecks,
)
public function __construct(private MixinCheck $check)
{
}

Expand All @@ -43,93 +24,7 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
$classReflection = $node->getClassReflection();
$mixinTags = $classReflection->getMixinTags();
$errors = [];
foreach ($mixinTags as $mixinTag) {
$type = $mixinTag->getType();
if (!$type->canCallMethods()->yes() || !$type->canAccessProperties()->yes()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains non-object type %s.', $type->describe(VerbosityLevel::typeOnly())))
->identifier('mixin.nonObject')
->build();
continue;
}

if (
$this->unresolvableTypeHelper->containsUnresolvableType($type)
) {
$errors[] = RuleErrorBuilder::message('PHPDoc tag @mixin contains unresolvable type.')
->identifier('mixin.unresolvableType')
->build();
continue;
}

$errors = array_merge($errors, $this->genericObjectTypeCheck->check(
$type,
'PHPDoc tag @mixin contains generic type %s but %s %s is not generic.',
'Generic type %s in PHPDoc tag @mixin does not specify all template types of %s %s: %s',
'Generic type %s in PHPDoc tag @mixin specifies %d template types, but %s %s supports only %d: %s',
'Type %s in generic type %s in PHPDoc tag @mixin is not subtype of template type %s of %s %s.',
'Call-site variance of %s in generic type %s in PHPDoc tag @mixin is in conflict with %s template type %s of %s %s.',
'Call-site variance of %s in generic type %s in PHPDoc tag @mixin is redundant, template type %s of %s %s has the same variance.',
));

foreach ($this->missingTypehintCheck->getNonGenericObjectTypesWithGenericClass($type) as [$innerName, $genericTypeNames]) {
$errors[] = RuleErrorBuilder::message(sprintf(
'PHPDoc tag @mixin contains generic %s but does not specify its types: %s',
$innerName,
implode(', ', $genericTypeNames),
))
->identifier('missingType.generics')
->build();
}

if ($this->absentTypeChecks) {
foreach ($this->missingTypehintCheck->getIterableTypesWithMissingValueTypehint($type) as $iterableType) {
$iterableTypeDescription = $iterableType->describe(VerbosityLevel::typeOnly());
$errors[] = RuleErrorBuilder::message(sprintf(
'%s %s has PHPDoc tag @mixin with no value type specified in iterable type %s.',
$classReflection->getClassTypeDescription(),
$classReflection->getDisplayName(),
$iterableTypeDescription,
))
->tip(MissingTypehintCheck::MISSING_ITERABLE_VALUE_TYPE_TIP)
->identifier('missingType.iterableValue')
->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingSignature($type) as $callableType) {
$errors[] = RuleErrorBuilder::message(sprintf(
'%s %s has PHPDoc tag @mixin with no signature specified for %s.',
$classReflection->getClassTypeDescription(),
$classReflection->getDisplayName(),
$callableType->describe(VerbosityLevel::typeOnly()),
))->identifier('missingType.callable')->build();
}
}

foreach ($type->getReferencedClasses() as $class) {
if (!$this->reflectionProvider->hasClass($class)) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains unknown class %s.', $class))
->identifier('class.notFound')
->discoveringSymbolsTip()
->build();
} elseif ($this->reflectionProvider->getClass($class)->isTrait()) {
$errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains invalid type %s.', $class))
->identifier('mixin.trait')
->build();
} else {
$errors = array_merge(
$errors,
$this->classCheck->checkClassNames([
new ClassNameNodePair($class, $node),
], $this->checkClassCaseSensitivity),
);
}
}
}

return $errors;
return $this->check->check($node->getClassReflection(), $node->getOriginalNode());
}

}
20 changes: 11 additions & 9 deletions tests/PHPStan/Rules/Classes/MixinRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,18 @@ protected function getRule(): Rule
$reflectionProvider = $this->createReflectionProvider();

return new MixinRule(
$reflectionProvider,
new ClassNameCheck(
new ClassCaseSensitivityCheck($reflectionProvider, true),
new ClassForbiddenNameCheck(self::getContainer()),
new MixinCheck(
$reflectionProvider,
new ClassNameCheck(
new ClassCaseSensitivityCheck($reflectionProvider, true),
new ClassForbiddenNameCheck(self::getContainer()),
),
new GenericObjectTypeCheck(),
new MissingTypehintCheck(true, true, true, true, []),
new UnresolvableTypeHelper(),
true,
true,
),
new GenericObjectTypeCheck(),
new MissingTypehintCheck(true, true, true, true, []),
new UnresolvableTypeHelper(),
true,
true,
);
}

Expand Down

0 comments on commit 57ccd8c

Please sign in to comment.