remove unmatched errors + require string in object types to avoid pre…

…fixing of generics objects (#5713)
rectorphp · Mar 1, 2021 · da5f4a0 · da5f4a0
1 parent b14ba76
commit da5f4a0
Show file tree

Hide file tree

Showing 23 changed files with 148 additions and 121 deletions.
diff --git a/packages/node-name-resolver/src/NodeNameResolver.php b/packages/node-name-resolver/src/NodeNameResolver.php
@@ -206,10 +206,18 @@ public function isLocalPropertyFetchNamed(Node $node, string $name): bool
             return false;
         }
 
+        if ($node->var instanceof MethodCall) {
+            return false;
+        }
+
         if (! $this->isName($node->var, 'this')) {
             return false;
         }
 
+        if ($node->name instanceof Expr) {
+            return false;
+        }
+
         return $this->isName($node->name, $name);
     }
 

diff --git a/packages/node-type-resolver/src/TypeAnalyzer/CountableTypeAnalyzer.php b/packages/node-type-resolver/src/TypeAnalyzer/CountableTypeAnalyzer.php
@@ -4,7 +4,6 @@
 
 namespace Rector\NodeTypeResolver\TypeAnalyzer;
 
-use Countable;
 use PhpParser\Node;
 use PHPStan\Type\NullType;
 use PHPStan\Type\ObjectType;
