strict-boolean-expressions conflicts with no-extra-boolean-cast (sometimes)

[WillSquire]

Given a non-boolean value, I'd like to coerce it to a boolean in an if statement or expression to check it's set. However, it's hard to satisfy tslint and eslint without creating an interim variable or using precise checks like lodash's isString (comparing type). To add context, here's an example:

const x: string | undefined = ''

// `no-extra-boolean-cast` issue
if (Boolean(x))
  //...

// `@typescript-eslint/strict-boolean-expressions` issue
if (x)
  //...

Reading both of the rules it seems like there's a bit of a conflict between 'un-necessary' casting (due to coercion) and explicit casting, but both of these rules add value. no-extra-boolean-cast helps in other situations, such as casting a boolean to a boolean, and strict-boolean-expressions helps keep the shotgun from my face :) .

Here's the plugins and extensions from eslintrc if it helps:

  plugins: ['@typescript-eslint', 'jest', 'react', 'react-hooks', 'sonarjs'],
  extends: [
    'standard-with-typescript',
    'plugin:jest/recommended',
    'plugin:react/recommended',
    'plugin:@typescript-eslint/recommended-requiring-type-checking',
    'plugin:sonarjs/recommended',
    'prettier',
    'prettier/@typescript-eslint',
    'prettier/react',
  ],

[bradzacher]

The point of the rule is to be explicit about your boolean comparisons so you explicitly handle every case, making it clear about your intentions.
Satisfying the rule with Boolean(x) is kind of going against the spirit of the rule - all you've done there is traded an implicit boolean conversion with an explicit boolean conversion.

The correct way to fix the lint error would be:

const x: string | undefined = ''
if (x != null && x !== '') {
  // ...
}

This will satisfy both lint rules.

[WillSquire]

Thanks for getting back so fast @bradzacher. "The point of the rule is to be explicit about your boolean comparisons" and "all you've done there is traded an implicit boolean conversion with an explicit boolean conversion" sound at odds to me though (: . I'd say that Boolean() is a function that returns a boolean and it's extremely explicit. The comparisons suggested are like isString except they're a bit more verbose (trying to step away from that).

[bradzacher]

I don't agree that it's explicitly handling the cases though - reading that code I still wouldn't be entirely certain if your intention is "handle the empty string case the same as the nullish case", or if you were just working around a lint rule / bug.

Looking around the FB codebase, the only times I see someone using the Boolean function is to coerce a nullable boolean to a boolean, though even then the majority case is just to do bool === true.

Regardless, though. This isn't something we can fix with our lint rule - it's reporting exactly as expected.
If it's something that is causing you issues, I'd consider submitting an issue with the eslint core package to add an option to allow if (Boolean(x)).

[WillSquire]

Thanks @bradzacher, I get where you're coming from and agree for the most part. My use case was for objects rather than strings, but I didn't stop to think before I simplified it to a string 🤦and I've just noticed the allowSafe option :) .

Going back to the original comment though, I think perhaps what was confusing (for me at least) is the wording around the rule itself. If I'm using a function that returns a boolean, I'd call the result a strict boolean. And from the comments above, it sounds like the root purpose of the rule is actually to enforce the precision or intended result of the coercion? In which case, I might be handy to extend this rule to a no-imprecise-boolean-coercion rule as I can still do something like this to shoot my foot off...

const x: string | undefined = ''
const y = Boolean(x)

// no lint issue
if (y)
  //...

[bradzacher]

Yeah the rule was implemented in a very strict way originally. Since then there's been a lot of learning and development, and there is a rewrite waiting for our next major version release (#1515 and the PR) (next major is waiting for the upcoming ESLint v7).

With that change I also want to eventually stack on some suggestion fixers to help inform users how to "properly" fix the error (eg replace if (x) with if (x != null && x !== ''))

I think adding Boolean support could be good behind a flag, as it's a thing that some people don't like, and some people use to say "I am treating all falsey values the same".

[WillSquire]

Ah nice :) . I'll close this issue in favour of #1515 (hopefully the comments here are applicable to that ticket).

Yeah it would be handy to handle Boolean, although I think the rule probably needs a name change to encompass things outside the realm of expressions.