phpcs-cognitive-complexity icon indicating copy to clipboard operation
phpcs-cognitive-complexity copied to clipboard

Case in switch should not increment nesting level

Open bkdotcom opened this issue 4 years ago • 4 comments

I don't know what this method's cognitive-complexity should be, but I know it shouldn't be 11

public function foo()
{
    $blah = array();
    for ($i = 0, $count = \count($this->things); $i < $count; $i++) {
        switch ($this->things[$i]['method']) {
            case 'foo':
            case 'bar':
                $blah[] = $i;
                break;
            case 'baz':
                \array_pop($blah);
                break;
            case 'ding':
            case 'dong':
                if ($this->something === false) {
                    break;
                }
                foreach ($blah as $i2) {
                    $this->things[$i2]['method'] = 'foo';
                }
                break;
        }
    }
}

9 maybe? ¯\_(ツ)_/¯

bkdotcom avatar Oct 22 '21 13:10 bkdotcom

Think the issue is that PHP CS tokenizer considers case a level and cognitive complexity doesn't. So stuff within case gets extra +1 from nesting each, as level is taken from PHP CS token, rather than tracked independently.

No immediate idea how to track this. There is tracking for try/catch, but switch/case syntax is a lot more elaborate.

Rarst avatar Oct 22 '21 15:10 Rarst

I have a solution... will create pull request soon... will also correctly handle nested trys

bkdotcom avatar Oct 24 '21 14:10 bkdotcom

Pull Request #4

This resolves both this issue and #2

I had trouble with the SniffTest unitTest The first half seemed to be a test of phpcs internals... The half I simply couldn't get to work.. I'm not sure what code was being sniffed... and the parser didnn't seem to be adding the "extended info" to the tokens... So... I took a look at how slevomat created their tests and borrowed from them

bkdotcom avatar Oct 26 '21 14:10 bkdotcom

Alternatively, Pull request #5 has been created It's like #4 but without modification to SniffTest (AnalyzerTest passes, but SniffTest fails)

bkdotcom avatar Oct 26 '21 16:10 bkdotcom