Skip to content

Commit

Permalink
Improve S3776: Exclude complexity of JSX short-circuits (#377)
Browse files Browse the repository at this point in the history
  • Loading branch information
francoismora committed Dec 5, 2022
1 parent 034ecc7 commit f757f28
Show file tree
Hide file tree
Showing 4 changed files with 235 additions and 57 deletions.
9 changes: 0 additions & 9 deletions ruling/snapshots/cognitive-complexity
Expand Up @@ -472,7 +472,6 @@ src/reveal.js/plugin/zoom-js/zoom.js: 34
src/socket.io/lib/index.js: 69,143
src/spectrum/admin/src/helpers/utils.js: 52
src/spectrum/admin/src/views/communities/components/search/index.js: 86
src/spectrum/admin/src/views/dashboard/components/coreMetrics.js: 114
src/spectrum/admin/src/views/users/components/search/index.js: 92
src/spectrum/api/authentication.js: 274
src/spectrum/api/models/community.js: 168,330
Expand Down Expand Up @@ -503,16 +502,13 @@ src/spectrum/shared/graphql/queries/thread/getThreadMessageConnection.js: 78
src/spectrum/src/components/avatar/hoverProfile.js: 44
src/spectrum/src/components/chatInput/index.js: 152
src/spectrum/src/components/composer/index.js: 131
src/spectrum/src/components/draftjs-editor/index.js: 177
src/spectrum/src/components/globals/index.js: 334
src/spectrum/src/components/listItems/index.js: 129
src/spectrum/src/components/modals/ChangeChannelModal/channelSelector.js: 22
src/spectrum/src/components/modals/DeleteDoubleCheckModal/index.js: 74
src/spectrum/src/components/profile/channel.js: 95
src/spectrum/src/components/profile/community.js: 60
src/spectrum/src/components/profile/user.js: 68
src/spectrum/src/components/threadComposer/components/composer.js: 120,215,346
src/spectrum/src/components/threadFeed/index.js: 195
src/spectrum/src/helpers/utils.js: 96
src/spectrum/src/registerServiceWorker.js: 24
src/spectrum/src/views/channel/index.js: 159
Expand All @@ -524,20 +520,15 @@ src/spectrum/src/views/communityMembers/components/communityMembers.js: 141
src/spectrum/src/views/communityMembers/components/importSlack.js: 117
src/spectrum/src/views/dashboard/components/sidebarChannels.js: 50
src/spectrum/src/views/dashboard/components/threadFeed.js: 93,213
src/spectrum/src/views/dashboard/index.js: 98
src/spectrum/src/views/directMessages/containers/newThread.js: 201
src/spectrum/src/views/directMessages/index.js: 83
src/spectrum/src/views/explore/components/search.js: 115
src/spectrum/src/views/explore/view.js: 109
src/spectrum/src/views/navbar/components/notificationsTab.js: 51,234
src/spectrum/src/views/navbar/index.js: 75
src/spectrum/src/views/newCommunity/index.js: 173
src/spectrum/src/views/notifications/components/sortAndGroupNotificationMessages.js: 3
src/spectrum/src/views/thread/components/actionBar.js: 209
src/spectrum/src/views/thread/components/messages.js: 142
src/spectrum/src/views/thread/index.js: 223,286
src/spectrum/src/views/user/components/communityList.js: 23
src/spectrum/src/views/user/index.js: 97
src/three.js/editor/js/Loader.js: 12,520
src/three.js/editor/js/Menubar.Edit.js: 125
src/three.js/editor/js/Script.js: 72,136
Expand Down
15 changes: 11 additions & 4 deletions src/rules/cognitive-complexity.ts
Expand Up @@ -19,17 +19,18 @@
*/
// https://sonarsource.github.io/rspec/#/rspec/S3776

import type { TSESTree, TSESLint } from '@typescript-eslint/experimental-utils';
import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
import { isArrowFunctionExpression, isIfStatement, isLogicalExpression } from '../utils/nodes';
import {
getMainFunctionTokenLocation,
getFirstToken,
getFirstTokenAfter,
report,
getMainFunctionTokenLocation,
IssueLocation,
issueLocation,
report,
} from '../utils/locations';
import docsUrl from '../utils/docs-url';
import { getJsxShortCircuitNodes } from '../utils/jsx';

const DEFAULT_THRESHOLD = 15;

Expand Down Expand Up @@ -339,13 +340,19 @@ const rule: TSESLint.RuleModule<string, (number | 'metric' | 'sonar-runtime')[]>
}

