[Naming] Remove matchesStringName() check completely from NodeNameRes…

…olver, including endsWith() method - use getName() and compare directly instead (#4954) Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Sep 9, 2023 · c97e4ff · c97e4ff
1 parent 416e52a
commit c97e4ff
Show file tree

Hide file tree

Showing 18 changed files with 72 additions and 113 deletions.
diff --git a/packages/NodeNameResolver/NodeNameResolver.php b/packages/NodeNameResolver/NodeNameResolver.php
@@ -15,9 +15,7 @@
 use Rector\CodingStyle\Naming\ClassNaming;
 use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\NodeAnalyzer\CallAnalyzer;
-use Rector\Core\Util\StringUtils;
 use Rector\NodeNameResolver\Contract\NodeNameResolverInterface;
-use Rector\NodeNameResolver\Regex\RegexPatternDetector;
 use Rector\NodeTypeResolver\Node\AttributeKey;
 
 final class NodeNameResolver
@@ -31,7 +29,6 @@ final class NodeNameResolver
      * @param NodeNameResolverInterface[] $nodeNameResolvers
      */
     public function __construct(
-        private readonly RegexPatternDetector $regexPatternDetector,
         private readonly ClassNaming $classNaming,
         private readonly CallAnalyzer $callAnalyzer,
         private readonly iterable $nodeNameResolvers = []
@@ -168,26 +165,6 @@ public function getNames(array $nodes): array
         return $names;
     }
 
-    /**
-     * Ends with ucname
-     * Starts with adjective, e.g. (Post $firstPost, Post $secondPost)
-     */
-    public function endsWith(string $currentName, string $expectedName): bool
-    {
-        $suffixNamePattern = '#\w+' . ucfirst($expectedName) . '#';
-        return StringUtils::isMatch($currentName, $suffixNamePattern);
-    }
-
-    public function startsWith(Node $node, string $prefix): bool
-    {
-        $name = $this->getName($node);
-        if (! is_string($name)) {
-            return false;
-        }
-
-        return str_starts_with($name, $prefix);
-    }
-
     public function getShortName(string | Name | Identifier | ClassLike $name): string
     {
         return $this->classNaming->getShortName($name);
@@ -207,21 +184,6 @@ public function isStringName(string $resolvedName, string $desiredName): bool
         return strcasecmp($resolvedName, $desiredName) === 0;
     }
 
-    public function matchesStringName(string|Identifier $resolvedName, string $desiredNamePattern): bool
-    {
-        if ($resolvedName instanceof Identifier) {
-            $resolvedName = $resolvedName->toString();
-        }
-
-        // is probably regex pattern
-        if ($this->regexPatternDetector->isRegexPattern($desiredNamePattern)) {
-            return StringUtils::isMatch($resolvedName, $desiredNamePattern);
-        }
-
-        // is probably fnmatch
-        return fnmatch($desiredNamePattern, $resolvedName, FNM_NOESCAPE);
-    }
-
     private function isCallOrIdentifier(Expr|Identifier $node): bool
     {
         if ($node instanceof Expr) {

diff --git a/...Naming/Rector/Class_/RenamePropertyToMatchTypeRector/Fixture/change_bare_property.php.inc b/...Naming/Rector/Class_/RenamePropertyToMatchTypeRector/Fixture/change_bare_property.php.inc
diff --git a/...ng/Rector/Class_/RenamePropertyToMatchTypeRector/Fixture/keep_underscore_property.php.inc b/...ng/Rector/Class_/RenamePropertyToMatchTypeRector/Fixture/keep_underscore_property.php.inc
@@ -0,0 +1,8 @@
+<?php
+
+namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixture;
+
+final class KeepUnderscoreProperty
+{
+    private ?\PDO $_pdo;
+}
diff --git a/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php b/rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php
@@ -154,30 +154,30 @@ private function shouldSkipIfAndReturn(If_ $if, Return_ $return): bool
         return ! $if->cond instanceof NotIdentical;
     }
 
-    private function processReturnTrue(If_ $if, Return_ $nextReturnNode): Return_
+    private function processReturnTrue(If_ $if, Return_ $nextReturn): Return_
     {
-        if ($if->cond instanceof BooleanNot && $nextReturnNode->expr instanceof Expr && $this->valueResolver->isTrue(
-            $nextReturnNode->expr
+        if ($if->cond instanceof BooleanNot && $nextReturn->expr instanceof Expr && $this->valueResolver->isTrue(
+            $nextReturn->expr
         )) {
             return new Return_($this->exprBoolCaster->boolCastOrNullCompareIfNeeded($if->cond->expr));
         }
 
         return new Return_($this->exprBoolCaster->boolCastOrNullCompareIfNeeded($if->cond));
     }
 
-    private function processReturnFalse(If_ $if, Return_ $nextReturnNode): ?Return_
+    private function processReturnFalse(If_ $if, Return_ $nextReturn): ?Return_
     {
         if ($if->cond instanceof Identical) {
             $notIdentical = new NotIdentical($if->cond->left, $if->cond->right);
 
             return new Return_($this->exprBoolCaster->boolCastOrNullCompareIfNeeded($notIdentical));
         }
 
-        if (! $nextReturnNode->expr instanceof Expr) {
+        if (! $nextReturn->expr instanceof Expr) {
             return null;
         }
 
-        if (! $this->valueResolver->isTrue($nextReturnNode->expr)) {
+        if (! $this->valueResolver->isTrue($nextReturn->expr)) {
             return null;
         }
 

diff --git a/rules/DeadCode/NodeManipulator/ControllerClassMethodManipulator.php b/rules/DeadCode/NodeManipulator/ControllerClassMethodManipulator.php
@@ -44,6 +44,11 @@ private function hasParentClassController(Class_ $class): bool
             return false;
         }
 
-        return $this->nodeNameResolver->matchesStringName($class->extends->toString(), '#(Controller|Presenter)$#');
+        $parentClassName = $this->nodeNameResolver->getName($class->extends);
+        if (str_ends_with($parentClassName, 'Controller')) {
+            return true;
+        }
+
+        return str_ends_with($parentClassName, 'Presenter');
     }
 }
diff --git a/rules/EarlyReturn/NodeFactory/InvertedIfFactory.php b/rules/EarlyReturn/NodeFactory/InvertedIfFactory.php
@@ -25,19 +25,19 @@ public function __construct(
      * @param Expr[] $conditions
      * @return If_[]
      */
-    public function createFromConditions(If_ $if, array $conditions, Return_ $return, ?Stmt $ifNextReturn): array
+    public function createFromConditions(If_ $if, array $conditions, Return_ $return, ?Stmt $ifNextReturnStmt): array
     {
         $ifs = [];
-        $stmt = $this->contextAnalyzer->isInLoop($if) && ! $ifNextReturn instanceof Return_
+        $stmt = $this->contextAnalyzer->isInLoop($if) && ! $ifNextReturnStmt instanceof Return_
             ? [new Continue_()]
             : [$return];
 
-        if ($ifNextReturn instanceof Return_) {
-            $stmt[0]->setAttribute(AttributeKey::COMMENTS, $ifNextReturn->getAttribute(AttributeKey::COMMENTS));
+        if ($ifNextReturnStmt instanceof Return_) {
+            $stmt[0]->setAttribute(AttributeKey::COMMENTS, $ifNextReturnStmt->getAttribute(AttributeKey::COMMENTS));
         }
 
-        if ($ifNextReturn instanceof Return_ && $ifNextReturn->expr instanceof Expr) {
-            $return->expr = $ifNextReturn->expr;
+        if ($ifNextReturnStmt instanceof Return_ && $ifNextReturnStmt->expr instanceof Expr) {
+            $return->expr = $ifNextReturnStmt->expr;
         }
 
         foreach ($conditions as $condition) {

diff --git a/rules/EarlyReturn/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php b/rules/EarlyReturn/Rector/Foreach_/ChangeNestedForeachIfsToEarlyContinueRector.php
@@ -131,9 +131,9 @@ private function processNestedIfsWithNonBreaking(Foreach_ $foreach, array $neste
         return $foreach;
     }
 
-    private function addInvertedIfStmtWithContinue(If_ $nestedIfWithOnlyReturn, Foreach_ $foreach): void
+    private function addInvertedIfStmtWithContinue(If_ $onlyReturnIf, Foreach_ $foreach): void
     {
-        $invertedCondExpr = $this->conditionInverter->createInvertedCondition($nestedIfWithOnlyReturn->cond);
+        $invertedCondExpr = $this->conditionInverter->createInvertedCondition($onlyReturnIf->cond);
 
         // special case
         if ($invertedCondExpr instanceof BooleanNot && $invertedCondExpr->expr instanceof BooleanAnd) {
@@ -147,18 +147,18 @@ private function addInvertedIfStmtWithContinue(If_ $nestedIfWithOnlyReturn, Fore
         }
 
         // should skip for weak inversion
-        if ($this->isBooleanOrWithWeakComparison($nestedIfWithOnlyReturn->cond)) {
-            $foreach->stmts[] = $nestedIfWithOnlyReturn;
+        if ($this->isBooleanOrWithWeakComparison($onlyReturnIf->cond)) {
+            $foreach->stmts[] = $onlyReturnIf;
 
             return;
         }
 
-        $nestedIfWithOnlyReturn->setAttribute(AttributeKey::ORIGINAL_NODE, null);
+        $onlyReturnIf->setAttribute(AttributeKey::ORIGINAL_NODE, null);
 
-        $nestedIfWithOnlyReturn->cond = $invertedCondExpr;
-        $nestedIfWithOnlyReturn->stmts = [new Continue_()];
+        $onlyReturnIf->cond = $invertedCondExpr;
+        $onlyReturnIf->stmts = [new Continue_()];
 
-        $foreach->stmts[] = $nestedIfWithOnlyReturn;
+        $foreach->stmts[] = $onlyReturnIf;
     }
 
     /**

diff --git a/rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php b/rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php
@@ -201,27 +201,27 @@ private function isInLoopWithoutContinueOrBreak(If_ $if): bool
     private function processReplaceIfs(
         If_ $if,
         array $conditions,
-        Return_ $ifNextReturnClone,
+        Return_ $ifNextReturn,
         array $afters,
         ?Stmt $nextStmt
     ): array {
-        $ifs = $this->invertedIfFactory->createFromConditions($if, $conditions, $ifNextReturnClone, $nextStmt);
+        $ifs = $this->invertedIfFactory->createFromConditions($if, $conditions, $ifNextReturn, $nextStmt);
         $this->mirrorComments($ifs[0], $if);
 
         $result = array_merge($ifs, $afters);
         if ($if->stmts[0] instanceof Return_) {
             return $result;
         }
 
-        if (! $ifNextReturnClone->expr instanceof Expr) {
+        if (! $ifNextReturn->expr instanceof Expr) {
             return $result;
         }
 
         if ($this->contextAnalyzer->isInLoop($if)) {
             return $result;
         }
 
-        return array_merge($result, [$ifNextReturnClone]);
+        return array_merge($result, [$ifNextReturn]);
     }
 
     private function shouldSkip(If_ $if, ?Stmt $nexStmt): bool

diff --git a/rules/EarlyReturn/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php b/rules/EarlyReturn/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php
@@ -141,9 +141,9 @@ private function processNestedIfsWithOnlyReturn(array $nestedIfsWithOnlyReturn,
     /**
      * @return If_[]
      */
-    private function createStandaloneIfsWithReturn(If_ $nestedIfWithOnlyReturn, Return_ $return): array
+    private function createStandaloneIfsWithReturn(If_ $onlyReturnIf, Return_ $return): array
     {
-        $invertedCondExpr = $this->conditionInverter->createInvertedCondition($nestedIfWithOnlyReturn->cond);
+        $invertedCondExpr = $this->conditionInverter->createInvertedCondition($onlyReturnIf->cond);
 
         // special case
         if ($invertedCondExpr instanceof BooleanNot && $invertedCondExpr->expr instanceof BooleanAnd) {
@@ -156,9 +156,9 @@ private function createStandaloneIfsWithReturn(If_ $nestedIfWithOnlyReturn, Retu
             return [$booleanNotPartIf, $secondBooleanNotPartIf];
         }
 
-        $nestedIfWithOnlyReturn->cond = $invertedCondExpr;
-        $nestedIfWithOnlyReturn->stmts = [$return];
+        $onlyReturnIf->cond = $invertedCondExpr;
+        $onlyReturnIf->stmts = [$return];
 
-        return [$nestedIfWithOnlyReturn];
+        return [$onlyReturnIf];
     }
 }
diff --git a/rules/Naming/ExpectedNameResolver/MatchPropertyTypeExpectedNameResolver.php b/rules/Naming/ExpectedNameResolver/MatchPropertyTypeExpectedNameResolver.php
@@ -52,9 +52,13 @@ public function resolve(Property $property, ClassLike $classLike): ?string
             return null;
         }
 
+        $propertyName = $this->nodeNameResolver->getName($property);
+
         // skip if already has suffix
-        $currentName = $this->nodeNameResolver->getName($property);
-        if ($this->nodeNameResolver->endsWith($currentName, $expectedName->getName())) {
+        if (str_ends_with($propertyName, $expectedName->getName()) || str_ends_with(
+            $propertyName,
+            ucfirst($expectedName->getName())
+        )) {
             return null;
         }
 

diff --git a/rules/Naming/Naming/ExpectedNameResolver.php b/rules/Naming/Naming/ExpectedNameResolver.php
@@ -47,11 +47,8 @@ public function resolveForParamIfNotYet(Param $param): ?string
 
         /** @var string $currentName */
         $currentName = $this->nodeNameResolver->getName($param->var);
-        if ($currentName === $expectedName) {
-            return null;
-        }
 
-        if ($this->nodeNameResolver->endsWith($currentName, $expectedName)) {
+        if ($currentName === $expectedName || str_ends_with($currentName, ucfirst($expectedName))) {
             return null;
         }
 

diff --git a/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php b/rules/Php55/Rector/String_/StringClassNameToClassConstantRector.php
@@ -180,7 +180,7 @@ private function shouldSkip(string $classLikeName): bool
                 return true;
             }
 
-            if ($this->nodeNameResolver->matchesStringName($classLikeName, $classToSkip)) {
+            if (fnmatch($classToSkip, $classLikeName, FNM_NOESCAPE)) {
                 return true;
             }
         }

diff --git a/rules/Privatization/Rector/ClassMethod/PrivatizeFinalClassMethodRector.php b/rules/Privatization/Rector/ClassMethod/PrivatizeFinalClassMethodRector.php
@@ -117,7 +117,9 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
     private function shouldSkipClassMethod(ClassMethod $classMethod): bool
     {
         // edge case in nette framework
-        if ($this->nodeNameResolver->startsWith($classMethod->name, 'createComponent')) {
+        /** @var string $methodName */
+        $methodName = $this->getName($classMethod->name);
+        if (str_starts_with($methodName, 'createComponent')) {
             return true;
         }
 

diff --git a/rules/Renaming/Rector/MethodCall/RenameMethodRector.php b/rules/Renaming/Rector/MethodCall/RenameMethodRector.php
@@ -11,6 +11,7 @@
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Interface_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
@@ -120,7 +121,7 @@ private function shouldSkipClassMethod(
     }
 
     private function hasClassNewClassMethod(
-        Class_|Interface_         $classOrInterface,
+        Class_|Interface_ $classOrInterface,
         MethodCallRenameInterface $methodCallRename
     ): bool {
         return (bool) $classOrInterface->getMethod($methodCallRename->getNewMethod());
@@ -163,7 +164,13 @@ private function refactorClass(Class_|Interface_ $classOrInterface, Scope $scope
             }
 
             foreach ($this->methodCallRenames as $methodCallRename) {
-                if ($this->shouldSkipRename($methodName, $classMethod, $methodCallRename, $classReflection, $classOrInterface)) {
+                if ($this->shouldSkipRename(
+                    $methodName,
+                    $classMethod,
+                    $methodCallRename,
+                    $classReflection,
+                    $classOrInterface
+                )) {
                     continue;
                 }
 
@@ -181,12 +188,11 @@ private function refactorClass(Class_|Interface_ $classOrInterface, Scope $scope
 
     private function shouldSkipRename(
         string $methodName,
-        Node\Stmt\ClassMethod $classMethod,
+        ClassMethod $classMethod,
         MethodCallRenameInterface $methodCallRename,
         ClassReflection $classReflection,
         Class_|Interface_ $classOrInterface
-    ): bool
-    {
+    ): bool {
         if (! $this->nodeNameResolver->isStringName($methodName, $methodCallRename->getOldMethod())) {
             return true;
         }
@@ -202,11 +208,7 @@ private function shouldSkipRename(
             return true;
         }
 
-        if ($this->hasClassNewClassMethod($classOrInterface, $methodCallRename)) {
-            return true;
-        }
-
-        return false;
+        return $this->hasClassNewClassMethod($classOrInterface, $methodCallRename);
     }
 
     private function refactorMethodCallAndStaticCall(

diff --git a/rules/Transform/Rector/Class_/AddAllowDynamicPropertiesAttributeRector.php b/rules/Transform/Rector/Class_/AddAllowDynamicPropertiesAttributeRector.php
@@ -146,7 +146,7 @@ private function shouldSkip(Class_ $class): bool
                     continue;
                 }
 
-                if (! $this->nodeNameResolver->matchesStringName($className, $transformOnNamespace)) {
+                if (! fnmatch($transformOnNamespace, $className, FNM_NOESCAPE)) {
                     return true;
                 }
             }

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeBasedOnPHPUnitDataProviderRector.php
@@ -235,9 +235,9 @@ private function resolveYieldStaticArrayTypeByParameterPosition(array $yields, i
         return $this->typeFactory->createMixedPassedOrUnionType($paramOnPositionTypes);
     }
 
-    private function getTypeFromClassMethodYield(Array_ $classMethodYieldArrayNode): MixedType | ConstantArrayType
+    private function getTypeFromClassMethodYield(Array_ $classMethodYieldArray): MixedType | ConstantArrayType
     {
-        $arrayType = $this->nodeTypeResolver->getType($classMethodYieldArrayNode);
+        $arrayType = $this->nodeTypeResolver->getType($classMethodYieldArray);
 
         // impossible to resolve
         if (! $arrayType instanceof ConstantArrayType) {