Option to complexity to reduce impact of switch statements

[Brian-Bartels]

What rule do you want to change?
complexity
Does this change cause the rule to produce more or fewer warnings?
yes
How will the change be implemented? (New option, new default behavior, etc.)?
A boolean option (optional) to complexity that changes switch statements from incrementing complexity by n number of cases to just incrementing it one
Please provide some example code that this change will affect:

switch(expression) {
  case x:
    // code block
    break;
  case y:
    // code block
    break;
  default:
    // code block
}

What does the rule currently do for this code?
returns a complexity of 3
What will the rule do after it's changed?
Return a complexity of 1
Are you willing to submit a pull request to implement this change?
Yes

[mdjermanovic]

Hi @Brian-Bartels, thanks for the issue!

This rule aims to work per the definition of Cyclomatic complexity, so in my opinion it shouldn't have any options that would change the measuring, but I'll defer to consensus if other team members have a different opinion.

[Brian-Bartels]

I understand that the rule is targeting a specific definition of complexity. There are many others available that also exist such as maintainability index. This suggestion is a targeted one, to resolve one of the main drawbacks of cyclomatic complexity. I have spent the last week thinking hard about how best to enforce maintainability of a large code base that I work on. I feel like this could be an easy win to improve the trust developers have in ESLint violations. My preferred solution would be to create an aggregate rule (IE only error when a function has too many lines, and too much complexity). But I ran into too many road blocks :(

[Brian-Bartels]

@mdjermanovic Does my above explanation change your opinion at all?

[mdjermanovic]

I think it's easy enough to create a custom rule based on the code from the core complexity rule and tweak it to get the desired behavior. If we add an option for switch statements, we could end up adding options for each construct, which would defy the purpose of this rule (which is to measure cyclomatic complexity).

That said, I won't be opposed if other team members are in favor of adding this particular option.

[Brian-Bartels]

Are rules extensible? Such that I am able to tweak the functionality of complexity without completely copy pasting the whole file?

[mdjermanovic]

Are rules extensible? Such that I am able to tweak the functionality of complexity without completely copy pasting the whole file?

It's technically possible to directly extend a core rule, but we do not recommend that approach. Rule implementations are not public API, so an extension rule that relies on the current interface of the core rule (e.g., on the set of listeners returned by create()) might break without notice.

For example, these are listeners in the current version of the complexity rule:

eslint/lib/rules/complexity.js

Lines 137 to 161 in 4338b74

	return {
	FunctionDeclaration: startFunction,
	FunctionExpression: startFunction,
	ArrowFunctionExpression: startFunction,
	"FunctionDeclaration:exit": endFunction,
	"FunctionExpression:exit": endFunction,
	"ArrowFunctionExpression:exit": endFunction,
	CatchClause: increaseComplexity,
	ConditionalExpression: increaseComplexity,
	LogicalExpression: increaseComplexity,
	ForStatement: increaseComplexity,
	ForInStatement: increaseComplexity,
	ForOfStatement: increaseComplexity,
	IfStatement: increaseComplexity,
	SwitchCase: increaseSwitchComplexity,
	WhileStatement: increaseComplexity,
	DoWhileStatement: increaseComplexity,
	AssignmentExpression(node) {
	if (astUtils.isLogicalAssignmentOperator(node.operator)) {
	increaseComplexity();
	}
	}
	};

and this is how they might look like after an upcoming change (#14957):

eslint/lib/rules/complexity.js

Lines 93 to 153 in 551e613

	return {
	onCodePathStart() {
	// The initial complexity is 1, representing one execution path in the CodePath
	complexities.push(1);
	},
	// Each branching in the code adds 1 to the complexity
	CatchClause: increaseComplexity,
	ConditionalExpression: increaseComplexity,
	LogicalExpression: increaseComplexity,
	ForStatement: increaseComplexity,
	ForInStatement: increaseComplexity,
	ForOfStatement: increaseComplexity,
	IfStatement: increaseComplexity,
	WhileStatement: increaseComplexity,
	DoWhileStatement: increaseComplexity,
	// Avoid `default`
	"SwitchCase[test]": increaseComplexity,
	// Logical assignment operators have short-circuiting behavior
	AssignmentExpression(node) {
	if (astUtils.isLogicalAssignmentOperator(node.operator)) {
	increaseComplexity();
	}
	},
	onCodePathEnd(codePath, node) {
	const complexity = complexities.pop();
	/*
	* This rule only evaluates complexity of functions, so "program" is excluded.
	* Class field initializers are implicit functions. Therefore, they shouldn't contribute
	* to the enclosing function's complexity, but their own complexity should be evaluated.
	*/
	if (
	codePath.origin !== "function" &&
	codePath.origin !== "class-field-initializer"
	) {
	return;
	}
	if (complexity > THRESHOLD) {
	const name = codePath.origin === "class-field-initializer"
	? "class field initializer"
	: astUtils.getFunctionNameWithKind(node);
	context.report({
	node,
	messageId: "complex",
	data: {
	name: upperCaseFirst(name),
	complexity,
	max: THRESHOLD
	}
	});
	}
	}
	};

This change is scheduled for a major release, but the same kind of changes can happen in semver-minor or even semver-patch upgrades.

Therefore, we recommend copy & paste & modify OR copy & paste & extend from the copy. The latter approach seems like a bit more work, but it could make synchronizing the extension rule with the original rule easier.

[nzakas]

I’m in agreement with @mdjermanovic — I don’t think it makes sense to alter the meaning of cyclomatic complexity in this rule. Creating a custom rule to cover your case is the best approach.