Skip to content

Commit

Permalink
JS-255 Restore cognitive complexity calculation at file level
Browse files Browse the repository at this point in the history
  • Loading branch information
yassin-kammoun-sonarsource committed Sep 19, 2024
1 parent bec1017 commit 2d4deac
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 48 deletions.
243 changes: 200 additions & 43 deletions packages/jsts/src/rules/S3776/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
getFirstTokenAfter,
getJsxShortCircuitNodes,
getMainFunctionTokenLocation,
isArrowFunctionExpression,
isIfStatement,
isLogicalExpression,
IssueLocation,
Expand All @@ -47,12 +48,7 @@ type LoopStatement =
| TSESTree.DoWhileStatement
| TSESTree.WhileStatement;

interface ScopeComplexity {
node: TSESTree.Program | TSESTree.FunctionLike;
nestingLevel: number;
nestingNodes: Set<TSESTree.Node>;
complexityPoints: ComplexityPoint[];
}
type OptionalLocation = TSESTree.SourceLocation | null | undefined;

const message =
'Refactor this function to reduce its Cognitive Complexity from {{complexityAmount}} to the {{threshold}} allowed.';
Expand All @@ -76,11 +72,60 @@ export const rule: Rule.RuleModule = {
/** Indicator if the file complexity should be reported */
const isFileComplexity = context.options.includes('metric');

/** Complexity of the file */
let fileComplexity = 0;

/** Complexity of the current function if it is *not* considered nested to the first level function */
let complexityIfNotNested: ComplexityPoint[] = [];

/** Complexity of the current function if it is considered nested to the first level function */
let complexityIfNested: ComplexityPoint[] = [];

/** Current nesting level (number of enclosing control flow statements and functions) */
let nesting = 0;

/** Indicator if the current top level function has a structural (generated by control flow statements) complexity */
let topLevelHasStructuralComplexity = false;

/** Indicator if the current top level function is React functional component */
const reactFunctionalComponent = {
nameStartsWithCapital: false,
returnsJsx: false,

isConfirmed() {
return this.nameStartsWithCapital && this.returnsJsx;
},

init(node: TSESTree.FunctionLike) {
this.nameStartsWithCapital = nameStartsWithCapital(node);
this.returnsJsx = false;
},
};

/** Own (not including nested functions) complexity of the current top function */
let topLevelOwnComplexity: ComplexityPoint[] = [];

/** Nodes that should increase nesting level */
const nestingNodes: Set<TSESTree.Node> = new Set();

/** Set of already considered (with already computed complexity) logical expressions */
const consideredLogicalExpressions: Set<TSESTree.Node> = new Set();

/** Stack of scopes that are either functions or the program */
const scopes: ScopeComplexity[] = [];
/** Stack of enclosing functions */
const enclosingFunctions: TSESTree.FunctionLike[] = [];

/** Stack of complexity points for each function without accumulated nested complexity */
const functionOwnComplexity: ComplexityPoint[][] = [];

const functionOwnControlFlowNesting: number[] = [];

let secondLevelFunctions: Array<{
node: TSESTree.FunctionLike;
parent: TSESTree.Node | undefined;
complexityIfThisSecondaryIsTopLevel: ComplexityPoint[];
complexityIfNested: ComplexityPoint[];
loc: OptionalLocation;
}> = [];

return {
':function': (node: estree.Node) => {
Expand All @@ -90,37 +135,32 @@ export const rule: Rule.RuleModule = {
onLeaveFunction(node as TSESTree.FunctionLike);
},
'*'(node: estree.Node) {
if (scopes[scopes.length - 1]?.nestingNodes.has(node as TSESTree.Node)) {
scopes[scopes.length - 1].nestingLevel++;
if (nestingNodes.has(node as TSESTree.Node)) {
nesting++;
if (functionOwnControlFlowNesting.length > 0) {
functionOwnControlFlowNesting[functionOwnControlFlowNesting.length - 1]++;
}
}
},
'*:exit'(node: estree.Node) {
if (scopes[scopes.length - 1]?.nestingNodes.has(node as TSESTree.Node)) {
scopes[scopes.length - 1].nestingLevel--;
scopes[scopes.length - 1].nestingNodes.delete(node as TSESTree.Node);
if (nestingNodes.has(node as TSESTree.Node)) {
nesting--;
nestingNodes.delete(node as TSESTree.Node);
if (functionOwnControlFlowNesting.length > 0) {
functionOwnControlFlowNesting[functionOwnControlFlowNesting.length - 1]--;
}
}
},
Program(node: estree.Program) {
scopes.push({
node: node as TSESTree.Program,
nestingLevel: 0,
nestingNodes: new Set(),
complexityPoints: [],
});
Program() {
fileComplexity = 0;
},
'Program:exit'(node: estree.Node) {
const programComplexity = scopes.pop()!;
if (isFileComplexity) {
// value from the message will be saved in SonarQube as file complexity metric
context.report({
node,
messageId: 'fileComplexity',
data: {
complexityAmount: programComplexity.complexityPoints.reduce(
(acc, cur) => acc + cur.complexity,
0,
) as any,
},
data: { complexityAmount: fileComplexity as any },
});
}
},
Expand Down Expand Up @@ -160,16 +200,73 @@ export const rule: Rule.RuleModule = {
ConditionalExpression(node: estree.Node) {
visitConditionalExpression(node as TSESTree.ConditionalExpression);
},
ReturnStatement(node: estree.Node) {
visitReturnStatement(node as TSESTree.ReturnStatement);
},
};

function onEnterFunction(node: TSESTree.FunctionLike) {
scopes.push({ node, nestingLevel: 0, nestingNodes: new Set(), complexityPoints: [] });
if (enclosingFunctions.length === 0) {
// top level function
topLevelHasStructuralComplexity = false;
reactFunctionalComponent.init(node);
topLevelOwnComplexity = [];
secondLevelFunctions = [];
} else if (enclosingFunctions.length === 1) {
// second level function
complexityIfNotNested = [];
complexityIfNested = [];
} else {
nesting++;
nestingNodes.add(node);
}

enclosingFunctions.push(node);
functionOwnComplexity.push([]);
functionOwnControlFlowNesting.push(0);
}

function onLeaveFunction(node: TSESTree.FunctionLike) {
const functionComplexity = scopes.pop()!;
const functionComplexity = functionOwnComplexity.pop();
functionOwnControlFlowNesting.pop();

enclosingFunctions.pop();
if (enclosingFunctions.length === 0) {
// top level function
if (topLevelHasStructuralComplexity && !reactFunctionalComponent.isConfirmed()) {
let totalComplexity = topLevelOwnComplexity;
secondLevelFunctions.forEach(secondLevelFunction => {
totalComplexity = totalComplexity.concat(secondLevelFunction.complexityIfNested);
});

fileComplexity += totalComplexity.reduce((acc, cur) => acc + cur.complexity, 0);
} else {
fileComplexity += topLevelOwnComplexity.reduce((acc, cur) => acc + cur.complexity, 0);

secondLevelFunctions.forEach(secondLevelFunction => {
fileComplexity += secondLevelFunction.complexityIfThisSecondaryIsTopLevel.reduce(
(acc, cur) => acc + cur.complexity,
0,
);
});
}
} else if (enclosingFunctions.length === 1) {
// second level function
secondLevelFunctions.push({
node,
parent: node.parent,
complexityIfNested,
complexityIfThisSecondaryIsTopLevel: complexityIfNotNested,
loc: getMainFunctionTokenLocation(node, node.parent, context as unknown as RuleContext),
});
}

if (isFileComplexity) {
return;
}

checkFunction(
functionComplexity.complexityPoints,
functionComplexity,
getMainFunctionTokenLocation(node, node.parent, context as unknown as RuleContext),
);
}
Expand All @@ -185,13 +282,13 @@ export const rule: Rule.RuleModule = {
}

// always increase nesting level inside `then` statement
scopes[scopes.length - 1].nestingNodes.add(ifStatement.consequent);
nestingNodes.add(ifStatement.consequent);

// if `else` branch is not `else if` then
// - increase nesting level inside `else` statement
// - add +1 complexity
if (ifStatement.alternate && !isIfStatement(ifStatement.alternate)) {
scopes[scopes.length - 1].nestingNodes.add(ifStatement.alternate);
nestingNodes.add(ifStatement.alternate);
const elseTokenLoc = getFirstTokenAfter(
ifStatement.consequent,
context as unknown as RuleContext,
Expand All @@ -202,15 +299,15 @@ export const rule: Rule.RuleModule = {

function visitLoop(loop: LoopStatement) {
addStructuralComplexity(getFirstToken(loop, context as unknown as RuleContext).loc);
scopes[scopes.length - 1].nestingNodes.add(loop.body);
nestingNodes.add(loop.body);
}

function visitSwitchStatement(switchStatement: TSESTree.SwitchStatement) {
addStructuralComplexity(
getFirstToken(switchStatement, context as unknown as RuleContext).loc,
);
for (const switchCase of switchStatement.cases) {
scopes[scopes.length - 1].nestingNodes.add(switchCase);
nestingNodes.add(switchCase);
}
}

Expand All @@ -224,7 +321,7 @@ export const rule: Rule.RuleModule = {

function visitCatchClause(catchClause: TSESTree.CatchClause) {
addStructuralComplexity(getFirstToken(catchClause, context as unknown as RuleContext).loc);
scopes[scopes.length - 1].nestingNodes.add(catchClause.body);
nestingNodes.add(catchClause.body);
}

function visitConditionalExpression(conditionalExpression: TSESTree.ConditionalExpression) {
Expand All @@ -233,8 +330,37 @@ export const rule: Rule.RuleModule = {
context as unknown as RuleContext,
)!.loc;
addStructuralComplexity(questionTokenLoc);
scopes[scopes.length - 1].nestingNodes.add(conditionalExpression.consequent);
scopes[scopes.length - 1].nestingNodes.add(conditionalExpression.alternate);
nestingNodes.add(conditionalExpression.consequent);
nestingNodes.add(conditionalExpression.alternate);
}

function visitReturnStatement({ argument }: TSESTree.ReturnStatement) {
// top level function
if (
enclosingFunctions.length === 1 &&
argument &&
['JSXElement', 'JSXFragment'].includes(argument.type as any)
) {
reactFunctionalComponent.returnsJsx = true;
}
}

function nameStartsWithCapital(node: TSESTree.FunctionLike) {
const checkFirstLetter = (name: string) => {
const firstLetter = name[0];
return firstLetter === firstLetter.toUpperCase();
};

if (!isArrowFunctionExpression(node) && node.id) {
return checkFirstLetter(node.id.name);
}

const { parent } = node;
if (parent && parent.type === 'VariableDeclarator' && parent.id.type === 'Identifier') {
return checkFirstLetter(parent.id.name);
}

return false;
}

function visitLogicalExpression(logicalExpression: TSESTree.LogicalExpression) {
Expand Down Expand Up @@ -301,20 +427,51 @@ export const rule: Rule.RuleModule = {
}

function addStructuralComplexity(location: TSESTree.SourceLocation) {
const added = scopes[scopes.length - 1].nestingLevel + 1;
const added = nesting + 1;
const complexityPoint = { complexity: added, location };
scopes[scopes.length - 1].complexityPoints.push(complexityPoint);
if (enclosingFunctions.length === 0) {
// top level scope
fileComplexity += added;
} else if (enclosingFunctions.length === 1) {
// top level function
topLevelHasStructuralComplexity = true;
topLevelOwnComplexity.push(complexityPoint);
} else {
// second+ level function
complexityIfNested.push({ complexity: added + 1, location });
complexityIfNotNested.push(complexityPoint);
}

if (functionOwnComplexity.length > 0) {
const addedWithoutFunctionNesting =
functionOwnControlFlowNesting[functionOwnControlFlowNesting.length - 1] + 1;
functionOwnComplexity[functionOwnComplexity.length - 1].push({
complexity: addedWithoutFunctionNesting,
location,
});
}
}

function addComplexity(location: TSESTree.SourceLocation) {
const complexityPoint = { complexity: 1, location };
scopes[scopes.length - 1].complexityPoints.push(complexityPoint);
if (functionOwnComplexity.length > 0) {
functionOwnComplexity[functionOwnComplexity.length - 1].push(complexityPoint);
}

if (enclosingFunctions.length === 0) {
// top level scope
fileComplexity += 1;
} else if (enclosingFunctions.length === 1) {
// top level function
topLevelOwnComplexity.push(complexityPoint);
} else {
// second+ level function
complexityIfNested.push(complexityPoint);
complexityIfNotNested.push(complexityPoint);
}
}

function checkFunction(complexity: ComplexityPoint[] = [], loc: TSESTree.SourceLocation) {
if (isFileComplexity) {
return;
}
const complexityAmount = complexity.reduce((acc, cur) => acc + cur.complexity, 0);
if (complexityAmount > threshold) {
const secondaryLocations: IssueLocation[] = complexity.map(complexityPoint => {
Expand Down
2 changes: 1 addition & 1 deletion packages/jsts/src/rules/S3776/unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ class TopLevel {
}
`,
options: [0, 'metric'],
errors: [{ messageId: 'fileComplexity', data: { complexityAmount: 5 } }],
errors: [{ messageId: 'fileComplexity', data: { complexityAmount: 25 } }],
},
],
});
Expand Down
2 changes: 1 addition & 1 deletion packages/jsts/tests/analysis/analyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ describe('analyzeJSTS', () => {
],
metrics: {
classes: 1,
cognitiveComplexity: 0,
cognitiveComplexity: 1,
commentLines: [2],
complexity: 2,
executableLines: [4],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ <h3>Which syntax in code does impact cognitive complexity score?</h3>
apply to recursive calls, those will increment cognitive score. </li>
</ul>
<p>The method of computation is fully detailed in the pdf linked in the resources.</p>
<p>Note that the calculation of cognitive complexity at function level deviates from the documented process. Given the functional nature of
JavaScript, nesting functions is a prevalent practice, especially within frameworks like React.js. Consequently, the cognitive complexity of functions
remains independent from one another. This means that the complexity of a nesting function does not increase with the complexity of nested
functions.</p>
<h3>What is the potential impact?</h3>
<p>Developers spend more time reading and understanding code than writing it. High cognitive complexity slows down changes and increases the cost of
maintenance.</p>
Expand All @@ -39,9 +43,6 @@ <h2>How to fix it</h2>
<li> <strong>Use null-safe operations (if available in the language).</strong><br> When available the <code>.?</code> or <code>??</code> operator
replaces multiple tests and simplifies the flow. </li>
</ul>
<p>Note that the calculation of cognitive complexity deviates from the documented process when functions are nested. Given the functional nature of
JavaScript, nesting functions is a prevalent practice, especially within frameworks like React.js. Consequently, the cognitive complexity of functions
remains independent of each other.</p>
<h3>Code examples</h3>
<p><strong>Extraction of a complex condition in a new function.</strong></p>
<h4>Noncompliant code example</h4>
Expand Down

0 comments on commit 2d4deac

Please sign in to comment.