Skip to content

Commit 41ec09f

Browse files
authored
Merge pull request #194 from cherryblossom000/193
fix(no-return-wrap): fix it not reporting for arrow functions without braces
2 parents f9a9ee0 + 12dfa12 commit 41ec09f

File tree

4 files changed

+52
-22
lines changed

4 files changed

+52
-22
lines changed

__tests__/no-return-wrap.js

+25-2
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,20 @@ ruleTester.run('no-return-wrap', rule, {
5454
},
5555

5656
// not function bind
57-
'doThing().then((function() { return Promise.resolve(4) }).toString())'
57+
'doThing().then((function() { return Promise.resolve(4) }).toString())',
58+
59+
{
60+
code: 'doThing().then(() => Promise.reject(4))',
61+
options: [{ allowReject: true }]
62+
},
63+
64+
// Call expressions that aren't Promise.resolve/reject
65+
'doThing().then(function() { return a() })',
66+
'doThing().then(function() { return Promise.a() })',
67+
'doThing().then(() => { return a() })',
68+
'doThing().then(() => { return Promise.a() })',
69+
'doThing().then(() => a())',
70+
'doThing().then(() => Promise.a())'
5871
],
5972

6073
invalid: [
@@ -115,7 +128,7 @@ ruleTester.run('no-return-wrap', rule, {
115128
// {code: 'doThing().catch(function(x) { return true ? Promise.resolve(4) : Promise.reject(5) })', errors: [{message: rejectMessage }, {message: resolveMessage}]},
116129
// {code: 'doThing().catch(function(x) { return x && Promise.reject(4) })', errors: [{message: rejectMessage}]}
117130

118-
// mltiple "ExpressionStatement"
131+
// multiple "ExpressionStatement"
119132
{
120133
code: `
121134
fn(function() {
@@ -214,6 +227,16 @@ ruleTester.run('no-return-wrap', rule, {
214227
}
215228
`,
216229
errors: [{ message: resolveMessage }]
230+
},
231+
232+
// issue #193
233+
{
234+
code: 'doThing().then(() => Promise.resolve(4))',
235+
errors: [{ message: resolveMessage }]
236+
},
237+
{
238+
code: 'doThing().then(() => Promise.reject(4))',
239+
errors: [{ message: rejectMessage }]
217240
}
218241
]
219242
})

rules/always-return.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ module.exports = {
6868
// funcInfoStack[i].branchInfoMap is an object representing information
6969
// about all branches within the given function
7070
// funcInfoStack[i].branchInfoMap[j].good is a boolean representing whether
71-
// the given branch explictly `return`s or `throw`s. It starts as `false`
71+
// the given branch explicitly `return`s or `throw`s. It starts as `false`
7272
// for every branch and is updated to `true` if a `return` or `throw`
7373
// statement is found
7474
// funcInfoStack[i].branchInfoMap[j].loc is a eslint SourceLocation object

rules/lib/has-promise-callback.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Library: Has Promis eCallback
2+
* Library: Has Promise Callback
33
* Makes sure that an Expression node is part of a promise
44
* with callback functions (like then() or catch())
55
*/

rules/no-return-wrap.js

+25-18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
* Rule: no-return-wrap function
3-
* Prevents uneccessary wrapping of results in Promise.resolve
3+
* Prevents unnecessary wrapping of results in Promise.resolve
44
* or Promise.reject as the Promise will do that for us
55
*/
66

@@ -49,26 +49,33 @@ module.exports = {
4949
const options = context.options[0] || {}
5050
const allowReject = options.allowReject
5151

52+
/**
53+
* Checks a call expression, reporting if necessary.
54+
* @param callExpression The call expression.
55+
* @param node The node to report.
56+
*/
57+
function checkCallExpression({ callee }, node) {
58+
if (
59+
isInPromise(context) &&
60+
callee.type === 'MemberExpression' &&
61+
callee.object.name === 'Promise'
62+
) {
63+
if (callee.property.name === 'resolve') {
64+
context.report({ node, messageId: 'resolve' })
65+
} else if (!allowReject && callee.property.name === 'reject') {
66+
context.report({ node, messageId: 'reject' })
67+
}
68+
}
69+
}
70+
5271
return {
5372
ReturnStatement(node) {
54-
if (isInPromise(context)) {
55-
if (node.argument) {
56-
if (node.argument.type === 'CallExpression') {
57-
if (node.argument.callee.type === 'MemberExpression') {
58-
if (node.argument.callee.object.name === 'Promise') {
59-
if (node.argument.callee.property.name === 'resolve') {
60-
context.report({ node, messageId: 'resolve' })
61-
} else if (
62-
!allowReject &&
63-
node.argument.callee.property.name === 'reject'
64-
) {
65-
context.report({ node, messageId: 'reject' })
66-
}
67-
}
68-
}
69-
}
70-
}
73+
if (node.argument && node.argument.type === 'CallExpression') {
74+
checkCallExpression(node.argument, node)
7175
}
76+
},
77+
'ArrowFunctionExpression > CallExpression'(node) {
78+
checkCallExpression(node, node)
7279
}
7380
}
7481
}

0 commit comments

Comments
 (0)