Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow truthiness coercions on known-true/known-false values #9041

Closed
RyanCavanaugh opened this issue Jun 9, 2016 · 9 comments
Closed
Labels
Effort: Difficult Good luck. Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@RyanCavanaugh
Copy link
Member

Inspired by #7746 but making a new issue for clarity

Bad code:

function func(): boolean { return Math.random() > 0.5; }
if (func) {
  // oops, meant to write func()
}

Change: Under --strictNullChecks, it becomes an error to use an expression in a truthiness position unless the type of the expression is possibly-falsy

Possibly-falsy types are:

  • any
  • The primitives: string, string literal types, number, enum types, and boolean
  • Type parameters
  • Union types containing possibly-falsy types
  • An intersection type where any member is possibly falsy

A truthiness position is:

  • The test expression of an if, while, or do/while
  • The middle expression of a for
  • The first operand of ?, && or ||
  • The operand of !
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Help Wanted You can do this Effort: Difficult Good luck. labels Jun 9, 2016
@RyanCavanaugh
Copy link
Member Author

cc @ahejlsberg

@RyanCavanaugh
Copy link
Member Author

I tried implementing this since it's pretty easy now under --strictNullChecks. It did not go well.

A nonexhaustive list of problems:

  • Interfacing with .d.ts files from non-strictnullcheck libraries is a disaster, since we assume all values from those libraries are truthy. This is perhaps a dealbreaker because this is going to be very common for a long time.
  • Need to special-case while (true) {
  • Since we don't include | undefined in the type of index access expressions, good checks like if (map[key]) { become errors. Not at all clear how we'd fix this except by perhaps trafficking around a "possibly-falsy but assumed-truthy" flag on those expressions, which would be a mess.
  • while(foo) { while(foo) { becomes an error because foo is assumed truthy inside the loop, but it may be mutated

@RyanCavanaugh
Copy link
Member Author

Idea from SBS is to check the specific syntactic forms

if (x) {

and

if (x.y) {

to see if x is non-nullable with call signatures. This should catch a few bugs while not having too many false positives.

@AnyhowStep
Copy link
Contributor

I've had times where I went if (somePromise()) when I meant if (await somePromise()).

Would be nice to also have an option to make it strictly require boolean expressions (or intersections with boolean types) without having to use a lint tool. Since, to me, it is closer to something like strictNullTypes but is strictBooleanTypes

@ulrichb
Copy link

ulrichb commented Aug 26, 2018

@AnyhowStep You could try the following TSLint rule as long this check isn't supported by TypeScript.

https://palantir.github.io/tslint/rules/strict-boolean-expressions/

@jwbay
Copy link
Contributor

jwbay commented Aug 10, 2019

Opened #32802 that implements this de-scoped suggestion #9041 (comment)

@falsandtru
Copy link
Contributor

I don't think this is the scope of strictNullChecks. This change injures the certainty and reliance of strictNullChecks. This change should be enabled by a new flag.

@RyanCavanaugh
Copy link
Member Author

This has been done to the best extent we believe currently possible

@AnyhowStep
Copy link
Contributor

You could try the following TSLint rule as long this check isn't supported by TypeScript.

https://palantir.github.io/tslint/rules/strict-boolean-expressions/

This and the ts-eslint rule for the same thing are not great at the moment.
They trip up over generics and such because the type relationship API isn't exposed.

So, lint rules requiring type information tend to be hacky and not always work.

#9879

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Difficult Good luck. Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants