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

Updates for PHPStan 2.0 #48

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
"license": "MIT",
"require": {
"php": "^8.2",
"phpstan/phpstan": "^1.11",
"phpstan/phpstan": "^2.0",
"webmozart/assert": "^1.11"
},
"require-dev": {
"nikic/php-parser": "^4.19",
"nikic/php-parser": "^5.0",
"symplify/phpstan-extensions": "^11.4",
"symplify/rule-doc-generator": "^12.1",
"phpunit/phpunit": "^10.5",
Expand Down
1 change: 0 additions & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ parameters:

# overly detailed generics
- '#Rector\\TypePerfect\\Tests\\Rules\\(.*?) generic (class|interface)#'
- '#Method Rector\\TypePerfect\\Tests\\Rules\\(.*?)testRule\(\) has parameter \$expectedErrorsWithLines with no value type specified in iterable type array#'
4 changes: 2 additions & 2 deletions src/Matcher/ClassMethodCallReferenceResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ public function resolve(MethodCall|MethodCallableNode $methodCallOrMethodCallabl
return null;
}

if (! $callerType instanceof TypeWithClassName) {
if (count($callerType->getObjectClassNames()) !== 1) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

return null;
}

// move to the class where method is defined, e.g. parent class defines the method, so it should be checked there
$className = $callerType->getClassName();
$className = $callerType->getObjectClassNames()[0];
$methodNameString = $methodName->toString();

return new MethodCallReference($className, $methodNameString);
Expand Down
23 changes: 11 additions & 12 deletions src/Printer/CollectorMetadataPrinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
use PhpParser\Node\NullableType;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\UnionType;
use PhpParser\PrettyPrinter\Standard;
use PHPStan\Node\Printer\Printer;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHPStan has removed the PhpParser\PrettyPrinter\Standard alias and now we need to use PHPStan\Node\Printer\Printer directly

use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
Expand All @@ -35,11 +35,9 @@

final readonly class CollectorMetadataPrinter
{
private Standard $printerStandard;

public function __construct()
{
$this->printerStandard = new Standard();
public function __construct(
private Printer $printer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHPStan complains if we try to instantiate this directly, it needs to be passed through dependency injection

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to create new Standard printer directly, as we lightweight. We don't need any special printer here

) {
}

public function printArgTypesAsString(MethodCall $methodCall, ExtendedMethodReflection $extendedMethodReflection, Scope $scope): string
Expand All @@ -56,6 +54,7 @@ public function printArgTypesAsString(MethodCall $methodCall, ExtendedMethodRefl
return ResolvedTypes::UNKNOWN_TYPES;
}

// @phpstan-ignore phpstanApi.instanceofType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case there is no good alternative to checking that it is an instance of IntersectionType (at least I was not able to find one), so I have added a ignore comment

Copy link
Member

Choose a reason for hiding this comment

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

Better place it in phpstan.neon, so we have all ignores in one place.

if ($argType instanceof IntersectionType) {
return ResolvedTypes::UNKNOWN_TYPES;
}
Expand Down Expand Up @@ -100,7 +99,7 @@ public function printParamTypesToString(ClassMethod $classMethod, ?string $class
$paramType = $this->resolveSortedTypes($paramType, $className);
}

$printedParamType = $this->printerStandard->prettyPrint([$paramType]);
$printedParamType = $this->printer->prettyPrint([$paramType]);
$printedParamType = str_replace('\Closure', 'callable', $printedParamType);
$printedParamType = ltrim($printedParamType, '\\');
$printedParamType = str_replace('|\\', '|', $printedParamType);
Expand Down Expand Up @@ -160,15 +159,15 @@ private function resolveSortedTypes(UnionType|NodeIntersectionType $paramType, ?

private function printTypeToString(Type $type): string
{
if ($type instanceof ClassStringType) {
if ($type->isClassString()->yes()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHPStan recommends using these alternatives

return 'string';
}

if ($type instanceof ArrayType) {
if ($type->isArray()->yes()) {
return 'array';
}

if ($type instanceof BooleanType) {
if ($type->isBoolean()->yes()) {
return 'bool';
}

Expand All @@ -180,8 +179,8 @@ private function printTypeToString(Type $type): string
return 'callable';
}

if ($type instanceof EnumCaseObjectType) {
return $type->getClassName();
if (count($type->getEnumCases()) === 1) {
return $type->getEnumCases()[0]->getClassName();
}

return $type->describe(VerbosityLevel::typeOnly());
Expand Down
6 changes: 3 additions & 3 deletions src/Printer/NodeComparator.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
namespace Rector\TypePerfect\Printer;

use PhpParser\Node;
use PhpParser\PrettyPrinter\Standard;
use PHPStan\Node\Printer\Printer;

final readonly class NodeComparator
{
public function __construct(
private Standard $standard
private Printer $printer
) {
}

Expand All @@ -20,6 +20,6 @@ public function areNodesEqual(Node $firstNode, Node $secondNode): bool
$firstNode->setAttribute('comments', null);
$secondNode->setAttribute('comments', null);

return $this->standard->prettyPrint([$firstNode]) === $this->standard->prettyPrint([$secondNode]);
return $this->printer->prettyPrint([$firstNode]) === $this->printer->prettyPrint([$secondNode]);
}
}
3 changes: 2 additions & 1 deletion src/Reflection/ReflectionParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PhpParser\NodeVisitor\NameResolver;
use PhpParser\Parser;
use PhpParser\ParserFactory;
use PhpParser\PhpVersion;
use PHPStan\Reflection\ClassReflection;
use Throwable;

Expand All @@ -26,7 +27,7 @@ final class ReflectionParser
public function __construct()
{
$parserFactory = new ParserFactory();
$this->parser = $parserFactory->create(ParserFactory::PREFER_PHP7);
$this->parser = $parserFactory->createForVersion(PhpVersion::fromString('7.4'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHPParser 5 has removed the create method. I used the createForVersion instead, setting this as 7.4. I am not really sure that this is the right solution

Copy link
Member

Choose a reason for hiding this comment

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

I think there is some createForLatestPhp(), that should be used instead so we don't create fixed value here

Copy link
Member

Choose a reason for hiding this comment

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

}

public function parseClassReflection(ClassReflection $classReflection): ?ClassLike
Expand Down
5 changes: 1 addition & 4 deletions src/Rules/NarrowPrivateClassMethodParamTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ private function validateArgVsParamTypes(array $args, MethodCall $methodCall, Sc
continue;
}

if (! $arg instanceof Arg) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

$arg will always be an Arg, this check is not needed

continue;
}

$paramRuleError = $this->validateParam($param, $position, $arg->value, $scope);
if (! $paramRuleError instanceof RuleError) {
continue;
Expand Down Expand Up @@ -149,6 +145,7 @@ private function validateParam(Param $param, int $position, Expr $expr, Scope $s
return null;
}

// @phpstan-ignore phpstanApi.instanceofType
if ($argType instanceof IntersectionType) {
return null;
}
Expand Down
14 changes: 8 additions & 6 deletions src/Rules/NarrowReturnObjectTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,11 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

/** @var TypeWithClassName $returnExprType */
$errorMessage = sprintf(self::ERROR_MESSAGE, $returnExprType->getClassName());
if (count($returnExprType->getObjectClassNames()) !== 1) {
return [];
}

$errorMessage = sprintf(self::ERROR_MESSAGE, $returnExprType->getObjectClassNames()[0]);

return [
RuleErrorBuilder::message($errorMessage)
Expand Down Expand Up @@ -127,15 +130,14 @@ private function shouldSkipScope(Scope $scope): bool

private function shouldSkipReturnExprType(Type $type): bool
{
if (! $type instanceof TypeWithClassName) {
if (count($type->getObjectClassNames()) !== 1) {
return true;
}

$classReflection = $type->getClassReflection();
if (! $classReflection instanceof ClassReflection) {
if (count($type->getObjectClassReflections()) !== 1) {
return true;
}

return $classReflection->isAnonymous();
return $type->getObjectClassReflections()[0]->isAnonymous();
}
}
1 change: 1 addition & 0 deletions src/Rules/NoArrayAccessOnObjectRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public function getNodeType(): string
public function processNode(Node $node, Scope $scope): array
{
$varType = $scope->getType($node->var);
// @phpstan-ignore phpstanApi.instanceofType
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, there is no good alternative or at least I was not able to find one

if (! $varType instanceof ObjectType) {
return [];
}
Expand Down
6 changes: 3 additions & 3 deletions src/Rules/NoMixedMethodCallerRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\PrettyPrinter\Standard;
use PHPStan\Node\Printer\Printer;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
Expand All @@ -27,7 +27,7 @@
public const ERROR_MESSAGE = 'Mixed variable in a `%s->...()` can skip important errors. Make sure the type is known';

public function __construct(
private Standard $printerStandard,
private Printer $printer,
private Configuration $configuration,
) {
}
Expand Down Expand Up @@ -65,7 +65,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

$printedMethodCall = $this->printerStandard->prettyPrintExpr($node->var);
$printedMethodCall = $this->printer->prettyPrintExpr($node->var);

return [
RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $printedMethodCall))
Expand Down
6 changes: 3 additions & 3 deletions src/Rules/NoMixedPropertyFetcherRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use PhpParser\Node;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\PrettyPrinter\Standard;
use PHPStan\Node\Printer\Printer;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
Expand All @@ -26,7 +26,7 @@
public const ERROR_MESSAGE = 'Mixed property fetch in a "%s->..." can skip important errors. Make sure the type is known';

public function __construct(
private Standard $standard,
private Printer $printer,
private Configuration $configuration,
) {
}
Expand Down Expand Up @@ -54,7 +54,7 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

$printedVar = $this->standard->prettyPrintExpr($node->var);
$printedVar = $this->printer->prettyPrintExpr($node->var);

return [
RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $printedVar))
Expand Down
1 change: 1 addition & 0 deletions src/Rules/NoParamTypeRemovalRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public function processNode(Node $node, Scope $scope): array
}

$parentClassMethodReflection = $this->methodNodeAnalyser->matchFirstParentClassMethod($scope, $classMethodName);
// @phpstan-ignore phpstanApi.instanceofAssumption
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not covered by PHPStan's BC guarantee, for now we ignore it

if (! $parentClassMethodReflection instanceof PhpMethodReflection) {
return [];
}
Expand Down
3 changes: 2 additions & 1 deletion src/Rules/ReturnNullOverFalseRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ public function processNode(Node $node, Scope $scope): array
}

$exprType = $scope->getType($return->expr);
// @phpstan-ignore phpstanApi.instanceofType
if (! $exprType instanceof ConstantBooleanType) {
if ($exprType instanceof BooleanType) {
if ($exprType->isBoolean()->yes()) {
return [];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
final class NarrowPrivateClassMethodParamTypeRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorsWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorsWithLines
Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHPStan asks us to add a more detailed type hint

*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorsWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ final class NarrowPublicClassMethodParamTypeRuleTest extends RuleTestCase
{
/**
* @param string[] $filePaths
* @param mixed[] $expectedErrorsWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorsWithLines
*/
#[DataProvider('provideData')]
public function testRule(array $filePaths, array $expectedErrorsWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
final class NarrowReturnObjectTypeRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorMessagesWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorMessagesWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
final class NoArrayAccessOnObjectRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorMessagesWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorMessagesWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
final class NoEmptyOnObjectRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorMessagesWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorMessagesWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
final class NoIssetOnObjectRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorMessagesWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorMessagesWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

final class NoMixedMethodCallerRuleTest extends RuleTestCase
{
/**
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorsWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorsWithLines): void
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@

final class NoMixedPropertyFetcherRuleTest extends RuleTestCase
{
/**
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorsWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorsWithLines): void
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
final class NoParamTypeRemovalRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorMessagesWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorMessagesWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
final class ReturnNullOverFalseRuleTest extends RuleTestCase
{
/**
* @param mixed[] $expectedErrorMessagesWithLines
* @param list<array{0: string, 1: int, 2?: string|null}> $expectedErrorMessagesWithLines
*/
#[DataProvider('provideData')]
public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void
Expand Down