Skip to content

Generic/CyclomaticComplexity: should nested structures be skipped ? #726

Open
@jrfnl

Description

@jrfnl

Describe the bug

Inspired by #684

I believe the Generic.Metrics.CyclomaticComplexity is incorrectly attributing complexity in nested, closed structures to the "outer" function.

Code sample

function hasNestedAnonFunction()
{
    while ($condition === true) {
        if ($condition) {
        } else if ($cond) {
        }
    }

    $callback = function($conditionA, $conditionB) {
        switch ($condition) {
            case '1':
                if ($condition) {
                } else if ($cond) {
                }
            break;
            case '2':
                while ($cond) {
                    echo 'hi';
                }
            break;
            case '3':
            break;
            default:
            break;
        }
    };

    $callback($conditionA, $conditionB);
}

function hasNestedAnonClass()
{
    $obj = new class() {
        function complexityEleven()
        {
            while ($condition === true) {
                if ($condition) {
                } else if ($cond) {
                }
            }

            switch ($condition) {
                case '1':
                    if ($condition) {
                    } else if ($cond) {
                    }
                break;
                case '2':
                    while ($cond) {
                        echo 'hi';
                    }
                break;
                case '3':
                break;
                default:
                break;
            }
        }
    };

    return $obj;
}

To reproduce

Steps to reproduce the behavior:

  1. Create a file called test.php with the code sample above...
  2. Run phpcs test.php --standard=Generic --sniffs=Generic.Metrics.CyclomaticComplexity
  3. See error message displayed
---------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
---------------------------------------------------------------------------------------------------------------------------------------------------
  3 | WARNING | Function's cyclomatic complexity (11) exceeds 10; consider refactoring the function (Generic.Metrics.CyclomaticComplexity.TooHigh)
 33 | WARNING | Function's cyclomatic complexity (11) exceeds 10; consider refactoring the function (Generic.Metrics.CyclomaticComplexity.TooHigh)
 36 | WARNING | Function's cyclomatic complexity (11) exceeds 10; consider refactoring the function (Generic.Metrics.CyclomaticComplexity.TooHigh)
---------------------------------------------------------------------------------------------------------------------------------------------------

Expected behavior

  • No error for line 3.
  • No error for line 33.

I think the sniff should also search for T_FUNCTION, T_CLOSURE, T_ANON_CLASS tokens (and possibly more) and skip over the body of these when these tokens are encountered, so as not to count complexity in a nested closed structure as complexity to be attributed to the "outer" structure.

Versions (please complete the following information)

Operating System not relevant
PHP version not relevant
PHP_CodeSniffer version master
Standard Generic
Install type not relevant

Additional context

Before addressing this issue, it should be confirmed that the proposed new behaviour for the sniff is in line with the official definition of Cyclomatic Complexity.

Please confirm

  • I have searched the issue list and am not opening a duplicate issue.
  • I have read the Contribution Guidelines and this is not a support question.
  • I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
  • I have verified the issue still exists in the master branch of PHP_CodeSniffer.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions