Skip to content

[dx] optimize cheaper early returns #8102

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.

Sign up for GitHub

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

Open
TomasVotruba wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[dx] optimize cheaper early returns #8102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions rules/CodeQuality/Rector/Expression/InlineIfToExplicitIfRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,6 @@ private function processExplicitIf(Expression $expression): ?Node
/** @var BooleanAnd|BooleanOr $booleanExpr */
$booleanExpr = $expression->expr;

$leftStaticType = $this->getType($booleanExpr->left);
if (! $leftStaticType->isBoolean()->yes()) {
return null;
}

$exprLeft = $booleanExpr->left instanceof BooleanNot
? $booleanExpr->left->expr
: $booleanExpr->left;
Expand All @@ -102,6 +97,11 @@ private function processExplicitIf(Expression $expression): ?Node
return null;
}

$leftStaticType = $this->getType($booleanExpr->left);
if (! $leftStaticType->isBoolean()->yes()) {
return null;
}

/** @var Expr $expr */
$expr = $booleanExpr instanceof BooleanAnd
? $booleanExpr->left
Expand Down
9 changes: 5 additions & 4 deletions rules/CodeQuality/Rector/LogicalAnd/LogicalToBooleanRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,11 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): BooleanAnd|BooleanOr|null
{
$type = $this->nodeTypeResolver->getNativeType($node->left);

if ($node->left instanceof Assign && ! $type->isBoolean()->yes()) {
return null;
if ($node->left instanceof Assign) {
$type = $this->nodeTypeResolver->getNativeType($node->left);
if (! $type->isBoolean()->yes()) {
return null;
}
}

return $this->refactorLogicalToBoolean($node);
Expand Down
11 changes: 8 additions & 3 deletions rules/CodeQuality/Rector/Ternary/NumberCompareToMaxFuncCallRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ public function refactor(Node $node): ?Node
}

$binaryOp = $node->cond;
if (! $this->areIntegersCompared($binaryOp)) {
return null;
}

if ($binaryOp instanceof Smaller || $binaryOp instanceof SmallerOrEqual) {
if (! $this->nodeComparator->areNodesEqual($binaryOp->left, $node->else)) {
Expand All @@ -79,6 +76,10 @@ public function refactor(Node $node): ?Node
return null;
}

if (! $this->areIntegersCompared($binaryOp)) {
return null;
}

return $this->nodeFactory->createFuncCall('max', [$node->if, $node->else]);
}

Expand All @@ -91,6 +92,10 @@ public function refactor(Node $node): ?Node
return null;
}

if (! $this->areIntegersCompared($binaryOp)) {
return null;
}

return $this->nodeFactory->createFuncCall('max', [$node->if, $node->else]);
}

Expand Down
8 changes: 4 additions & 4 deletions rules/CodingStyle/Rector/FuncCall/ConsistentImplodeRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ public function refactor(Node $node): ?Node
return $node;
}

if (count($node->getArgs()) !== 2) {
return null;
}

$firstArg = $node->getArgs()[0];
$firstArgumentValue = $firstArg->value;

Expand All @@ -92,10 +96,6 @@ public function refactor(Node $node): ?Node
return null;
}

if (count($node->getArgs()) !== 2) {
return null;
}

$secondArg = $node->getArgs()[1];

if ($this->stringTypeAnalyzer->isStringOrUnionStringOnlyType($secondArg->value)) {
Expand Down
18 changes: 9 additions & 9 deletions rules/DeadCode/Rector/Stmt/RemoveNextSameValueConditionRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,6 @@ public function refactor(Node $node): ?Node
continue;
}

// first condition must be without side effect
if ($this->sideEffectNodeDetector->detect($stmt->cond)) {
continue;
}

if ($this->isCondVariableUsedInIfBody($stmt)) {
continue;
}

$nextStmt = $node->stmts[$key + 1] ?? null;
if (! $nextStmt instanceof If_) {
continue;
Expand All @@ -128,6 +119,15 @@ public function refactor(Node $node): ?Node
continue;
}

