Cyclomatic complexity calculation often wrong

[abelbraaksma]

I've often wondered why my code had a lot of orange squigglies with "This binding has a cyclomatic complexity of X, expecting a cyclomatic complexity of less than 10". I just thought I had a lot of dependencies, but then it occurred to me that these warnings occur on code that have zero dependencies.

Since I now have a repro, I decided to share it.

Repro steps

Here's an example that, when you copy it in VS 2015, shows a cyclomatic complexity of 73 (yes, it is ugly, it is autogenerated, but it helps as an example):

let private encodingTest c = 
    c <= 0xa0 || c = 0xA4 || c = 0xA7 || c = 0xA8 || c = 0xAD || c = 0xB0 || c = 0xB4 || c = 0xB8 || c = 0xC1 || c = 0xC2 || c = 0xC4 || c = 0xC7 || c = 0xC9 || c = 0xCB || c = 0xCD || c = 0xCE 
    || c = 0xD3 || c = 0xD4 || c = 0xD6 || c = 0xD7 || c = 0xDA || c = 0xDC || c = 0xDD || c = 0xDF || c = 0xE1 || c = 0xE2 || c = 0xE4 || c = 0xE7 || c = 0xE9 || c = 0xEB || c = 0xED || c = 0xEE 
    || c = 0xF3 || c = 0xF4 || c = 0xF6 || c = 0xF7 || c = 0xFA || c = 0xFC || c = 0xFD || (c >= 0x102 && c <= 0x107) || (c >= 0x10C && c <= 0x111) || (c >= 0x118 && c <= 0x11B) 
    || c = 0x139 || c = 0x13A || c = 0x13D || c = 0x13E || (c >= 0x141 && c <= 0x144) || c = 0x147 || c = 0x148 || c = 0x150 || c = 0x151 || c = 0x154 || c = 0x155 || c = 0x158 || c = 0x159 || c = 0x15A || c = 0x15B 
    || (c >= 0x15E && c <= 0x165) || c = 0x16E || c = 0x16F || c = 0x170 || c = 0x171 || (c >= 0x179 && c <= 0x17E) || c = 0x2C7 || c = 0x2D8 || c = 0x2D9 || c = 0x2DB || c = 0x2DD

I suspected that the count of 73 had to do with the number of || in the boolean expression, so I wondered what'd happen if I split it up:

let private encodingTest c = 
    let x1 = c <= 0xa0 || c = 0xA4 || c = 0xA7 || c = 0xA8 || c = 0xAD || c = 0xB0 || c = 0xB4 || c = 0xB8 || c = 0xC1 || c = 0xC2 || c = 0xC4 || c = 0xC7 || c = 0xC9 || c = 0xCB || c = 0xCD || c = 0xCE 
    let x2 = c = 0xD3 || c = 0xD4 || c = 0xD6 || c = 0xD7 || c = 0xDA || c = 0xDC || c = 0xDD || c = 0xDF || c = 0xE1 || c = 0xE2 || c = 0xE4 || c = 0xE7 || c = 0xE9 || c = 0xEB || c = 0xED || c = 0xEE 
    let x3 = c = 0xF3 || c = 0xF4 || c = 0xF6 || c = 0xF7 || c = 0xFA || c = 0xFC || c = 0xFD || (c >= 0x102 && c <= 0x107) || (c >= 0x10C && c <= 0x111) || (c >= 0x118 && c <= 0x11B) 
    let x4 = c = 0x139 || c = 0x13A || c = 0x13D || c = 0x13E || (c >= 0x141 && c <= 0x144) || c = 0x147 || c = 0x148 || c = 0x150 || c = 0x151 || c = 0x154 || c = 0x155 || c = 0x158 || c = 0x159 || c = 0x15A || c = 0x15B 
    let x5 = (c >= 0x15E && c <= 0x165) || c = 0x16E || c = 0x16F || c = 0x170 || c = 0x171 || (c >= 0x179 && c <= 0x17E) || c = 0x2C7 || c = 0x2D8 || c = 0x2D9 || c = 0x2DB || c = 0x2DD
    x1 || x2 || x3 || x4 || x5

Still shows 73. If I remove any of the || the complexity goes down with one.

If I rewrite it as follows, the warning drops from 73 to a complexity level of 13, which is surprising, because one could argue that there actually is a higher cyclomatic complexity than before (though i would argue it is 2 in this case). Note that again the || and && counted towards the complexity, not the |> orAnd. Remove them and the complexity count becomes 1.

let private encodingTest (c: int) : bool = 
    let orAnd b a = a || (c = b)
    c <= 0xa0 
    |> orAnd 0xA7 
    |> orAnd 0xA8 
    |> orAnd 0xAD 
    |> orAnd 0xB0 
    |> orAnd 0xB4 
    |> orAnd 0xB8 
    |> orAnd 0xC1 
    |> orAnd 0xC2 
    |> orAnd 0xC4 
    |> orAnd 0xC7 
    |> orAnd 0xC9 
    |> orAnd 0xCB 
    |> orAnd 0xCD 
    |> orAnd 0xCE 

    |> orAnd 0xD3 
    |> orAnd 0xD4 
    |> orAnd 0xD6 
    |> orAnd 0xD7 
    |> orAnd 0xDA 
    |> orAnd 0xDC 
    |> orAnd 0xDD 
    |> orAnd 0xDF 
    |> orAnd 0xE1 
    |> orAnd 0xE2 
    |> orAnd 0xE4 
    |> orAnd 0xE7 
    |> orAnd 0xE9 
    |> orAnd 0xEB 
    |> orAnd 0xED 
    |> orAnd 0xEE 

    |> orAnd 0xF3 
    |> orAnd 0xF4 
    |> orAnd 0xF6 
    |> orAnd 0xF7 
    |> orAnd 0xFA 
    |> orAnd 0xFC 
    |> orAnd 0xFD || (c >= 0x102 && c <= 0x107) || (c >= 0x10C && c <= 0x111) || (c >= 0x118 && c <= 0x11B) 

    |> orAnd 0x139 
    |> orAnd 0x13A 
    |> orAnd 0x13D 
    |> orAnd 0x13E || (c >= 0x141 && c <= 0x144) 
    |> orAnd 0x147 
    |> orAnd 0x148 
    |> orAnd 0x150 
    |> orAnd 0x151 
    |> orAnd 0x154 
    |> orAnd 0x155 
    |> orAnd 0x158 
    |> orAnd 0x159 
    |> orAnd 0x15A 
    |> orAnd 0x15B || (c >= 0x15E && c <= 0x165) 
    |> orAnd 0x16E 
    |> orAnd 0x16F 
    |> orAnd 0x170 
    |> orAnd 0x171 || (c >= 0x179 && c <= 0x17E) 
    |> orAnd 0x2C7 
    |> orAnd 0x2D8 
    |> orAnd 0x2D9 
    |> orAnd 0x2DB 
    |> orAnd 0x2DD

Even more suprisingly (or maybe not, I don't know), if you use the following syntax, the complexity goes up again:

let private encodingTest c = 
    let orAnd b a = a || (c = b)
    c <= 0xa0 
    |> ((||) (c = 0xA7))   // counts towards complexity with +2
    |> ((||) (c = 0xA7))
    |> ((||) (c = 0xA7))
    |> ((||) (c = 0xA7))
    |> orAnd 0xA8 
    |> orAnd 0xAD 
    .....

Expected behavior

There is no cyclomatic complexity in this case, it should be 1.

Actual behavior

As can be seen above: the cyclomatic complexity is calculated way too high.

Known workarounds

None that I know of, unless of course to write the code differently. While the above is an extreme example, I am wondering how the calculation is done, because this warning happens a lot even in cases where there are no (inter)dependencies within the code.

Sometimes it gets it right as well, of course, in which case it is a good warning too have.

Related information

Tested with VS 2015 Update 3 and latest version of F# PowerPack.

[vasily-kirichenko]

We just use FSharpLint library for this, please report your issue here https://github.com/fsprojects/FSharpLint

[duckmatt]

@vasily-kirichenko could we update the linter? This rule no longer exists + the default config is now much saner

The last scenario (+2) looks like a bug, but the first and and second scenarios look as expected, examples like this is why the rule is gone ;)

[vasily-kirichenko]

@duckmatt ok, I will update everything.

[abelbraaksma]

this is why the rule is gone ;)

Do you mean to say that this rule is supposed to be off by default? Or permanently switched off?

[abelbraaksma]

but the first and and second scenarios look as expected,

From your reference doc I understand that a || b && c || d should be 3. But I don't understand why:

let a = b || c
let d = e || f
a && d

also counts as 3. As I'd say there are three blocks of code here, each with a complexity of 1. But as can be seen in my 2nd example, the count just counts up.

But it is true that I have misunderstood, at least in part, what cyclomatic complexity is supposed to mean. I thought it dealt with inter-function or class dependencies (aka as coupling), but I was wrong to assume that.

[vasily-kirichenko]

https://github.com/fsprojects/VisualFSharpPowerTools/pull/1472

[duckmatt]

Do you mean to say that this rule is supposed to be off by default? Or permanently switched off?

The most recent version of the linter contains no implementation of the rule

The cyclomatic complexity was being counted on a per function basis, so you're right in that each block would have a CC of 1, but the linter would add the three blocks together to get the CC of the function. The rule you thought it was sounds useful :)

@vasily-kirichenko thank you

[vasily-kirichenko]

Version 2.5.4 was just released with fresh FSharpLint.