Rule S4158: Empty collections should not be accessed or iterated (#232)

README.md

@@ -10,6 +10,7 @@ Rules in this category aim to find places in code which have a high chance of be
 
 * All branches in a conditional structure should not have exactly the same implementation ([`no-all-duplicated-branches`])
 * Collection elements should not be replaced unconditionally ([`no-element-overwrite`])
+* Empty collections should not be accessed or iterated ([`no-empty-collection`])
 * Function calls should not pass extra arguments ([`no-extra-arguments`])
 * Related "if/else if" statements should not have the same condition ([`no-identical-conditions`])
 * 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
 [`no-duplicate-string`]: ./docs/rules/no-duplicate-string.md
 [`no-duplicated-branches`]: ./docs/rules/no-duplicated-branches.md
 [`no-element-overwrite`]: ./docs/rules/no-element-overwrite.md
+[`no-empty-collection`]: ./docs/rules/no-empty-collection.md
 [`no-extra-arguments`]: ./docs/rules/no-extra-arguments.md
 [`no-identical-conditions`]: ./docs/rules/no-identical-conditions.md
 [`no-identical-expressions`]: ./docs/rules/no-identical-expressions.md

docs/rules/no-empty-collection.md

@@ -0,0 +1,15 @@
+# no-empty-collection
+
+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.
+
+## Noncompliant Code Example
+
+```javascript
+let strings = [];
+
+if (strings.includes("foo")) {}  // Noncompliant
+
+for (str of strings) {}  // Noncompliant
+
+strings.forEach(str => doSomething(str)); // Noncompliant
+```

ruling/snapshots/no-empty-collection

@@ -0,0 +1 @@
+src/freeCodeCamp/server/boot/react.js: 46

src/index.ts

@@ -28,6 +28,7 @@ const sonarjsRules: [string, TSESLint.Linter.RuleLevel][] = [
   ['no-duplicate-string', 'error'],
   ['no-duplicated-branches', 'error'],
   ['no-element-overwrite', 'error'],
+  ['no-empty-collection', 'error'],
   ['no-extra-arguments', 'error'],
   ['no-identical-conditions', 'error'],
   ['no-identical-functions', 'error'],

src/rules/no-empty-collection.ts

@@ -0,0 +1,219 @@
+/*
+ * 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.
+ */
+// https://jira.sonarsource.com/browse/RSPEC-4158
+
+import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
+import {
+  isIdentifier,
+  findFirstMatchingAncestor,
+  isReferenceTo,
+  collectionConstructor,
+  ancestorsChain,
+} from '../utils';
+import { Rule } from '../utils/types';
+
+// Methods that mutate the collection but can't add elements
+const nonAdditiveMutatorMethods = [
+  // array methods
+  'copyWithin',
+  'pop',
+  'reverse',
+  'shift',
+  'sort',
+  // map, set methods
+  'clear',
+  'delete',
+];
+const accessorMethods = [
+  // array methods
+  'concat',
+  'flat',
+  'flatMap',
+  'includes',
+  'indexOf',
+  'join',
+  'lastIndexOf',
+  'slice',
+  'toSource',
+  'toString',
+  'toLocaleString',
+  // map, set methods
+  'get',
+  'has',
+];
+const iterationMethods = [
+  'entries',
+  'every',
+  'filter',
+  'find',
+  'findIndex',
+  'forEach',
+  'keys',
+  'map',
+  'reduce',
+  'reduceRight',
+  'some',
+  'values',
+];
+
+const strictlyReadingMethods = new Set([
+  ...nonAdditiveMutatorMethods,
+  ...accessorMethods,
+  ...iterationMethods,
+]);
+
+const rule: Rule.RuleModule = {
+  meta: {
+    type: 'problem',
+  },
+  create(context: Rule.RuleContext) {
+    return {
+      'Program:exit': () => {
+        reportEmptyCollectionsUsage(context.getScope(), context);
+      },
+    };
+  },
+};
+
+function reportEmptyCollectionsUsage(scope: TSESLint.Scope.Scope, context: Rule.RuleContext) {
+  if (scope.type !== 'global') {
+    scope.variables.forEach(v => {
+      reportEmptyCollectionUsage(v, context);
+    });
+  }
+
+  scope.childScopes.forEach(childScope => {
+    reportEmptyCollectionsUsage(childScope, context);
+  });
+}
+
+function reportEmptyCollectionUsage(variable: TSESLint.Scope.Variable, context: Rule.RuleContext) {
+  if (variable.references.length <= 1) {
+    return;
+  }
+
+  if (variable.defs.some(d => d.type === 'Parameter' || d.type === 'ImportBinding')) {
+    // Bound value initialized elsewhere, could be non-empty.
+    return;
+  }
+
+  const readingUsages = [];
+  let hasAssignmentOfEmptyCollection = false;
+
+  for (const ref of variable.references) {
+    if (ref.isWriteOnly()) {
+      if (isReferenceAssigningEmptyCollection(ref)) {
+        hasAssignmentOfEmptyCollection = true;
+      } else {
+        // There is at least one operation that might make the collection non-empty.
+        // We ignore the order of usages, and consider all reads to be safe.
+        return;
+      }
+    } else if (isReadingCollectionUsage(ref)) {
+      readingUsages.push(ref);
+    } else {
+      // some unknown operation on the collection.
+      // To avoid any FPs, we assume that it could make the collection non-empty.
+      return;
+    }
+  }
+
+  if (hasAssignmentOfEmptyCollection) {
+    readingUsages.forEach(ref => {
+      context.report({
+        message: `Review this usage of "${ref.identifier.name}" as it can only be empty here.`,
+        node: ref.identifier,
+      });
+    });
+  }
+}
+
+function isReferenceAssigningEmptyCollection(ref: TSESLint.Scope.Reference) {
+  const declOrExprStmt = findFirstMatchingAncestor(
+    ref.identifier as TSESTree.Node,
+    n => n.type === 'VariableDeclarator' || n.type === 'ExpressionStatement',
+  ) as TSESTree.Node;
+  if (declOrExprStmt) {
+    if (declOrExprStmt.type === 'VariableDeclarator' && declOrExprStmt.init) {
+      return isEmptyCollectionType(declOrExprStmt.init);
+    }
+
+    if (declOrExprStmt.type === 'ExpressionStatement') {
+      const { expression } = declOrExprStmt;
+      return (
+        expression.type === 'AssignmentExpression' &&
+        isReferenceTo(ref, expression.left) &&
+        isEmptyCollectionType(expression.right)
+      );
+    }
+  }
+  return false;
+}
+
+function isEmptyCollectionType(node: TSESTree.Node) {
+  if (node && node.type === 'ArrayExpression') {
+    return node.elements.length === 0;
+  } else if (node && (node.type === 'CallExpression' || node.type === 'NewExpression')) {
+    return isIdentifier(node.callee, ...collectionConstructor) && node.arguments.length === 0;
+  }
+  return false;
+}
+
+function isReadingCollectionUsage(ref: TSESLint.Scope.Reference) {
+  return isStrictlyReadingMethodCall(ref) || isForIterationPattern(ref) || isElementRead(ref);
+}
+
+function isStrictlyReadingMethodCall(usage: TSESLint.Scope.Reference) {
+  const { parent } = usage.identifier as TSESTree.Node;
+  if (parent && parent.type === 'MemberExpression') {
+    const memberExpressionParent = parent.parent;
+    if (memberExpressionParent && memberExpressionParent.type === 'CallExpression') {
+      return isIdentifier(parent.property as TSESTree.Node, ...strictlyReadingMethods);
+    }
+  }
+  return false;
+}
+
+function isForIterationPattern(ref: TSESLint.Scope.Reference) {
+  const forInOrOfStatement = findFirstMatchingAncestor(
+    ref.identifier as TSESTree.Node,
+    n => n.type === 'ForOfStatement' || n.type === 'ForInStatement',
+  ) as TSESTree.ForOfStatement | TSESTree.ForInStatement;
+
+  return forInOrOfStatement && forInOrOfStatement.right === ref.identifier;
+}
+
+function isElementRead(ref: TSESLint.Scope.Reference) {
+  const { parent } = ref.identifier as TSESTree.Node;
+  return parent && parent.type === 'MemberExpression' && parent.computed && !isElementWrite(parent);
+}
+
+function isElementWrite(memberExpression: TSESTree.MemberExpression) {
+  const ancestors = ancestorsChain(memberExpression, new Set());
+  const assignment = ancestors.find(
+    n => n.type === 'AssignmentExpression',
+  ) as TSESTree.AssignmentExpression;
+  if (assignment && assignment.operator === '=') {
+    return [memberExpression, ...ancestors].includes(assignment.left);
+  }
+  return false;
+}
+
+export = rule;

src/utils/index.ts

@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+export * from './utils-ast';
+export * from './utils-collection';
+export * from './utils-parent';

src/utils/utils-ast.ts

@@ -0,0 +1,32 @@
+/*
+ * 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 { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
+
+export function isIdentifier(
+  node: TSESTree.Node,
+  ...values: string[]
+): node is TSESTree.Identifier {
+  return node.type === 'Identifier' && values.some(value => value === node.name);
+}
+
+export function isReferenceTo(ref: TSESLint.Scope.Reference, node: TSESTree.Node) {
+  return node.type === 'Identifier' && node === ref.identifier;
+}

src/utils/utils-collection.ts

@@ -0,0 +1,20 @@
+/*
+ * 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.
+ */
+export const collectionConstructor = ['Array', 'Map', 'Set', 'WeakSet', 'WeakMap'];

src/utils/utils-parent.ts

@@ -0,0 +1,41 @@
+/*
+ * 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 { TSESTree } from '@typescript-eslint/experimental-utils';
+
+export function findFirstMatchingAncestor(
+  node: TSESTree.Node,
+  predicate: (node: TSESTree.Node) => boolean,
+) {
+  return ancestorsChain(node, new Set()).find(predicate);
+}
+
+export function ancestorsChain(node: TSESTree.Node, boundaryTypes: Set<string>) {
+  const chain: TSESTree.Node[] = [];
+
+  let currentNode = node.parent;
+  while (currentNode) {
+    chain.push(currentNode);
+    if (boundaryTypes.has(currentNode.type)) {
+      break;
+    }
+    currentNode = currentNode.parent;
+  }
+  return chain;
+}