Rule Change: Array methods to highlight when return type does not match expected

[mikedidomizio]

What rule do you want to change?

array-callback-return

What change to do you want to make?

Implement suggestions

How do you think the change should be implemented?

A new option

Example code

// Here is a Typescript Playground [link](https://www.typescriptlang.org/play?#code/PQKhCgAI0hBAneBDAngOgGYEsB2ATKEYccYYSAdwFMByeKyAWyQBcBjAC1wHNIl9I9FgFd4OHpBYcG2eAGcWkAG5IANsIZTWTVpypzIAIgBGSeIfBsA9jgWRuVq3kgBeSAG0aGRzQA0kGlN4GgBdTFw8AApIrAAuSAV4HgBKVwA+SCxXFzdAsxpkkjJIKgAPAAcqNkUg0nIkauE1SCDIQFBySxs5K1UqNFUrbkiAAykGIVEcZTUNSCsMAIcnGniAEgBvJbwAX2HCusoGZjwGJCYsBSQAawZpej4BajpTxrVVFD45OSxucRxeM4qdRUTq2GpIZxuTzeKx+AJBULhfDROIJFhJf6pFwZLK5BH7YplSrVFpmA4NETNGGQQAy5KDur1+oMRmNBFQRGJpsC5gs8ngVpANqYdnsSKQwIQQHBEKg0FQlFR4ChCMR6Yp5YqUABxRyQjx5YL+A2IjVK6JA+KJFLpSDrKBsjlTIHZPH5cDbAnkIlVRTojTk16qSTwWYday2Hp9AZDUbSB2TLmzeYBU3a3UCjapnVOXb7cN2VMAIQhrn1CKNCLCqfNakt6Ot2Nt9udOVdwXdnpKFR9wf9xQpTSDGDUcgYdPzkaZMdZE05QKTvKLEIz6yXIoJEugUoQyHQzHKKpI+cU++zes85fh+TC+5rqjrGO4WIydsg8bnzVbV-bHqKXu7JJ+lQ-hAQGlJBkBIEhgwYZdJO0YsnGs5OjMDDJjQp7pms6yYTmooHFgCwoFYwiQMIo72FQOCKlgbAGHcwGQAAKgAypQWCqEG1gKvAkDEcIACEkBqjo5TFuexoVteaC3pEFpoo+z5Nm+LY5N+NAdn+XbEr60FQX29SBpA-i0vS8HMrG4zsgm85oby+7iSuDkQrmJBAA)

// but for anyone that doesn't want to click through


/**
 ** Array.find
 */

// we're matching and returning the first value that matches "bar"
const good = ['foo', 'bar'].find((i: string) => i === 'bar')

// expect bar
// actual bar ✅
console.log(`the return value of 'good': ${good}`)

// we made a mistake here and we're actually assigning a value
const bad = ['foo', 'bar'].find((i: string) => i = 'bar')

// expect bar
// actual foo ❌
console.log(`the return value of 'bad': ${bad}`)


/**
 ** Array.every
 */

const everyGood = ['bar', 'bar'].every((val: string) => {
  return val === 'bar'
})

// expect true
// actual true ✅
console.log(`the return value of 'everyGood': ${everyGood}`)

const everyBad = ['bar', 'bar'].every((val: string) => {
  val === 'bar'
})

// expect true
// actual false ❌
console.log(`the return value of 'everyBad': ${everyBad}`)

/**
 ** Array.map
 */

const mapGood = ['bar', 'bar'].map((val: string) => {
  return val === 'bar'
})

// expect true, true
// actual true, true ✅
console.log(`the return value of 'mapGood': ${mapGood}`)

// if you use generics here, TS will cover you! 
const mapBad = ['bar', 'bar'].map((val: string) => {
  val === 'bar'
})

// expect true, true
// actual ,  ❌
console.log(`the return value of 'mapBad': ${mapBad}`)

What does the rule currently do for this code?

It can have it so that a return is required, but what is returned (the type) does not matter

What will the rule do after it's changed?

It will be able to highlight places where certain Array methods return values may be not what is expected.

Participation

• I am willing to submit a pull request to implement this change.

Additional comments

In Typescript it was boolean before, they made it unknown/any. There has been some discussion on having stricter types but nothing has come out of it.

My thinking is for the following methods:

Array.from - don't need
Array.every - boolean
Array.filter - boolean
Array.find - boolean
Array.findIndex - boolean
Array.findLast - boolean
Array.findLastIndex
Array.forEach - undefined
Array.map - TS covered if using generics!
Array.reduce - TS covered if using generics!
Array.reduceRight - TS covered if using generics!
Array.some - boolean
Array.sort - not sure
Array.toSorted - not sure

A bit worried in the difficulty of doing it for all Array methods in one PR but I also am not sure if you would allow doing it gradually as an option.

Should this also be the default, or an option? A part of me thinks why wouldn't someone want this, but maybe I haven't put enough thought into this.

[mdjermanovic]

Hi @mikedidomizio, thanks for the issue!

Can you please provide a few examples of JavaScript code where array-callback-return would report new errors after this change?

In two of the three examples marked as bad in the original post, this rule already reports errors:

/* eslint array-callback-return: "error" */

const everyBad = ['bar', 'bar'].every((val) => { // error: Array.prototype.every() expects a return value from arrow function.  (array-callback-return)
  val === 'bar'
})

const mapBad = ['bar', 'bar'].map((val) => { // error: Array.prototype.map() expects a return value from arrow function.  (array-callback-return)
  val === 'bar'
})

Playground demo

The third example is covered by the no-return-assign rule:

/* eslint no-return-assign: "error" */

const bad = ['foo', 'bar'].find((i) => i = 'bar') // error: Arrow function should not return assignment.  (no-return-assign)

Playground demo

[mikedidomizio]

Hey @mdjermanovic thanks for the quick reply. Sorry, I should've probably used the eslint playground to build a proper example demo.

Investigating more I think with the existing no-return-assign rule with the "always" option, I can cover where it bit us. In my initial investigation the rules didn't seem to cover us (we had parentheses), but the "always" option is what will cover us.

/* eslint no-return-assign: ["error", "always"] */

const findBad = ['bar', 'bar'].find((val) => val = 'foo')
const findBad2 = ['bar', 'bar'].find((val) => (val = 'foo'))

const everyBad = ['bar', 'bar'].every((val) => val = 'foo')
const everyBad2 = ['bar', 'bar'].every((val) => (val = 'foo'))

const mapBad = ['bar', 'bar'].map((val) => val = 'foo')
const mapBad2 = ['bar', 'bar'].map((val) => (val = 'foo'))

Playground demo

Going to close this as I think there's nothing to do here.

[eslint-github-bot]

It looks like there wasn't enough information for us to know how to help you, so we're closing the issue.

Thanks for your understanding.