diff --git a/conf/eslint-recommended.js b/conf/eslint-recommended.js index 0d8d19e447d..1a2e2f8e7ff 100755 --- a/conf/eslint-recommended.js +++ b/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", diff --git a/docs/rules/getter-return.md b/docs/rules/getter-return.md new file mode 100644 index 00000000000..57f92065d42 --- /dev/null +++ b/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) diff --git a/lib/rules/getter-return.js b/lib/rules/getter-return.js new file mode 100644 index 00000000000..8a095ad296f --- /dev/null +++ b/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 + }; + } +}; diff --git a/tests/lib/rules/getter-return.js b/tests/lib/rules/getter-return.js new file mode 100644 index 00000000000..646d4489389 --- /dev/null +++ b/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 }] } + ] +});