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

[Naming] Remove matchesStringName() check completely from NodeNameResolver, including endsWith() method - use getName() and compare directly instead #4954

Merged
merged 5 commits into from
Sep 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 0 additions & 38 deletions packages/NodeNameResolver/NodeNameResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = []
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixture;

final class KeepUnderscoreProperty
{
private ?\PDO $_pdo;
}
12 changes: 6 additions & 6 deletions rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
12 changes: 6 additions & 6 deletions rules/EarlyReturn/NodeFactory/InvertedIfFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 1 addition & 4 deletions rules/Naming/Naming/ExpectedNameResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
22 changes: 12 additions & 10 deletions rules/Renaming/Rector/MethodCall/RenameMethodRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down