New: getter-return rule (fixes #8449) (#8460)

conf/eslint-recommended.js

@@ -48,6 +48,7 @@ module.exports = {
         "func-names": "off",
         "func-style": "off",
         "generator-star-spacing": "off",
+        "getter-return": "off",
         "global-require": "off",
         "guard-for-in": "off",
         "handle-callback-err": "off",

docs/rules/getter-return.md

@@ -0,0 +1,97 @@
+# Enforces that a return statement is present in property getters (getter-return)
+
+The get syntax binds an object property to a function that will be called when that property is looked up. It was first introduced in ECMAScript 5:
+
+```js
+    var p = {
+        get name(){
+            return "nicholas";
+        }
+    };
+
+    Object.defineProperty(p, "age", {
+        get: function (){
+            return 17;
+        }
+    });
+```
+
+Note that every `getter` is expected to return a value.
+
+## Rule Details
+
+This rule enforces that a return statement is present in property getters.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint getter-return: "error"*/
+
+p = {
+    get name(){
+        // no returns.
+    }
+};
+
+Object.defineProperty(p, "age", {
+    get: function (){
+        // no returns.
+    }
+});
+
+class P{
+    get name(){
+        // no returns.
+    }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint getter-return: "error"*/
+
+p = {
+    get name(){
+        return "nicholas";
+    }
+};
+
+Object.defineProperty(p, "age", {
+    get: function (){
+        return 18;
+    }
+});
+
+class P{
+    get name(){
+        return "nicholas";
+    }
+}
+```
+
+## Options
+
+This rule has an object option:
+
+* `"allowImplicit": false` (default) disallows implicitly returning undefined with a return; statement.
+
+Examples of **correct** code for the `{ "allowImplicit": true }` option:
+
+```js
+/*eslint getter-return: ["error", { allowImplicit: true }]*/
+p = {
+    get name(){
+        return; // return undefined implicitly.
+    }
+};
+```
+
+## When Not To Use It
+
+If your project will not be using ES5 property getters you do not need this rule.
+
+## Further Reading
+
+* [MDN: Functions getter](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/get)
+* [Understanding ES6: Accessor Properties](https://leanpub.com/understandinges6/read/#leanpub-auto-accessor-properties)

lib/rules/getter-return.js

@@ -0,0 +1,151 @@
+/**
+ * @fileoverview Enforces that a return statement is present in property getters.
+ * @author Aladdin-ADD(hh_2013@foxmail.com)
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const astUtils = require("../ast-utils");
+
+//------------------------------------------------------------------------------
+// Helpers
+//------------------------------------------------------------------------------
+
+/**
+ * Checks a given code path segment is reachable.
+ *
+ * @param {CodePathSegment} segment - A segment to check.
+ * @returns {boolean} `true` if the segment is reachable.
+ */
+function isReachable(segment) {
+    return segment.reachable;
+}
+
+/**
+ * Gets a readable location.
+ *
+ * - FunctionExpression -> the function name or `function` keyword.
+ *
+ * @param {ASTNode} node - A function node to get.
+ * @returns {ASTNode|Token} The node or the token of a location.
+ */
+function getId(node) {
+    return node.id || node;
+}
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "enforce `return` statements in getters",
+            category: "Possible Errors",
+            recommended: false
+        },
+        fixable: null,
+        schema: [
+            {
+                type: "object",
+                properties: {
+                    allowImplicit: {
+                        type: "boolean"
+                    }
+                },
+                additionalProperties: false
+            }
+        ]
+    },
+
+    create(context) {
+
+        const options = context.options[0] || { allowImplicit: false };
+
+        let funcInfo = {
+            upper: null,
+            codePath: null,
+            hasReturn: false,
+            shouldCheck: false,
+            node: null
+        };
+
+        /**
+         * Checks whether or not the last code path segment is reachable.
+         * Then reports this function if the segment is reachable.
+         *
+         * If the last code path segment is reachable, there are paths which are not
+         * returned or thrown.
+         *
+         * @param {ASTNode} node - A node to check.
+         * @returns {void}
+         */
+        function checkLastSegment(node) {
+            if (funcInfo.shouldCheck &&
+                funcInfo.codePath.currentSegments.some(isReachable)
+            ) {
+                context.report({
+                    node,
+                    loc: getId(node).loc.start,
+                    message: funcInfo.hasReturn
+                        ? "Expected {{name}} to always return a value."
+                        : "Expected to return a value in {{name}}.",
+                    data: {
+                        name: astUtils.getFunctionNameWithKind(funcInfo.node)
+                    }
+                });
+            }
+        }
+
+        return {
+
+            // Stacks this function's information.
+            onCodePathStart(codePath, node) {
+                const parent = node.parent;
+
+                funcInfo = {
+                    upper: funcInfo,
+                    codePath,
+                    hasReturn: false,
+                    shouldCheck:
+                        node.type === "FunctionExpression" &&
+                        node.body.type === "BlockStatement" &&
+
+                        // check if it is a "getter", or a method named "get".
+                        (parent.kind === "get" || astUtils.getStaticPropertyName(parent) === "get"),
+                    node
+                };
+            },
+
+            // Pops this function's information.
+            onCodePathEnd() {
+                funcInfo = funcInfo.upper;
+            },
+
+            // Checks the return statement is valid.
+            ReturnStatement(node) {
+                if (funcInfo.shouldCheck) {
+                    funcInfo.hasReturn = true;
+
+                    // if allowImplicit: false, should also check node.argument
+                    if (!options.allowImplicit && !node.argument) {
+                        context.report({
+                            node,
+                            message: "Expected to return a value in {{name}}.",
+                            data: {
+                                name: astUtils.getFunctionNameWithKind(funcInfo.node)
+                            }
+                        });
+                    }
+                }
+            },
+
+            // Reports a given function if the last path is reachable.
+            "FunctionExpression:exit": checkLastSegment
+        };
+    }
+};

tests/lib/rules/getter-return.js

@@ -0,0 +1,109 @@
+/**
+ * @fileoverview Enforces that a return statement is present in property getters.
+ * @author Aladdin-ADD(hh_2013@foxmail.com)
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/getter-return");
+const RuleTester = require("../../../lib/testers/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 6 } });
+
+// data is not working, so specify a name: "getter 'bar'"
+const name = "getter 'bar'";
+const noReturnMessage = `Expected to return a value in ${name}.`;
+const noLastReturnMessage = `Expected ${name} to always return a value.`;
+const options = [{ allowImplicit: true }];
+
+ruleTester.run("getter-return", rule, {
+
+    valid: [
+
+        // test obj: get
+        // option: {allowImplicit: false}
+        { code: "var foo = { get bar(){return true;} };" },
+
+        // option: {allowImplicit: true}
+        { code: "var foo = { get bar() {return;} };", options },
+        { code: "var foo = { get bar(){return true;} };", options },
+        { code: "var foo = { get bar(){if(bar) {return;} return true;} };", options },
+
+        // test class: get
+        // option: {allowImplicit: false}
+        { code: "class foo { get bar(){return true;} }" },
+        { code: "class foo { get bar(){if(baz){return true;} else {return false;} } }" },
+        { code: "class foo { get(){return true;} }" },
+
+        // option: {allowImplicit: true}
+        { code: "class foo { get bar(){return true;} }", options },
+        { code: "class foo { get bar(){return;} }", options },
+
+        // test object.defineProperty(s)
+        // option: {allowImplicit: false}
+        { code: "Object.defineProperty(foo, \"bar\", { get: function () {return true;}});" },
+        { code: "Object.defineProperty(foo, \"bar\", { get: function () { ~function (){ return true; }();return true;}});" },
+        { code: "Object.defineProperies(foo, { bar: { get: function () {return true;}} });" },
+        { code: "Object.defineProperies(foo, { bar: { get: function () { ~function (){ return true; }(); return true;}} });" },
+
+        // option: {allowImplicit: true}
+        { code: "Object.defineProperty(foo, \"bar\", { get: function () {return true;}});", options },
+        { code: "Object.defineProperty(foo, \"bar\", { get: function (){return;}});", options },
+        { code: "Object.defineProperies(foo, { bar: { get: function () {return true;}} });", options },
+        { code: "Object.defineProperies(foo, { bar: { get: function () {return;}} });", options },
+
+        // not getter.
+        { code: "var get = function(){};" },
+        { code: "var get = function(){ return true; };" },
+        { code: "var foo = { bar(){} };" },
+        { code: "var foo = { bar(){ return true; } };" },
+        { code: "var foo = { bar: function(){} };" },
+        { code: "var foo = { bar(){ return true; } };" },
+        { code: "var foo = { bar: function(){return;} };" },
+        { code: "var foo = { bar: function(){return true;} };" }
+    ],
+
+    invalid: [
+
+        // test obj: get
+        // option: {allowImplicit: false}
+        { code: "var foo = { get bar() {} };", errors: [{ message: noReturnMessage }] },
+        { code: "var foo = { get bar(){if(baz) {return true;}} };", errors: [{ message: noLastReturnMessage }] },
+        { code: "var foo = { get bar() { ~function () {return true;}} };", errors: [{ message: noReturnMessage }] },
+
+        // option: {allowImplicit: true}
+        { code: "var foo = { get bar() {} };", options, errors: [{ message: noReturnMessage }] },
+        { code: "var foo = { get bar() {if (baz) {return;}} };", options, errors: [{ message: noLastReturnMessage }] },
+
+        // test class: get
+        // option: {allowImplicit: false}
+        { code: "class foo { get bar(){} }", errors: [{ message: noReturnMessage }] },
+        { code: "class foo { get bar(){ if (baz) { return true; }}}", errors: [{ noLastReturnMessage }] },
+        { code: "class foo { get bar(){ ~function () { return true; }()}}", errors: [{ noLastReturnMessage }] },
+
+        // option: {allowImplicit: true}
+        { code: "class foo { get bar(){} }", options, errors: [{ message: noReturnMessage }] },
+        { code: "class foo { get bar(){if (baz) {return true;} } }", options, errors: [{ message: noLastReturnMessage }] },
+
+        // test object.defineProperty(s)
+        // option: {allowImplicit: false}
+        { code: "Object.defineProperty(foo, \"bar\", { get: function (){}});", errors: [{ noReturnMessage }] },
+        { code: "Object.defineProperty(foo, \"bar\", { get: function (){if(bar) {return true;}}});", errors: [{ message: "Expected method 'get' to always return a value." }] },
+        { code: "Object.defineProperty(foo, \"bar\", { get: function (){ ~function () { return true; }()}});", errors: [{ noReturnMessage }] },
+        { code: "Object.defineProperies(foo, { bar: { get: function () {}} });", options, errors: [{ noReturnMessage }] },
+        { code: "Object.defineProperies(foo, { bar: { get: function (){if(bar) {return true;}}}});", options, errors: [{ message: "Expected method 'get' to always return a value." }] },
+        { code: "Object.defineProperies(foo, { bar: { get: function () {~function () { return true; }()}} });", options, errors: [{ noReturnMessage }] },
+
+        // option: {allowImplicit: true}
+        { code: "Object.defineProperty(foo, \"bar\", { get: function (){}});", options, errors: [{ message: "Expected to return a value in method 'get'." }] },
+        { code: "Object.defineProperies(foo, { bar: { get: function () {}} });", options, errors: [{ noReturnMessage }] }
+    ]
+});