Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve S3776: Exclude complexity of JSX short-circuits #377

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
36 changes: 31 additions & 5 deletions src/rules/cognitive-complexity.ts
Expand Up @@ -19,15 +19,15 @@
*/
// 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';

Expand Down Expand Up @@ -153,7 +153,6 @@ const rule: TSESLint.RuleModule<string, (number | 'metric' | 'sonar-runtime')[]>
});
}
},

IfStatement(node: TSESTree.Node) {
visitIfStatement(node as TSESTree.IfStatement);
},
Expand Down Expand Up @@ -339,20 +338,47 @@ const rule: TSESLint.RuleModule<string, (number | 'metric' | 'sonar-runtime')[]>
}

function visitLogicalExpression(logicalExpression: TSESTree.LogicalExpression) {
if (isJsxShortCircuitNode(logicalExpression)) {
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;
ilia-kebets-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
const operatorTokenLoc = getFirstTokenAfter(current.left, context)!.loc;
addComplexity(operatorTokenLoc);
}
previous = current;
}
}
}

function isJsxShortCircuitNode(logicalExpression: TSESTree.LogicalExpression) {
const isShortCircuit =
logicalExpression.parent?.type === 'JSXExpressionContainer' &&
isJsxShortCircuitSubNode(logicalExpression, logicalExpression.left) &&
isJsxShortCircuitSubNode(logicalExpression, logicalExpression.right);
if (isShortCircuit) {
consideredLogicalExpressions.add(logicalExpression);
}
return isShortCircuit;
}

function isJsxShortCircuitSubNode(root: TSESTree.LogicalExpression, node: TSESTree.Node) {
ilia-kebets-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
if (node.type === 'LogicalExpression') {
const isShortCircuit = node.operator === root.operator;
if (isShortCircuit) {
consideredLogicalExpressions.add(node);
}
return isShortCircuit;
} else {
return node.type !== 'ConditionalExpression';
}
}

function flattenLogicalExpression(node: TSESTree.Node): TSESTree.LogicalExpression[] {
if (isLogicalExpression(node)) {
consideredLogicalExpressions.add(node);
Expand Down
145 changes: 144 additions & 1 deletion tests/rules/cognitive-complexity.test.ts
Expand Up @@ -22,7 +22,63 @@ import { ruleTester } from '../rule-tester';
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 (
<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 @@ -516,6 +572,93 @@ ruleTester.run('cognitive-complexity', rule, {
options: [0],
errors: [message(1, { line: 2 }), message(1, { line: 3 })],
},
{
code: `
function Component(obj) {
return (
<>
<span title={ obj.user?.name ?? (obj.isDemo ? 'demo' : 'none') }>Text</span>
</>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0, 'sonar-runtime'],
errors: [
{
messageId: 'sonarRuntime',
data: {
threshold: 0,
sonarRuntimeData: JSON.stringify({
secondaryLocations: [
{ line: 5, column: 41, endLine: 5, endColumn: 43, message: '+1' }, // ??
{ line: 5, column: 56, endLine: 5, endColumn: 57, message: '+1' }, // ?:
],
message:
'Refactor this function to reduce its Cognitive Complexity from 2 to the 0 allowed.',
cost: 2,
}),
},
},
],
},
{
code: `
function Component(obj) {
return (
<>
{ obj.isUser && (obj.name || obj.surname) }
</>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0, 'sonar-runtime'],
errors: [
{
messageId: 'sonarRuntime',
data: {
threshold: 0,
sonarRuntimeData: JSON.stringify({
secondaryLocations: [
{ line: 5, column: 25, endLine: 5, endColumn: 27, message: '+1' }, // &&
{ line: 5, column: 38, endLine: 5, endColumn: 40, message: '+1' }, // ||
],
message:
'Refactor this function to reduce its Cognitive Complexity from 2 to the 0 allowed.',
cost: 2,
}),
},
},
],
},
{
code: `
function Component(obj) {
return (
<>
{ obj.isUser && (obj.isDemo ? 'Demo' : 'None') }
</>
);
}`,
parserOptions: { ecmaFeatures: { jsx: true } },
options: [0, 'sonar-runtime'],
errors: [
{
messageId: 'sonarRuntime',
data: {
threshold: 0,
sonarRuntimeData: JSON.stringify({
secondaryLocations: [
{ line: 5, column: 25, endLine: 5, endColumn: 27, message: '+1' }, // &&
{ line: 5, column: 40, endLine: 5, endColumn: 41, message: '+1' }, // ||
],
message:
'Refactor this function to reduce its Cognitive Complexity from 2 to the 0 allowed.',
cost: 2,
}),
},
},
],
},
],
});

Expand Down