Skip to content

Commit

Permalink
Revert "Remove unneded abstraction"
Browse files Browse the repository at this point in the history 
This reverts commit ad6703d.
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Nov 19, 2023
1 parent fc23154 commit d0c0938
Show file tree
Hide file tree
Showing 7 changed files with 232 additions and 21 deletions.
8 changes: 4 additions & 4 deletions src/Reflection/Native/NativeMethodReflection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@

namespace PHPStan\Reflection\Native;

use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
use PHPStan\Reflection\Assertions;
use PHPStan\Reflection\ClassMemberReflection;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\MethodPrototypeReflection;
use PHPStan\Reflection\ParametersAcceptorWithPhpDocs;
use PHPStan\Reflection\Php\BuiltinMethodReflection;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\TrinaryLogic;
use PHPStan\Type\Type;
Expand All @@ -26,7 +26,7 @@ class NativeMethodReflection implements ExtendedMethodReflection
public function __construct(
private ReflectionProvider $reflectionProvider,
private ClassReflection $declaringClass,
private ReflectionMethod $reflection,
private BuiltinMethodReflection $reflection,
private array $variants,
private ?array $namedArgumentsVariants,
private TrinaryLogic $hasSideEffects,
Expand Down Expand Up @@ -116,7 +116,7 @@ public function getDeprecatedDescription(): ?string

public function isDeprecated(): TrinaryLogic
{
return TrinaryLogic::createFromBoolean($this->reflection->isDeprecated());
return $this->reflection->isDeprecated();
}

public function isInternal(): TrinaryLogic
Expand Down Expand Up @@ -181,7 +181,7 @@ public function getSelfOutType(): ?Type

public function returnsByReference(): TrinaryLogic
{
return TrinaryLogic::createFromBoolean($this->reflection->returnsReference());
return $this->reflection->returnsByReference();
}

}
60 changes: 60 additions & 0 deletions src/Reflection/Php/BuiltinMethodReflection.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php declare(strict_types = 1);

namespace PHPStan\Reflection\Php;

use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionIntersectionType;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionNamedType;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionUnionType;
use PHPStan\TrinaryLogic;

interface BuiltinMethodReflection
{

public function getName(): string;

public function getReflection(): ReflectionMethod;

This comment has been minimized.

Sign in to view

Copy link
@buffcode

buffcode Nov 21, 2023

@ondrejmirtes The methods returned nullable types before. Was the removal of the nullable types intentional as part of the revert commit?

Before:
ad6703d#diff-118ce1aee3922aeae65327b5e79f318af404a8ea22ed773ea31e3ab3e8a2c83bL19

This comment has been minimized.

Sign in to view

Copy link
@ondrejmirtes

ondrejmirtes Nov 21, 2023

Author Member

Hi, please note that I reverted this as a courtesy to nesbot/carbon package. You shouldn't be interested in this interface because it's not part of the backward compatibility promise: https://phpstan.org/developing-extensions/backward-compatibility-promise

It will be removed in the future.

Please open a discussion with your use-case and I'll guide you what to replace the usage with.

This comment has been minimized.

Sign in to view

Copy link
@kylekatarnls

kylekatarnls Nov 21, 2023

I recommend you mark as @internal any interface that should not be implemented outside the PHPStan package.

Also if we cannot extend, I'm not sure we can actually continue to expose user custom macro methods and make them work.

I didn't introduce myself the support of PHPStan in carbon library. With no alternatives to this I don't know if we can still support.

This comment has been minimized.

Sign in to view

Copy link
@ondrejmirtes

ondrejmirtes Nov 21, 2023

Author Member

@kylekatarnls The linked article describes what classes and interfaces from PHPStan one can refer in their own code. Plus there are PHPStan rules that cover that too.

I tried reimplementing the Carbon PHPStan extension over the weekend and it can work cleanly with the current APIs, except for anonymous classes. I got stuck on that for now. Will dive into it again soon.

The BuiltinMethodReflection will only be removed in a major version, so don't worry for now.

This comment has been minimized.

Sign in to view

Copy link
@kylekatarnls

kylekatarnls Nov 21, 2023 •
edited

Having your own @api annotation system is nice, but advantage of standard @internal is interoperability. Most of the IDEs would flag it as an error to implement/extend such classes/interfaces so it get better chances to be discovered.

If I understand well basically any class not having @api should have @internal, if you're interested I could send a PR for this.

This comment has been minimized.

Sign in to view

Copy link
@ondrejmirtes

ondrejmirtes Nov 21, 2023

Author Member

No, the problem in Carbon is that you don’t analyse the extension source at all. It’s excluded in the config and should not be.

I don’t want to spam the sources with unnecessary annotations, this is opt-in by design.

This comment has been minimized.

Sign in to view

Copy link
@calebdw

calebdw Nov 21, 2023 •
edited

Contributor

@ondrejmirtes, can we re-add the null type signature so that carbon continues to work in the meantime?

briannesbitt/Carbon#2887

This comment has been minimized.

Sign in to view

Copy link
@ondrejmirtes

ondrejmirtes Nov 21, 2023

Author Member

Sure, please send a PR to phpstan-src branch 1.10.x and I’ll release it today

This comment has been minimized.

Sign in to view

Copy link
@calebdw

calebdw Nov 21, 2023

Contributor

Done: #2762

public function getFileName(): ?string;

public function getDeclaringClass(): ReflectionClass|ReflectionEnum;

public function getStartLine(): ?int;

Check failure on line 26 in src/Reflection/Php/BuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.2, ubuntu-latest) 

