Skip to content

[CodeQuality] Add ForRepeatedCountToOwnVariableRector#2559

Merged
TomasVotruba merged 1 commit intomasterfrom
count-for
Jan 4, 2020
Merged

[CodeQuality] Add ForRepeatedCountToOwnVariableRector#2559
TomasVotruba merged 1 commit intomasterfrom
count-for

Conversation

@TomasVotruba
Copy link
Copy Markdown
Member

Closes #1573

@TomasVotruba TomasVotruba force-pushed the count-for branch 2 times, most recently from ed1f6da to fbc6b4c Compare January 4, 2020 12:58
@TomasVotruba TomasVotruba merged commit b7d6078 into master Jan 4, 2020
@TomasVotruba TomasVotruba deleted the count-for branch January 4, 2020 13:35
} else {
$countedValueName = $valueName . 'Count';
}

Copy link
Copy Markdown
Contributor

@staabm staabm Jan 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the rector make sure that there is no other - already existing variable - in the current scope which might collide with the newly introduced one?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide code example?

How would you name new variable in case of conflict?

Copy link
Copy Markdown
Contributor

@staabm staabm Jan 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class SomeClass
 {
     public function run($items)
     {
         $itemsCount = 25;
         for ($i = 5; $i <= count($items); $i++) {
             echo $items[$i];
         }
         If ($itemsCount == 25 ) echo „whoo“;
     }
 }

I guess you would need just make sure the var name is unique, e.g. by appending a 1 or similar (after you checked the final var name is unique)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Covered in #2562

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thx

public function run($items, \stdClass $someObject)
{
for ($i = 5; $i <= count($someObject->getItems() + 10); $i++) {
echo $items[$i];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case the loop body changes the number of elements of the array the count() cannot be calculated beforehand .. could happen when the array get re-assigned, changed by reference or elements get added/deleted

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are many theoretical edge cases. Not worth covering unless really exist

TomasVotruba added a commit that referenced this pull request Jun 23, 2022
rectorphp/rector-src@bab3835 [TypeDeclaration] Skip string type override nullable string return on AddReturnTypeDeclarationRector (#2559)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CodeQuality] Looping count to variable with count

2 participants