Skip to content

Commit

Permalink
Clean PHPStan errors (#2475)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomasVotruba committed Jun 10, 2022
1 parent a6fb826 commit 797cb38
Show file tree
Hide file tree
Showing 18 changed files with 204 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ protected function setUp(): void
* @param class-string<T> $type
* @return T[]
*/
protected function getNodesForFileOfType(string $file, string $type): array
protected function getNodesForFileOfType(string $filepath, string $type): array
{
$nodes = $this->testingParser->parseFileToDecoratedNodes($file);
$nodes = $this->testingParser->parseFileToDecoratedNodes($filepath);
return $this->betterNodeFinder->findInstanceOf($nodes, $type);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use PHPStan\PhpDocParser\Ast\Type\IdentifierTypeNode;
use PHPStan\PhpDocParser\Ast\Type\TypeNode;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use Rector\BetterPhpDocParser\ValueObject\PhpDocAttributeKey;
use Rector\Core\Configuration\CurrentNodeProvider;
use Rector\Core\Exception\ShouldNotHappenException;
Expand Down Expand Up @@ -82,12 +83,10 @@ public function enterNode(Node $node): ?Node
}

// make sure to compare FQNs
if ($staticType instanceof ShortenedObjectType) {
$staticType = new ObjectType($staticType->getFullyQualifiedName());
}
$objectType = $this->expandShortenedObjectType($staticType);

foreach ($this->oldToNewTypes as $oldToNewType) {
if (! $staticType->equals($oldToNewType->getOldType())) {
if (! $objectType->equals($oldToNewType->getOldType())) {
continue;
}

Expand Down Expand Up @@ -169,4 +168,13 @@ private function resolveNamefromUse(array $uses, string $name): string

return $name;
}

private function expandShortenedObjectType(Type $type): ObjectType|Type
{
if ($type instanceof ShortenedObjectType) {
return new ObjectType($type->getFullyQualifiedName());
}

return $type;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,7 @@ public function mapToPHPStanPhpDocTypeNode(Type $type, string $typeKind): TypeNo
$attributeAwareIdentifierTypeNode = new IdentifierTypeNode('class-string');

if ($type instanceof GenericClassStringType) {
$genericType = $type->getGenericType();
if ($genericType instanceof ObjectType) {
$className = $genericType->getClassName();
$className = $this->normalizeType($className);
$genericType = new ObjectType($className);
}
$genericType = $this->resolveGenericObjectType($type);

$genericTypeNode = $this->phpStanStaticTypeMapper->mapToPHPStanPhpDocTypeNode($genericType, $typeKind);
return new GenericTypeNode($attributeAwareIdentifierTypeNode, [$genericTypeNode]);
Expand Down Expand Up @@ -81,4 +76,17 @@ private function normalizeType(string $classType): string

return $classType;
}

private function resolveGenericObjectType(GenericClassStringType $genericClassStringType): ObjectType|Type
{
$genericType = $genericClassStringType->getGenericType();

if (! $genericType instanceof ObjectType) {
return $genericType;
}

$className = $genericType->getClassName();
$className = $this->normalizeType($className);
return new ObjectType($className);
}
}
23 changes: 15 additions & 8 deletions packages/StaticTypeMapper/Mapper/PhpParserNodeMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,33 @@ public function __construct(

public function mapToPHPStanType(Node $node): Type
{
if ($node::class === Name::class && $node->hasAttribute(AttributeKey::NAMESPACED_NAME)) {
$node = new FullyQualified($node->getAttribute(AttributeKey::NAMESPACED_NAME));
}
$nameOrExpr = $this->expandedNamespacedName($node);

foreach ($this->phpParserNodeMappers as $phpParserNodeMapper) {
if (! is_a($node, $phpParserNodeMapper->getNodeType())) {
if (! is_a($nameOrExpr, $phpParserNodeMapper->getNodeType())) {
continue;
}

// do not let Expr collect all the types
// note: can be solve later with priorities on mapper interface, making this last
if ($phpParserNodeMapper->getNodeType() !== Expr::class) {
return $phpParserNodeMapper->mapToPHPStan($node);
return $phpParserNodeMapper->mapToPHPStan($nameOrExpr);
}

if (! $node instanceof String_) {
return $phpParserNodeMapper->mapToPHPStan($node);
if (! $nameOrExpr instanceof String_) {
return $phpParserNodeMapper->mapToPHPStan($nameOrExpr);
}
}

throw new NotImplementedYetException($node::class);
throw new NotImplementedYetException($nameOrExpr::class);
}

private function expandedNamespacedName(Node $node): Node|FullyQualified
{
if ($node::class === Name::class && $node->hasAttribute(AttributeKey::NAMESPACED_NAME)) {
return new FullyQualified($node->getAttribute(AttributeKey::NAMESPACED_NAME));
}

return $node;
}
}
8 changes: 4 additions & 4 deletions packages/Testing/TestingParser/TestingParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ public function parseFilePathToFile(string $filePath): File
/**
* @return Node[]
*/
public function parseFileToDecoratedNodes(string $file): array
public function parseFileToDecoratedNodes(string $filepath): array
{
// autoload file
require_once $file;
require_once $filepath;

$smartFileInfo = new SmartFileInfo($file);
$this->parameterProvider->changeParameter(Option::SOURCE, [$file]);
$smartFileInfo = new SmartFileInfo($filepath);
$this->parameterProvider->changeParameter(Option::SOURCE, [$filepath]);

$nodes = $this->rectorParser->parseFile($smartFileInfo);

Expand Down
53 changes: 52 additions & 1 deletion phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -731,4 +731,55 @@ parameters:
message: '#@\\ini_set\(.*\)" is forbidden to use#'
path: bin/rector.php

- '#New objects with "\$.*" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
# should be fixed in symplify next
-
message: '#New objects with "\$arrayItem" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: src/PhpParser/Node/NodeFactory.php

# reducing original node
-
message: '#New objects with "\$bitwiseOr" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: rules/DowngradePhp72/NodeManipulator/BitwiseFlagCleaner.php

# faking node to invoke scope callable on attribute
-
message: '#New objects with "\$node" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: src/Application/ChangedNodeScopeRefresher.php

# added nullable type
-
message: '#New objects with "\$mappedCurrentParamType" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: rules/DowngradePhp72/PhpDoc/NativeParamToPhpDocDecorator.php

-
message: '#New objects with "\$node" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: src/Rector/AbstractRector.php

# joined items
-
message: '#New objects with "\$expr" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: rules/DowngradePhp55/Rector/Isset_/DowngradeArbitraryExpressionArgsToEmptyAndIssetRector.php

# mixed type correction
-
message: '#New objects with "\$(first|second)KeyType" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: packages/NodeTypeResolver/TypeComparator/TypeComparator.php

-
message: '#New objects with "\$staticType" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: packages/NodeTypeResolver/PhpDocNodeVisitor/NameImportingPhpDocNodeVisitor.php

-
message: '#New objects with "\$replaceIntoType" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: rules/CodeQuality/NodeManipulator/ClassMethodReturnTypeManipulator.php

-
path: rules/DowngradePhp74/Rector/ClassMethod/DowngradeCovariantReturnTypeRector.php
message: '#New objects with "\$parentReturnTypeNode" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
-
path: rules/DowngradePhp80/NodeAnalyzer/UnnamedArgumentResolver.php
message: '#New objects with "\$functionLikeReflection" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'

-
message: '#New objects with "\$fullyQualifiedObjectType" name are overridden\. This can lead to unwanted bugs, please pick a different name to avoid it#'
path: packages/NodeTypeResolver/PhpDocNodeVisitor/NameImportingPhpDocNodeVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,13 @@
use PhpParser\Node\Param;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Expression;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\UnionType;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
use Rector\Core\Exception\ShouldNotHappenException;
use Rector\Core\NodeAnalyzer\ParamAnalyzer;
use Rector\DowngradePhp72\UnionTypeFactory;
use Rector\NodeNameResolver\NodeNameResolver;
use Rector\NodeTypeResolver\Node\AttributeKey;
use Rector\NodeTypeResolver\NodeTypeResolver;
Expand All @@ -36,7 +35,8 @@ public function __construct(
private readonly NodeTypeResolver $nodeTypeResolver,
private readonly ParamAnalyzer $paramAnalyzer,
private readonly NodeNameResolver $nodeNameResolver,
private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser
private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
private readonly UnionTypeFactory $unionTypeFactory,
) {
}

Expand Down Expand Up @@ -69,10 +69,10 @@ public function refactorFunctionParameters(
private function refactorParamTypeHint(Param $param, Identifier|Name|NullableType $replaceIntoType): void
{
if ($this->paramAnalyzer->isNullable($param) && ! $replaceIntoType instanceof NullableType) {
$replaceIntoType = new NullableType($replaceIntoType);
$param->type = new NullableType($replaceIntoType);
} else {
$param->type = $replaceIntoType;
}

$param->type = $replaceIntoType;
}

private function refactorParamDocBlock(Param $param, ClassMethod $classMethod, Type $phpDocType): void
Expand All @@ -83,12 +83,7 @@ private function refactorParamDocBlock(Param $param, ClassMethod $classMethod, T
}

if ($this->paramAnalyzer->isNullable($param)) {
if ($phpDocType instanceof UnionType) {
// Adding a UnionType into a new UnionType throws an exception so we need to "unpack" the types
$phpDocType = new UnionType([...$phpDocType->getTypes(), new NullType()]);
} else {
$phpDocType = new UnionType([$phpDocType, new NullType()]);
}
$phpDocType = $this->unionTypeFactory->createNullableUnionType($phpDocType);
}

$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,25 @@

namespace Rector\CodeQuality\NodeManipulator;

use PhpParser\Node;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\NullableType;
use PhpParser\Node\Stmt\ClassMethod;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\UnionType;
use Rector\BetterPhpDocParser\PhpDocInfo\PhpDocInfoFactory;
use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
use Rector\DowngradePhp72\UnionTypeFactory;
use Rector\NodeTypeResolver\NodeTypeResolver;

final class ClassMethodReturnTypeManipulator
{
public function __construct(
private readonly PhpDocInfoFactory $phpDocInfoFactory,
private readonly PhpDocTypeChanger $phpDocTypeChanger,
private readonly NodeTypeResolver $nodeTypeResolver
private readonly NodeTypeResolver $nodeTypeResolver,
private readonly UnionTypeFactory $unionTypeFactory,
) {
}

Expand All @@ -32,7 +33,7 @@ public function refactorFunctionReturnType(
Type $phpDocType
): ?ClassMethod {
$returnType = $classMethod->returnType;
if ($returnType === null) {
if (! $returnType instanceof Node) {
return null;
}

Expand All @@ -52,12 +53,7 @@ public function refactorFunctionReturnType(
}

if ($isNullable) {
if ($phpDocType instanceof UnionType) {
// Adding a UnionType into a new UnionType throws an exception so we need to "unpack" the types
$phpDocType = new UnionType([...$phpDocType->getTypes(), new NullType()]);
} else {
$phpDocType = new UnionType([$phpDocType, new NullType()]);
}
$phpDocType = $this->unionTypeFactory->createNullableUnionType($phpDocType);

if (! $replaceIntoType instanceof NullableType) {
$replaceIntoType = new NullableType($replaceIntoType);
Expand Down
8 changes: 4 additions & 4 deletions rules/CodeQuality/Rector/If_/ExplicitBoolCompareRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,9 @@ private function resolveArray(bool $isNegated, Expr $expr): ?BinaryOp

private function resolveString(bool $isNegated, Expr $expr): Identical | NotIdentical | BooleanAnd | BooleanOr
{
$string = new String_('');
$emptyString = new String_('');

$identical = $this->resolveIdentical($expr, $isNegated, $string);
$identical = $this->resolveIdentical($expr, $isNegated, $emptyString);

$value = $this->valueResolver->getValue($expr);

Expand All @@ -220,8 +220,8 @@ private function resolveString(bool $isNegated, Expr $expr): Identical | NotIden
$length = strlen((string) $value);

if ($length === 1) {
$string = new String_('0');
return $this->resolveIdentical($expr, $isNegated, $string);
$zeroString = new String_('0');
return $this->resolveIdentical($expr, $isNegated, $zeroString);
}

return $identical;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ private function removeAndReturn(Return_ $return, Expression $expression, Expr $
$this->removeNode($return);
$this->removeNode($expression);

$return = new Return_($expr);
$exprReturn = new Return_($expr);
$this->varTagRemover->removeVarPhpTagValueNodeIfNotComment($expression, $unionType);
$this->mirrorComments($return, $expression);
$this->mirrorComments($exprReturn, $expression);

return $return;
return $exprReturn;
}

private function shouldSkip(If_ $if): bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Stmt\ClassMethod;
use PHPStan\Type\ObjectType;
use Rector\CodingStyle\Enum\PreferenceSelfThis;
Expand Down Expand Up @@ -161,11 +162,11 @@ private function processToThis(MethodCall | StaticCall $node): ?MethodCall
// avoid adding dynamic method call to static method
$classMethod = $this->betterNodeFinder->findParentByTypes($node, [ClassMethod::class]);
if (! $classMethod instanceof ClassMethod) {
return $this->nodeFactory->createMethodCall(self::THIS, $name, $node->args);
return $this->nodeFactory->createMethodCall(new Variable(self::THIS), $name, $node->args);
}

if (! $classMethod->isStatic()) {
return $this->nodeFactory->createMethodCall(self::THIS, $name, $node->args);
return $this->nodeFactory->createMethodCall(new Variable(self::THIS), $name, $node->args);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ private function refactorIsset(Isset_ $isset): Expr
private function joinWithBooleanAnd(array $exprs): Expr
{
$expr = $exprs[0];
$nbExprs = count($exprs);
for ($i = 1; $i < $nbExprs; ++$i) {

$exprCount = count($exprs);
for ($i = 1; $i < $exprCount; ++$i) {
$expr = new BooleanAnd($expr, $exprs[$i]);
}

Expand Down

0 comments on commit 797cb38

Please sign in to comment.