Method PHPStan\Reflection\Php\BuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.

Check failure on line 26 in src/Reflection/Php/BuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.2, windows-latest) 

Method PHPStan\Reflection\Php\BuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.

Check failure on line 26 in src/Reflection/Php/BuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3, ubuntu-latest) 

Method PHPStan\Reflection\Php\BuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.

Check failure on line 26 in src/Reflection/Php/BuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3, windows-latest) 

Method PHPStan\Reflection\Php\BuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.
public function getEndLine(): ?int;

public function getDocComment(): ?string;

public function isStatic(): bool;

public function isPrivate(): bool;

public function isPublic(): bool;

public function getPrototype(): self;

public function isDeprecated(): TrinaryLogic;

public function isVariadic(): bool;

public function getReturnType(): ReflectionIntersectionType|ReflectionNamedType|ReflectionUnionType|null;

public function getTentativeReturnType(): ReflectionIntersectionType|ReflectionNamedType|ReflectionUnionType|null;

/**
* @return ReflectionParameter[]
*/
public function getParameters(): array;

public function isFinal(): bool;

public function isInternal(): bool;

public function isAbstract(): bool;

public function returnsByReference(): TrinaryLogic;

}
149 changes: 149 additions & 0 deletions src/Reflection/Php/NativeBuiltinMethodReflection.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
<?php declare(strict_types = 1);

namespace PHPStan\Reflection\Php;

use PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionIntersectionType;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionNamedType;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionUnionType;
use PHPStan\TrinaryLogic;

class NativeBuiltinMethodReflection implements BuiltinMethodReflection
{

public function __construct(private ReflectionMethod $reflection)
{
}

public function getName(): string
{
return $this->reflection->getName();
}

public function getReflection(): ReflectionMethod
{
return $this->reflection;
}

public function getFileName(): ?string
{
$fileName = $this->reflection->getFileName();
if ($fileName === false) {
return null;
}

return $fileName;
}

public function getDeclaringClass(): ReflectionClass|ReflectionEnum
{
return $this->reflection->getDeclaringClass();
}

public function getStartLine(): ?int
{
$line = $this->reflection->getStartLine();
if ($line === false) {

Check failure on line 49 in src/Reflection/Php/NativeBuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.2, ubuntu-latest) 

Method PHPStan\Reflection\Php\NativeBuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.

Check failure on line 49 in src/Reflection/Php/NativeBuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.2, windows-latest) 

Method PHPStan\Reflection\Php\NativeBuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.

Check failure on line 49 in src/Reflection/Php/NativeBuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3, ubuntu-latest) 

Method PHPStan\Reflection\Php\NativeBuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.

Check failure on line 49 in src/Reflection/Php/NativeBuiltinMethodReflection.php

View workflow job for this annotation

