Incorrect reporting of `no-callback-literal`

[tunnckoCore]

I have a iterator function, that is called recursively. Something like

function iterator (foo, bar, baz) {
  return function next (index) {
    // ... code
    // that `next` function is named intentionally
    // because it is called recursively when
    // some condition.
    if (foo === 123) {
      next(index + 1)
    }
  }
}

If that's not so clear enough, see what i mean here each-promise/utils.js#L44-L83

Basically this next function in parallel flow is called for each item in array, passing the index of the item next(index). And in serial flow, that next function is just called recursively.

And it reports that the literal passed as first argument to next is not valid. Easiest fix for me is to just rename that function to, say, nextStep... but yea. I believe the implementation in eslint-plugin-standard can be done better, to report only if there is argument called next, cb or callback.

It is just the more correct approach.

For me, it should report that warning only if the case is such like that

function iterator (foo, bar, baz) {
  return function someAsyncFn (index, cb) {
    // ... code
    if (foo === 123) {
      cb(index + 1)
    }
  }
}

only when there is last argument named next/cb/callback and is passed with wrong literal.

[feross]

@xjamundx Thoughts?

Maybe we should only check cb and callback?

[tunnckoCore]

Maybe we should only check cb and callback?

They are good, even can add done? Actually I have package called common-callback-names, but it is for different purpose - used in another lib that checks if there is last argument called like that names. Notice that some of them are with trailing _, because there is some internal node core methods that use such argument names - for example fs.readFile, fs.appendFile, fs.stat and etc.

Here we don't need this, but done is pretty used too.

edit: also is-async-function may be in help in the impl. actually it can't ;d

[juliangruber]

also discovered this in https://github.com/juliangruber/hyperdrive-import-files, in a setting like this:

function next (mode) {
  // 
}

next('created')

Using common callback names sounds good to me, or maybe checking if the value of the last argument is ever called as a function.

[xjamundx]

done is tricky, because grunt uses it with a status code, but we could try it. i'm pretty ok with just cb and callback though as that should grab the majority of use cases.

[tunnckoCore]

yea it can stay as it is currently, but the main point is to detect and warn only if there is argument with such name and it is called incorrectlly

[dcousens]

Shouldn't the callback detection be limited to a function argument?

[tunnckoCore]

@dcousens absolutely.

[feross]

I don't have the bandwidth to change this rule to only detect callbacks that are passed in as function arguments before getting out the 10.0.0 release.

I'm going to remove next from the list of possible callback function names, since it's more likely to be used in other contexts like iterators, recursive functions, etc. We'll just ship with callback and cb for now and see how it goes.