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

do/while loops are double-counted in Generic.Metrics.CyclomaticComplexity #3468

Closed
MarkBaker opened this issue Nov 11, 2021 · 0 comments
Closed

Comments

@MarkBaker
Copy link
Contributor

MarkBaker commented Nov 11, 2021

Both while loops and do/while loops should increment the Cyclomatic Complexity score by one, because there are two paths through the code in both cases.

In the case of a while loop, the while code block will only be executed if the while condition is met, otherwise code execution will branch over the while code block. This gives just two branches in the code.

while ($condition) {            |\
    ...                         | \
    ...                         | /
}                               |<
                                v

When calculating the score, the T_WHILE token is recognised, and increases the CYC value correctly by one.

For a do/while loop, the do code block will always be executed at least once, and code execution may then branch back to repeat execution of that block, or may continue on through the code based on the while condition. This also has only two branches in the code.

do {                            |<
    ...                         | \
    ...                         | /
} while ($condition);           |/
                                |
                                v

However, when calculating the score, the sniff checks for both the T_DO and the terminating T_WHILE tokens, incrementing CYC by two (once for each token) in the do/while loop, rather than just by one (as it should).

This double-count error can easily be eliminated by removing the increment for the T_DO token.

Note that some the existing tests also erroneously assume that do/while should increment the CYC count by two, so they also require modification.

MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Nov 11, 2021
Resolve double-increment of CYC count for do/while loop
MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Nov 12, 2021
Resolve double-increment of CYC count for do/while loop

Refactor unit test file into a numbered file. This should make it easier to have several test cases for different aspects of CYC counting without merge conflicts that would occur with a single file
MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Nov 12, 2021
Resolve double-increment of CYC count for do/while loop

Refactor unit test file into a numbered file. This should make it easier to have several test cases for different aspects of CYC counting without merge conflicts that would occur with a single file
MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Dec 3, 2021
…le loop

This is a bugfix for Issue squizlabs#3468, which identified that the count was being incremented twice when calculating the score because the sniff was checking for both the T_DO and the terminating T_WHILE tokens, incrementing CYC by two (once for each token) in the do/while loop, rather than just by one (as it should).

Code in the unit test .inc file has ben modified to reflect this change, and the test php file has also been modified to reflect the change in error line numbers.
@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Dec 5, 2021
@gsherwood gsherwood added this to the 3.6.2 milestone Dec 5, 2021
@gsherwood gsherwood changed the title do/while loops are double-counted in the Generic.Metrics.CyclomaticComplexity sniff do/while loops are double-counted in Generic.Metrics.CyclomaticComplexity Dec 6, 2021
gsherwood pushed a commit that referenced this issue Dec 6, 2021
…le loop

This is a bugfix for Issue #3468, which identified that the count was being incremented twice when calculating the score because the sniff was checking for both the T_DO and the terminating T_WHILE tokens, incrementing CYC by two (once for each token) in the do/while loop, rather than just by one (as it should).

Code in the unit test .inc file has ben modified to reflect this change, and the test php file has also been modified to reflect the change in error line numbers.
PHPCS v3 Development automation moved this from Idea Bank to Ready for Release Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PHPCS v3 Development
Ready for Release
Development

No branches or pull requests

2 participants