Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: Improve report location for array-callback-return (refs #12334) #13109

Merged
merged 1 commit into from Apr 24, 2020
Merged

Update: Improve report location for array-callback-return (refs #12334) #13109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 3 additions & 18 deletions lib/rules/array-callback-return.js
View file
Open in desktop
Expand Up @@ -29,22 +29,6 @@ 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;
}

/**
* Checks a given node is a MemberExpression node which has the specified name's
* property.
Expand Down Expand Up @@ -179,6 +163,7 @@ module.exports = {
create(context) {

const options = context.options[0] || { allowImplicit: false, checkForEach: false };
const sourceCode = context.getSourceCode();

let funcInfo = {
arrayMethodName: null,
Expand Down Expand Up @@ -217,12 +202,12 @@ module.exports = {
}

if (messageId) {
let name = astUtils.getFunctionNameWithKind(funcInfo.node);
let name = astUtils.getFunctionNameWithKind(node);

name = messageId === "expectedNoReturnValue" ? lodash.upperFirst(name) : name;
context.report({
node,
loc: getLocation(node, context.getSourceCode()).loc.start,
loc: astUtils.getFunctionHeadLoc(node, sourceCode),
messageId,
data: { name }
});
Expand Down
249 changes: 246 additions & 3 deletions tests/lib/rules/array-callback-return.js
View file
Open in desktop
Expand Up @@ -147,7 +147,7 @@ ruleTester.run("array-callback-return", rule, {
{ code: "foo[`every`](function foo() {})", parserOptions: { ecmaVersion: 6 }, errors: [{ messageId: "expectedInside", data: { name: "function 'foo'" } }] },
{ code: "foo.every(() => {})", parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected to return a value in arrow function.", column: 14 }] },
{ code: "foo.every(function() { if (a) return true; })", errors: [{ message: "Expected to return a value at the end of function.", column: 11 }] },
{ code: "foo.every(function cb() { if (a) return true; })", errors: [{ message: "Expected to return a value at the end of function 'cb'.", column: 20 }] },
{ code: "foo.every(function cb() { if (a) return true; })", errors: [{ message: "Expected to return a value at the end of function 'cb'.", column: 11 }] },
{ code: "foo.every(function() { switch (a) { case 0: break; default: return true; } })", errors: [{ messageId: "expectedAtEnd", data: { name: "function" } }] },
{ code: "foo.every(function foo() { switch (a) { case 0: break; default: return true; } })", errors: [{ messageId: "expectedAtEnd", data: { name: "function 'foo'" } }] },
{ code: "foo.every(function() { try { bar(); } catch (err) { return true; } })", errors: [{ messageId: "expectedAtEnd", data: { name: "function" } }] },
Expand All @@ -163,7 +163,7 @@ ruleTester.run("array-callback-return", rule, {
{ code: "foo.every(a ? function() {} : function() {})", errors: ["Expected to return a value in function.", "Expected to return a value in function."] },
{ code: "foo.every(a ? function foo() {} : function bar() {})", errors: ["Expected to return a value in function 'foo'.", "Expected to return a value in function 'bar'."] },
{ code: "foo.every(function(){ return function() {}; }())", errors: [{ message: "Expected to return a value in function.", column: 30 }] },
{ code: "foo.every(function(){ return function foo() {}; }())", errors: [{ message: "Expected to return a value in function 'foo'.", column: 39 }] },
{ code: "foo.every(function(){ return function foo() {}; }())", errors: [{ message: "Expected to return a value in function 'foo'.", column: 30 }] },
{ code: "foo.every(() => {})", options: [{ allowImplicit: false }], parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected to return a value in arrow function." }] },
{ code: "foo.every(() => {})", options: [{ allowImplicit: true }], parserOptions: { ecmaVersion: 6 }, errors: [{ message: "Expected to return a value in arrow function." }] },

Expand Down Expand Up @@ -198,7 +198,250 @@ ruleTester.run("array-callback-return", rule, {
{ code: "foo.every(function() {})", options: checkForEachOptions, errors: [{ messageId: "expectedInside", data: { name: "function" } }] },
{ code: "foo.filter(function foo() {})", options: checkForEachOptions, errors: [{ messageId: "expectedInside", data: { name: "function 'foo'" } }] },
{ code: "foo.filter(function foo() { return; })", options: checkForEachOptions, errors: [{ messageId: "expectedReturnValue", data: { name: "Function 'foo'" } }] },
{ code: "foo.every(cb || function() {})", options: checkForEachOptions, errors: ["Expected to return a value in function."] }
{ code: "foo.every(cb || function() {})", options: checkForEachOptions, errors: ["Expected to return a value in function."] },

// full location tests
{
code: "foo.filter(bar => { baz(); } )",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedInside",
data: { name: "arrow function" },
type: "ArrowFunctionExpression",
line: 1,
column: 16,
endLine: 1,
endColumn: 18
}]
},
{
code: "foo.filter(\n() => {} )",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedInside",
data: { name: "arrow function" },
type: "ArrowFunctionExpression",
line: 2,
column: 4,
endLine: 2,
endColumn: 6
}]
},
{
code: "foo.filter(bar || ((baz) => {}) )",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedInside",
data: { name: "arrow function" },
type: "ArrowFunctionExpression",
line: 1,
column: 26,
endLine: 1,
endColumn: 28
}]
},
{
code: "foo.filter(bar => { return; })",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedReturnValue",
data: { name: "Arrow function" },
type: "ReturnStatement",
line: 1,
column: 21,
endLine: 1,
endColumn: 28
}]
},
{
code: "Array.from(foo, bar => { bar })",
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedInside",
data: { name: "arrow function" },
type: "ArrowFunctionExpression",
line: 1,
column: 21,
endLine: 1,
endColumn: 23
}]
},
{
code: "foo.forEach(bar => bar)",
options: checkForEachOptions,
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedNoReturnValue",
data: { name: "Arrow function" },
type: "ArrowFunctionExpression",
line: 1,
column: 17,
endLine: 1,
endColumn: 19
}]
},
{
code: "foo.forEach((function () { return (bar) => bar; })())",
options: checkForEachOptions,
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedNoReturnValue",
data: { name: "Arrow function" },
type: "ArrowFunctionExpression",
line: 1,
column: 41,
endLine: 1,
endColumn: 43
}]
},
{
code: "foo.forEach((() => {\n return bar => bar; })())",
options: checkForEachOptions,
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedNoReturnValue",
data: { name: "Arrow function" },
type: "ArrowFunctionExpression",
line: 2,
column: 13,
endLine: 2,
endColumn: 15
}]
},
{
code: "foo.forEach((bar) => { if (bar) { return; } else { return bar ; } })",
options: checkForEachOptions,
parserOptions: { ecmaVersion: 6 },
errors: [{
messageId: "expectedNoReturnValue",
data: { name: "Arrow function" },
type: "ReturnStatement",
line: 1,
column: 52,
endLine: 1,
endColumn: 64
}]
},
{
code: "foo.filter(function(){})",
errors: [{
messageId: "expectedInside",
data: { name: "function" },
type: "FunctionExpression",
line: 1,
column: 12,
endLine: 1,
endColumn: 20
}]
},
{
code: "foo.filter(function (){})",
errors: [{
messageId: "expectedInside",
data: { name: "function" },
type: "FunctionExpression",
line: 1,
column: 12,
endLine: 1,
endColumn: 21
}]
},
{
code: "foo.filter(function\n(){})",
errors: [{
messageId: "expectedInside",
data: { name: "function" },
type: "FunctionExpression",
line: 1,
column: 12,
endLine: 2,
endColumn: 1
}]
},
{
code: "foo.filter(function bar(){})",
errors: [{
messageId: "expectedInside",
data: { name: "function 'bar'" },
type: "FunctionExpression",
line: 1,
column: 12,
endLine: 1,
endColumn: 24
}]
},
{
code: "foo.filter(function bar (){})",
errors: [{
messageId: "expectedInside",
data: { name: "function 'bar'" },
type: "FunctionExpression",
line: 1,
column: 12,
endLine: 1,
endColumn: 26
}]
},
{
code: "foo.filter(function\n bar() {})",
errors: [{
messageId: "expectedInside",
data: { name: "function 'bar'" },
type: "FunctionExpression",
line: 1,
column: 12,
endLine: 2,
endColumn: 5
}]
},
{
code: "Array.from(foo, function bar(){})",
errors: [{
messageId: "expectedInside",
data: { name: "function 'bar'" },
type: "FunctionExpression",
line: 1,
column: 17,
endLine: 1,
endColumn: 29
}]
},
{
code: "Array.from(foo, bar ? function (){} : baz)",
errors: [{
messageId: "expectedInside",
data: { name: "function" },
type: "FunctionExpression",
line: 1,
column: 23,
endLine: 1,
endColumn: 32
}]
},
{
code: "foo.filter(function bar() { return \n })",
errors: [{
messageId: "expectedReturnValue",
data: { name: "Function 'bar'" },
type: "ReturnStatement",
line: 1,
column: 29,
endLine: 1,
endColumn: 35
}]
},
{
code: "foo.forEach(function () { \nif (baz) return bar\nelse return\n })",
options: checkForEachOptions,
errors: [{
messageId: "expectedNoReturnValue",
data: { name: "Function" },
type: "ReturnStatement",
line: 2,
column: 10,
endLine: 2,
endColumn: 20
}]
}
]
});