Skip to content

Commit

Permalink
Update: callback-return allows for object methods (fixes #4711)
Browse files Browse the repository at this point in the history
  • Loading branch information
kaicataldo committed May 29, 2016
1 parent 0e4c9c8 commit 5349499
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 5 deletions.
7 changes: 4 additions & 3 deletions lib/rules/callback-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ module.exports = {

create: function(context) {

var callbacks = context.options[0] || ["callback", "cb", "next"];
var callbacks = context.options[0] || ["callback", "cb", "next"],
sourceCode = context.getSourceCode();

//--------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -52,7 +53,7 @@ module.exports = {
* @returns {Boolean} Whether or not this function matches our callback name.
*/
function isCallback(node) {
return node.callee.type === "Identifier" && callbacks.indexOf(node.callee.name) > -1;
return callbacks.indexOf(sourceCode.getText(node.callee)) > -1;
}

/**
Expand Down Expand Up @@ -90,7 +91,7 @@ module.exports = {
return {
CallExpression: function(node) {

// if we"re not a callback we can return
// if we're not a callback we can return
if (!isCallback(node)) {
return;
}
Expand Down
99 changes: 97 additions & 2 deletions tests/lib/rules/callback-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var ruleTester = new RuleTester();
ruleTester.run("callback-return", rule, {
valid: [

// callbacks inside of functions should return
// callbacks inside of functions should return
"function a(err) { if (err) return callback (err); }",
"function a(err) { if (err) return callback (err); callback(); }",
"function a(err) { if (err) { return callback (err); } callback(); }",
Expand All @@ -38,6 +38,7 @@ ruleTester.run("callback-return", rule, {
"function x() { switch(x) { case 'a': return next(); } }",
"function x() { for(x = 0; x < 10; x++) { return next(); } }",
"function x() { while(x) { return next(); } }",
"function a(err) { if (err) { obj.method (err); } }",

// callback() all you want outside of a function
"callback()",
Expand Down Expand Up @@ -73,7 +74,7 @@ ruleTester.run("callback-return", rule, {

// options (only warns with the correct callback name)
{
code: "if (err) { callback(err) }",
code: "function a(err) { if (err) { callback(err) } }",
options: [["cb"]]
},
{
Expand All @@ -85,6 +86,40 @@ ruleTester.run("callback-return", rule, {
options: [["cb", "next"]]
},

// allow object methods (https://github.com/eslint/eslint/issues/4711)
{
code: "function a(err) { if (err) { callback(err); } }",
options: [["obj.method"]]
},
{
code: "function a(err) { if (err) { return obj.method(err); } }",
options: [["obj.method"]]
},
{
code: "function a(err) { if (err) { obj.method(err); } }",
options: [["obj().method"]]
},
{
code: "function a(err) { if (err) { //comment\nreturn obj().method(err); } }",
options: [["obj().method"]]
},
{
code: "function a(err) { if (err) { /*comment*/return obj().method(err); } }",
options: [["obj().method"]]
},
{
code: "function a(err) { if (err) { return obj().method(err); //comment\n } }",
options: [["obj().method"]]
},
{
code: "function a(err) { if (err) { return obj().method(err); /*comment*/ } }",
options: [["obj().method"]]
},
{
code: "function a(err) { if (err) { obj.method(err); } obj().method(); }",
options: [["obj().method"]]
},

// known bad examples that we know we are ignoring
"function x(err) { if (err) { setTimeout(callback, 0); } callback(); }", // callback() called twice
"function x(err) { if (err) { process.nextTick(function(err) { callback(); }); } callback(); }" // callback() called twice
Expand Down Expand Up @@ -332,6 +367,66 @@ ruleTester.run("callback-return", rule, {
nodeType: "CallExpression"
}
]
},
{
code: "function a(err) { if (err) { obj().method(err); } }",
options: [["obj().method"]],
errors: [
{
message: "Expected return with your callback function.",
line: 1,
column: 30,
nodeType: "CallExpression"
}
]
},
{
code: "function a(err) { if (err) { /*comment*/obj().method(err); } }",
options: [["obj().method"]],
errors: [
{
message: "Expected return with your callback function.",
line: 1,
column: 41,
nodeType: "CallExpression"
}
]
},
{
code: "function a(err) { if (err) { //comment\nobj().method(err); } }",
options: [["obj().method"]],
errors: [
{
message: "Expected return with your callback function.",
line: 2,
column: 1,
nodeType: "CallExpression"
}
]
},
{
code: "function a(err) { if (err) { obj().method(err); /*comment*/ } }",
options: [["obj().method"]],
errors: [
{
message: "Expected return with your callback function.",
line: 1,
column: 30,
nodeType: "CallExpression"
}
]
},
{
code: "function a(err) { if (err) { obj().method(err); //comment\n } }",
options: [["obj().method"]],
errors: [
{
message: "Expected return with your callback function.",
line: 1,
column: 30,
nodeType: "CallExpression"
}
]
}
]
});

0 comments on commit 5349499

Please sign in to comment.