diff --git a/composer.json b/composer.json index d5cd3b7d3..d0c747e29 100644 --- a/composer.json +++ b/composer.json @@ -79,12 +79,13 @@ "contribute": [ "@fix-rector", "@fix-style", - "@analyze" + "@analyze", + "@phpstan-generate-baseline" ], "check-style": "@php vendor/bin/ecs check --ansi", "fix-style": "@php vendor/bin/ecs check --fix --ansi", "analyze": "@php vendor/bin/phpstan analyse --memory-limit=-1 --ansi", - "phpstan-generate-baseline": "@php vendor/bin/phpstan analyse --generate-baseline --memory-limit=-1 --ansi", + "phpstan-generate-baseline": "@php vendor/bin/phpstan analyse --generate-baseline --memory-limit=-1 --ansi --allow-empty-baseline", "test": "@php vendor/bin/phpunit", "docs": "@php vendor/bin/rule-doc-generator generate src --output-file docs/all_rectors_overview.md --ansi", "rector": "@php vendor/bin/rector process --dry-run --ansi", diff --git a/config/v12/typo3-120.php b/config/v12/typo3-120.php index ba172b0c7..3e56e5537 100644 --- a/config/v12/typo3-120.php +++ b/config/v12/typo3-120.php @@ -11,6 +11,7 @@ use Ssch\TYPO3Rector\Rector\v12\v0\typo3\AddMethodToWidgetInterfaceClassesRector; use Ssch\TYPO3Rector\Rector\v12\v0\typo3\ChangeExtbaseValidatorsRector; use Ssch\TYPO3Rector\Rector\v12\v0\typo3\ContentObjectRegistrationViaServiceConfigurationRector; +use Ssch\TYPO3Rector\Rector\v12\v0\typo3\HardenMethodSignatureOfLogicalAndAndLogicalOrRector; use Ssch\TYPO3Rector\Rector\v12\v0\typo3\HintNecessaryUploadedFileChangesRector; use Ssch\TYPO3Rector\Rector\v12\v0\typo3\ImplementSiteLanguageAwareInterfaceRector; use Ssch\TYPO3Rector\Rector\v12\v0\typo3\MigrateQueryBuilderExecuteRector; @@ -170,4 +171,5 @@ $rectorConfig->rule(RegisterExtbaseTypeConvertersAsServicesRector::class); $rectorConfig->rule(ChangeExtbaseValidatorsRector::class); $rectorConfig->rule(ContentObjectRegistrationViaServiceConfigurationRector::class); + $rectorConfig->rule(HardenMethodSignatureOfLogicalAndAndLogicalOrRector::class); }; diff --git a/src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php b/src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php new file mode 100644 index 000000000..4355ca2ed --- /dev/null +++ b/src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php @@ -0,0 +1,349 @@ +nodesToAddCollector = $nodesToAddCollector; + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [MethodCall::class]; + } + + /** + * @param MethodCall $node + */ + public function refactor(Node $node): ?Node + { + if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType( + $node, + new ObjectType('TYPO3\CMS\Extbase\Persistence\QueryInterface') + )) { + return null; + } + + if (! $this->isNames($node->name, ['logicalAnd', 'logicalOr'])) { + return null; + } + + $args = $node->getArgs(); + if (! isset($args[0])) { + return null; + } + + if (count($args) > 1) { + return null; + } + + $firstArgument = $args[0]->value; + + if ($firstArgument instanceof Variable) { + // In this case, the code looks like this: $query->logicalAnd($constraints); + return $this->handleArgumentIsVariable($node, $firstArgument); + } + + if (! ($firstArgument instanceof Array_)) { + return null; + } + + // In this case, the code looks like this: $query->logicalAnd([...]) + + // Maybe add "new \PhpParser\Node\Stmt\Nop()" somehow for long lines? + $node->args = $this->nodeFactory->createArgs([...$firstArgument->items]); + + return $node; + } + + /** + * @codeCoverageIgnore + */ + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Use multiple parameters instead of array for logicalOr and logicalAnd of Extbase Query class', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use TYPO3\CMS\Extbase\Persistence\Repository; + +class ProductRepositoryLogicalAnd extends Repository +{ + public function findAllForList() + { + $query = $this->createQuery(); + $query->matching($query->logicalAnd([ + $query->equals('propertyName1', 'value1'), + $query->equals('propertyName2', 'value2'), + $query->equals('propertyName3', 'value3'), + ])); + } + public function findAllForSomething() + { + $query = $this->createQuery(); + $constraints[] = $query->lessThan('foo', 1); + $constraints[] = $query->lessThan('bar', 1); + $query->matching($query->logicalAnd($constraints)); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use TYPO3\CMS\Extbase\Persistence\Repository; + +class ProductRepositoryLogicalAnd extends Repository +{ + public function findAllForList() + { + $query = $this->createQuery(); + $query->matching($query->logicalAnd( + $query->equals('propertyName1', 'value1'), + $query->equals('propertyName2', 'value2'), + $query->equals('propertyName3', 'value3'), + )); + } + public function findAllForSomething() + { + $query = $this->createQuery(); + $constraints[] = $query->lessThan('foo', 1); + $constraints[] = $query->lessThan('bar', 1); + $query->matching($query->logicalAnd(...$constraints)); + } +} +CODE_SAMPLE + ), + ] + ); + } + + private function handleArgumentIsVariable(MethodCall $node, Variable $firstArgument): ?MethodCall + { + $parentIfStatement = $this->betterNodeFinder->findParentType($node, If_::class); + + $currentStmt = $this->betterNodeFinder->resolveCurrentStatement($node); + if (! $currentStmt instanceof Stmt) { + return null; + } + + $parentNode = $node->getAttribute('parent'); + + $logicalAndOrOr = $this->isName($node->name, 'logicalAnd') ? 'logicalAnd' : 'logicalOr'; + + $queryVariable = $node->var instanceof Variable ? $node->var : new Variable('query'); + + if ($parentNode instanceof Assign && $parentNode->expr instanceof MethodCall) { + // FIXME: This case is quite complicated as we don't really know how the code looks like. This is WIP for now... + // This is how a real world example could look like. See: https://review.typo3.org/c/Packages/TYPO3.CMS/+/72244/8/typo3/sysext/beuser/Classes/Domain/Repository/BackendUserRepository.php + // $constraints = []; + // $query = $this->createQuery(); + // $query->setOrderings(['userName' => QueryInterface::ORDER_ASCENDING]); + // // Username + // if ($demand->getUserName() !== '') { + // $searchConstraints = []; + // $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('be_users'); + // foreach (['userName', 'realName'] as $field) { + // $searchConstraints[] = $query->like( + // $field, + // '%' . $queryBuilder->escapeLikeWildcards($demand->getUserName()) . '%' + // ); + // } + // if (MathUtility::canBeInterpretedAsInteger($demand->getUserName())) { + // $searchConstraints[] = $query->equals('uid', (int)$demand->getUserName()); + // } + // $constraints[] = $query->logicalOr($searchConstraints); + // } + $if = $this->createIfForAssignment($parentNode, $firstArgument); + } else { + $if = $this->createIfForNormalMethod($firstArgument, $queryVariable, $logicalAndOrOr); + } + + if (! ($parentIfStatement instanceof If_)) { + $this->nodesToAddCollector->addNodeBeforeNode($if, $node); + return $node; + } + + if ($this->isBinaryOpAndNameAppearsInConditions($parentIfStatement, $firstArgument) + || $this->isBooleanNotAndNameAppears($parentIfStatement, $firstArgument) + || $this->isEmptyAndNameAppears($parentIfStatement, $firstArgument) + || $this->isVariableAndNameAppears($parentIfStatement, $firstArgument) + ) { + $this->removeNode($parentIfStatement); + $this->nodesToAddCollector->addNodeBeforeNode($if, $parentIfStatement); + } else { + #$this->removeNode($node); + $this->nodesToAddCollector->addNodeBeforeNode($if, $node); + } + + return $node; + } + + private function createIfForAssignment(Assign $parentNode, Variable $firstArgument): If_ + { + $ifExpression = clone $parentNode; + $ifExpression->expr = $this->nodeFactory->createFuncCall('reset', [$firstArgument]); + + return new If_( + new Identical($this->nodeFactory->createFuncCall('count', [$firstArgument]), new LNumber(1)), + [ + 'stmts' => [$ifExpression], + 'elseifs' => [ + new ElseIf_( + new GreaterOrEqual( + $this->nodeFactory->createFuncCall('count', [$firstArgument]), + new LNumber(2) + ), + [new Expression($parentNode->expr)] + ), + ], + ] + ); + } + + private function createIfForNormalMethod( + Variable $firstArgument, + Variable $queryVariable, + string $logicalAndOrOr + ): If_ { + return new If_( + new Identical($this->nodeFactory->createFuncCall('count', [$firstArgument]), new LNumber(1)), + [ + 'stmts' => [ + new Expression( + $this->nodeFactory->createMethodCall( + $queryVariable, + 'matching', + [new Arg($this->nodeFactory->createFuncCall('reset', [$firstArgument]))] + ) + ), + ], + 'elseifs' => [ + new ElseIf_( + new GreaterOrEqual( + $this->nodeFactory->createFuncCall('count', [$firstArgument]), + new LNumber(2) + ), + [ + new Expression( + $this->nodeFactory->createMethodCall( + $queryVariable, + 'matching', + [ + new Arg( + $this->nodeFactory->createMethodCall( + $queryVariable, + $logicalAndOrOr, + [new Arg($firstArgument, false, true)] + ), + ), + ] + ) + ), + ] + ), + ], + ] + ); + } + + private function isBinaryOpAndNameAppearsInConditions(If_ $parentIfStatement, Variable $firstArgument): bool + { + if (! ($parentIfStatement->cond instanceof BinaryOp)) { + return false; + } + + /** @var string $name */ + $name = $firstArgument->name; + + return ($parentIfStatement->cond->left instanceof Variable && $this->isName( + $parentIfStatement->cond->left, + $name + )) + || ($parentIfStatement->cond->right instanceof Variable && $this->isName( + $parentIfStatement->cond->right, + $name + )); + } + + private function isBooleanNotAndNameAppears(If_ $parentIfStatement, Variable $firstArgument): bool + { + if (! ($parentIfStatement->cond instanceof BooleanNot)) { + return false; + } + + /** @var string $name */ + $name = $firstArgument->name; + + if ($parentIfStatement->cond->expr instanceof Variable) { + return $this->isName($parentIfStatement->cond->expr, $name); + } + + if ($parentIfStatement->cond->expr instanceof Empty_) { + return $this->isName($parentIfStatement->cond->expr->expr, $name); + } + + return false; + } + + private function isEmptyAndNameAppears(If_ $parentIfStatement, Variable $firstArgument): bool + { + if (! ($parentIfStatement->cond instanceof Empty_)) { + return false; + } + + /** @var string $name */ + $name = $firstArgument->name; + + return $this->isName($parentIfStatement->cond->expr, $name); + } + + private function isVariableAndNameAppears(If_ $parentIfStatement, Variable $firstArgument): bool + { + if (! ($parentIfStatement->cond instanceof Variable)) { + return false; + } + + /** @var string $name */ + $name = $firstArgument->name; + + return $this->isName($parentIfStatement->cond, $name); + } +} diff --git a/stubs/TYPO3/CMS/Extbase/Persistence/Generic/Qom/AndInterface.php b/stubs/TYPO3/CMS/Extbase/Persistence/Generic/Qom/AndInterface.php new file mode 100644 index 000000000..98cf270b1 --- /dev/null +++ b/stubs/TYPO3/CMS/Extbase/Persistence/Generic/Qom/AndInterface.php @@ -0,0 +1,14 @@ +createQuery(); + $query->matching( + $query->logicalAnd([ + $query->lessThan('foo', 1), + $query->lessThan('bar', 1) + ]) + ); + } +} + +?> +----- +createQuery(); + $query->matching( + $query->logicalAnd($query->lessThan('foo', 1), $query->lessThan('bar', 1)) + ); + } +} + +?> diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/log_entry_repository.php.inc b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/log_entry_repository.php.inc new file mode 100644 index 000000000..7d16555ce --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/log_entry_repository.php.inc @@ -0,0 +1,105 @@ +createQuery(); + $queryConstraints = $this->createQueryConstraints($query, $constraint); + if (!empty($queryConstraints)) { + $query->matching($query->logicalAnd($queryConstraints)); + } + $query->setOrderings(['uid' => QueryInterface::ORDER_DESCENDING]); + $query->setLimit($constraint->getNumber()); + return $query->execute(); + } + + public function findByConstraintWithVariableComparison(Constraint $constraint): QueryResultInterface + { + $query = $this->createQuery(); + $queryConstraints = $this->createQueryConstraints($query, $constraint); + if ($queryConstraints) { + $query->matching($query->logicalAnd($queryConstraints)); + } + $query->setOrderings(['uid' => QueryInterface::ORDER_DESCENDING]); + $query->setLimit($constraint->getNumber()); + return $query->execute(); + } + + /** + * @param QueryInterface $query + * @param Constraint $constraint + * @return ConstraintInterface[] + */ + protected function createQueryConstraints(QueryInterface $query, Constraint $constraint): array + { + $queryConstraints = []; + $queryConstraints[] = $query->greaterThanOrEqual('tstamp', $constraint->getStartTimestamp()); + $queryConstraints[] = $query->lessThan('tstamp', $constraint->getEndTimestamp()); + return $queryConstraints; + } +} +?> +----- +createQuery(); + $queryConstraints = $this->createQueryConstraints($query, $constraint); + if (count($queryConstraints) === 1) { + $query->matching(reset($queryConstraints)); + } elseif (count($queryConstraints) >= 2) { + $query->matching($query->logicalAnd(...$queryConstraints)); + } + $query->setOrderings(['uid' => QueryInterface::ORDER_DESCENDING]); + $query->setLimit($constraint->getNumber()); + return $query->execute(); + } + + public function findByConstraintWithVariableComparison(Constraint $constraint): QueryResultInterface + { + $query = $this->createQuery(); + $queryConstraints = $this->createQueryConstraints($query, $constraint); + if (count($queryConstraints) === 1) { + $query->matching(reset($queryConstraints)); + } elseif (count($queryConstraints) >= 2) { + $query->matching($query->logicalAnd(...$queryConstraints)); + } + $query->setOrderings(['uid' => QueryInterface::ORDER_DESCENDING]); + $query->setLimit($constraint->getNumber()); + return $query->execute(); + } + + /** + * @param QueryInterface $query + * @param Constraint $constraint + * @return ConstraintInterface[] + */ + protected function createQueryConstraints(QueryInterface $query, Constraint $constraint): array + { + $queryConstraints = []; + $queryConstraints[] = $query->greaterThanOrEqual('tstamp', $constraint->getStartTimestamp()); + $queryConstraints[] = $query->lessThan('tstamp', $constraint->getEndTimestamp()); + return $queryConstraints; + } +} +?> diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_individual_arguments.php.inc b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_individual_arguments.php.inc new file mode 100644 index 000000000..a130405c1 --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_individual_arguments.php.inc @@ -0,0 +1,18 @@ +createQuery(); + $query->matching( + $query->logicalAnd($query->lessThan('foo', 1), $query->lessThan('bar', 1)) + ); + } +} + +?> diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_one_individual_argument.php.inc b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_one_individual_argument.php.inc new file mode 100644 index 000000000..2d4d65ffe --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_one_individual_argument.php.inc @@ -0,0 +1,18 @@ +createQuery(); + $query->matching( + $query->logicalAnd($query->lessThan('foo', 1)) + ); + } +} + +?> diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_variables_as_arguments.php.inc b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_variables_as_arguments.php.inc new file mode 100644 index 000000000..d052f775e --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_and_with_variables_as_arguments.php.inc @@ -0,0 +1,81 @@ +createQuery(); + if ($constraints !== []) { + $query->matching($query->logicalAnd(...$constraints)); + } + } + + public function findAllForListWithDifferentQueryVariableName() + { + $constraints = []; + $q = $this->createQuery(); + if ($constraints !== []) { + $q->matching($q->logicalAnd(...$constraints)); + } + } + + public function findAllForListWithLogicalOr() + { + $constraints = []; + $q = $this->createQuery(); + if ($constraints !== []) { + $q->matching($q->logicalOr(...$constraints)); + } + } +} + +?> +----- +createQuery(); + if (count($constraints) === 1) { + $query->matching(reset($constraints)); + } elseif (count($constraints) >= 2) { + $query->matching($query->logicalAnd(...$constraints)); + } + } + + public function findAllForListWithDifferentQueryVariableName() + { + $constraints = []; + $q = $this->createQuery(); + if (count($constraints) === 1) { + $q->matching(reset($constraints)); + } elseif (count($constraints) >= 2) { + $q->matching($q->logicalAnd(...$constraints)); + } + } + + public function findAllForListWithLogicalOr() + { + $constraints = []; + $q = $this->createQuery(); + if (count($constraints) === 1) { + $q->matching(reset($constraints)); + } elseif (count($constraints) >= 2) { + $q->matching($q->logicalOr(...$constraints)); + } + } +} + +?> diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_or.php.inc b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_or.php.inc new file mode 100644 index 000000000..9aa1cc7fc --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/Fixture/query_logical_or.php.inc @@ -0,0 +1,40 @@ +createQuery(); + $query->matching( + $query->logicalOr([ + $query->lessThan('foo', 1), + $query->lessThan('bar', 1), + ]) + ); + } +} + +?> +----- +createQuery(); + $query->matching( + $query->logicalOr($query->lessThan('foo', 1), $query->lessThan('bar', 1)) + ); + } +} + +?> diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest.php b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest.php new file mode 100644 index 000000000..304ab4e55 --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest.php @@ -0,0 +1,32 @@ +doTestFile($filePath); + } + + /** + * @return Iterator> + */ + public function provideData(): Iterator + { + return $this->yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/config/configured_rule.php b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/config/configured_rule.php new file mode 100644 index 000000000..ba0bb42cc --- /dev/null +++ b/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/config/configured_rule.php @@ -0,0 +1,11 @@ +import(__DIR__ . '/../../../../../../../config/config_test.php'); + $rectorConfig->rule(HardenMethodSignatureOfLogicalAndAndLogicalOrRector::class); +};