microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101k stars 12.48k forks source link

Add support for type guarding to if statements #53201

Open brandonmcconnell opened 1 year ago

brandonmcconnell commented 1 year ago

Suggestion

🔍 Search Terms

type guard guarding assertion

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

This proposal introduces a new feature that would allow developers to use type guard statements with if-statements rather than only with functions. This feature would provide a more concise and expressive syntax for type guards, making code easier to read and maintain.

📃 Motivating Example

With the proposed feature, developers could use the is keyword directly within an if-statement to perform a type guard. For example…

Before — requires separate function for type guarding

function isIterable<T>(obj: Iterable<T> | ArrayLike<T>): obj is Iterable<T> {
  return typeof (obj as Iterable<T>)[Symbol.iterator] === 'function';
}

function iterateOver<T>(obj: Iterable<T> | ArrayLike<T>, expose: Function) {
  if (isIterable(obj)) {
    // some logic here
  } else {
    // some logic here
  }
}

After — can handle type guarding inline via if statement

function iterateOver<T>(obj: Iterable<T> | ArrayLike<T>, expose: Function) {
  const isIterable = typeof (obj as Iterable<T>)[Symbol.iterator] === 'function';
  if (isIterable): obj is Iterable<T> {
    // some logic here
  } else {
    // some logic here
  }
}

** this could just as easily be rewritten to get rid of the isIterable placeholder altogether, like this:

function iterateOver<T>(obj: Iterable<T> | ArrayLike<T>, expose: Function) {
  if (typeof (obj as Iterable<T>)[Symbol.iterator] === 'function'): obj is Iterable<T> {
    // some logic here
  } else {
    // some logic here
  }
}

💻 Use Cases

Currently, if developers want to use a type guard to narrow down the type of a value within an if statement, they need to define a separate function (AFAIK) that returns a boolean value indicating whether the value is of a certain type. This approach can be cumbersome and can lead to code that is harder to read and understand.

whzx5byb commented 1 year ago

Duplicate of https://github.com/microsoft/TypeScript/issues/41710

brandonmcconnell commented 1 year ago

Head & open issue: #6474

koshic commented 1 year ago

@brandonmcconnell theoretically it's a good idea, to remove function-only guards limitation. In the reality, more or less meaningful check + 'obj is' + type name will lead to extremely long, unreadable 'if' statements: instead of read guard name and go next you have to go trough visual garbage and find 'is xxx' statement to understand code logic. As a result, we will have language feature doomed to be permanently banned in various linters / style guides due to above reason.

brandonmcconnell commented 1 year ago

@koshic I don't think the syntax would be banned as you mentioned. I could certainly see the syntax being used in ways that make expressions harder to read but that's not so different from any convention in JS or TS, or programming general for that matter— ternaries, overrides, etc.

In cases where an if-statement might get too long, it would likely be recommended to move the condition into its own variable as I did in my first "after" example, above:

function iterateOver<T>(obj: Iterable<T> | ArrayLike<T>, expose: Function) {
  const isIterable = typeof (obj as Iterable<T>)[Symbol.iterator] === 'function';
  if (isIterable): obj is Iterable<T> {
    // some logic here
  } else {
    // some logic here
  }
}
koshic commented 1 year ago

@brandonmcconnell yeah, it will work - but there is no real difference between

if (isIterable): obj is Iterable<T> {
  // some logic here
} else {
  // some logic here
}

and

if (isIterable(obj) {
  // some logic here
} else {
  // some logic here
}

Also, 'dedicated variable' pattern will split guard into 2 independent parts, which is unsafe: variable declared without any information about expected type. Obviously, check logic & expected type should be defined inside single syntax construct (guard function, or extended 'if' statement from your proposal). So, the chain will be a bit longer: 'do not use long guards in conditions -> extract them to variables -> keep variables near appropriate condition -> do we really need 3 eslint rules for that feature? let's ban it instead'.

What I want to say - every new feature should be fine balanced: flexibility, readability, maintainability & safety (== foolproof) are equal important. From my standpoint, ability to mark boolean variable with 'obj is Xxx' (and check that metadata in conditions) is more realistic: no visual garbage near conditions, single construct, very similar to existing guard functions.

Thx!

brandonmcconnell commented 1 year ago

Great feedback. I appreciate you putting so much time into this.

My only counter-thought would be that the same vulnerability you mentioned is also present with functional type guards.

koshic commented 1 year ago

Hmm, we can't separate guard function definition and 'o is Xxx' statement. Or may be I missed something obvious?

brandonmcconnell commented 1 year ago

Ah I wasn't aware of this error. Got it

image
matthew-dean commented 1 year ago

I think this is a good idea, but related to https://github.com/microsoft/TypeScript/issues/54270, you may want the ability to narrow within the else statement as well (or instead of), so I think this should be supported:

if ([condition]) {

} else: obj is Iterable<T> {

}

In addition, I would expand this to all scopes, because you should also be able to do:

while ([condition]): obj is Iterable<T> {
}

and:

do: obj is Iterable<T> {
} while ([condition])
// OR?
// do {
// } while ([condition]): obj is Iterable<T>

This breaks down with ternary statements, but at that level it adds too much complexity, and the as keyword should probably just be used there.

matthew-dean commented 1 year ago

If you don't want to complicate all the above control flow statements, you should just be able to do:

do {
  assert obj is Iterable<T>
} while ([condition])

if ([condition]) {

} else {
  assert obj is Iterable<T>
}

Right now, it's sometimes very complicated getting TypeScript to magically narrow a type within a scope. Ideally this could just be triggered manually.