GitHub Actions / PHPStan (7.3, windows-latest) 

Method PHPStan\Reflection\Php\NativeBuiltinMethodReflection::getDeclaringClass() has invalid return type PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum.
return null;
}

return $line;
}

public function getEndLine(): ?int
{
$line = $this->reflection->getEndLine();
if ($line === false) {
return null;
}

return $line;
}

public function getDocComment(): ?string
{
$docComment = $this->reflection->getDocComment();
if ($docComment === false) {
return null;
}

return $docComment;
}

public function isStatic(): bool
{
return $this->reflection->isStatic();
}

public function isPrivate(): bool
{
return $this->reflection->isPrivate();
}

public function isPublic(): bool
{
return $this->reflection->isPublic();
}

public function isConstructor(): bool
{
return $this->reflection->isConstructor();
}

public function getPrototype(): BuiltinMethodReflection
{
return new self($this->reflection->getPrototype());
}

public function isDeprecated(): TrinaryLogic
{
return TrinaryLogic::createFromBoolean($this->reflection->isDeprecated());
}

public function isFinal(): bool
{
return $this->reflection->isFinal();
}

public function isInternal(): bool
{
return $this->reflection->isInternal();
}

public function isAbstract(): bool
{
return $this->reflection->isAbstract();
}

public function isVariadic(): bool
{
return $this->reflection->isVariadic();
}

public function getReturnType(): ReflectionIntersectionType|ReflectionNamedType|ReflectionUnionType|null
{
return $this->reflection->getReturnType();
}

public function getTentativeReturnType(): ReflectionIntersectionType|ReflectionNamedType|ReflectionUnionType|null
{
return $this->reflection->getTentativeReturnType();
}

/**
* @return ReflectionParameter[]
*/
public function getParameters(): array
{
return $this->reflection->getParameters();
}

public function returnsByReference(): TrinaryLogic
{
return TrinaryLogic::createFromBoolean($this->reflection->returnsReference());
}

}
23 changes: 13 additions & 10 deletions src/Reflection/Php/PhpClassReflectionExtension.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use PHPStan\Analyser\NodeScopeResolver;
use PHPStan\Analyser\ScopeContext;
use PHPStan\Analyser\ScopeFactory;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionProperty;
use PHPStan\Parser\Parser;
Expand Down Expand Up @@ -381,7 +380,7 @@ public function getMethod(ClassReflection $classReflection, string $methodName):
return $this->methodsIncludingAnnotations[$classReflection->getCacheKey()][$methodName];
}