@@ -55,7 +54,7 @@ public function isCountableType(Node $node): bool
     private function isCountableObjectType(Type $type): bool
     {
         $countableObjectTypes = [
-            new ObjectType(Countable::class),
+            new ObjectType('Countable'),
             new ObjectType('SimpleXMLElement'),
             new ObjectType('ResourceBundle'),
         ];

diff --git a/packages/read-write/src/ReadNodeAnalyzer/PropertyFetchReadNodeAnalyzer.php b/packages/read-write/src/ReadNodeAnalyzer/PropertyFetchReadNodeAnalyzer.php
@@ -7,9 +7,26 @@
 use PhpParser\Node;
 use PhpParser\Node\Expr\PropertyFetch;
 use Rector\ReadWrite\Contract\ReadNodeAnalyzerInterface;
+use Rector\ReadWrite\NodeFinder\NodeUsageFinder;
 
-final class PropertyFetchReadNodeAnalyzer extends AbstractReadNodeAnalyzer implements ReadNodeAnalyzerInterface
+final class PropertyFetchReadNodeAnalyzer implements ReadNodeAnalyzerInterface
 {
+    /**
+     * @var ReadExprAnalyzer
+     */
+    private $readExprAnalyzer;
+
+    /**
+     * @var NodeUsageFinder
+     */
+    private $nodeUsageFinder;
+
+    public function __construct(ReadExprAnalyzer $readExprAnalyzer, NodeUsageFinder $nodeUsageFinder)
+    {
+        $this->readExprAnalyzer = $readExprAnalyzer;
+        $this->nodeUsageFinder = $nodeUsageFinder;
+    }
+
     public function supports(Node $node): bool
     {
         return $node instanceof PropertyFetch;
@@ -22,7 +39,7 @@ public function isRead(Node $node): bool
     {
         $propertyFetchUsages = $this->nodeUsageFinder->findPropertyFetchUsages($node);
         foreach ($propertyFetchUsages as $propertyFetchUsage) {
-            if ($this->isCurrentContextRead($propertyFetchUsage)) {
+            if ($this->readExprAnalyzer->isReadContext($propertyFetchUsage)) {
                 return true;
             }
         }

diff --git a/...NodeAnalyzer/AbstractReadNodeAnalyzer.php → ...src/ReadNodeAnalyzer/ReadExprAnalyzer.php b/...NodeAnalyzer/AbstractReadNodeAnalyzer.php → ...src/ReadNodeAnalyzer/ReadExprAnalyzer.php
@@ -10,34 +10,11 @@
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
-use Rector\NodeNestingScope\ParentScopeFinder;
 use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\ReadWrite\NodeFinder\NodeUsageFinder;
 
-abstract class AbstractReadNodeAnalyzer
+final class ReadExprAnalyzer
 {
-    /**
-     * @var ParentScopeFinder
-     */
-    protected $parentScopeFinder;
-
-    /**
-     * @var NodeUsageFinder
-     */
-    protected $nodeUsageFinder;
-
-    /**
-     * @required
-     */
-    public function autowireAbstractReadNodeAnalyzer(
-        ParentScopeFinder $parentScopeFinder,
-        NodeUsageFinder $nodeUsageFinder
-    ): void {
-        $this->parentScopeFinder = $parentScopeFinder;
-        $this->nodeUsageFinder = $nodeUsageFinder;
-    }
-
-    protected function isCurrentContextRead(Expr $expr): bool
+    public function isReadContext(Expr $expr): bool
     {
         $parent = $expr->getAttribute(AttributeKey::PARENT_NODE);
         if ($parent instanceof Return_) {

diff --git a/packages/read-write/src/ReadNodeAnalyzer/VariableReadNodeAnalyzer.php b/packages/read-write/src/ReadNodeAnalyzer/VariableReadNodeAnalyzer.php
@@ -6,10 +6,37 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\Variable;
+use Rector\NodeNestingScope\ParentScopeFinder;
 use Rector\ReadWrite\Contract\ReadNodeAnalyzerInterface;
+use Rector\ReadWrite\NodeFinder\NodeUsageFinder;
 
-final class VariableReadNodeAnalyzer extends AbstractReadNodeAnalyzer implements ReadNodeAnalyzerInterface
+final class VariableReadNodeAnalyzer implements ReadNodeAnalyzerInterface
 {
+    /**
+     * @var ParentScopeFinder
+     */
+    private $parentScopeFinder;
+
+    /**
+     * @var NodeUsageFinder
+     */
+    private $nodeUsageFinder;
+
+    /**
+     * @var ReadExprAnalyzer
+     */
+    private $readExprAnalyzer;
+
+    public function __construct(
+        ParentScopeFinder $parentScopeFinder,
+        NodeUsageFinder $nodeUsageFinder,
+        ReadExprAnalyzer $readExprAnalyzer
+    ) {
+        $this->parentScopeFinder = $parentScopeFinder;
+        $this->nodeUsageFinder = $nodeUsageFinder;
+        $this->readExprAnalyzer = $readExprAnalyzer;
+    }
+
     public function supports(Node $node): bool
     {
         return $node instanceof Variable;
@@ -27,7 +54,7 @@ public function isRead(Node $node): bool
 
         $variableUsages = $this->nodeUsageFinder->findVariableUsages((array) $parentScope->stmts, $node);
         foreach ($variableUsages as $variableUsage) {
-            if ($this->isCurrentContextRead($variableUsage)) {
+            if ($this->readExprAnalyzer->isReadContext($variableUsage)) {
                 return true;
             }
         }

diff --git a/phpstan.neon b/phpstan.neon
@@ -99,8 +99,6 @@ parameters:
         # broken
         - '#Parameter \#2 \$name of method Rector\\NodeNameResolver\\NodeNameResolver\:\:isName\(\) expects string, string\|null given#'
 
-        - '#Parameter \#1 \$funcCall of method Rector\\Php80\\MatchAndRefactor\\StrStartsWithMatchAndRefactor\\AbstractMatchAndRefactor\:\:createStrStartsWithValueObjectFromFuncCall\(\) expects PhpParser\\Node\\Expr\\FuncCall, PhpParser\\Node\\Expr given#'
-
         # mostly strings in tests
         - '#Method Rector\\Naming\\Naming\\PropertyNaming\:\:resolveShortClassName\(\) should return string but returns string\|null#'
 
@@ -232,7 +230,6 @@ parameters:
                 - packages/better-php-doc-parser/src/PhpDocInfo/PhpDocInfo.php # 108
                 - rules/coding-style/src/Rector/ClassMethod/YieldClassMethodToArrayClassMethodRector.php # 47
                 - rules/php70/src/EregToPcreTransformer.php # 66
-                - rules/type-declaration/src/Rector/FunctionLike/ReturnTypeDeclarationRector.php # 82
 
         # trait in trait call
         - '#Parameter \#1 \$expr of class PhpParser\\Node\\Stmt\\Expression constructor expects PhpParser\\Node\\Expr, PhpParser\\Node\\Expr\|PhpParser\\Node\\Stmt given#'
@@ -437,8 +434,6 @@ parameters:
             paths:
                 - rules/removing-static/tests/Rector/Property/DesiredPropertyClassMethodTypeToDynamicRector/config/some_config.php
 
-        - '#Content of method "configure\(\)" is duplicated with method "configure\(\)" in "Rector\\Composer\\Rector\\AddPackageToRequireComposerRector" class\. Use unique content or abstract service instead#'
-
         # buggy phpstan clas-string
         - '#Method (.*?) should return class\-string but returns string#'
 
@@ -504,20 +499,13 @@ parameters:
         # known values
         - '#Method Rector\\Testing\\Finder\\RectorsFinder\:\:findClassesInDirectoriesByName\(\) should return array<class\-string\> but returns array<int, \(int\|string\)\>#'
         - '#Property PhpParser\\Node\\Param\:\:\$type \(PhpParser\\Node\\Identifier\|PhpParser\\Node\\Name\|PhpParser\\Node\\NullableType\|PhpParser\\Node\\UnionType\|null\) does not accept PhpParser\\Node#'
-        - '#Content of method "getFunctionLikePhpDocInfo\(\)" is duplicated with method "getFunctionLikePhpDocInfo\(\)" in "Rector\\TypeDeclaration\\TypeInferer\\ParamTypeInferer\\PHPUnitDataProviderParamTypeInferer" class\. Use unique content or abstract service instead#'
 
         -
             message: '#"%s" in sprintf\(\) format must be quoted#'
             paths:
                 - packages/attribute-aware-php-doc/src/Ast/PhpDoc/AttributeAwareParamTagValueNode.php
 
-        # @todo bug, solve in Symplify
-        -
-             message: '#Constant type should be "class\-string", but is "string"#'
-             path: rules/symfony/src/Rector/BinaryOp/ResponseStatusCodeRector.php
-
         - '#Property Rector\\Core\\PhpParser\\Node\\AssignAndBinaryMap\:\:\$binaryOpToAssignClasses \(array<class\-string<PhpParser\\Node\\Expr\\BinaryOp\>, class\-string<PhpParser\\Node\\Expr\\BinaryOp\>\>\) does not accept array#'
-        - '#Content of method "configure\(\)" is duplicated with method "configure\(\)" in "Rector\\RemovingStatic\\Rector\\Class_\\NewUniqueObjectToEntityFactoryRector" class\. Use unique content or abstract service instead#'
         - '#Content of method "configure\(\)" is duplicated with method "configure\(\)" in "Rector\\RemovingStatic\\Rector\\Class_\\PassFactoryToUniqueObjectRector" class\. Use unique content or abstract service instead#'
 
         -
@@ -558,9 +546,6 @@ parameters:
 
         - '#Method Rector\\TypeDeclaration\\PhpParserTypeAnalyzer\:\:unwrapNullableAndToString\(\) should return string but returns string\|null#'
 
-        # @todo resolve in next PR!!! (Feb 2021)
-        - '#Function "array_filter\(\)" cannot be used/left in the code#'
-        - '#Cognitive complexity for "Rector\\CodeQuality\\Rector\\Isset_\\IssetOnPropertyObjectToPropertyExistsRector\:\:refactor\(\)" is 11, keep it under 9#'
         - '#Cognitive complexity for "Rector\\DeadCode\\UnusedNodeResolver\\ClassUnusedPrivateClassMethodResolver\:\:filterOutParentAbstractMethods\(\)" is 10, keep it under 9#'
 
         # known types
@@ -576,11 +561,6 @@ parameters:
             paths:
                 - rules/restoration/src/ClassMap/ExistingClassesProvider.php
 
-        # false positives
-        -
-             message: '#Empty array passed to foreach#'
-             paths:
-                 - rules/transform/src/Rector/New_/NewToConstructorInjectionRector.php
         -
             message: '#Parameter \#1 \$types of method Rector\\Defluent\\NodeAnalyzer\\FluentCallStaticTypeResolver\:\:filterOutAlreadyPresentParentClasses\(\) expects array<class\-string\>, array<int, string\> given#'
             paths:
@@ -594,15 +574,28 @@ parameters:
             paths:
                 - packages/read-write/src/Guard/VariableToConstantGuard.php
 
-        - '#Cognitive complexity for "Rector\\NetteCodeQuality\\FormControlTypeResolver\\MagicNetteFactoryInterfaceFormControlTypeResolver\:\:resolve\(\)" is 11, keep it under 9#'
         - '#Cognitive complexity for "Rector\\NetteCodeQuality\\FormControlTypeResolver\\MagicNetteFactoryInterfaceFormControlTypeResolver\:\:resolve\(\)" is 12, keep it under 9#'
         - '#Content of method "matchAssignExprToPropertyName\(\)" is duplicated with method "matchAssignExprToPropertyName\(\)" in "Rector\\TypeDeclaration\\AlreadyAssignDetector\\ConstructorAssignDetector" class\. Use unique content or abstract service instead#'
+
         # make more tolerand, e.g. at least 2-3 lines
-        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\AbstractPropertyConflictingNameGuard" class\. Use unique content or abstract service instead#'
-        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\MatchPropertyTypeConflictingNameGuard" class\. Use unique content or abstract service instead#'
-        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\UnderscoreCamelCaseConflictingNameGuard" class\. Use unique content or abstract service instead#'
         - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\UnderscoreCamelCaseConflictingNameGuard" class\. Use unique content or abstract service instead#'
-        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\UnderscoreCamelCaseConflictingNameGuard" class\. Use unique content or abstract service instead#'
-        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\BoolPropertyConflictingNameGuard" class\. Use unique content or abstract service instead#'
-        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\BoolPropertyConflictingNameGuard" class\. Use unique content or abstract service instead#'
 
+
+        -
+            message: '#Offset 0 does not exist on array<PhpParser\\Node\\Stmt\>\|null#'
+            paths:
+                - rules/naming/src/Naming/PropertyNaming.php
+
+        -
+            message: '#Parameter \#1 \$value of function count expects array\|Countable, array<PhpParser\\Node\\Stmt\>\|null given#'
+            paths:
+                -  rules/naming/src/Naming/PropertyNaming.php
+
+        -
+            message: '#Use quoted string in constructor "new PHPStan\\Type\\ObjectType\(\)" argument on position 0 instead of "\:\:class\. It prevent scoping of the class in building prefixed package#'
+            paths:
+                - packages/node-type-resolver/tests
+
+        - '#Content of method "configure\(\)" is duplicated with method "configure\(\)" in "Rector\\Composer\\Rector\\AddPackageToRequireComposerRector" class\. Use unique content or abstract service instead#'
+        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\MatchPropertyTypeConflictingNameGuard" class\. Use unique content or abstract service instead#'
+        - '#Content of method "isConflicting\(\)" is duplicated with method "isConflicting\(\)" in "Rector\\Naming\\Guard\\PropertyConflictingNameGuard\\BoolPropertyConflictingNameGuard" class\. Use unique content or abstract service instead#'
diff --git a/...s/doctrine-code-quality/src/Rector/Class_/MoveRepositoryFromParentToConstructorRector.php b/...s/doctrine-code-quality/src/Rector/Class_/MoveRepositoryFromParentToConstructorRector.php
@@ -4,11 +4,8 @@
 
 namespace Rector\DoctrineCodeQuality\Rector\Class_;
 
-use Doctrine\ORM\EntityManagerInterface;
-use Doctrine\ORM\EntityRepository;
 use Nette\Utils\Strings;
 use PhpParser\Node;
-use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Stmt\Class_;
 use PHPStan\Type\ObjectType;
 use Rector\Core\NodeManipulator\ClassDependencyManipulator;
@@ -105,7 +102,7 @@ public function refactor(Node $node): ?Node
         }
 
         $parentClassName = $node->getAttribute(AttributeKey::PARENT_CLASS_NAME);
-        if ($parentClassName !== EntityRepository::class) {
+        if ($parentClassName !== 'Doctrine\ORM\EntityRepository') {
             return null;
         }
 
@@ -123,15 +120,19 @@ public function refactor(Node $node): ?Node
         $node->extends = null;
 
         // add $repository property
-        $this->classInsertManipulator->addPropertyToClass($node, 'repository', new ObjectType(EntityRepository::class));
+        $this->classInsertManipulator->addPropertyToClass(
+            $node,
+            'repository',
+            new ObjectType('Doctrine\ORM\EntityRepository')
+        );
 
         // add $entityManager and assign to constuctor
         $repositoryAssign = $this->repositoryAssignFactory->create($node);
 
         $this->classDependencyManipulator->addConstructorDependencyWithCustomAssign(
             $node,
             'entityManager',
-            new ObjectType(EntityManagerInterface::class),
+            new ObjectType('Doctrine\ORM\EntityManagerInterface'),
             $repositoryAssign
         );
 

diff --git a/...doctrine-code-quality/tests/Rector/DoctrineRepositoryAsService/Fixture/PostController.php b/...doctrine-code-quality/tests/Rector/DoctrineRepositoryAsService/Fixture/PostController.php
@@ -24,8 +24,7 @@ public function anythingAction(int $id): Response
 namespace Rector\DoctrineCodeQuality\Tests\Rector\DoctrineRepositoryAsService\Fixture;
 
 use Rector\DoctrineCodeQuality\Tests\Rector\DoctrineRepositoryAsService\Source\Entity\Post;
-use Rector\Core\Tests\Rector\Architecture\DoctrineRepositoryAsService\Source\SymfonyController;
-use Symfony\Component\HttpFoundation\Response;
+use Rector\Core\Tests\Rector\Architecture\DoctrineRepositoryAsService\Source\SymfonyController;use Symfony\Component\HttpFoundation\Response;
 
 final class PostController extends SymfonyController
 {

diff --git a/rules/downgrade-php70/src/Rector/FunctionLike/AbstractDowngradeReturnDeclarationRector.php b/rules/downgrade-php70/src/Rector/FunctionLike/AbstractDowngradeReturnDeclarationRector.php
@@ -15,7 +15,6 @@
 use Rector\BetterPhpDocParser\PhpDocManipulator\PhpDocTypeChanger;
 use Rector\Core\Rector\AbstractRector;
 use Rector\DowngradePhp70\Contract\Rector\DowngradeReturnDeclarationRectorInterface;
-use Traversable;
 
 abstract class AbstractDowngradeReturnDeclarationRector extends AbstractRector implements DowngradeReturnDeclarationRectorInterface
 {
@@ -68,7 +67,7 @@ private function decorateFunctionLikeWithReturnTagValueNode(FunctionLike $functi
 
         $type = $this->staticTypeMapper->mapPhpParserNodePHPStanType($functionLike->returnType);
         if ($type instanceof IterableType) {
-            $type = new UnionType([$type, new IntersectionType([new ObjectType(Traversable::class)])]);
+            $type = new UnionType([$type, new IntersectionType([new ObjectType('Traversable')])]);
         }
 
         $this->phpDocTypeChanger->changeReturnType($phpDocInfo, $type);

diff --git a/rules/naming/src/Naming/PropertyNaming.php b/rules/naming/src/Naming/PropertyNaming.php
@@ -330,16 +330,11 @@ private function filterClassMethodsWithPropertyFetchReturnOnly(
         $currentName = $this->nodeNameResolver->getName($property);
 
         return array_filter($prefixedClassMethods, function (ClassMethod $classMethod) use ($currentName): bool {
-            $stmts = $classMethod->stmts;
-            if ($stmts === null) {
+            if ((array) $classMethod->stmts === []) {
                 return false;
             }
 
-            if (! array_key_exists(0, $stmts)) {
-                return false;
-            }
-
-            $return = $stmts[0];
+            $return = $classMethod->stmts[0];
             if (! $return instanceof Return_) {
                 return false;
             }

diff --git a/rules/naming/src/PropertyRenamer/BoolPropertyRenamer.php b/rules/naming/src/PropertyRenamer/BoolPropertyRenamer.php
@@ -8,16 +8,24 @@
 use Rector\Naming\Guard\PropertyConflictingNameGuard\BoolPropertyConflictingNameGuard;
 use Rector\Naming\ValueObject\PropertyRename;
 
-final class BoolPropertyRenamer extends AbstractPropertyRenamer
+final class BoolPropertyRenamer
 {
     /**
      * @var BoolPropertyConflictingNameGuard
      */
     private $boolPropertyConflictingNameGuard;
 
-    public function __construct(BoolPropertyConflictingNameGuard $boolPropertyConflictingNameGuard)
-    {
+    /**
+     * @var PropertyRenamer
+     */
+    private $propertyRenamer;
+
+    public function __construct(
+        BoolPropertyConflictingNameGuard $boolPropertyConflictingNameGuard,
+        PropertyRenamer $propertyRenamer
+    ) {
         $this->boolPropertyConflictingNameGuard = $boolPropertyConflictingNameGuard;
+        $this->propertyRenamer = $propertyRenamer;
     }
 
     public function rename(PropertyRename $propertyRename): ?Property
@@ -26,6 +34,6 @@ public function rename(PropertyRename $propertyRename): ?Property
             return null;
         }
 
-        return parent::rename($propertyRename);
+        return $this->propertyRenamer->rename($propertyRename);
     }
 }