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 all commits
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
14 changes: 13 additions & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,16 @@ 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#'

-
identifier: phpstanApi.instanceofType
paths:
- src/Printer/CollectorMetadataPrinter.php
- src/Rules/NarrowPrivateClassMethodParamTypeRule.php
- src/Rules/NoArrayAccessOnObjectRule.php
- src/Rules/ReturnNullOverFalseRule.php

-
identifier: phpstanApi.instanceofAssumption
paths:
- src/Rules/NoParamTypeRemovalRule.php
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
25 changes: 11 additions & 14 deletions src/Printer/CollectorMetadataPrinter.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,11 @@
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;
use PHPStan\Type\ArrayType;
use PHPStan\Type\BooleanType;
use PHPStan\Type\ClassStringType;
use PHPStan\Type\ClosureType;
use PHPStan\Type\Enum\EnumCaseObjectType;
use PHPStan\Type\IntegerRangeType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\MixedType;
Expand All @@ -35,11 +32,11 @@

final readonly class CollectorMetadataPrinter
{
private Standard $printerStandard;
private Standard $printer;

public function __construct()
{
$this->printerStandard = new Standard();
public function __construct(
) {
$this->printer = new Standard();
}

public function printArgTypesAsString(MethodCall $methodCall, ExtendedMethodReflection $extendedMethodReflection, Scope $scope): string
Expand Down Expand Up @@ -100,7 +97,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 +157,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 +177,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]);
}
}
2 changes: 1 addition & 1 deletion src/Reflection/ReflectionParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ final class ReflectionParser
public function __construct()
{
$parserFactory = new ParserFactory();
$this->parser = $parserFactory->create(ParserFactory::PREFER_PHP7);
$this->parser = $parserFactory->createForNewestSupportedVersion();
Copy link
Member

Choose a reason for hiding this comment

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

it needs verify under php 7.x, since rector downgraded to php 7.2, will verify after PR on rector side finished

currently still wip :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's why I said I wasn't sure what to do here and why I initially used createForVersion(PhpVersion::fromString('7.4')) I'll let you decide what really needs to go here.

BTW the new Rector probably needs to downgrade to PHP 7.4 since PHPStan dropped support for 7.2 and 7.3, but again not really sure

Copy link
Member

Choose a reason for hiding this comment

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

I see, Rector require phpstan, so php 7.4 will be minimum then ;)

Copy link
Member

Choose a reason for hiding this comment

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

I tested on rector side, with ->createForNewestSupportedVersion(), it got error on curly array $string{0}

There was 1 error:

1) Rector\Tests\Php74\Rector\ArrayDimFetch\CurlyToSquareBracketArrayStringRector\CurlyToSquareBracketArrayStringRectorTest::test with data set #0 ('/Users/samsonasik/www/rector-...hp.inc')
PHPStan\Parser\ParserErrorsException: Syntax error, unexpected '{', expecting ';', Syntax error, unexpected '{', expecting ';', Syntax error, unexpected '{', expecting ';'

phar:///Users/samsonasik/www/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Parser/RichParser.php:70

updating to :

createForVersion(PhpVersion::fromString('7.0'))

make it working:

PHPUnit 10.5.38 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.12
Configuration: /Users/samsonasik/www/rector-src/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 00:00.316, Memory: 62.50 MB

OK (1 test, 2 assertions)

see rectorphp/rector-src#6431 (comment)

this needs fallback to php 8.0 tho, I will keep updating ;)

Copy link
Contributor Author

@carlos-granados carlos-granados Nov 16, 2024

Choose a reason for hiding this comment

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

Let me know if I can help in any way

Copy link
Member

@samsonasik samsonasik Nov 17, 2024

Choose a reason for hiding this comment

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

I got it, on rector side, using ->createForNewestSupportedVersion() needs to be on default, then if it catch PHPStan\Parser\ParserErrorsException, fallback to use php 7 parser when doing parsing, see rectorphp/rector-src@3bb03c6

If parsing process doesn't involve phpstan, you may need to use PhpParser\Error for catch and retry with createForVersion(PhpVersion::fromString('7.0')), something like:

 try {
            // parse with newest supported version
        } catch (\PhpParser\Error) {
            // parse with php 7 parser
        }

}

public function parseClassReflection(ClassReflection $classReflection): ?ClassLike
Expand Down
4 changes: 0 additions & 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
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();
}
}
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
3 changes: 1 addition & 2 deletions src/Rules/ReturnNullOverFalseRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\BooleanType;
use PHPStan\Type\Constant\ConstantBooleanType;
use Rector\TypePerfect\Configuration;

Expand Down Expand Up @@ -75,7 +74,7 @@ public function processNode(Node $node, Scope $scope): array

$exprType = $scope->getType($return->expr);
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
Loading