$nativeMethodReflection = $classReflection->getNativeReflection()->getMethod($methodName);
$nativeMethodReflection = new NativeBuiltinMethodReflection($classReflection->getNativeReflection()->getMethod($methodName));
if (!isset($this->methodsIncludingAnnotations[$classReflection->getCacheKey()][$nativeMethodReflection->getName()])) {
$method = $this->createMethod($classReflection, $nativeMethodReflection, true);
$this->methodsIncludingAnnotations[$classReflection->getCacheKey()][$nativeMethodReflection->getName()] = $method;
Expand All @@ -408,7 +407,8 @@ public function getNativeMethod(ClassReflection $classReflection, string $method
throw new ShouldNotHappenException();
}

$nativeMethodReflection = $classReflection->getNativeReflection()->getMethod($methodName);
$reflectionMethod = $classReflection->getNativeReflection()->getMethod($methodName);
$nativeMethodReflection = new NativeBuiltinMethodReflection($reflectionMethod);

if (!isset($this->nativeMethods[$classReflection->getCacheKey()][$nativeMethodReflection->getName()])) {
$method = $this->createMethod($classReflection, $nativeMethodReflection, false);
Expand All @@ -420,7 +420,7 @@ public function getNativeMethod(ClassReflection $classReflection, string $method

private function createMethod(
ClassReflection $classReflection,
ReflectionMethod $methodReflection,
BuiltinMethodReflection $methodReflection,
bool $includingAnnotations,
): ExtendedMethodReflection
{
Expand Down Expand Up @@ -621,14 +621,14 @@ private function createMethod(
return $this->createUserlandMethodReflection($declaringClass, $declaringClass, $methodReflection);
}

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

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

if ($stubPhpDocPair === null && $methodDeclaringClass->isTrait()) {
if (! $methodReflection->getDeclaringClass()->isTrait() || $methodDeclaringClass->getName() !== $methodReflection->getDeclaringClass()->getName()) {
Expand All @@ -648,7 +648,7 @@ public function createUserlandMethodReflection(ClassReflection $fileDeclaringCla
}

if ($resolvedPhpDoc === null) {
$docComment = $methodReflection->getDocComment() !== false ? $methodReflection->getDocComment() : null;
$docComment = $methodReflection->getDocComment();
$positionalParameterNames = array_map(static fn (ReflectionParameter $parameter): string => $parameter->getName(), $methodReflection->getParameters());

$resolvedPhpDoc = $this->phpDocInheritanceResolver->resolvePhpDocForMethod(
Expand All @@ -671,7 +671,10 @@ public function createUserlandMethodReflection(ClassReflection $fileDeclaringCla
}

$phpDocParameterTypes = [];
if ($methodReflection->isConstructor()) {
if (
$methodReflection instanceof NativeBuiltinMethodReflection
&& $methodReflection->isConstructor()
) {
foreach ($methodReflection->getParameters() as $parameter) {
if (!$parameter->isPromoted()) {
continue;
Expand Down Expand Up @@ -862,10 +865,10 @@ private function findPropertyTrait(ReflectionProperty $propertyReflection): ?str
}

private function findMethodTrait(
ReflectionMethod $methodReflection,
BuiltinMethodReflection $methodReflection,
): ?string
{
$declaringClass = $methodReflection->getBetterReflection()->getDeclaringClass();
$declaringClass = $methodReflection->getReflection()->getBetterReflection()->getDeclaringClass();
if ($declaringClass->isTrait()) {
if ($methodReflection->getDeclaringClass()->isTrait() && $declaringClass->getName() === $methodReflection->getDeclaringClass()->getName()) {
return null;
Expand Down
7 changes: 3 additions & 4 deletions src/Reflection/Php/PhpMethodReflection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use PhpParser\Node\Stmt\Declare_;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\Namespace_;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
use PHPStan\BetterReflection\Reflection\Adapter\ReflectionParameter;
use PHPStan\Cache\Cache;
use PHPStan\Parser\FunctionCallStatementFinder;
Expand Down Expand Up @@ -67,7 +66,7 @@ public function __construct(
private InitializerExprTypeResolver $initializerExprTypeResolver,
private ClassReflection $declaringClass,
private ?ClassReflection $declaringTrait,
private ReflectionMethod $reflection,
private BuiltinMethodReflection $reflection,
private ReflectionProvider $reflectionProvider,
private Parser $parser,
private FunctionCallStatementFinder $functionCallStatementFinder,
Expand Down Expand Up @@ -391,7 +390,7 @@ public function isDeprecated(): TrinaryLogic
return TrinaryLogic::createYes();
}

return TrinaryLogic::createFromBoolean($this->reflection->isDeprecated());
return $this->reflection->isDeprecated();
}

public function isInternal(): TrinaryLogic
Expand Down Expand Up @@ -451,7 +450,7 @@ public function getDocComment(): ?string

public function returnsByReference(): TrinaryLogic
{
return TrinaryLogic::createFromBoolean($this->reflection->returnsReference());
return $this->reflection->returnsByReference();
}

}
3 changes: 1 addition & 2 deletions src/Reflection/Php/PhpMethodReflectionFactory.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace PHPStan\Reflection\Php;

use PHPStan\BetterReflection\Reflection\Adapter\ReflectionMethod;
use PHPStan\Reflection\Assertions;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Type\Generic\TemplateTypeMap;
Expand All @@ -18,7 +17,7 @@ interface PhpMethodReflectionFactory
public function create(
ClassReflection $declaringClass,
?ClassReflection $declaringTrait,
ReflectionMethod $reflection,
BuiltinMethodReflection $reflection,
TemplateTypeMap $templateTypeMap,
array $phpDocParameterTypes,
?Type $phpDocReturnType,
Expand Down

0 comments on commit d0c0938

Please sign in to comment.