Enhancement: [strict-boolean-expressions] Check array filter/find predicate functions as boolean locations.

Before You File a Bug Report Please Confirm You Have Done The Following...

• [X] I have tried restarting my IDE and the issue persists.
• [X] I have updated to the latest version of the packages.
• [X] I have searched for related issues and found none that matched my issue.
• [X] I have read the FAQ and my problem is not listed.

Description

The strict-boolean-expressions does not prevent the following bug:

const numbers: number[] = [];
const filteredNumbers = numbers.filter((element) => {
  return element; // BUG - SHOULD BE SOME KIND OF BOOLEAN COMPARISON!!!
});

This is because it does not examine function return values, it only examines comparisons.

Note that the no-unnecessary-condition rule is a similar rule, and it does support function return values. Thus, the function-examining logic needs to also be copy-pasted to this rule, so that this rule can catch the bug above.

For reference, the rule should be able to handle the following Array methods:

• Array.every
• Array.filter
• Array.find
• Array.findIndex
• Array.findLast
• Array.findLastIndex
• Array.some

Duplicate of https://github.com/typescript-eslint/typescript-eslint/issues/1038?

In the meantime I programmed the strict-array-methods rule.

Duplicate of https://github.com/typescript-eslint/typescript-eslint/issues/1038?

Technically yeah - but that issue is so generic that it's forgettable and hence nobody has found it or actioned it. So in okay for this to be open.

@bradzacher Should I submit my strict-array-methods rule to typescript-eslint?

I think this problem is kind of specific to Array methods because they are not "properly" typed as having a return type of boolean in the upstream definitions.

Yeah I think this fits well into the strict-boolean-expressions rule. The only hangup I have is that maybe people wouldn't want this check? I'm on board with just adding this as an always-on feature and if someone complains in future we can consider an option. I don't see why you'd be against it personally.

Should this also warn on implicit returns for these predicates?

Should this also warn on implicit returns for these predicates?

My understanding of the question is - should we check function literals with implicit return type, i.e. ['foo'].filter(x => x) (contrasted with function literals with explicit return type, i.e. ['foo'].filter((x): string => x))? If that's what you mean, I would say yes.

Should this also warn on implicit returns for these predicates?

My understanding of the question is - should we check function literals with implicit return type, i.e. ['foo'].filter(x => x) (contrasted with function literals with explicit return type, i.e. ['foo'].filter((x): string => x))? If that's what you mean, I would say yes.

Oh, I asked this in a confusing way (for sure short-hand arrow-function expressions should be checked), I meant the following example:

[1, null].filter(x => {
  if (x) {
    return true;
  }

  // implicitly returns `undefined`
});

If I understand this correctly, the rule should check that return statements (and short-hand arrow-function-expression returns) are strict boolean expressions, not necessarily that the predicate itself returns a boolean.

Edit: Oh, I misread your comment, I just understand that you meant explicit or implicit return type annotation 🙈

I meant the following example:

[1, null].filter(x => {
  if (x) {
    return true;
  }

  // implicitly returns `undefined`
});

If I understand this correctly, the rule should check that return statements (and short-hand arrow-function-expression returns) are strict boolean expressions, not necessarily that the predicate itself returns a boolean.

Ah - super helpful clarification!

Hmm, yeah, I see why that's a less obvious answer. My personal preference would be - yes, we should choose whether to flag or not based on the inferred or explicit return type of the callback. That shifts the report to the function as a whole, rather than to individual returns or implicit returns.... and I'm ok with that. Note that that's how the solution to the related issue #1038 works, see in playground.

Any obvious objections or counterexamples come to mind with that approach?

👍 from me on checking both inferred and explicit undefined return types.