function visitLogicalExpression(logicalExpression: TSESTree.LogicalExpression) {
const jsxShortCircuitNodes = getJsxShortCircuitNodes(logicalExpression);
if (jsxShortCircuitNodes != null) {
jsxShortCircuitNodes.forEach(node => consideredLogicalExpressions.add(node));
return;
}

if (!consideredLogicalExpressions.has(logicalExpression)) {
const flattenedLogicalExpressions = flattenLogicalExpression(logicalExpression);

let previous: TSESTree.LogicalExpression | undefined;
for (const current of flattenedLogicalExpressions) {
if (!previous || previous.operator !== current.operator) {
const operatorTokenLoc = getFirstTokenAfter(logicalExpression.left, context)!.loc;
const operatorTokenLoc = getFirstTokenAfter(current.left, context)!.loc;
addComplexity(operatorTokenLoc);
}
previous = current;
Expand Down
50 changes: 50 additions & 0 deletions src/utils/jsx.ts
@@ -0,0 +1,50 @@
/*
* eslint-plugin-sonarjs
* Copyright (C) 2018-2021 SonarSource SA
* mailto:info AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

import type { TSESTree } from '@typescript-eslint/experimental-utils';

export function getJsxShortCircuitNodes(logicalExpression: TSESTree.LogicalExpression) {
if (logicalExpression.parent?.type !== 'JSXExpressionContainer') {
return null;
} else {
return flattenJsxShortCircuitNodes(logicalExpression, logicalExpression);
}
}

function flattenJsxShortCircuitNodes(
root: TSESTree.LogicalExpression,
node: TSESTree.Node,
): TSESTree.LogicalExpression[] | null {
if (
node.type === 'ConditionalExpression' ||
(node.type === 'LogicalExpression' && node.operator !== root.operator)
) {
return null;
} else if (node.type !== 'LogicalExpression') {
return [];
} else {
const leftNodes = flattenJsxShortCircuitNodes(root, node.left);
const rightNodes = flattenJsxShortCircuitNodes(root, node.right);
if (leftNodes == null || rightNodes == null) {
return null;
}
return [...leftNodes, node, ...rightNodes];
}
}
218 changes: 174 additions & 44 deletions tests/rules/cognitive-complexity.test.ts
Expand Up @@ -19,10 +19,79 @@
*/
import { TSESLint } from '@typescript-eslint/experimental-utils';
import { ruleTester } from '../rule-tester';
import { IssueLocation } from '../../src/utils/locations';
import rule = require('../../src/rules/cognitive-complexity');

ruleTester.run('cognitive-complexity', rule, {
valid: [{ code: `function zero_complexity() {}`, options: [0] }],
valid: [
{ code: `function zero_complexity() {}`, options: [0] },
{
code: `
function Component(obj) {
return (
<span>{ obj.title?.text }</span>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0],
},
{
code: `
function Component(obj) {
return (
<>
{ obj.isFriendly && <strong>Welcome</strong> }
</>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0],
},
{
code: `
function Component(obj) {
return (
<>
{ obj.isFriendly && obj.isLoggedIn && <strong>Welcome</strong> }
</>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0],
},
{
code: `
function Component(obj) {
return (
<>
{ obj.x && obj.y && obj.z && <strong>Welcome</strong> }
</>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0],
},
{
code: `
function Component(obj) {
return (
<span title={ obj.title || obj.disclaimer }>Text</span>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0],
},
{
code: `
function Component(obj) {
return (
<button type="button" disabled={ obj.user?.isBot ?? obj.isDemo }>Logout</button>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0],
},
],
invalid: [
// if
{
Expand Down Expand Up @@ -196,8 +265,8 @@ ruleTester.run('cognitive-complexity', rule, {
options: [0],
errors: [message(2)],
},
{
code: `
testCaseWithSonarRuntime(
`
function check_secondaries() {
if (condition) { // +1 "if"
if (condition) {} else {} // +2 "if", +1 "else"
Expand All @@ -217,51 +286,46 @@ ruleTester.run('cognitive-complexity', rule, {
return foo(a && b) && c; // +1 "&&", +1 "&&"
}`,
options: [0, 'sonar-runtime'],
errors: [
[
{ line: 3, column: 8, endLine: 3, endColumn: 10, message: '+1' }, // if
{ line: 7, column: 10, endLine: 7, endColumn: 14, message: '+1' }, // else
{
messageId: 'sonarRuntime',
data: {
complexityAmount: 13,
threshold: 0,
sonarRuntimeData: JSON.stringify({
secondaryLocations: [
{ line: 3, column: 8, endLine: 3, endColumn: 10, message: '+1' }, // if
{ line: 7, column: 10, endLine: 7, endColumn: 14, message: '+1' }, // else
{
line: 4,
column: 10,
endLine: 4,
endColumn: 12,
message: '+2 (incl. 1 for nesting)',
}, // if
{ line: 4, column: 28, endLine: 4, endColumn: 32, message: '+1' }, // else
{
line: 6,
column: 10,
endLine: 6,
endColumn: 15,
message: '+2 (incl. 1 for nesting)',
}, // catch
{ line: 11, column: 8, endLine: 11, endColumn: 13, message: '+1' }, // while
{ line: 12, column: 10, endLine: 12, endColumn: 15, message: '+1' }, // break
{ line: 15, column: 10, endLine: 15, endColumn: 11, message: '+1' }, // ?
{ line: 17, column: 8, endLine: 17, endColumn: 14, message: '+1' }, // switch
{ line: 19, column: 27, endLine: 19, endColumn: 29, message: '+1' }, // &&
{ line: 19, column: 21, endLine: 19, endColumn: 23, message: '+1' }, // &&
],
message:
'Refactor this function to reduce its Cognitive Complexity from 13 to the 0 allowed.',
cost: 13,
}),
...message(13),
cost: 13,
},
},
line: 4,
column: 10,
endLine: 4,
endColumn: 12,
message: '+2 (incl. 1 for nesting)',
}, // if
{ line: 4, column: 28, endLine: 4, endColumn: 32, message: '+1' }, // else
{
line: 6,
column: 10,
endLine: 6,
endColumn: 15,
message: '+2 (incl. 1 for nesting)',
}, // catch
{ line: 11, column: 8, endLine: 11, endColumn: 13, message: '+1' }, // while
{ line: 12, column: 10, endLine: 12, endColumn: 15, message: '+1' }, // break
{ line: 15, column: 10, endLine: 15, endColumn: 11, message: '+1' }, // ?
{ line: 17, column: 8, endLine: 17, endColumn: 14, message: '+1' }, // switch
{ line: 19, column: 27, endLine: 19, endColumn: 29, message: '+1' }, // &&
{ line: 19, column: 21, endLine: 19, endColumn: 23, message: '+1' }, // &&
],
},
13,
),

// expressions
testCaseWithSonarRuntime(
`
function and_or_locations() {
foo(1 && 2 || 3 && 4);
}`,
[
{ line: 3, column: 14, endLine: 3, endColumn: 16, message: '+1' }, // &&
{ line: 3, column: 19, endLine: 3, endColumn: 21, message: '+1' }, // ||
{ line: 3, column: 24, endLine: 3, endColumn: 26, message: '+1' }, // &&
],
),
{
code: `
function and_or() {
Expand Down Expand Up @@ -516,6 +580,48 @@ ruleTester.run('cognitive-complexity', rule, {
options: [0],
errors: [message(1, { line: 2 }), message(1, { line: 3 })],
},
testCaseWithSonarRuntime(
`
function Component(obj) {
return (
<>
<span title={ obj.user?.name ?? (obj.isDemo ? 'demo' : 'none') }>Text</span>
</>
);
}`,
[
{ line: 5, column: 41, endLine: 5, endColumn: 43, message: '+1' }, // ??
{ line: 5, column: 56, endLine: 5, endColumn: 57, message: '+1' }, // ?:
],
),
testCaseWithSonarRuntime(
`
function Component(obj) {
return (
<>
{ obj.isUser && (obj.name || obj.surname) }
</>
);
}`,
[
{ line: 5, column: 25, endLine: 5, endColumn: 27, message: '+1' }, // &&
{ line: 5, column: 38, endLine: 5, endColumn: 40, message: '+1' }, // ||
],
),
testCaseWithSonarRuntime(
`
function Component(obj) {
return (
<>
{ obj.isUser && (obj.isDemo ? <strong>Demo</strong> : <em>None</em>) }
</>
);
}`,
[
{ line: 5, column: 25, endLine: 5, endColumn: 27, message: '+1' }, // &&
{ line: 5, column: 40, endLine: 5, endColumn: 41, message: '+1' }, // ||
],
),
],
});

Expand Down Expand Up @@ -655,6 +761,30 @@ class TopLevel {
],
});

function testCaseWithSonarRuntime(
code: string,
secondaryLocations: IssueLocation[],
complexity?: number,
): TSESLint.InvalidTestCase<string, (number | 'sonar-runtime')[]> {
const cost = complexity ?? secondaryLocations.length;
const message = `Refactor this function to reduce its Cognitive Complexity from ${cost} to the 0 allowed.`;
const sonarRuntimeData = JSON.stringify({ secondaryLocations, message, cost });
return {
code,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0, 'sonar-runtime'],
errors: [
{
messageId: 'sonarRuntime',
data: {
threshold: 0,
sonarRuntimeData,
},
},
],
};
}

function message(complexityAmount: number, other: Partial<TSESLint.TestCaseError<string>> = {}) {
return {
messageId: 'refactorFunction',
Expand Down

0 comments on commit f757f28

Please sign in to comment.