Rule S3531: Generators should yield something (#236)

Co-authored-by: Tibor Blenessy <[email protected]>

Author: yassin-kammoun-sonarsource

README.md

@@ -8,6 +8,7 @@ SonarJS rules for ESLint to detect bugs and suspicious patterns in your code.
 
 Rules in this category aim to find places in code which have a high chance of being bugs, i.e. don't work as intended.
 
+* Generators should "yield" something ([`generator-without-yield`])
 * 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`])
@@ -44,6 +45,7 @@ Code Smells, or maintainability issues, are raised for places of code which migh
 * A "while" loop should be used instead of a "for" loop ([`prefer-while`]) (:wrench: *fixable*)
 
 [`cognitive-complexity`]: ./docs/rules/cognitive-complexity.md
+[`generator-without-yield`]: ./docs/rules/generator-without-yield.md
 [`max-switch-cases`]: ./docs/rules/max-switch-cases.md
 [`no-all-duplicated-branches`]: ./docs/rules/no-all-duplicated-branches.md
 [`no-collapsible-if`]: ./docs/rules/no-collapsible-if.md

docs/rules/generator-without-yield.md

@@ -0,0 +1,24 @@
+# generator-without-yield
+
+A generator without a `yield` statement is at best confusing, and at worst a bug in your code, since the iterator produced by your code will always be empty.
+
+## Noncompliant Code Example
+
+```javascript
+function* myGen(a, b) {  // Noncompliant
+  let answer = 0;
+  answer += a * b;
+}
+```
+
+## Compliant Solution
+
+```javascript
+function* myGen(a, b) {
+  let answer = 0;
+  while (answer < 42) {
+    answer += a * b;
+    yield answer;
+  }
+}
+```

src/index.ts

@@ -21,6 +21,7 @@ import { TSESLint } from '@typescript-eslint/experimental-utils';
 
 const sonarjsRules: [string, TSESLint.Linter.RuleLevel][] = [
   ['cognitive-complexity', 'error'],
+  ['generator-without-yield', 'error'],
   ['max-switch-cases', 'error'],
   ['no-all-duplicated-branches', 'error'],
   ['no-collapsible-if', 'error'],

src/rules/generator-without-yield.ts

@@ -0,0 +1,61 @@
+/*
+ * 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';
+import { getMainFunctionTokenLocation } from '../utils/locations';
+import { Rule } from '../utils/types';
+
+const MESSAGE = 'Add a "yield" statement to this generator.';
+
+const rule: Rule.RuleModule = {
+  meta: {
+    type: 'problem',
+  },
+  create(context: Rule.RuleContext) {
+    const yieldStack: number[] = [];
+
+    function enterFunction() {
+      yieldStack.push(0);
+    }
+
+    function exitFunction(node: TSESTree.Node) {
+      const functionNode = node as TSESTree.FunctionExpression | TSESTree.FunctionDeclaration;
+      const countYield = yieldStack.pop();
+      if (countYield === 0 && functionNode.body.body.length > 0) {
+        context.report({
+          message: MESSAGE,
+          loc: getMainFunctionTokenLocation(functionNode, node.parent, context),
+        });
+      }
+    }
+
+    return {
+      ':function[generator=true]': enterFunction,
+      ':function[generator=true]:exit': exitFunction,
+      YieldExpression() {
+        if (yieldStack.length > 0) {
+          yieldStack[yieldStack.length - 1] += 1;
+        }
+      },
+    };
+  },
+};
+
+export = rule;

tests/rules/generator-without-yield.test.ts

@@ -0,0 +1,128 @@
+/*
+ * 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 { ruleTester } from '../rule-tester';
+import * as rule from '../../src/rules/generator-without-yield';
+
+ruleTester.run('Generator without yield', rule, {
+  valid: [
+    {
+      code: `
+            var foo = function * () {
+            }
+            `,
+    },
+    {
+      code: `
+            var foo = function * () {
+              let a = 3;
+              yield a;
+            }
+            `,
+    },
+    {
+      code: `
+            function someFunction() {
+              doSomething();
+            }
+            `,
+    },
+  ],
+  invalid: [
+    {
+      code: `
+            function * foo() {
+            //         ^^^
+              return 1;
+            }
+            `,
+      errors: [
+        {
+          message: `Add a "yield" statement to this generator.`,
+          line: 2,
+          endLine: 2,
+          column: 24,
+          endColumn: 27,
+        },
+      ],
+    },
+    {
+      code: `
+            var foo = function * () {
+            //        ^^^^^^^^
+              doSomething();
+            }
+            `,
+      errors: [
+        {
+          message: `Add a "yield" statement to this generator.`,
+          line: 2,
+          endLine: 2,
+          column: 23,
+          endColumn: 31,
+        },
+      ],
+    },
+    {
+      code: `
+            var foo = function * bar () {
+              doSomething();
+            }
+            `,
+      errors: 1,
+    },
+    {
+      code: `
+            function * foo() {  // Noncompliant
+            //         ^^^
+              function * bar() {  // OK
+                yield 1;
+              }
+            }
+            `,
+      errors: [
+        {
+          message: `Add a "yield" statement to this generator.`,
+          line: 2,
+          endLine: 2,
+          column: 24,
+          endColumn: 27,
+        },
+      ],
+    },
+    {
+      code: `
+            class A {
+              *foo() {
+                doSomething();
+              }
+            }
+            `,
+      errors: [
+        {
+          message: `Add a "yield" statement to this generator.`,
+          line: 3,
+          endLine: 3,
+          column: 16,
+          endColumn: 19,
+        },
+      ],
+    },
+  ],
+});