Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

ignore-rhs should be configurable to only apply outside of conditionals #4671

Closed
lukewlms opened this issue Apr 17, 2019 · 4 comments
Closed

Comments

@lukewlms
Copy link

lukewlms commented Apr 17, 2019

Feature request

We'd like to use ignore-rhs but do not want to use it inside conditionals or other contexts (like the result of a filter() function) which require a boolean. This could potentially be called ignore-rhs-when-bool-not-required though that's wordy.

Here's an example. I'd like the first two statements to get flagged but the third to not be flagged - so in places where we absolutely must have a boolean, we're sure to get a strict one and not, for example, a truthy empty string.

const func = (b: boolean, s: string): boolean | string => {
  // Gets flagged with ignore-rhs on - correct
  if (s) {
    // ...
  }

  // Does not get flagged with ignore-rhs on, but I'd like it to!  We don't want strings inside of if/while/ternaries/filter or anywhere a bool is required
  if (b || s) {
    // ...
  }

  // This does not get flagged and that's good!  Function is able to return a boolean or a string
  return b || s;
}
@tadhgmister
Copy link

I don't see where the confusion comes in, typescript has no problems if you add an intermediate variable:

const func = (b: boolean, s: string) => {
    // typescript knows `typeof thing === true | string` according to how || evaluates.
    const thing = b || s;
    // since true | string is not a valid boolean type this gets flagged.
    if(thing){
        // ...
    }
    // however this, even though it evaluates to true | string it is not flagged when passed to an if statement.
    if(b || s){
        // ...
    }
  }

can't the rule do the same? If the expression passed to if/while/ternary is not a boolean then throw an error, I don't even see how ignoring the right hand side would even cause it to ignore that.

@JoshuaKGoldberg
Copy link
Contributor

Yeah, this seems reasonable as a rule option. We could take it in.

Please keep #4534 in mind: we're nearing the deadline for no more non-essential PRs.

@JoshuaKGoldberg
Copy link
Contributor

☠️ TSLint's time has come! ☠️

TSLint is no longer accepting most feature requests per #4534. See typescript-eslint.io for the new, shiny way to lint your TypeScript code with ESLint. ✨

It was a pleasure open sourcing with you all!

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants