Skip to content
This repository was archived by the owner on Oct 3, 2024. It is now read-only.

Commit 54dbe24

Browse files
yassin-kammoun-sonarsourceyassin-kammoun-sonarsource
authored
Rule: Return values from functions without side effects should not be ignored (#253)
1 parent f5a24c5 commit 54dbe24

File tree

6 files changed

+535
-0
lines changed

6 files changed

+535
-0
lines changed

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Rules in this category aim to find places in code which have a high chance of be
1515
* Function calls should not pass extra arguments ([`no-extra-arguments`])
1616
* Related "if/else if" statements should not have the same condition ([`no-identical-conditions`])
1717
* Identical expressions should not be used on both sides of a binary operator ([`no-identical-expressions`])
18+
* Return values from functions without side effects should not be ignored ([`no-ignored-return`])
1819
* Loops with at most one iteration should be refactored ([`no-one-iteration-loop`])
1920
* The output of functions that don't return anything should not be used ([`no-use-of-empty-return-value`])
2021
* Non-existent operators '=+', '=-' and '=!' should not be used ([`non-existent-operator`])
@@ -62,6 +63,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
6263
[`no-identical-conditions`]: ./docs/rules/no-identical-conditions.md
6364
[`no-identical-expressions`]: ./docs/rules/no-identical-expressions.md
6465
[`no-identical-functions`]: ./docs/rules/no-identical-functions.md
66+
[`no-ignored-return`]: ./docs/rules/no-ignored-return.md
6567
[`no-inverted-boolean-check`]: ./docs/rules/no-inverted-boolean-check.md
6668
[`no-nested-switch`]: ./docs/rules/no-nested-switch.md
6769
[`no-nested-template-literals`]: ./docs/rules/no-nested-template-literals.md

‎docs/rules/no-ignored-return.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# no-ignored-return
2+
3+
When the call to a function doesn’t have any side effects, what is the point of making the call if the results are ignored? In such case, either the function call is useless and should be dropped or the source code doesn’t behave as expected.
4+
5+
To prevent generating any false-positives, this rule triggers an issues only on a predefined list of known objects & functions.
6+
7+
## Noncompliant Code Example
8+
9+
```javascript
10+
'hello'.lastIndexOf('e'); // Noncompliant
11+
```
12+
13+
## Compliant Solution
14+
15+
```javascript
16+
let char = 'hello'.lastIndexOf('e');
17+
```
18+
19+
## See
20+
21+
<ul>
22+
<li> <a href="https://wiki.sei.cmu.edu/confluence/x/mtYxBQ">CERT, EXP12-C.</a> - Do not ignore values returned by functions</li>
23+
<li> <a href="https://wiki.sei.cmu.edu/confluence/x/xzdGBQ">CERT, EXP00-J.</a> - Do not ignore values returned by methods</li>
24+
</ul>

‎src/rules/no-ignored-return.ts‎

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
/*
2+
* eslint-plugin-sonarjs
3+
* Copyright (C) 2018-2021 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
// https://sonarsource.github.io/rspec/#/rspec/S2201
21+
22+
import * as ts from 'typescript';
23+
import { TSESTree } from '@typescript-eslint/experimental-utils';
24+
import { isRequiredParserServices, RequiredParserServices } from '../utils/parser-services';
25+
import { Rule } from '../utils/types';
26+
import docsUrl from '../utils/docs-url';
27+
import { getTypeFromTreeNode } from '../utils';
28+
29+
const METHODS_WITHOUT_SIDE_EFFECTS: { [index: string]: Set<string> } = {
30+
array: new Set([
31+
'concat',
32+
'includes',
33+
'join',
34+
'slice',
35+
'indexOf',
36+
'lastIndexOf',
37+
'entries',
38+
'filter',
39+
'findIndex',
40+
'keys',
41+
'map',
42+
'values',
43+
'find',
44+
'reduce',
45+
'reduceRight',
46+
'toString',
47+
'toLocaleString',
48+
]),
49+
date: new Set([
50+
'getDate',
51+
'getDay',
52+
'getFullYear',
53+
'getHours',
54+
'getMilliseconds',
55+
'getMinutes',
56+
'getMonth',
57+
'getSeconds',
58+
'getTime',
59+
'getTimezoneOffset',
60+
'getUTCDate',
61+
'getUTCDay',
62+
'getUTCFullYear',
63+
'getUTCHours',
64+
'getUTCMilliseconds',
65+
'getUTCMinutes',
66+
'getUTCMonth',
67+
'getUTCSeconds',
68+
'getYear',
69+
'toDateString',
70+
'toISOString',
71+
'toJSON',
72+
'toGMTString',
73+
'toLocaleDateString',
74+
'toLocaleTimeString',
75+
'toTimeString',
76+
'toUTCString',
77+
'toString',
78+
'toLocaleString',
79+
]),
80+
math: new Set([
81+
'abs',
82+
'E',
83+
'LN2',
84+
'LN10',
85+
'LOG2E',
86+
'LOG10E',
87+
'PI',
88+
'SQRT1_2',
89+
'SQRT2',
90+
'abs',
91+
'acos',
92+
'acosh',
93+
'asin',
94+
'asinh',
95+
'atan',
96+
'atanh',
97+
'atan2',
98+
'cbrt',
99+
'ceil',
100+
'clz32',
101+
'cos',
102+
'cosh',
103+
'exp',
104+
'expm1',
105+
'floor',
106+
'fround',
107+
'hypot',
108+
'imul',
109+
'log',
110+
'log1p',
111+
'log10',
112+
'log2',
113+
'max',
114+
'min',
115+
'pow',
116+
'random',
117+
'round',
118+
'sign',
119+
'sin',
120+
'sinh',
121+
'sqrt',
122+
'tan',
123+
'tanh',
124+
'trunc',
125+
]),
126+
number: new Set(['toExponential', 'toFixed', 'toPrecision', 'toLocaleString', 'toString']),
127+
regexp: new Set(['test', 'toString']),
128+
string: new Set([
129+
'charAt',
130+
'charCodeAt',
131+
'codePointAt',
132+
'concat',
133+
'includes',
134+
'endsWith',
135+
'indexOf',
136+
'lastIndexOf',
137+
'localeCompare',
138+
'match',
139+
'normalize',
140+
'padEnd',
141+
'padStart',
142+
'repeat',
143+
'replace',
144+
'search',
145+
'slice',
146+
'split',
147+
'startsWith',
148+
'substr',
149+
'substring',
150+
'toLocaleLowerCase',
151+
'toLocaleUpperCase',
152+
'toLowerCase',
153+
'toUpperCase',
154+
'trim',
155+
'length',
156+
'toString',
157+
'valueOf',
158+
159+
// HTML wrapper methods
160+
'anchor',
161+
'big',
162+
'blink',
163+
'bold',
164+
'fixed',
165+
'fontcolor',
166+
'fontsize',
167+
'italics',
168+
'link',
169+
'small',
170+
'strike',
171+
'sub',
172+
'sup',
173+
]),
174+
};
175+
176+
const rule: Rule.RuleModule = {
177+
meta: {
178+
type: 'problem',
179+
docs: {
180+
description: 'Return values from functions without side effects should not be ignored',
181+
category: 'Possible Errors',
182+
recommended: 'error',
183+
url: docsUrl(__filename),
184+
},
185+
},
186+
create(context: Rule.RuleContext) {
187+
if (!isRequiredParserServices(context.parserServices)) {
188+
return {};
189+
}
190+
const services = context.parserServices;
191+
return {
192+
CallExpression: (node: TSESTree.Node) => {
193+
const call = node as TSESTree.CallExpression;
194+
const { callee } = call;
195+
if (callee.type === 'MemberExpression') {
196+
const { parent } = node;
197+
if (parent && parent.type === 'ExpressionStatement') {
198+
const methodName = context.getSourceCode().getText(callee.property);
199+
const objectType = services.program
200+
.getTypeChecker()
201+
.getTypeAtLocation(
202+
services.esTreeNodeToTSNodeMap.get(callee.object as TSESTree.Node),
203+
);
204+
if (
205+
!hasSideEffect(methodName, objectType, services) &&
206+
!isReplaceWithCallback(methodName, call.arguments, services)
207+
) {
208+
context.report({
209+
message: message(methodName),
210+
node,
211+
});
212+
}
213+
}
214+
}
215+
},
216+
};
217+
},
218+
};
219+
220+
function isReplaceWithCallback(
221+
methodName: string,
222+
callArguments: Array<TSESTree.Expression | TSESTree.SpreadElement>,
223+
services: RequiredParserServices,
224+
) {
225+
if (methodName === 'replace' && callArguments.length > 1) {
226+
const type = getTypeFromTreeNode(callArguments[1], services);
227+
const typeNode = services.program.getTypeChecker().typeToTypeNode(type, undefined, undefined);
228+
return typeNode && ts.isFunctionTypeNode(typeNode);
229+
}
230+
return false;
231+
}
232+
233+
function message(methodName: string): string {
234+
if (methodName === 'map') {
235+
return `Consider using "forEach" instead of "map" as its return value is not being used here.`;
236+
} else {
237+
return `The return value of "${methodName}" must be used.`;
238+
}
239+
}
240+
241+
function hasSideEffect(methodName: string, objectType: any, services: RequiredParserServices) {
242+
const typeAsString = typeToString(objectType, services);
243+
if (typeAsString !== null) {
244+
const methods = METHODS_WITHOUT_SIDE_EFFECTS[typeAsString];
245+
return !(methods && methods.has(methodName));
246+
}
247+
return true;
248+
}
249+
250+
function typeToString(tp: any, services: RequiredParserServices): string | null {
251+
const typechecker = services.program.getTypeChecker();
252+
253+
const baseType = typechecker.getBaseTypeOfLiteralType(tp);
254+
const typeAsString = typechecker.typeToString(baseType);
255+
if (typeAsString === 'number' || typeAsString === 'string') {
256+
return typeAsString;
257+
}
258+
259+
const symbol = tp.getSymbol();
260+
if (symbol) {
261+
const name = symbol.getName();
262+
switch (name) {
263+
case 'Array':
264+
case 'Date':
265+
case 'Math':
266+
case 'RegExp':
267+
return name.toLowerCase();
268+
}
269+
}
270+
271+
return null;
272+
}
273+
274+
export = rule;

‎src/utils/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@
2121
export * from './utils-ast';
2222
export * from './utils-collection';
2323
export * from './utils-parent';
24+
export * from './utils-type';

‎src/utils/utils-type.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* eslint-plugin-sonarjs
3+
* Copyright (C) 2018-2021 SonarSource SA
4+
* mailto:info AT sonarsource DOT com
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public License
17+
* along with this program; if not, write to the Free Software Foundation,
18+
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
19+
*/
20+
21+
import { TSESTree } from '@typescript-eslint/experimental-utils';
22+
import { RequiredParserServices } from './parser-services';
23+
24+
export function getTypeFromTreeNode(node: TSESTree.Node, services: RequiredParserServices) {
25+
const checker = services.program.getTypeChecker();
26+
return checker.getTypeAtLocation(services.esTreeNodeToTSNodeMap.get(node));
27+
}

0 commit comments

Comments
 (0)