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

Commit 87d2bc1

Browse files
Rule S4158: Empty collections should not be accessed or iterated (#232)
1 parent 9b9c682 commit 87d2bc1

File tree

10 files changed

+665
-0
lines changed

10 files changed

+665
-0
lines changed

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Rules in this category aim to find places in code which have a high chance of be
1010

1111
* All branches in a conditional structure should not have exactly the same implementation ([`no-all-duplicated-branches`])
1212
* Collection elements should not be replaced unconditionally ([`no-element-overwrite`])
13+
* Empty collections should not be accessed or iterated ([`no-empty-collection`])
1314
* Function calls should not pass extra arguments ([`no-extra-arguments`])
1415
* Related "if/else if" statements should not have the same condition ([`no-identical-conditions`])
1516
* Identical expressions should not be used on both sides of a binary operator ([`no-identical-expressions`])
@@ -48,6 +49,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
4849
[`no-duplicate-string`]: ./docs/rules/no-duplicate-string.md
4950
[`no-duplicated-branches`]: ./docs/rules/no-duplicated-branches.md
5051
[`no-element-overwrite`]: ./docs/rules/no-element-overwrite.md
52+
[`no-empty-collection`]: ./docs/rules/no-empty-collection.md
5153
[`no-extra-arguments`]: ./docs/rules/no-extra-arguments.md
5254
[`no-identical-conditions`]: ./docs/rules/no-identical-conditions.md
5355
[`no-identical-expressions`]: ./docs/rules/no-identical-expressions.md

docs/rules/no-empty-collection.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# no-empty-collection
2+
3+
When a collection is empty it makes no sense to access or iterate it. Doing so anyway is surely an error; either population was accidentally omitted or the developer doesn’t understand the situation.
4+
5+
## Noncompliant Code Example
6+
7+
```javascript
8+
let strings = [];
9+
10+
if (strings.includes("foo")) {} // Noncompliant
11+
12+
for (str of strings) {} // Noncompliant
13+
14+
strings.forEach(str => doSomething(str)); // Noncompliant
15+
```
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
src/freeCodeCamp/server/boot/react.js: 46

src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const sonarjsRules: [string, TSESLint.Linter.RuleLevel][] = [
2828
['no-duplicate-string', 'error'],
2929
['no-duplicated-branches', 'error'],
3030
['no-element-overwrite', 'error'],
31+
['no-empty-collection', 'error'],
3132
['no-extra-arguments', 'error'],
3233
['no-identical-conditions', 'error'],
3334
['no-identical-functions', 'error'],

src/rules/no-empty-collection.ts

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,219 @@
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://jira.sonarsource.com/browse/RSPEC-4158
21+
22+
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
23+
import {
24+
isIdentifier,
25+
findFirstMatchingAncestor,
26+
isReferenceTo,
27+
collectionConstructor,
28+
ancestorsChain,
29+
} from '../utils';
30+
import { Rule } from '../utils/types';
31+
32+
// Methods that mutate the collection but can't add elements
33+
const nonAdditiveMutatorMethods = [
34+
// array methods
35+
'copyWithin',
36+
'pop',
37+
'reverse',
38+
'shift',
39+
'sort',
40+
// map, set methods
41+
'clear',
42+
'delete',
43+
];
44+
const accessorMethods = [
45+
// array methods
46+
'concat',
47+
'flat',
48+
'flatMap',
49+
'includes',
50+
'indexOf',
51+
'join',
52+
'lastIndexOf',
53+
'slice',
54+
'toSource',
55+
'toString',
56+
'toLocaleString',
57+
// map, set methods
58+
'get',
59+
'has',
60+
];
61+
const iterationMethods = [
62+
'entries',
63+
'every',
64+
'filter',
65+
'find',
66+
'findIndex',
67+
'forEach',
68+
'keys',
69+
'map',
70+
'reduce',
71+
'reduceRight',
72+
'some',
73+
'values',
74+
];
75+
76+
const strictlyReadingMethods = new Set([
77+
...nonAdditiveMutatorMethods,
78+
...accessorMethods,
79+
...iterationMethods,
80+
]);
81+
82+
const rule: Rule.RuleModule = {
83+
meta: {
84+
type: 'problem',
85+
},
86+
create(context: Rule.RuleContext) {
87+
return {
88+
'Program:exit': () => {
89+
reportEmptyCollectionsUsage(context.getScope(), context);
90+
},
91+
};
92+
},
93+
};
94+
95+
function reportEmptyCollectionsUsage(scope: TSESLint.Scope.Scope, context: Rule.RuleContext) {
96+
if (scope.type !== 'global') {
97+
scope.variables.forEach(v => {
98+
reportEmptyCollectionUsage(v, context);
99+
});
100+
}
101+
102+
scope.childScopes.forEach(childScope => {
103+
reportEmptyCollectionsUsage(childScope, context);
104+
});
105+
}
106+
107+
function reportEmptyCollectionUsage(variable: TSESLint.Scope.Variable, context: Rule.RuleContext) {
108+
if (variable.references.length <= 1) {
109+
return;
110+
}
111+
112+
if (variable.defs.some(d => d.type === 'Parameter' || d.type === 'ImportBinding')) {
113+
// Bound value initialized elsewhere, could be non-empty.
114+
return;
115+
}
116+
117+
const readingUsages = [];
118+
let hasAssignmentOfEmptyCollection = false;
119+
120+
for (const ref of variable.references) {
121+
if (ref.isWriteOnly()) {
122+
if (isReferenceAssigningEmptyCollection(ref)) {
123+
hasAssignmentOfEmptyCollection = true;
124+
} else {
125+
// There is at least one operation that might make the collection non-empty.
126+
// We ignore the order of usages, and consider all reads to be safe.
127+
return;
128+
}
129+
} else if (isReadingCollectionUsage(ref)) {
130+
readingUsages.push(ref);
131+
} else {
132+
// some unknown operation on the collection.
133+
// To avoid any FPs, we assume that it could make the collection non-empty.
134+
return;
135+
}
136+
}
137+
138+
if (hasAssignmentOfEmptyCollection) {
139+
readingUsages.forEach(ref => {
140+
context.report({
141+
message: `Review this usage of "${ref.identifier.name}" as it can only be empty here.`,
142+
node: ref.identifier,
143+
});
144+
});
145+
}
146+
}
147+
148+
function isReferenceAssigningEmptyCollection(ref: TSESLint.Scope.Reference) {
149+
const declOrExprStmt = findFirstMatchingAncestor(
150+
ref.identifier as TSESTree.Node,
151+
n => n.type === 'VariableDeclarator' || n.type === 'ExpressionStatement',
152+
) as TSESTree.Node;
153+
if (declOrExprStmt) {
154+
if (declOrExprStmt.type === 'VariableDeclarator' && declOrExprStmt.init) {
155+
return isEmptyCollectionType(declOrExprStmt.init);
156+
}
157+
158+
if (declOrExprStmt.type === 'ExpressionStatement') {
159+
const { expression } = declOrExprStmt;
160+
return (
161+
expression.type === 'AssignmentExpression' &&
162+
isReferenceTo(ref, expression.left) &&
163+
isEmptyCollectionType(expression.right)
164+
);
165+
}
166+
}
167+
return false;
168+
}
169+
170+
function isEmptyCollectionType(node: TSESTree.Node) {
171+
if (node && node.type === 'ArrayExpression') {
172+
return node.elements.length === 0;
173+
} else if (node && (node.type === 'CallExpression' || node.type === 'NewExpression')) {
174+
return isIdentifier(node.callee, ...collectionConstructor) && node.arguments.length === 0;
175+
}
176+
return false;
177+
}
178+
179+
function isReadingCollectionUsage(ref: TSESLint.Scope.Reference) {
180+
return isStrictlyReadingMethodCall(ref) || isForIterationPattern(ref) || isElementRead(ref);
181+
}
182+
183+
function isStrictlyReadingMethodCall(usage: TSESLint.Scope.Reference) {
184+
const { parent } = usage.identifier as TSESTree.Node;
185+
if (parent && parent.type === 'MemberExpression') {
186+
const memberExpressionParent = parent.parent;
187+
if (memberExpressionParent && memberExpressionParent.type === 'CallExpression') {
188+
return isIdentifier(parent.property as TSESTree.Node, ...strictlyReadingMethods);
189+
}
190+
}
191+
return false;
192+
}
193+
194+
function isForIterationPattern(ref: TSESLint.Scope.Reference) {
195+
const forInOrOfStatement = findFirstMatchingAncestor(
196+
ref.identifier as TSESTree.Node,
197+
n => n.type === 'ForOfStatement' || n.type === 'ForInStatement',
198+
) as TSESTree.ForOfStatement | TSESTree.ForInStatement;
199+
200+
return forInOrOfStatement && forInOrOfStatement.right === ref.identifier;
201+
}
202+
203+
function isElementRead(ref: TSESLint.Scope.Reference) {
204+
const { parent } = ref.identifier as TSESTree.Node;
205+
return parent && parent.type === 'MemberExpression' && parent.computed && !isElementWrite(parent);
206+
}
207+
208+
function isElementWrite(memberExpression: TSESTree.MemberExpression) {
209+
const ancestors = ancestorsChain(memberExpression, new Set());
210+
const assignment = ancestors.find(
211+
n => n.type === 'AssignmentExpression',
212+
) as TSESTree.AssignmentExpression;
213+
if (assignment && assignment.operator === '=') {
214+
return [memberExpression, ...ancestors].includes(assignment.left);
215+
}
216+
return false;
217+
}
218+
219+
export = rule;

src/utils/index.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
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+
export * from './utils-ast';
22+
export * from './utils-collection';
23+
export * from './utils-parent';

src/utils/utils-ast.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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 { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
22+
23+
export function isIdentifier(
24+
node: TSESTree.Node,
25+
...values: string[]
26+
): node is TSESTree.Identifier {
27+
return node.type === 'Identifier' && values.some(value => value === node.name);
28+
}
29+
30+
export function isReferenceTo(ref: TSESLint.Scope.Reference, node: TSESTree.Node) {
31+
return node.type === 'Identifier' && node === ref.identifier;
32+
}

src/utils/utils-collection.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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+
export const collectionConstructor = ['Array', 'Map', 'Set', 'WeakSet', 'WeakMap'];

src/utils/utils-parent.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
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+
import { TSESTree } from '@typescript-eslint/experimental-utils';
21+
22+
export function findFirstMatchingAncestor(
23+
node: TSESTree.Node,
24+
predicate: (node: TSESTree.Node) => boolean,
25+
) {
26+
return ancestorsChain(node, new Set()).find(predicate);
27+
}
28+
29+
export function ancestorsChain(node: TSESTree.Node, boundaryTypes: Set<string>) {
30+
const chain: TSESTree.Node[] = [];
31+
32+
let currentNode = node.parent;
33+
while (currentNode) {
34+
chain.push(currentNode);
35+
if (boundaryTypes.has(currentNode.type)) {
36+
break;
37+
}
38+
currentNode = currentNode.parent;
39+
}
40+
return chain;
41+
}

0 commit comments

Comments
 (0)