diff --git a/ruling/snapshots/cognitive-complexity b/ruling/snapshots/cognitive-complexity index 16d87e1d..961ca769 100644 --- a/ruling/snapshots/cognitive-complexity +++ b/ruling/snapshots/cognitive-complexity @@ -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 @@ -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 @@ -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 diff --git a/src/rules/cognitive-complexity.ts b/src/rules/cognitive-complexity.ts index 6f0b68a0..421cb914 100644 --- a/src/rules/cognitive-complexity.ts +++ b/src/rules/cognitive-complexity.ts @@ -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; @@ -339,13 +340,19 @@ const rule: TSESLint.RuleModule } 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; diff --git a/src/utils/jsx.ts b/src/utils/jsx.ts new file mode 100644 index 00000000..af583144 --- /dev/null +++ b/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]; + } +} diff --git a/tests/rules/cognitive-complexity.test.ts b/tests/rules/cognitive-complexity.test.ts index aa588848..da5b2dbc 100644 --- a/tests/rules/cognitive-complexity.test.ts +++ b/tests/rules/cognitive-complexity.test.ts @@ -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 ( + { obj.title?.text } + ); + }`, + parserOptions: { ecmaFeatures: { jsx: true } }, + options: [0], + }, + { + code: ` + function Component(obj) { + return ( + <> + { obj.isFriendly && Welcome } + + ); + }`, + parserOptions: { ecmaFeatures: { jsx: true } }, + options: [0], + }, + { + code: ` + function Component(obj) { + return ( + <> + { obj.isFriendly && obj.isLoggedIn && Welcome } + + ); + }`, + parserOptions: { ecmaFeatures: { jsx: true } }, + options: [0], + }, + { + code: ` + function Component(obj) { + return ( + <> + { obj.x && obj.y && obj.z && Welcome } + + ); + }`, + parserOptions: { ecmaFeatures: { jsx: true } }, + options: [0], + }, + { + code: ` + function Component(obj) { + return ( + Text + ); + }`, + parserOptions: { ecmaFeatures: { jsx: true } }, + options: [0], + }, + { + code: ` + function Component(obj) { + return ( + + ); + }`, + parserOptions: { ecmaFeatures: { jsx: true } }, + options: [0], + }, + ], invalid: [ // if { @@ -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" @@ -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() { @@ -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 ( + <> + Text + + ); + }`, + [ + { 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 ? Demo : None) } + + ); + }`, + [ + { line: 5, column: 25, endLine: 5, endColumn: 27, message: '+1' }, // && + { line: 5, column: 40, endLine: 5, endColumn: 41, message: '+1' }, // || + ], + ), ], }); @@ -655,6 +761,30 @@ class TopLevel { ], }); +function testCaseWithSonarRuntime( + code: string, + secondaryLocations: IssueLocation[], + complexity?: number, +): TSESLint.InvalidTestCase { + 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> = {}) { return { messageId: 'refactorFunction',