// first condition must be without side effect
if ($this->sideEffectNodeDetector->detect($stmt->cond)) {
continue;
}

if ($this->isCondVariableUsedInIfBody($stmt)) {
continue;
}

$stmt->setAttribute(AttributeKey::COMMENTS, array_merge(
$stmt->getAttribute(AttributeKey::COMMENTS) ?? [],
$nextStmt->getAttribute(AttributeKey::COMMENTS) ?? []
Expand Down
8 changes: 4 additions & 4 deletions rules/Php74/Rector/StaticCall/ExportToReflectionFunctionRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,16 @@ public function refactor(Node $node): ?Node
return null;
}

$callerType = $this->nodeTypeResolver->getType($node->class);
if (! $callerType->isSuperTypeOf(new ObjectType('ReflectionFunction'))->yes()) {
if (! $this->isName($node->name, 'export')) {
return null;
}

if (! $this->isName($node->name, 'export')) {
if ($node->isFirstClassCallable()) {
return null;
}

if ($node->isFirstClassCallable()) {
$callerType = $this->nodeTypeResolver->getType($node->class);
if (! $callerType->isSuperTypeOf(new ObjectType('ReflectionFunction'))->yes()) {
return null;
}

Expand Down
4 changes: 2 additions & 2 deletions rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,13 +177,13 @@ public function refactor(Node $node): ?Node
return null;
}

$constructorPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($constructClassMethod);

$classReflection = $this->reflectionResolver->resolveClassReflection($node);
if (! $classReflection instanceof ClassReflection) {
return null;
}

$constructorPhpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($constructClassMethod);

$hasChanged = false;
foreach ($promotionCandidates as $promotionCandidate) {
$param = $promotionCandidate->getParam();
Expand Down
4 changes: 2 additions & 2 deletions rules/Php82/Rector/New_/FilesystemIteratorSkipDotsRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,11 @@ public function refactor(Node $node): ?New_
return null;
}

if (! $this->isObjectType($node->class, new ObjectType('FilesystemIterator'))) {
if (! isset($node->args[1])) {
return null;
}

if (! isset($node->args[1])) {
if (! $this->isObjectType($node->class, new ObjectType('FilesystemIterator'))) {
return null;
}

Expand Down
8 changes: 4 additions & 4 deletions rules/Php84/Rector/FuncCall/AddEscapeArgumentRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,16 +75,16 @@ public function refactor(Node $node): null|FuncCall|MethodCall
return $node;
}

if (! $this->isObjectType($node->var, new ObjectType('SplFileObject'))) {
return null;
}

$name = $this->getName($node->name);

if (! in_array($name, ['setCsvControl', 'fputcsv', 'fgetcsv'], true)) {
return null;
}

if (! $this->isObjectType($node->var, new ObjectType('SplFileObject'))) {
return null;
}

if ($this->shouldSkipNamedArg($node)) {
return null;
}
Expand Down
15 changes: 8 additions & 7 deletions rules/Php85/Rector/FuncCall/OrdSingleByteRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,24 @@ public function refactor(Node $node): ?Node
$firstArg = $args[0];

$argExpr = $firstArg->value;
$type = $this->nodeTypeResolver->getNativeType($argExpr);

if (! $type->isString()->yes() && ! $type->isInteger()->yes()) {
if ($argExpr instanceof String_ && strlen($argExpr->value) === 1) {
return null;
}

$value = $this->valueResolver->getValue($argExpr);
$isInt = is_int($value);

if ($argExpr instanceof String_ && strlen($argExpr->value) === 1) {
if ($argExpr instanceof Int_ && strlen((string) $argExpr->value) === 1) {
return null;
}

if ($argExpr instanceof Int_ && strlen((string) $argExpr->value) === 1) {
$type = $this->nodeTypeResolver->getNativeType($argExpr);

if (! $type->isString()->yes() && ! $type->isInteger()->yes()) {
return null;
}

$value = $this->valueResolver->getValue($argExpr);
$isInt = is_int($value);

if (! $argExpr instanceof Int_) {
return $this->refactorStringType($argExpr, $isInt, $args, $node);
}
Expand Down
9 changes: 4 additions & 5 deletions rules/Php85/Rector/FuncCall/RemoveFinfoBufferContextArgRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,10 @@ public function refactor(Node $node): ?Node
return null;
}

$objectType = new ObjectType('finfo');
if ($node instanceof MethodCall && (! $this->nodeTypeResolver->isObjectType(
$node->var,
$objectType
) || ! $this->isName($node->name, 'buffer'))) {
if ($node instanceof MethodCall && (! $this->isName(
$node->name,
'buffer'
) || ! $this->nodeTypeResolver->isObjectType($node->var, new ObjectType('finfo')))) {
return null;
}

Expand Down
4 changes: 2 additions & 2 deletions ...peDeclarationDocblocks/Rector/ClassMethod/AddParamArrayDocblockFromDataProviderRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ public function refactor(Node $node): ?Node
continue;
}

$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);

$dataProviderNodes = $this->dataProviderMethodsFinder->findDataProviderNodes($node, $classMethod);
if ($dataProviderNodes->getClassMethods() === []) {
continue;
}

$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($classMethod);

foreach ($classMethod->getParams() as $paramPosition => $param) {
// we are interested only in array params
if (! $param->type instanceof Node) {
Expand Down
10 changes: 5 additions & 5 deletions ...larationDocblocks/Rector/ClassMethod/AddReturnDocblockForArrayDimAssignedObjectRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
// definitely not an array return
if ($node->returnType instanceof Node && ! $this->isName($node->returnType, 'array')) {
return null;
}

$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
$returnType = $phpDocInfo->getReturnType();

Expand All @@ -109,11 +114,6 @@ public function refactor(Node $node): ?Node
return null;
}

// definitely not an array return
if ($node->returnType instanceof Node && ! $this->isName($node->returnType, 'array')) {
return null;
}

$onlyReturnWithExpr = $this->returnNodeFinder->findOnlyReturnWithExpr($node);
if (! $onlyReturnWithExpr instanceof Return_ || ! $onlyReturnWithExpr->expr instanceof Variable) {
return null;
Expand Down
8 changes: 4 additions & 4 deletions ...arationDocblocks/Rector/ClassMethod/AddReturnDocblockForCommonObjectDenominatorRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,13 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
// definitely not an array return
if ($node->returnType instanceof Node && ! $this->isName($node->returnType, 'array')) {
return null;
}

// definitely not an array return
if ($node->returnType instanceof Node && ! $this->isName($node->returnType, 'array')) {
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
return null;
}

Expand Down
7 changes: 2 additions & 5 deletions rules/TypeDeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockForJsonArrayRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);

// definitely not an array return
if ($node->returnType instanceof Node && ! $this->isName($node->returnType, 'array')) {
return null;
Expand All @@ -106,9 +104,8 @@ public function refactor(Node $node): ?Node
return null;
}

$classMethodDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);

if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($classMethodDocInfo->getReturnTagValue())) {
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
return null;
}

Expand Down
8 changes: 4 additions & 4 deletions ...DeclarationDocblocks/Rector/ClassMethod/AddReturnDocblockFromMethodCallDocblockRector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ public function getNodeTypes(): array
*/
public function refactor(Node $node): ?Node
{
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
// definitely not an array return
if (! $node->returnType instanceof Node || ! $this->isName($node->returnType, 'array')) {
return null;
}

// definitely not an array return
if (! $node->returnType instanceof Node || ! $this->isName($node->returnType, 'array')) {
$phpDocInfo = $this->phpDocInfoFactory->createFromNodeOrEmpty($node);
if ($this->usefulArrayTagNodeAnalyzer->isUsefulArrayTag($phpDocInfo->getReturnTagValue())) {
return null;
}

Expand Down
Loading