Skip to content

Commit

Permalink
Fix signature check of method from trait
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Nov 19, 2023
1 parent 3863da9 commit be2b415
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 47 deletions.
3 changes: 2 additions & 1 deletion src/PhpDoc/StubValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PHPStan\Php\PhpVersion;
use PHPStan\PhpDocParser\Lexer\Lexer;
use PHPStan\PhpDocParser\Parser\PhpDocParser;
use PHPStan\Reflection\Php\PhpClassReflectionExtension;
use PHPStan\Reflection\PhpVersionStaticAccessor;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Reflection\ReflectionProviderStaticAccessor;
Expand Down Expand Up @@ -164,7 +165,7 @@ private function getRuleRegistry(Container $container): RuleRegistry
new ExistingClassesInTypehintsRule($functionDefinitionCheck),
new \PHPStan\Rules\Functions\ExistingClassesInTypehintsRule($functionDefinitionCheck),
new ExistingClassesInPropertiesRule($reflectionProvider, $classCaseSensitivityCheck, $unresolvableTypeHelper, $phpVersion, true, false),
new OverridingMethodRule($phpVersion, new MethodSignatureRule(true, true, $container->getParameter('featureToggles')['abstractTraitMethod']), true, new MethodParameterComparisonHelper($phpVersion, $container->getParameter('featureToggles')['genericPrototypeMessage']), $container->getParameter('featureToggles')['genericPrototypeMessage'], $container->getParameter('featureToggles')['finalByPhpDoc'], $container->getParameter('checkMissingOverrideMethodAttribute')),
new OverridingMethodRule($phpVersion, new MethodSignatureRule(true, true, $container->getParameter('featureToggles')['abstractTraitMethod']), true, new MethodParameterComparisonHelper($phpVersion, $container->getParameter('featureToggles')['genericPrototypeMessage']), $container->getByType(PhpClassReflectionExtension::class), $container->getParameter('featureToggles')['genericPrototypeMessage'], $container->getParameter('featureToggles')['finalByPhpDoc'], $container->getParameter('checkMissingOverrideMethodAttribute')),
new DuplicateDeclarationRule(),
new LocalTypeAliasesRule($localTypeAliasesCheck),
new LocalTypeTraitAliasesRule($localTypeAliasesCheck, $reflectionProvider),
Expand Down
23 changes: 14 additions & 9 deletions src/Reflection/Php/PhpClassReflectionExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,10 +618,15 @@ private function createMethod(
);
}

return $this->createUserlandMethodReflection($declaringClass, $declaringClass, $methodReflection);
}

public function createUserlandMethodReflection(ClassReflection $fileDeclaringClass, ClassReflection $actualDeclaringClass, ReflectionMethod $methodReflection): PhpMethodReflection
{
$declaringTraitName = $this->findMethodTrait($methodReflection);
$resolvedPhpDoc = null;
$stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($declaringClass, $methodReflection->getName(), array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters()));
$phpDocBlockClassReflection = $declaringClass;
$stubPhpDocPair = $this->findMethodPhpDocIncludingAncestors($fileDeclaringClass, $methodReflection->getName(), array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters()));
$phpDocBlockClassReflection = $fileDeclaringClass;

$methodDeclaringClass = $methodReflection->getBetterReflection()->getDeclaringClass();

Expand All @@ -648,13 +653,13 @@ private function createMethod(

$resolvedPhpDoc = $this->phpDocInheritanceResolver->resolvePhpDocForMethod(
$docComment,
$declaringClass->getFileName(),
$declaringClass,
$fileDeclaringClass->getFileName(),
$fileDeclaringClass,
$declaringTraitName,
$methodReflection->getName(),
$positionalParameterNames,
);
$phpDocBlockClassReflection = $declaringClass;
$phpDocBlockClassReflection = $fileDeclaringClass;
}

$declaringTrait = null;
Expand Down Expand Up @@ -685,8 +690,8 @@ private function createMethod(
}

