From a8379bcc1907c8425e9c6c909f7b75832f56195f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=96=9B=E5=AE=9A=E8=B0=94=E7=9A=84=E7=8C=AB?= Date: Fri, 14 Apr 2017 22:46:22 +0800 Subject: [PATCH] New: getter-return (#8449) --- conf/eslint-recommended.js | 1 + docs/rules/getter-return.md | 5 ++ lib/rules/getter-return.js | 143 +++++++++++++++++++++++++++++++ tests/lib/rules/getter-return.js | 48 +++++++++++ 4 files changed, 197 insertions(+) create mode 100644 docs/rules/getter-return.md create mode 100644 lib/rules/getter-return.js create mode 100644 tests/lib/rules/getter-return.js diff --git a/conf/eslint-recommended.js b/conf/eslint-recommended.js index 17b0f448e8f4..f5e2c4a2a834 100755 --- a/conf/eslint-recommended.js +++ b/conf/eslint-recommended.js @@ -47,6 +47,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 000000000000..e71ad4c9903e --- /dev/null +++ b/docs/rules/getter-return.md @@ -0,0 +1,5 @@ +# Enforces that a return statement is present in property getters (getter-return) + +## Rule Details + +## When Not To Use It diff --git a/lib/rules/getter-return.js b/lib/rules/getter-return.js new file mode 100644 index 000000000000..e708997cd792 --- /dev/null +++ b/lib/rules/getter-return.js @@ -0,0 +1,143 @@ +/** + * @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 +//------------------------------------------------------------------------------ + +const TARGET_NODE_TYPE = /^(?:Arrow)?FunctionExpression$/; + +/** + * 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. + * - ArrowFunctionExpression -> `=>` token. + * + * @param {ASTNode} node - A function node to get. + * @param {SourceCode} sourceCode - A source code to get tokens. + * @returns {ASTNode|Token} The node or the token of a location. + */ +function getLocation(node, sourceCode) { + if (node.type === "ArrowFunctionExpression") { + return sourceCode.getTokenBefore(node.body); + } + return node.id || node; +} + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: "enforce `return` statements in getters", + category: "Best Practices", + recommended: false + }, + + schema: [] + }, + + create(context) { + 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: getLocation(node, context.getSourceCode()).loc.start, + message: funcInfo.hasReturn + ? "Expected to return a value at the end of {{name}}." + : "Expected to return a value in {{name}}.", + data: { + name: astUtils.getFunctionNameWithKind(funcInfo.node) + } + }); + } + } + + return { + + // Stacks this function's information. + onCodePathStart(codePath, node) { + funcInfo = { + upper: funcInfo, + codePath, + hasReturn: false, + shouldCheck: + TARGET_NODE_TYPE.test(node.type) && + node.body.type === "BlockStatement" && + node.parent.kind === "get" && + !node.async && + !node.generator, + 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 (!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, + "ArrowFunctionExpression:exit": checkLastSegment + }; + } +}; diff --git a/tests/lib/rules/getter-return.js b/tests/lib/rules/getter-return.js new file mode 100644 index 000000000000..19ab85fcc5ff --- /dev/null +++ b/tests/lib/rules/getter-return.js @@ -0,0 +1,48 @@ +/** + * @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(); + +// TODO: data is not working, so specify a name: "getter 'bar'" +const noReturnMessage = "Expected to return a value in getter 'bar'."; +const noLastReturnMessage = "Expected to return a value at the end of getter 'bar'."; + +ruleTester.run("enforce-return-in-getter", rule, { + + valid: [ + { code: "var foo = { get: function(){} };" }, + { code: "var foo = { get bar(){return true;} };" }, + { code: "var foo = { bar(){return true;} };", env: { es6: true } }, + { code: "var foo = { bar(){} };", env: { es6: true } }, + + { code: "class foo { get bar(){return true;} }", env: { es6: true } }, + { code: "class foo { get(){} }", env: { es6: true } }, + { code: "class foo { get(){return true;} }", env: { es6: true } }, + + { code: "var get = function(){};" } + ], + + invalid: [ + + // TODO: why data: { name: "getter 'bar'"} is not working? + { code: "var foo = { get bar() {return;} }", env: { es6: true }, errors: [{ message: noReturnMessage, data: { name: "getter 'bar'" } }] }, + { code: "var foo = { get bar(){if(bar) {return true;}} }", env: { es6: true }, errors: [{ message: noLastReturnMessage, data: { name: "getter 'bar'" } }] }, + { code: "var foo = { get bar(){if(bar) {return;} return true;} }", env: { es6: true }, errors: [{ message: noReturnMessage, data: { name: "getter 'bar'" } }] } + + ] +});