Skip to content

Commit

Permalink
Fix: getter-return reporting method named 'get' (fixes #8919) (#9004)
Browse files Browse the repository at this point in the history
  • Loading branch information
aladdin-add authored and not-an-aardvark committed Aug 1, 2017
1 parent d0f78ec commit c794f86
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 10 deletions.
43 changes: 34 additions & 9 deletions lib/rules/getter-return.js
Expand Up @@ -14,6 +14,7 @@ const astUtils = require("../ast-utils");
//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------
const TARGET_NODE_TYPE = /^(?:Arrow)?FunctionExpression$/;

/**
* Checks a given code path segment is reachable.
Expand Down Expand Up @@ -101,22 +102,45 @@ module.exports = {
}
}

/** Checks whether a node means a getter function.
* @param {ASTNode} node - a node to check.
* @returns {boolean} if node means a getter, return true; else return false.
*/
function isGetter(node) {
const parent = node.parent;

if (TARGET_NODE_TYPE.test(node.type) && node.body.type === "BlockStatement") {
if (parent.kind === "get") {
return true;
}
if (parent.type === "Property" && astUtils.getStaticPropertyName(parent) === "get" && parent.parent.type === "ObjectExpression") {

// Object.defineProperty()
if (parent.parent.parent.type === "CallExpression" &&
astUtils.getStaticPropertyName(parent.parent.parent.callee) === "defineProperty") {
return true;
}

// Object.defineProperties()
if (parent.parent.parent.type === "Property" &&
parent.parent.parent.parent.type === "ObjectExpression" &&
parent.parent.parent.parent.parent.type === "CallExpression" &&
astUtils.getStaticPropertyName(parent.parent.parent.parent.parent.callee) === "defineProperties") {
return true;
}
}
}
return false;
}
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"),
shouldCheck: isGetter(node),
node
};
},
Expand Down Expand Up @@ -145,7 +169,8 @@ module.exports = {
},

// Reports a given function if the last path is reachable.
"FunctionExpression:exit": checkLastSegment
"FunctionExpression:exit": checkLastSegment,
"ArrowFunctionExpression:exit": checkLastSegment
};
}
};
5 changes: 4 additions & 1 deletion tests/lib/rules/getter-return.js
Expand Up @@ -67,7 +67,9 @@ ruleTester.run("getter-return", rule, {
{ code: "var foo = { bar(){ return true; } };" },
{ code: "var foo = { bar: function(){} };" },
{ code: "var foo = { bar: function(){return;} };" },
{ code: "var foo = { bar: function(){return true;} };" }
{ code: "var foo = { bar: function(){return true;} };" },
{ code: "var foo = { get: function () {} }" },
{ code: "var foo = { get: () => {}};" }
],

invalid: [
Expand Down Expand Up @@ -95,6 +97,7 @@ ruleTester.run("getter-return", rule, {
// test object.defineProperty(s)
// option: {allowImplicit: false}
{ code: "Object.defineProperty(foo, \"bar\", { get: function (){}});", errors: [{ noReturnMessage }] },
{ code: "Object.defineProperty(foo, \"bar\", { get: () => {}});", 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.defineProperties(foo, { bar: { get: function () {}} });", options, errors: [{ noReturnMessage }] },
Expand Down

0 comments on commit c794f86

Please sign in to comment.