Skip to content
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.

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

Infinite loop in logicalAnd migration #3928

Closed
nhovratov opened this issue Nov 27, 2023 · 4 comments · Fixed by #3929
Closed

Infinite loop in logicalAnd migration #3928

nhovratov opened this issue Nov 27, 2023 · 4 comments · Fixed by #3929
Labels

Comments

@nhovratov
Copy link
Contributor

Bug Report

This issue is related to #2861

When I ran rector, it took oddly long. I canceled the run and had a look, what happened:

Rector tried to migrate:

        if (\count($constraints) > 0) {
            $query->matching(
                $query->logicalAnd($constraints)
            );
        }

The result was:

        if (\count($constraints) > 0) {
            if (count($constraints) === 1) {
                $query->matching(reset($constraints));
            } elseif (count($constraints) >= 2) {
                if (count($constraints) === 1) {
                    $query->matching(reset($constraints));
                } elseif (count($constraints) >= 2) {
                    if (count($constraints) === 1) {
                        $query->matching(reset($constraints));
                    }
                    ...
                        }
                        $query->matching($query->logicalAnd(...$constraints));
                    }
                    $query->matching($query->logicalAnd(...$constraints));
                }
                $query->matching($query->logicalAnd(...$constraints));
            }


@nhovratov nhovratov added the Bug label Nov 27, 2023
@nhovratov
Copy link
Contributor Author

This seems to be an issue with a composer dependency. When I ran the test suite in my local rector environment, it failed:

/usr/bin/php /home/nikita/projects/typo3-rector/vendor/phpunit/phpunit/phpunit --configuration /home/nikita/projects/typo3-rector/phpunit.xml --filter Ssch\\TYPO3Rector\\Tests\\Rector\\v12\\v0\\typo3\\HardenMethodSignatureOfLogicalAndAndLogicalOrRector\\HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest --test-suffix HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest.php /home/nikita/projects/typo3-rector/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector --teamcity
Testing started at 12:43 ...
PHPUnit 9.6.5 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.13
Configuration: /home/nikita/projects/typo3-rector/phpunit.xml


Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
         if (count($queryConstraints) === 1) {
             $query->matching(reset($queryConstraints));
         } elseif (count($queryConstraints) >= 2) {
+            if (count($queryConstraints) === 1) {
+                $query->matching(reset($queryConstraints));
+            } elseif (count($queryConstraints) >= 2) {
+                $query->matching($query->logicalAnd(...$queryConstraints));
+            }
             $query->matching($query->logicalAnd(...$queryConstraints));
         }
         $query->setOrderings(['uid' => QueryInterface::ORDER_DESCENDING]);
@@ @@
         if (count($queryConstraints) === 1) {
             $query->matching(reset($queryConstraints));
         } elseif (count($queryConstraints) >= 2) {
+            if (count($queryConstraints) === 1) {
+                $query->matching(reset($queryConstraints));
+            } elseif (count($queryConstraints) >= 2) {
+                $query->matching($query->logicalAnd(...$queryConstraints));
+            }
             $query->matching($query->logicalAnd(...$queryConstraints));
         }
         $query->setOrderings(['uid' => QueryInterface::ORDER_DESCENDING]);

 /home/nikita/projects/typo3-rector/vendor/rector/rector/packages/Testing/PHPUnit/AbstractRectorTestCase.php:145
 /home/nikita/projects/typo3-rector/vendor/rector/rector/packages/Testing/PHPUnit/AbstractRectorTestCase.php:116
 /home/nikita/projects/typo3-rector/tests/Rector/v12/v0/typo3/HardenMethodSignatureOfLogicalAndAndLogicalOrRector/HardenMethodSignatureOfLogicalAndAndLogicalOrRectorTest.php:17

After composer update the test was green. I have not figured out yet, which dependency is responsible.

@nhovratov
Copy link
Contributor Author

Added a PR to showcase the error: #3929
Seems to be an error when running the rector multiple times.

@simonschaufi simonschaufi linked a pull request Dec 1, 2023 that will close this issue
@simonschaufi
Copy link
Collaborator

Fixed! This rule is super complex and I hope it works now as expected.

@nhovratov
Copy link
Contributor Author

Thanks! Yes, it is indeed. It works fine otherwise :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants