diff --git a/conf/config.level2.neon b/conf/config.level2.neon index 9ff871b3d4..6b84041f1d 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -21,12 +21,14 @@ rules: - PHPStan\Rules\Generics\MethodTemplateTypeRule - PHPStan\Rules\Generics\MethodSignatureVarianceRule - PHPStan\Rules\Generics\TraitTemplateTypeRule + - PHPStan\Rules\Methods\FriendMethodCallRule - PHPStan\Rules\Methods\IncompatibleDefaultParameterTypeRule - PHPStan\Rules\Operators\InvalidBinaryOperationRule - PHPStan\Rules\Operators\InvalidUnaryOperationRule - PHPStan\Rules\Operators\InvalidComparisonOperationRule - PHPStan\Rules\PhpDoc\IncompatiblePhpDocTypeRule - PHPStan\Rules\PhpDoc\IncompatiblePropertyPhpDocTypeRule + - PHPStan\Rules\PhpDoc\InvalidFriendTagTargetRule - PHPStan\Rules\PhpDoc\InvalidPhpDocTagValueRule - PHPStan\Rules\PhpDoc\InvalidPHPStanDocTagRule - PHPStan\Rules\PhpDoc\InvalidThrowsPhpDocValueRule diff --git a/src/PhpDoc/PhpDocNodeResolver.php b/src/PhpDoc/PhpDocNodeResolver.php index 31a8e79639..4924eaed89 100644 --- a/src/PhpDoc/PhpDocNodeResolver.php +++ b/src/PhpDoc/PhpDocNodeResolver.php @@ -5,6 +5,7 @@ use PHPStan\Analyser\NameScope; use PHPStan\PhpDoc\Tag\DeprecatedTag; use PHPStan\PhpDoc\Tag\ExtendsTag; +use PHPStan\PhpDoc\Tag\FriendTag; use PHPStan\PhpDoc\Tag\ImplementsTag; use PHPStan\PhpDoc\Tag\MethodTag; use PHPStan\PhpDoc\Tag\MethodTagParameter; @@ -17,10 +18,13 @@ use PHPStan\PhpDoc\Tag\UsesTag; use PHPStan\PhpDoc\Tag\VarTag; use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprNullNode; +use PHPStan\PhpDocParser\Ast\PhpDoc\GenericTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\MixinTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode; +use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocTagNode; use PHPStan\PhpDocParser\Ast\PhpDoc\TemplateTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\ThrowsTagValueNode; +use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode; use PHPStan\Reflection\PassedByReference; use PHPStan\Type\ErrorType; use PHPStan\Type\Generic\TemplateTypeVariance; @@ -28,6 +32,7 @@ use PHPStan\Type\NeverType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; +use PHPStan\Type\TypeWithClassName; class PhpDocNodeResolver { @@ -373,6 +378,34 @@ public function resolveIsFinal(PhpDocNode $phpDocNode): bool return count($finalTags) > 0; } + /** + * @return array + */ + public function resolveFriends(PhpDocNode $phpDocNode, NameScope $nameScope): array + { + $potentialFriends = array_map(function (PhpDocTagNode $node) use ($nameScope): ?FriendTag { + $genericTagValue = $node->value; + if (!$genericTagValue instanceof GenericTagValueNode) { + return null; + } + $type = $genericTagValue->value; + if ($type === '') { + return null; + } + $method = null; + if (strpos($type, '::') !== false) { + [$type, $method] = explode('::', $type, 2); + } + $type = $this->typeNodeResolver->resolve(new IdentifierTypeNode($type), $nameScope); + if (!$type instanceof TypeWithClassName) { + return null; + } + return new FriendTag($type, $method); + }, $phpDocNode->getTagsByName('@friend')); + + return array_values(array_filter($potentialFriends)); + } + private function shouldSkipType(string $tagName, Type $type): bool { if (strpos($tagName, '@psalm-') !== 0) { diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index 0a6af89e6f..3914a0ce75 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -3,6 +3,7 @@ namespace PHPStan\PhpDoc; use PHPStan\Analyser\NameScope; +use PHPStan\PhpDoc\Tag\FriendTag; use PHPStan\PhpDoc\Tag\MixinTag; use PHPStan\PhpDoc\Tag\ParamTag; use PHPStan\PhpDoc\Tag\ReturnTag; @@ -70,6 +71,9 @@ class ResolvedPhpDocBlock private ?bool $isFinal = null; + /** @var array|false */ + private $friends = false; + private function __construct() { } @@ -129,6 +133,7 @@ public static function createEmpty(): self $self->isDeprecated = false; $self->isInternal = false; $self->isFinal = false; + $self->friends = []; return $self; } @@ -164,6 +169,7 @@ public function merge(array $parents, array $parentPhpDocBlocks): self $result->isDeprecated = $result->deprecatedTag !== null; $result->isInternal = $this->isInternal(); $result->isFinal = $this->isFinal(); + $result->friends = self::mergeFriendsTags($this->getFriends(), $parents); return $result; } @@ -204,6 +210,7 @@ public function changeParameterNamesByMapping(array $parameterNameMapping): self $self->isDeprecated = $this->isDeprecated; $self->isInternal = $this->isInternal; $self->isFinal = $this->isFinal; + $self->friends = $this->friends; return $self; } @@ -402,6 +409,20 @@ public function isFinal(): bool return $this->isFinal; } + /** + * @return array + */ + public function getFriends(): array + { + if ($this->friends === false) { + $this->friends = $this->phpDocNodeResolver->resolveFriends( + $this->phpDocNode, + $this->nameScope + ); + } + return $this->friends; + } + public function getTemplateTypeMap(): TemplateTypeMap { return $this->templateTypeMap; @@ -544,6 +565,25 @@ private static function mergeThrowsTags(?ThrowsTag $throwsTag, array $parents): return null; } + /** + * @param array $friendsTag + * @param array $parents + * @return array + */ + private static function mergeFriendsTags(array $friendsTag, array $parents): array + { + $merged = []; + foreach ($friendsTag as $friendTag) { + $merged[(string) $friendTag] = $friendTag; + } + foreach ($parents as $parent) { + foreach ($parent->getFriends() as $inheritedFriend) { + $merged[(string) $inheritedFriend] = $inheritedFriend; + } + } + return array_values($merged); + } + /** * @template T of \PHPStan\PhpDoc\Tag\TypedTag * @param T $tag diff --git a/src/PhpDoc/Tag/FriendTag.php b/src/PhpDoc/Tag/FriendTag.php new file mode 100644 index 0000000000..a68d7d7605 --- /dev/null +++ b/src/PhpDoc/Tag/FriendTag.php @@ -0,0 +1,40 @@ +type = $type; + $this->method = $method; + } + + public function getType(): TypeWithClassName + { + return $this->type; + } + + public function getMethod(): ?string + { + return $this->method; + } + + public function __toString(): string + { + $string = $this->type->describe(VerbosityLevel::precise()); + if ($this->method !== null) { + $string .= '::' . $this->method; + } + return $string; + } + +} diff --git a/src/Rules/Methods/FriendMethodCallRule.php b/src/Rules/Methods/FriendMethodCallRule.php new file mode 100644 index 0000000000..a3d9fe55e3 --- /dev/null +++ b/src/Rules/Methods/FriendMethodCallRule.php @@ -0,0 +1,92 @@ + + */ +class FriendMethodCallRule implements \PHPStan\Rules\Rule +{ + + private FileTypeMapper $fileTypeMapper; + + public function __construct(FileTypeMapper $fileTypeMapper) + { + $this->fileTypeMapper = $fileTypeMapper; + } + + public function getNodeType(): string + { + return MethodCall::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node->name instanceof Node\Identifier) { + return []; + } + $methodName = $node->name->name; + $callee = $scope->getType($node->var); + if (!$callee->hasMethod($methodName)->yes()) { + return []; + } + $method = $callee->getMethod($methodName, $scope); + if (!$scope->canCallMethod($method)) { + return []; + } + $docComment = $method->getDocComment(); + if ($docComment === null || $method->getDeclaringClass()->getFileName() === false) { + return []; + } + $friends = $this->fileTypeMapper->getResolvedPhpDoc( + $method->getDeclaringClass()->getFileName(), + $method->getDeclaringClass()->getName(), + null, + $method->getName(), + $docComment + )->getFriends(); + if (\count($friends) === 0) { + return []; + } + + if (!$scope->isInClass()) { + return [ + RuleErrorBuilder::message(sprintf( + 'You may not not call %s::%s() from the global scope as the method lists allowed callers through friends.', + $method->getDeclaringClass()->getName(), + $method->getName() + ))->build(), + ]; + } + + $callerClass = $scope->getClassReflection(); + $callerMethod = $scope->getFunctionName(); + foreach ($friends as $friend) { + $type = $friend->getType(); + if ($callerClass->getName() !== $type->getClassName()) { + continue; + } + if ($friend->getMethod() !== null && $friend->getMethod() !== $callerMethod) { + continue; + } + return []; // caller is whitelisted + } + + return [ + RuleErrorBuilder::message(sprintf( + '%s::%s() may not call %s::%s() as it is not listed as a friend.', + $callerClass->getName(), + $callerMethod, + $method->getDeclaringClass()->getName(), + $method->getName() + ))->build(), + ]; + } + +} diff --git a/src/Rules/PhpDoc/InvalidFriendTagTargetRule.php b/src/Rules/PhpDoc/InvalidFriendTagTargetRule.php new file mode 100644 index 0000000000..4cd94f8dcc --- /dev/null +++ b/src/Rules/PhpDoc/InvalidFriendTagTargetRule.php @@ -0,0 +1,69 @@ + + */ +class InvalidFriendTagTargetRule implements \PHPStan\Rules\Rule +{ + + private FileTypeMapper $fileTypeMapper; + + private ReflectionProvider $reflectionProvider; + + public function __construct(FileTypeMapper $fileTypeMapper, ReflectionProvider $reflectionProvider) + { + $this->fileTypeMapper = $fileTypeMapper; + $this->reflectionProvider = $reflectionProvider; + } + + public function getNodeType(): string + { + return \PhpParser\Node\Stmt\ClassMethod::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $docComment = $node->getDocComment(); + if ($docComment === null) { + return []; + } + $resolvedPhpDoc = $this->fileTypeMapper->getResolvedPhpDoc( + $scope->getFile(), + $scope->isInClass() ? $scope->getClassReflection()->getName() : null, + $scope->isInTrait() ? $scope->getTraitReflection()->getName() : null, + $node->name->name, + $docComment->getText() + ); + $errors = []; + foreach ($resolvedPhpDoc->getFriends() as $friendTag) { + $className = $friendTag->getType()->getClassName(); + if (!$this->reflectionProvider->hasClass($className)) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Class %s specified by a @friend tag does not exist.', + $className + ))->build(); + continue; + } + $reflection = $this->reflectionProvider->getClass($className); + $methodName = $friendTag->getMethod(); + if ($methodName === null || $reflection->hasMethod($methodName)) { + continue; + } + $errors[] = RuleErrorBuilder::message(sprintf( + 'Method %s::%s() specified by a @friend tag does not exist.', + $className, + $methodName + ))->build(); + } + return $errors; + } + +} diff --git a/tests/PHPStan/Rules/Methods/FriendMethodCallRuleTest.php b/tests/PHPStan/Rules/Methods/FriendMethodCallRuleTest.php new file mode 100644 index 0000000000..b35a626f29 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/FriendMethodCallRuleTest.php @@ -0,0 +1,61 @@ + + */ +class FriendMethodCallRuleTest extends \PHPStan\Testing\RuleTestCase +{ + + protected function getRule(): Rule + { + return new FriendMethodCallRule(self::getContainer()->getByType(FileTypeMapper::class)); + } + + public function testFriendMethodCall(): void + { + $this->analyse([ __DIR__ . '/data/friend-method-call.php'], [ + [ + 'You may not not call FriendMethodCallTest\Foo::friendWithEntireBar() from the global scope as the method lists allowed callers through friends.', + 6, + ], + [ + 'FriendMethodCallTest\Bar::notBaz() may not call FriendMethodCallTest\Foo::friendWithBarBaz() as it is not listed as a friend.', + 34, + ], + [ + 'FriendMethodCallTest\BarBar::baz() may not call FriendMethodCallTest\Foo::friendWithEntireBar() as it is not listed as a friend.', + 44, + ], + [ + 'FriendMethodCallTest\BarBar::baz() may not call FriendMethodCallTest\Foo::friendWithBarBaz() as it is not listed as a friend.', + 45, + ], + [ + 'FriendMethodCallTest\BarBar::notBaz() may not call FriendMethodCallTest\Foo::friendWithEntireBar() as it is not listed as a friend.', + 52, + ], + [ + 'FriendMethodCallTest\BarBar::notBaz() may not call FriendMethodCallTest\Foo::friendWithBarBaz() as it is not listed as a friend.', + 53, + ], + [ + 'FriendMethodCallTest\Qux::smh() may not call FriendMethodCallTest\Foo::friendWithEntireBar() as it is not listed as a friend.', + 63, + ], + [ + 'FriendMethodCallTest\Qux::smh() may not call FriendMethodCallTest\Foo::friendWithBarBaz() as it is not listed as a friend.', + 64, + ], + [ + 'FriendMethodCallTest\SelfFriend::caller() may not call FriendMethodCallTest\SelfFriend::notFriend() as it is not listed as a friend.', + 72, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Methods/data/friend-method-call.php b/tests/PHPStan/Rules/Methods/data/friend-method-call.php new file mode 100644 index 0000000000..cbba012047 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/friend-method-call.php @@ -0,0 +1,89 @@ +friendWithEntireBar(); + +class Foo +{ + public function casual() { } + + /** @friend Bar */ + public function friendWithEntireBar() { } + + /** @friend Bar::baz */ + public function friendWithBarBaz() { } +} + +class Bar +{ + public function baz() + { + $foo = new Foo(); + $foo->casual(); + $foo->friendWithEntireBar(); + $foo->friendWithBarBaz(); + } + + public function notBaz() + { + $foo = new Foo(); + $foo->casual(); + $foo->friendWithEntireBar(); + $foo->friendWithBarBaz(); + } +} + +class BarBar +{ + public function baz() + { + $foo = new Foo(); + $foo->casual(); + $foo->friendWithEntireBar(); + $foo->friendWithBarBaz(); + } + + public function notBaz() + { + $foo = new Foo(); + $foo->casual(); + $foo->friendWithEntireBar(); + $foo->friendWithBarBaz(); + } +} + +class Qux +{ + public function smh() + { + $foo = new Foo(); + $foo->casual(); + $foo->friendWithEntireBar(); + $foo->friendWithBarBaz(); + } +} + +class SelfFriend +{ + public function caller() + { + $this->notFriend(); + $this->friendWithSelfFriend(); + $this->friendWithSelf(); + $this->friendWithStatic(); + } + + /** @friend Foo */ + public function notFriend() { } + + /** @friend SelfFriend */ + public function friendWithSelfFriend() { } + + /** @friend self */ + public function friendWithSelf() { } + + /** @friend static */ + public function friendWithStatic() { } +} diff --git a/tests/PHPStan/Rules/PhpDoc/InvalidFriendTagTargetRuleTest.php b/tests/PHPStan/Rules/PhpDoc/InvalidFriendTagTargetRuleTest.php new file mode 100644 index 0000000000..3e67bb64b3 --- /dev/null +++ b/tests/PHPStan/Rules/PhpDoc/InvalidFriendTagTargetRuleTest.php @@ -0,0 +1,36 @@ + + */ +class InvalidFriendTagTargetRuleTest extends \PHPStan\Testing\RuleTestCase +{ + + protected function getRule(): Rule + { + return new InvalidFriendTagTargetRule( + self::getContainer()->getByType(FileTypeMapper::class), + $this->createReflectionProvider() + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/invalid-friends.php'], [ + [ + 'Class InvalidFriends\Bar specified by a @friend tag does not exist.', + 11, + ], + [ + 'Method InvalidFriends\Foo::notExists() specified by a @friend tag does not exist.', + 11, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/PhpDoc/data/invalid-friends.php b/tests/PHPStan/Rules/PhpDoc/data/invalid-friends.php new file mode 100644 index 0000000000..405298ff8b --- /dev/null +++ b/tests/PHPStan/Rules/PhpDoc/data/invalid-friends.php @@ -0,0 +1,12 @@ +getByType(FileTypeMapper::class); + + $realpath = realpath(__DIR__ . '/data/friend-phpdocs.php'); + if ($realpath === false) { + throw new \PHPStan\ShouldNotHappenException(); + } + + $resolved = $fileTypeMapper->getResolvedPhpDoc($realpath, \FriendPhpDocs\Foo::class, null, 'noFriends', ''); + $this->assertSame([], $resolved->getFriends()); + + $resolved = $fileTypeMapper->getResolvedPhpDoc($realpath, \FriendPhpDocs\Foo::class, null, 'invalidFriends', '/** + * @friend + * @friend bool + */'); + $this->assertSame([], $resolved->getFriends()); + + $resolved = $fileTypeMapper->getResolvedPhpDoc($realpath, \FriendPhpDocs\Foo::class, null, 'classAsFriend', '/** + * @friend Bar + */'); + $this->assertCount(1, $resolved->getFriends()); + $this->assertSame( + 'FriendPhpDocs\Bar', + $resolved->getFriends()[0]->getType()->describe(VerbosityLevel::precise()) + ); + $this->assertNull($resolved->getFriends()[0]->getMethod()); + + $resolved = $fileTypeMapper->getResolvedPhpDoc($realpath, \FriendPhpDocs\Foo::class, null, 'methodAsFriend', '/** + * @friend Bar::baz + */'); + $this->assertCount(1, $resolved->getFriends()); + $this->assertSame( + 'FriendPhpDocs\Bar', + $resolved->getFriends()[0]->getType()->describe(VerbosityLevel::precise()) + ); + $this->assertSame('baz', $resolved->getFriends()[0]->getMethod()); + + $resolved = $fileTypeMapper->getResolvedPhpDoc($realpath, \FriendPhpDocs\Foo::class, null, 'multipleFriends', '/** + * @friend Bar + * @friend Bar::baz + */'); + $this->assertCount(2, $resolved->getFriends()); + $this->assertSame( + 'FriendPhpDocs\Bar', + $resolved->getFriends()[0]->getType()->describe(VerbosityLevel::precise()) + ); + $this->assertNull($resolved->getFriends()[0]->getMethod()); + $this->assertSame( + 'FriendPhpDocs\Bar', + $resolved->getFriends()[1]->getType()->describe(VerbosityLevel::precise()) + ); + $this->assertSame('baz', $resolved->getFriends()[1]->getMethod()); + } + public function testFileWithCyclicPhpDocs(): void { self::getContainer()->getByType(\PHPStan\Broker\Broker::class); diff --git a/tests/PHPStan/Type/data/friend-phpdocs.php b/tests/PHPStan/Type/data/friend-phpdocs.php new file mode 100644 index 0000000000..de371b1a63 --- /dev/null +++ b/tests/PHPStan/Type/data/friend-phpdocs.php @@ -0,0 +1,38 @@ +