$propertyDocblock = $this->fileTypeMapper->getResolvedPhpDoc(
$declaringClass->getFileName(),
$declaringClassName,
$fileDeclaringClass->getFileName(),
$fileDeclaringClass->getName(),
$declaringTraitName,
$methodReflection->getName(),
$parameterProperty->getDocComment(),
Expand Down Expand Up @@ -734,7 +739,7 @@ private function createMethod(
$nativeReturnType = TypehintHelper::decideTypeFromReflection(
$methodReflection->getReturnType(),
null,
$declaringClass,
$actualDeclaringClass,
);
$phpDocReturnType = $this->getPhpDocReturnType($phpDocBlockClassReflection, $resolvedPhpDoc, $nativeReturnType);
$phpDocThrowType = $resolvedPhpDoc->getThrowsTag() !== null ? $resolvedPhpDoc->getThrowsTag()->getType() : null;
Expand All @@ -751,7 +756,7 @@ private function createMethod(
}

return $this->methodReflectionFactory->create(
$declaringClass,
$actualDeclaringClass,
$declaringTrait,
$methodReflection,
$templateTypeMap,
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Methods/ConsistentConstructorRule.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

return $this->methodParameterComparisonHelper->compare($parentConstructor, $method, true);
return $this->methodParameterComparisonHelper->compare($parentConstructor, $parentConstructor->getDeclaringClass(), $method, true);
}

}
23 changes: 12 additions & 11 deletions src/Rules/Methods/MethodParameterComparisonHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Rules\Methods;

use PHPStan\Php\PhpVersion;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\Php\PhpMethodFromParserNodeReflection;
Expand Down Expand Up @@ -30,7 +31,7 @@ public function __construct(private PhpVersion $phpVersion, private bool $generi
/**
* @return RuleError[]
*/
public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParserNodeReflection $method, bool $ignorable = false): array
public function compare(ExtendedMethodReflection $prototype, ClassReflection $prototypeDeclaringClass, PhpMethodFromParserNodeReflection $method, bool $ignorable = false): array
{
/** @var RuleError[] $messages */
$messages = [];
Expand All @@ -46,7 +47,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
'Method %s::%s() overrides method %s::%s() but misses parameter #%d $%s.',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
$i + 1,
$prototypeParameter->getName(),
Expand All @@ -72,7 +73,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -91,7 +92,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -132,7 +133,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -177,7 +178,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$i + $j + 1,
$remainingPrototypeParameter->getName(),
$remainingPrototypeParameter->getNativeType()->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -197,7 +198,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -219,7 +220,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$method->getName(),
$i + 1,
$prototypeParameter->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -245,7 +246,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$i + 1,
$prototypeParameter->getName(),
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down Expand Up @@ -273,7 +274,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$i + 1,
$prototypeParameter->getName(),
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand All @@ -293,7 +294,7 @@ public function compare(ExtendedMethodReflection $prototype, PhpMethodFromParser
$i + 1,
$prototypeParameter->getName(),
$prototypeParameterType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
));

Expand Down
58 changes: 33 additions & 25 deletions src/Rules/Methods/OverridingMethodRule.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use PHPStan\Reflection\MethodPrototypeReflection;
use PHPStan\Reflection\Native\NativeMethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\Php\PhpClassReflectionExtension;
use PHPStan\Reflection\Php\PhpMethodReflection;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
Expand All @@ -35,6 +36,7 @@ public function __construct(
private MethodSignatureRule $methodSignatureRule,
private bool $checkPhpDocMethodSignatures,
private MethodParameterComparisonHelper $methodParameterComparisonHelper,
private PhpClassReflectionExtension $phpClassReflectionExtension,
private bool $genericPrototypeMessage,
private bool $finalByPhpDoc,
private bool $checkMissingOverrideMethodAttribute,
Expand Down Expand Up @@ -94,7 +96,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

[$prototype, $checkVisibility] = $prototypeData;
[$prototype, $prototypeDeclaringClass, $checkVisibility] = $prototypeData;

$messages = [];
if (
Expand All @@ -106,7 +108,7 @@ public function processNode(Node $node, Scope $scope): array
'Method %s::%s() overrides method %s::%s() but is missing the #[\Override] attribute.',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->build();
}
Expand All @@ -115,15 +117,15 @@ public function processNode(Node $node, Scope $scope): array
'Method %s::%s() overrides final method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
} elseif ($prototype->isFinal()->yes() && $this->finalByPhpDoc) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Method %s::%s() overrides @final method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->build();
}
Expand All @@ -134,7 +136,7 @@ public function processNode(Node $node, Scope $scope): array
'Non-static method %s::%s() overrides static method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -143,7 +145,7 @@ public function processNode(Node $node, Scope $scope): array
'Static method %s::%s() overrides non-static method %s::%s().',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -156,7 +158,7 @@ public function processNode(Node $node, Scope $scope): array
$method->isPrivate() ? 'Private' : 'Protected',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -165,7 +167,7 @@ public function processNode(Node $node, Scope $scope): array
'Private method %s::%s() overriding protected method %s::%s() should be protected or public.',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand All @@ -188,7 +190,7 @@ public function processNode(Node $node, Scope $scope): array
&& $this->phpVersion->hasTentativeReturnTypes()
&& $realPrototype->getTentativeReturnType() !== null
&& !$this->hasReturnTypeWillChangeAttribute($node->getOriginalNode())
&& count($prototype->getDeclaringClass()->getNativeReflection()->getMethod($prototype->getName())->getAttributes('ReturnTypeWillChange')) === 0
&& count($prototypeDeclaringClass->getNativeReflection()->getMethod($prototype->getName())->getAttributes('ReturnTypeWillChange')) === 0
) {
if (!$this->methodParameterComparisonHelper->isReturnTypeCompatible($realPrototype->getTentativeReturnType(), $methodVariant->getNativeReturnType(), true)) {
$messages[] = RuleErrorBuilder::message(sprintf(
Expand All @@ -203,7 +205,7 @@ public function processNode(Node $node, Scope $scope): array
}
}

$messages = array_merge($messages, $this->methodParameterComparisonHelper->compare($prototype, $method));
$messages = array_merge($messages, $this->methodParameterComparisonHelper->compare($prototype, $prototypeDeclaringClass, $method));

if (!$prototypeVariant instanceof FunctionVariantWithPhpDocs) {
return $this->addErrors($messages, $node, $scope);
Expand All @@ -215,7 +217,7 @@ public function processNode(Node $node, Scope $scope): array
$reportReturnType = !$realPrototype instanceof MethodPrototypeReflection || $realPrototype->getTentativeReturnType() === null || $prototype->isInternal()->no();
} else {
if ($realPrototype instanceof MethodPrototypeReflection && $realPrototype->isInternal()) {
if ($prototype->isInternal()->yes() && $prototype->getDeclaringClass()->getName() !== $realPrototype->getDeclaringClass()->getName()) {
if ($prototype->isInternal()->yes() && $prototypeDeclaringClass->getName() !== $realPrototype->getDeclaringClass()->getName()) {
$realPrototypeVariant = $realPrototype->getVariants()[0];
if (
$prototypeReturnType instanceof MixedType
Expand Down Expand Up @@ -243,7 +245,7 @@ public function processNode(Node $node, Scope $scope): array
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototypeReturnType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
} else {
Expand All @@ -253,7 +255,7 @@ public function processNode(Node $node, Scope $scope): array
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
$prototypeReturnType->describe(VerbosityLevel::typeOnly()),
$prototype->getDeclaringClass()->getDisplayName($this->genericPrototypeMessage),
$prototypeDeclaringClass->getDisplayName($this->genericPrototypeMessage),
$prototype->getName(),
))->nonIgnorable()->build();
}
Expand Down Expand Up @@ -310,30 +312,36 @@ private function hasOverrideAttribute(Node\Stmt\ClassMethod $method): bool
}

/**
* @return array{ExtendedMethodReflection, bool}|null
* @return array{ExtendedMethodReflection, ClassReflection, bool}|null
*/
private function findPrototype(ClassReflection $classReflection, string $methodName): ?array
{
foreach ($classReflection->getImmediateInterfaces() as $immediateInterface) {
if ($immediateInterface->hasNativeMethod($methodName)) {
return [$immediateInterface->getNativeMethod($methodName), true];
$method = $immediateInterface->getNativeMethod($methodName);
return [$method, $method->getDeclaringClass(), true];
}
}

if ($this->phpVersion->supportsAbstractTraitMethods()) {
foreach ($classReflection->getTraits(true) as $trait) {
if (!$trait->hasNativeMethod($methodName)) {
$nativeTraitReflection = $trait->getNativeReflection();
if (!$nativeTraitReflection->hasMethod($methodName)) {
continue;
}

$method = $trait->getNativeMethod($methodName);
$isAbstract = $method->isAbstract();
if (is_bool($isAbstract)) {
if ($isAbstract) {
return [$method, false];
}
} elseif ($isAbstract->yes()) {
return [$method, false];
$methodReflection = $nativeTraitReflection->getMethod($methodName);
$isAbstract = $methodReflection->isAbstract();
if ($isAbstract) {
return [
$this->phpClassReflectionExtension->createUserlandMethodReflection(
$trait,
$classReflection,
$methodReflection,
),
$trait->getNativeMethod($methodName)->getDeclaringClass(),
false,
];
}
}
}
Expand Down Expand Up @@ -369,7 +377,7 @@ private function findPrototype(ClassReflection $classReflection, string $methodN
}
}

return [$method, true];
return [$method, $method->getDeclaringClass(), true];
}

}
Loading

0 comments on commit be2b415

Please sign in to comment.