From a2ae659e0a3db3d68086f38eaabfa9579f419ac4 Mon Sep 17 00:00:00 2001 From: Abdul Malik Ikhsan Date: Fri, 30 Oct 2020 03:47:12 +0700 Subject: [PATCH] [CodeQualityStrict] Enable MoveOutMethodCallInsideIfConditionRector (#4502) Co-authored-by: rector-bot --- .../Printer/MultilineSpaceFormatPreserver.php | 3 +- rector-ci.php | 2 - .../MethodCallToVariableNameResolver.php | 73 +++++++++++++------ ...g_class_constant_with_class_suffix.php.inc | 48 ++++++++++++ .../Rector/Class_/LoggableBehaviorRector.php | 6 +- .../Class_/SoftDeletableBehaviorRector.php | 3 +- .../Class_/TranslationBehaviorRector.php | 6 +- .../src/Rector/Class_/TreeBehaviorRector.php | 3 +- .../PhpDocParser/DoctrineDocBlockResolver.php | 6 +- src/Rector/AbstractRector.php | 6 +- 10 files changed, 119 insertions(+), 37 deletions(-) create mode 100644 rules/code-quality/tests/Rector/If_/MoveOutMethodCallInsideIfConditionRector/Fixture/if_condition_method_call_with_arg_class_constant_with_class_suffix.php.inc diff --git a/packages/better-php-doc-parser/src/Printer/MultilineSpaceFormatPreserver.php b/packages/better-php-doc-parser/src/Printer/MultilineSpaceFormatPreserver.php index a73900cde49..f70a7e760ed 100644 --- a/packages/better-php-doc-parser/src/Printer/MultilineSpaceFormatPreserver.php +++ b/packages/better-php-doc-parser/src/Printer/MultilineSpaceFormatPreserver.php @@ -55,7 +55,8 @@ final class MultilineSpaceFormatPreserver public function fixMultilineDescriptions( AttributeAwareNodeInterface $attributeAwareNode ): AttributeAwareNodeInterface { - if (! $attributeAwareNode->getAttribute(Attribute::ORIGINAL_CONTENT)) { + $originalContent = $attributeAwareNode->getAttribute(Attribute::ORIGINAL_CONTENT); + if (! $originalContent) { return $attributeAwareNode; } diff --git a/rector-ci.php b/rector-ci.php index 5ac488fe85d..a5a6839eb62 100644 --- a/rector-ci.php +++ b/rector-ci.php @@ -2,7 +2,6 @@ declare(strict_types=1); -use Rector\CodeQuality\Rector\If_\MoveOutMethodCallInsideIfConditionRector; use Rector\CodingStyle\Rector\String_\SplitStringClassConstantToClassConstFetchRector; use Rector\Core\Configuration\Option; use Rector\Core\Rector\AbstractRector; @@ -78,7 +77,6 @@ return static function (ContainerConfigurator $containerConfigurator): void { SplitStringClassConstantToClassConstFetchRector::class, // false positives on constants used in rector-ci.php RemoveUnusedClassConstantRector::class, - MoveOutMethodCallInsideIfConditionRector::class, ]); # so Rector code is still PHP 7.2 compatible diff --git a/rules/code-quality/src/Naming/MethodCallToVariableNameResolver.php b/rules/code-quality/src/Naming/MethodCallToVariableNameResolver.php index 4b333b266e5..e96e8457d47 100644 --- a/rules/code-quality/src/Naming/MethodCallToVariableNameResolver.php +++ b/rules/code-quality/src/Naming/MethodCallToVariableNameResolver.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; +use PhpParser\Node\Name; use PhpParser\Node\Scalar\String_; use Rector\Naming\Naming\ExpectedNameResolver; use Rector\NodeNameResolver\NodeNameResolver; @@ -48,41 +49,34 @@ final class MethodCallToVariableNameResolver public function resolveVariableName(MethodCall $methodCall): ?string { $methodCallVarName = $this->nodeNameResolver->getName($methodCall->var); - if ($methodCallVarName === null) { - return null; - } - $methodCallName = $this->nodeNameResolver->getName($methodCall->name); - if ($methodCallName === null) { + if ($methodCallVarName === null || $methodCallName === null) { return null; } + return $this->getVariableName($methodCall, $methodCallVarName, $methodCallName); + } + + private function getVariableName(MethodCall $methodCall, string $methodCallVarName, string $methodCallName): string + { $variableName = $this->expectedNameResolver->resolveForCall($methodCall); if ($methodCall->args === [] && $variableName !== null && $variableName !== $methodCallVarName) { return $variableName; } + $fallbackVarName = $this->getFallbackVarName($methodCallVarName, $methodCallName); $argValue = $methodCall->args[0]->value; if ($argValue instanceof ClassConstFetch && $argValue->name instanceof Identifier) { - return Strings::replace( - strtolower($argValue->name->toString()), - self::CONSTANT_REGEX, - function ($matches): string { - return strtoupper($matches[2]); - } - ); + return $this->getClassConstFetchVarName($argValue, $methodCallName); } - $fallbackVarName = $this->getFallbackVarName($methodCallVarName, $methodCallName); if ($argValue instanceof String_) { return $this->getStringVarName($argValue, $methodCallVarName, $fallbackVarName); } - if ($argValue instanceof Variable) { - $argumentName = $this->nodeNameResolver->getName($argValue); - if ($argumentName !== null && $variableName !== null) { - return $argumentName . ucfirst($variableName); - } + $argumentName = $this->nodeNameResolver->getName($argValue); + if ($argValue instanceof Variable && $argumentName !== null && $variableName !== null) { + return $argumentName . ucfirst($variableName); } return $fallbackVarName; @@ -93,16 +87,47 @@ final class MethodCallToVariableNameResolver return $methodCallVarName . ucfirst($methodCallName); } + private function getClassConstFetchVarName(ClassConstFetch $classConstFetch, string $methodCallName): string + { + /** @var Identifier $name */ + $name = $classConstFetch->name; + $argValueName = strtolower($name->toString()); + + if ($argValueName !== 'class') { + return Strings::replace( + $argValueName, + self::CONSTANT_REGEX, + function ($matches): string { + return strtoupper($matches[2]); + } + ); + } + + if ($classConstFetch->class instanceof Name) { + return $this->normalizeStringVariableName($methodCallName) . $classConstFetch->class->getLast(); + } + + return $this->normalizeStringVariableName($methodCallName); + } + private function getStringVarName(String_ $string, string $methodCallVarName, string $fallbackVarName): string { - $get = str_ireplace('get', '', $string->value . ucfirst($fallbackVarName)); - $by = str_ireplace('by', '', $get); - $by = str_replace('-', '', $by); - - if (Strings::match($by, self::START_ALPHA_REGEX) && $by !== $methodCallVarName) { - return $by; + $normalizeStringVariableName = $this->normalizeStringVariableName($string->value . ucfirst($fallbackVarName)); + if (Strings::match( + $normalizeStringVariableName, + self::START_ALPHA_REGEX + ) && $normalizeStringVariableName !== $methodCallVarName) { + return $normalizeStringVariableName; } return $fallbackVarName; } + + private function normalizeStringVariableName(string $string): string + { + $get = str_ireplace('get', '', $string); + $by = str_ireplace('by', '', $get); + + return str_replace('-', '', $by); + } } diff --git a/rules/code-quality/tests/Rector/If_/MoveOutMethodCallInsideIfConditionRector/Fixture/if_condition_method_call_with_arg_class_constant_with_class_suffix.php.inc b/rules/code-quality/tests/Rector/If_/MoveOutMethodCallInsideIfConditionRector/Fixture/if_condition_method_call_with_arg_class_constant_with_class_suffix.php.inc new file mode 100644 index 00000000000..0dbcb4137a5 --- /dev/null +++ b/rules/code-quality/tests/Rector/If_/MoveOutMethodCallInsideIfConditionRector/Fixture/if_condition_method_call_with_arg_class_constant_with_class_suffix.php.inc @@ -0,0 +1,48 @@ +condition(SomeClassWithConstants::class) === 1) { + + } + } + + public function condition($arg): int + { + return 1; + } +} + +?> +----- +condition(SomeClassWithConstants::class); + if ($conditionSomeClassWithConstants === 1) { + + } + } + + public function condition($arg): int + { + return 1; + } +} + +?> diff --git a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/LoggableBehaviorRector.php b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/LoggableBehaviorRector.php index 0d5857323cb..fb4b3d00d48 100644 --- a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/LoggableBehaviorRector.php +++ b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/LoggableBehaviorRector.php @@ -95,8 +95,9 @@ CODE_SAMPLE if ($classPhpDocInfo === null) { return null; } + $hasTypeLoggableTagValueNode = $classPhpDocInfo->hasByType(LoggableTagValueNode::class); - if (! $classPhpDocInfo->hasByType(LoggableTagValueNode::class)) { + if (! $hasTypeLoggableTagValueNode) { return null; } @@ -119,8 +120,9 @@ CODE_SAMPLE if ($propertyPhpDocInfo === null) { continue; } + $hasTypeVersionedTagValueNode = $propertyPhpDocInfo->hasByType(VersionedTagValueNode::class); - if (! $propertyPhpDocInfo->hasByType(VersionedTagValueNode::class)) { + if (! $hasTypeVersionedTagValueNode) { continue; } diff --git a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/SoftDeletableBehaviorRector.php b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/SoftDeletableBehaviorRector.php index e22209183f8..1ce1c7f2272 100644 --- a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/SoftDeletableBehaviorRector.php +++ b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/SoftDeletableBehaviorRector.php @@ -94,8 +94,9 @@ CODE_SAMPLE if ($classPhpDocInfo === null) { return null; } + $hasTypeSoftDeleteableTagValueNode = $classPhpDocInfo->hasByType(SoftDeleteableTagValueNode::class); - if (! $classPhpDocInfo->hasByType(SoftDeleteableTagValueNode::class)) { + if (! $hasTypeSoftDeleteableTagValueNode) { return null; } diff --git a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TranslationBehaviorRector.php b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TranslationBehaviorRector.php index cd0a341ec01..aa58c345f29 100644 --- a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TranslationBehaviorRector.php +++ b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TranslationBehaviorRector.php @@ -190,13 +190,15 @@ CODE_SAMPLE if ($propertyPhpDocInfo === null) { continue; } + $hasTypeLocaleTagValueNode = $propertyPhpDocInfo->hasByType(LocaleTagValueNode::class); - if ($propertyPhpDocInfo->hasByType(LocaleTagValueNode::class)) { + if ($hasTypeLocaleTagValueNode) { $this->removeNode($property); continue; } + $hasTypeTranslatableTagValueNode = $propertyPhpDocInfo->hasByType(TranslatableTagValueNode::class); - if (! $propertyPhpDocInfo->hasByType(TranslatableTagValueNode::class)) { + if (! $hasTypeTranslatableTagValueNode) { continue; } diff --git a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TreeBehaviorRector.php b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TreeBehaviorRector.php index 0a1d310941e..1386df3bb05 100644 --- a/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TreeBehaviorRector.php +++ b/rules/doctrine-gedmo-to-knplabs/src/Rector/Class_/TreeBehaviorRector.php @@ -141,8 +141,9 @@ CODE_SAMPLE if ($classPhpDocInfo === null) { return null; } + $hasTypeTreeTagValueNode = $classPhpDocInfo->hasByType(TreeTagValueNode::class); - if (! $classPhpDocInfo->hasByType(TreeTagValueNode::class)) { + if (! $hasTypeTreeTagValueNode) { return null; } diff --git a/rules/doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php b/rules/doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php index d70d462e053..c1fb755af3e 100644 --- a/rules/doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php +++ b/rules/doctrine/src/PhpDocParser/DoctrineDocBlockResolver.php @@ -99,8 +99,9 @@ final class DoctrineDocBlockResolver if ($phpDocInfo === null) { return false; } + $hasTypeColumnTagValueNode = $phpDocInfo->hasByType(ColumnTagValueNode::class); - if ($phpDocInfo->hasByType(ColumnTagValueNode::class)) { + if ($hasTypeColumnTagValueNode) { return true; } @@ -124,8 +125,9 @@ final class DoctrineDocBlockResolver if ($phpDocInfo === null) { return false; } + $hasTypeEntityTagValueNode = $phpDocInfo->hasByType(EntityTagValueNode::class); - if ($phpDocInfo->hasByType(EntityTagValueNode::class)) { + if ($hasTypeEntityTagValueNode) { return true; } diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php index 2742f88feb5..fe17afb51fc 100644 --- a/src/Rector/AbstractRector.php +++ b/src/Rector/AbstractRector.php @@ -431,11 +431,13 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn private function keepFileInfoAttribute(Node $node, Node $originalNode): void { - if ($node->getAttribute(AttributeKey::FILE_INFO) instanceof SmartFileInfo) { + $fileInfo = $node->getAttribute(AttributeKey::FILE_INFO); + if ($fileInfo instanceof SmartFileInfo) { return; } + $fileInfo = $originalNode->getAttribute(AttributeKey::FILE_INFO); - if ($originalNode->getAttribute(AttributeKey::FILE_INFO) !== null) { + if ($fileInfo !== null) { $node->setAttribute(AttributeKey::FILE_INFO, $originalNode->getAttribute(AttributeKey::FILE_INFO)); } elseif ($originalNode->getAttribute(AttributeKey::PARENT_NODE) !== null) { /** @var Node $parentOriginalNode */