Skip to content

Commit

Permalink
[TASK] Harden method signature of logicalAnd() and logicalOr()
Browse files Browse the repository at this point in the history
Resolves: #2861
  • Loading branch information
simonschaufi committed Oct 19, 2023
1 parent 0a32294 commit a783e6d
Show file tree
Hide file tree
Showing 18 changed files with 911 additions and 6 deletions.
2 changes: 2 additions & 0 deletions config/v12/typo3-120.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,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;
Expand Down Expand Up @@ -170,4 +171,5 @@
$rectorConfig->rule(RegisterExtbaseTypeConvertersAsServicesRector::class);
$rectorConfig->rule(ChangeExtbaseValidatorsRector::class);
$rectorConfig->rule(ContentObjectRegistrationViaServiceConfigurationRector::class);
$rectorConfig->rule(HardenMethodSignatureOfLogicalAndAndLogicalOrRector::class);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,285 @@
<?php

declare(strict_types=1);

namespace Ssch\TYPO3Rector\Rector\v12\v0\typo3;

use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\Array_;
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\BinaryOp\GreaterOrEqual;
use PhpParser\Node\Expr\BinaryOp\Identical;
use PhpParser\Node\Expr\BooleanNot;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Scalar\LNumber;
use PhpParser\Node\Stmt;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\If_;
use PhpParser\Node\Stmt\Nop;
use PHPStan\Type\ObjectType;
use Rector\Core\Rector\AbstractRector;
use Rector\PostRector\Collector\NodesToAddCollector;
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;

/**
* @changelog https://docs.typo3.org/c/typo3/cms-core/main/en-us/Changelog/12.0/Breaking-96044-HardenMethodSignatureOfLogicalAndAndLogicalOr.html
* @see \Ssch\TYPO3Rector\Tests\Rector\v12\v0\typo3\HardenMethodSignatureOfLogicalAndAndLogicalOrRector\HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest
*/
final class HardenMethodSignatureOfLogicalAndAndLogicalOrRector extends AbstractRector
{
/**
* @readonly
*/
public NodesToAddCollector $nodesToAddCollector;

public function __construct(NodesToAddCollector $nodesToAddCollector)
{
$this->nodesToAddCollector = $nodesToAddCollector;
}

/**
* @return array<class-string<Node>>
*/
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) {
$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 : '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);

Check failure on line 117 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $queryVariable of method Ssch\TYPO3Rector\Rector\v12\v0\typo3\HardenMethodSignatureOfLogicalAndAndLogicalOrRector::createIfForNormalMethod() expects PhpParser\Node\Expr\Variable, PhpParser\Node\Expr\Variable|string given.

Check failure on line 117 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $queryVariable of method Ssch\TYPO3Rector\Rector\v12\v0\typo3\HardenMethodSignatureOfLogicalAndAndLogicalOrRector::createIfForNormalMethod() expects PhpParser\Node\Expr\Variable, PhpParser\Node\Expr\Variable|string given.

Check failure on line 117 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $queryVariable of method Ssch\TYPO3Rector\Rector\v12\v0\typo3\HardenMethodSignatureOfLogicalAndAndLogicalOrRector::createIfForNormalMethod() expects PhpParser\Node\Expr\Variable, PhpParser\Node\Expr\Variable|string given.
}

if ($parentIfStatement instanceof If_ && (($parentIfStatement->cond instanceof BinaryOp
&& (($parentIfStatement->cond->left instanceof Variable && $this->isName(
$parentIfStatement->cond->left,
$firstArgument->name

Check failure on line 123 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.

Check failure on line 123 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.

Check failure on line 123 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.
))
|| ($parentIfStatement->cond->right instanceof Variable && $this->isName(
$parentIfStatement->cond->right,
$firstArgument->name

Check failure on line 127 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.

Check failure on line 127 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.

Check failure on line 127 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.
)))) ||
($parentIfStatement->cond instanceof BooleanNot && $this->isName(
$parentIfStatement->cond->expr->expr,

Check failure on line 130 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Access to an undefined property PhpParser\Node\Expr::$expr.

Check failure on line 130 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Access to an undefined property PhpParser\Node\Expr::$expr.

Check failure on line 130 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Access to an undefined property PhpParser\Node\Expr::$expr.
$firstArgument->name

Check failure on line 131 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.

Check failure on line 131 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.

Check failure on line 131 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Parameter #2 $name of method Rector\Core\Rector\AbstractRector::isName() expects string, PhpParser\Node\Expr|string given.
)))
) {
$this->removeNode($parentIfStatement);
$this->nodesToAddCollector->addNodeBeforeNode($if, $parentIfStatement);
} else {
#$this->removeNode($node);
$this->nodesToAddCollector->addNodeBeforeNode($if, $node);
}

return $node;
}

if (! ($firstArgument instanceof Array_)) {
return null;
}

$args = [];
foreach ($firstArgument->items as $item) {
$args[] = $this->nodeFactory->createArg($item->value);

Check failure on line 150 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Cannot access property $value on PhpParser\Node\Expr\ArrayItem|null.

Check failure on line 150 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Cannot access property $value on PhpParser\Node\Expr\ArrayItem|null.

Check failure on line 150 in src/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector.php

View workflow job for this annotation

GitHub Actions / PHPStan

Cannot access property $value on PhpParser\Node\Expr\ArrayItem|null.
// TODO: add "new Nop()"
}

return new MethodCall($node->var, $node->name, $args);
}

/**
* @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 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)]
),
),
]
)
),
]
),
],
]
);
}
}
23 changes: 23 additions & 0 deletions stubs/TYPO3/CMS/Extbase/Domain/Model/Constraint.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

namespace TYPO3\CMS\Extbase\Domain\Model;

class Constraint
{
public function getStartTimestamp(): int
{
return 1;
}

public function getEndTimestamp(): int
{
return 2;
}

public function getNumber(): int
{
return 42;
}
}
17 changes: 17 additions & 0 deletions stubs/TYPO3/CMS/Extbase/Domain/Model/Demand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace TYPO3\CMS\Extbase\Domain\Model;

class Demand
{

public function getUserName()
{
}

public function getBackendUserGroup()
{
}
}
14 changes: 14 additions & 0 deletions stubs/TYPO3/CMS/Extbase/Persistence/Generic/Qom/AndInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace TYPO3\CMS\Extbase\Persistence\Generic\Qom;

if (interface_exists('TYPO3\CMS\Extbase\Persistence\Generic\Qom\AndInterface')) {
return;
}

interface AndInterface extends ConstraintInterface
{

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

declare(strict_types=1);

namespace TYPO3\CMS\Extbase\Persistence\Generic\Qom;

if (interface_exists('TYPO3\CMS\Extbase\Persistence\Generic\Qom\ConstraintInterface')) {
return;
}

interface ConstraintInterface
{

}
14 changes: 14 additions & 0 deletions stubs/TYPO3/CMS/Extbase/Persistence/Generic/Qom/OrInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace TYPO3\CMS\Extbase\Persistence\Generic\Qom;

if (interface_exists('TYPO3\CMS\Extbase\Persistence\Generic\Qom\OrInterface')) {
return;
}

interface OrInterface extends ConstraintInterface
{

}
Loading

0 comments on commit a783e6d

Please sign in to comment.