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

Ternary Operator and Null Coalescing Operator are not counted in Generic.Metrics.CyclomaticComplexity #3469

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

Comments

@MarkBaker
Copy link
Contributor

MarkBaker commented Nov 11, 2021

The Ternary and Null Coalescing Operator (and the Corresponding Null Coalescing Assignment Operator) are not taken into consideration when calculating complexity in the Generic.Metrics.CyclomaticComplexity sniff.

All three of these operators should increment the Cyclomatic Complexity score by 1, because there are two paths through the code each cases.

A ternary like

$result = ($condition) ? $value1 : $value2;

can be re-written as an if/else statement

if ($condition) {
    $result = $value1
} else {
    $result = $value2;
}

with two branches through the code, which should increment the CYC count by one.

Similarly, the Null Coalescing Operator

$result = $value1 ?? $value2;

can be written as

if ($value1 !== null) {
    $result = $value1
} else {
    $result = $value2;
}

while the Null Coalescing Assignment Operator

$value1 ??= $value1 ?? $value2;

can be written as

if ($value1 === null) {
    $value1 = $value2;
}

also two branches through the code, which should increment the CYC count by one in each case.

These cases can be resolved by adding the T_INLINE_THEN, T_COALESCE and T_COALESCE_EQUAL tokens to the list of tokens that increase CYC by one.

MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Nov 11, 2021
Add ternary operator, null coalescence operator and null coalescence assignment operator to tokens that trigger an increase in cyclomatic complexity
MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Nov 12, 2021
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
Add ternary operator, null coalescence operator and null coalescence assignment operator to tokens that trigger an increase in cyclomatic complexity
@gsherwood gsherwood added this to the 3.6.2 milestone Dec 3, 2021
MarkBaker pushed a commit to MarkBaker/PHP_CodeSniffer that referenced this issue Dec 3, 2021
…into CyclomaticComplexity calculation

This is for Issue squizlabs#3469, which identified that these operators were not being included in the count, when they should have been (treated as an if/else condition).

Unit tests provided.
@gsherwood gsherwood changed the title Ternary Operator and Null Coalescing Operator are not counted in the Generic.Metrics.CyclomaticComplexity sniff Ternary Operator and Null Coalescing Operator are not counted in Generic.Metrics.CyclomaticComplexity Dec 6, 2021
gsherwood pushed a commit that referenced this issue Dec 6, 2021
…into CyclomaticComplexity calculation

This is for Issue #3469, which identified that these operators were not being included in the count, when they should have been (treated as an if/else condition).

Unit tests provided.
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions-WikibaseLexeme that referenced this issue Mar 20, 2023
The newer version of codesniffer is counting the ternary operator now
into the complexity and that result in a complexity of 11
Move one try/catch into own function and do some more clean up

squizlabs/PHP_CodeSniffer#3469

Change-Id: I7a049a2ad12b2fa67793f448124204f000eed073
wmfgerrit pushed a commit to wikimedia/mediawiki-extensions that referenced this issue Mar 20, 2023
* Update WikibaseLexeme from branch 'master'
  to fba865d575069c35e2c9cc8d722eb6fb4bfdc300
  - Merge "AddSense: Reduce cyclomatic complexity"
  - AddSense: Reduce cyclomatic complexity
    
    The newer version of codesniffer is counting the ternary operator now
    into the complexity and that result in a complexity of 11
    Move one try/catch into own function and do some more clean up
    
    squizlabs/PHP_CodeSniffer#3469
    
    Change-Id: I7a049a2ad12b2fa67793f448124204f000eed073
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants