Complexity rule prefers and && over or ||

[edi9999]

• ESLint Version: 3.19.0
• Node Version: 7.7.3
• npm Version: 4.5.0

What parser (default, Babel-ESLint, etc.) are you using? : babel-eslint

Please show your full configuration:

{
  "parser": "babel-eslint",
  "rules": {
    "complexity": [2, 1]
  }
}

What did you do? Please include the actual source code causing the issue.

let foo, bar, baz, bang;

function check(foo, bar, baz, bang) {
	if (!bar || !bang) {
		baz = foo;
	}
}
function check2(foo, bar, baz, bang) {
	if (!(bar && bang)) {
		baz = foo;
	}
}
function check3(foo, bar, baz, bang) {
	if (bar && bang) {
		baz = foo;
	}
}
function check4(foo, bar, baz, bang) {
	if (!bar && !bang) {
		baz = foo;
	}
}
function check5(foo, bar, baz, bang) {
	if (bar || bang) {
		baz = foo;
	}
}

check(foo, bar, baz, bang);
check2(foo, bar, baz, bang);
check3(foo, bar, baz, bang);
check4(foo, bar, baz, bang);
check5(foo, bar, baz, bang);

What did you expect to happen?

All functions check to check5 should have the same complexity

What actually happened? Please include the actual, raw output from ESLint.

   3:1  error  Function 'check' has a complexity of 3   complexity
   8:1  error  Function 'check2' has a complexity of 2  complexity
  13:1  error  Function 'check3' has a complexity of 2  complexity
  18:1  error  Function 'check4' has a complexity of 2  complexity
  23:1  error  Function 'check5' has a complexity of 3  complexity

It seems that || has more complexity than && in the current implementation, but I don't think that is true. All the check have a complexity of 3. (That is what http://jsmeter.info/axtsgb/1#results reports)

[soda0289]

There seems to be hardcoded logic in the rule that well only increase complexity for ||:
https://github.com/eslint/eslint/blob/master/lib/rules/complexity.js#L138-L140

So it is working as intended.

I think the logic is saying that since an || could always be rewritten as a nested if statement it is more complex than and && statement. This is not true in the worse case scenario, where each condition must be checked, and I believe they should have the same complexity. Or at the very least update the documentation.

[not-an-aardvark]

Closing because this seems to be working as intended.

[edi9999]

No it is not :

See @soda0289 statement :

This is not true in the worse case scenario, where each condition must be checked, and I believe they should have the same complexity

Also according to Wikipedia :

McCabe showed that the cyclomatic complexity of any structured program with only one entrance point and one exit point is equal to the number of decision points (i.e., "if" statements or conditional loops) contained in that program plus one. However, this is true only for decision points counted at the lowest, machine-level instructions. Decisions involving compound predicates like those found in high-level languages like IF cond1 AND cond2 THEN ... should be counted in terms of predicate variables involved, i.e. in this example one should count two decision points, because at machine level it is equivalent to IF cond1 THEN IF cond2 THEN ....[2][4]

So this means that to count the complexity, we need to change our program to remove the "or" and "and", that would be :

function check(foo, bar, baz, bang) {
	if (!bar || !bang) {
		baz = foo;
	}
}

=>

function check(foo, bar, baz, bang) {
	if (!bar) {
           baz = foo
        } 
        if (!bang) {
             baz = foo;
	}
}

Complexitiy = 1 + number of if statements = 1+2 = 3

For check3 :

function check3(foo, bar, baz, bang) {
	if (bar && bang) {
		baz = foo;
	}
}

function check3(foo, bar, baz, bang) {
	if (bar) {
                if (bang) {
         		baz = foo;
                }
	}
}

Complexity = 1 + number of if statements = 3.

So please, reopen this issue :-)

[platinumazure]

I think I agree. Assuming a negation does not count towards cyclomatic complexity, by DeMorgan's law it is always the case that !(a && b) is equal to !a || !b, so those expressions (again, assuming we can ignore negation) should be equal in cyclomatic complexity.

I'm convinced this is a bug. @soda0289 @not-an-aardvark Okay if I reopen?

[not-an-aardvark]

Ok, sounds good.

[soda0289]

Do you think we should add an option to count or statements the same as and? That way if people preferred the old behavior they could always go back or fix it in the future.

[not-an-aardvark]

I just found this: jshint/jshint#840

If I had to guess, this bug exists because we were originally mimicing/copying the logic from JSHint, but JSHint also had a bug in the logic related to &&.

[not-an-aardvark]

Marking this accepted because I'm also now convinced that this is a bug.