microsoft / TypeScript

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

Support user-defined type guard functions #1007

Closed RyanCavanaugh closed 9 years ago

RyanCavanaugh commented 9 years ago

We currently only support some baked-in forms of type checking for type guards -- typeof and instanceof. In many cases, users will have their own functions that can provide run-time type information.

Proposal: a new syntax, valid only in return type annotations, of the form x is T where x is a declared parameter in the signature, or this, and T is any type. This type is actually considered as boolean, but "lights up" type guards. Examples:

function isCat(a: Animal): a is Cat {
  return a.name === 'kitty';
}

var x: Animal;
if(isCat(x)) {
  x.meow(); // OK, x is Cat in this block
}

class Node {
  isLeafNode(): this is LeafNode { throw new Error('abstract'); }
}
class ParentNode extends Node {
  isLeafNode(): this is LeafNode { return false; }
}
class LeafNode extends Node {
  isLeafNode(): this is LeafNode { return true; }
}
var someNode: LeafNode|ParentNode;
if(someNode.isLeafNode()) {
  // someNode: LeafNode in this block
}

The forms if(userCheck([other args,] expr [, other args])) { and if(expr.userCheck([any args])) would apply the type guard to expr the same way that expr instanceof t and typeof expr === 'literal' do today.

Nevor commented 9 years ago

The second example does not seem to scale very well :

class Node {
  isLeafNode(): this is LeafNode { throw new Error('abstract'); }
  isOtherNode(): this is OtherNode { throw new Error('abstract'); }
}
class ParentNode extends Node {
  isLeafNode(): this is LeafNode { return false; } 
  isOtherNode(): this is OtherNode { return false; } 
}
class OtherNode extends Node {
  isLeafNode(): this is LeafNode { return false; }
  isOtherNode(): this is OtherNode { return true; } 
}
class LeafNode extends Node {
  isLeafNode(): this is LeafNode { return true; }
  isOtherNode(): this is OtherNode { return false; } 
}
var someNode: LeafNode|ParentNode|OtherNode;
if(someNode.isLeafNode()) {
  // someNode: LeafNode in this block
}

Furthermore, we are back with typing that is dependent on class construction while one would expect this to work with interface only for items as simple as these.

Finally, given that we are stuck with classes, we can currently encode this idiom more lightly with instanceof :

class SuperNode { }

class ParentNode extends SuperNode { 
  private constrain;  // using dummy private to disjoint classes
}
class OtherNode extends SuperNode { 
  private constrain; 
}
class LeafNode extends SuperNode { 
  private constrain; 
  leafNode : string;
}

var someNode : ParentNode | OtherNode | LeafNode;

if(someNode instanceof LeafNode) {
  someNode.leafNode;
}

The first example seems to be an excellent case for #1003

RyanCavanaugh commented 9 years ago

Perhaps the second example was not clear in its intent. Consider something like this, where instanceof would not work:

interface Sortable {
  sort(): void;
}

class BaseCollection {
  isSortable(): this is Sortable { return false; }
}

class List extends BaseCollection implements Sortable {
  isSortable(): this is Sortable { return true; }
  sort() { ... }
}

class HashSet extends BaseCollection {
  isSortable(): this is Sortable { return false; }
}

class LinkedList extends BaseCollection implements Sortable {
  isSortable(): this is Sortable { return true; }
  sort() { ... }
}
Nevor commented 9 years ago

Indeed its intent is indeed clearer. So I have two comments :

1) With the current compiler, we can leverage that everything is either "undefined" or something to create an optional conversion :

interface Sortable  {
  sort(): void;
}

class BaseCollection {
  asSortable() : Sortable { return undefined }
}

class List extends BaseCollection implements Sortable {
  asSortable() { return this }
  sort(){}
}

class HashSet extends BaseCollection {
}

class LinkedList extends BaseCollection implements Sortable {
  asSortable() { return this; }
  sort() { }
}

function someFun(collection : BaseCollection) {
  var asSortable = collection.asSortable();
  if(asSortable) {
    asSortable.sort();
  }
}

But I agree that it would be strange to do something through asSortable and then come back on collection to call other methods.

2) I assume that the compiler verifies that the result of "this is Sortable" is in accordance with what the class actually implements (the verification might be done structurally). In that case, what about generating the "return true" ?

class List extends BaseCollection implements Sortable {
  isSortable() : this is Sortable; // would return true
  sort() { ... }
}

class OtherList extends BaseCollection {
  isSortable() : this is Sortable; // would return true
  sort() { ... }
}

class OtherList2 extends BaseCollection {
  isSortable() : this is Sortable; // would return false
}

class OtherList3  {
  isSortable() : this is Sortable; // would return true
  sort() { ... }
}

class OtherList4 {
  isSortable() : this is Sortable; // would return false
}
bgever commented 9 years ago

This is a cool idea. Have you considered combining it with generics? Then you could do something like this for array checks:

function isNumber(nbr): nbr is number {
    return typeof nbr === 'number';
}

function isString(str): str is string {
    return typeof str === 'string';
}

function isArrayOf<T>(of: (item) => item is T, a: any[]): a is T[] {
  // Ensure that there's at least one item of T in the array, and all others are of T too.
}

function isArrayOrEmptyOf<T>(of: (item) => item is T, a: any[]): a is T[] {
  // Accepts an empty array, or where all items are of T.
}

function(input: string|string[]|number[]) {
    if (isString(input)) {
        // It's a string.
    }
    else if (isArrayOrEmptyOf(isString, input)) {
        // It's an array of string, or
        // an empty array (could be either of string or number!)
    }
    else if (isArrayOf(isNumber, input)) {
        // It's an array of number.
    }
}
ahejlsberg commented 9 years ago

@bgever We did indeed consider that. It breaks down to (a) allowing x is T where T is a type parameter, and (b) extending type inference such that a type argument can be inferred from the target type of a user defined type guard function. All up I think this could be really useful.

Arnavion commented 9 years ago

One example usage for the standard library - Array.isArray can be changed to:

interface ArrayConstructor {
    isArray<T>(arg: any): arg is T[];
}
awerlang commented 9 years ago

@Arnavion I believe that for Array.isArray it would be more correct to express in a non-generic way:

interface ArrayConstructor {
    isArray(arg: any): arg is any[];
}

This would support union types like MyCustomObject | any[]

And how about:

interface Object {
    hasOwnProperty<T | any>(v: string): v is T;
}

Object.hasOwnProperty<MyCustomObject>.call(obj, 'id');

Or it could narrow to possible matching types obj could be:

var obj: { id: number; description: string; } | any;
if ('id' in obj) {
  // obj.id is valid here  
}
Arnavion commented 9 years ago

@awerlang

Actually, I suppose isArray<T>(): T[] would give the false assurance that the array only has elements of type T. It might be better to require the user to give an explicit assertion <T[]> so that they know they have to perform some validation after the call to isArray to be sure they actually have a T[].

Your last example results in obj being of type any because of union type behavior, so obj.id is also allowed already. If you meant you wanted obj to be considered of the first type in the union (and thus obj.id would have type number) inside the if block, then that doesn't seem right. obj could be { id: "foo" } which would not be assignable to the first type.

For your second example, I assume you meant hasOwnProperty<T>(obj: T | any, v: string): obj is T;. This has the same problem - the presence of an 'id' member doesn't really tell you that obj is of MyCustomObject type and not some other type that also has an id property. So it should be something like hasOwnProperty(obj: any, v: string): obj is { [v]: any };.

Even if the obj was of type { id: number, description: string } | { foo: string } (i.e., the union doesn't contain any), the second and third example should still result in obj being of type { id: any } inside the if-block, because obj could actually be { foo: "5", id: "bar" } which isn't assignable to the first type but is assignable to the second.

awerlang commented 9 years ago

@Arnavion

I think having isArray(arg): arg is any[] is convenient, even though we don't assert element type.

About inferring an object type by duck-typing, I consider it is not a problem in its own, although I see the problems it may lead to. This is a concern in #1427 . How would you handle this?

Arnavion commented 9 years ago

Sorry, perhaps I wasn't clear. I was agreeing with you that isArray should return arg is any[].

And yes, #1427 is exactly the same as what I said for your other two examples.

mintern commented 9 years ago

Here's another use case based on a filesystem Entry as defined in the not-yet-standard File System API:

interface Entry {
    isFile: boolean;
    isDirectory: boolean;
    name: string;
    ...
}

interface DirectoryEntry extends Entry { ... }
interface FileEntry extends Entry { ... }

When (<Entry>entry).isFile, entry is a FileEntry, and when entry.isDirectory, entry is a DirectoryEntry.

Note that the *Entry classes are not easily accessible, so instanceof can't be used.

RyanCavanaugh commented 9 years ago

Approved; assignment to Anders is tentative (someone else can give this a shot if they're feeling up to the challenge)

tinganho commented 9 years ago

@RyanCavanaugh I decided to take shot on this. Just one question. Can the type guard function take multiple arguments?

function isCat(a: Animal, b: number): a is Cat {
  return a.name === 'kitty';
}

and if so does the argument index need to match the parameter index?

if(isCat(b, a)) {
  a.meow();// error
}

No error if matched index:

if(isCat(a, b)) {
  a.meow();
}

I'm not sure of its's usefulness though.

RyanCavanaugh commented 9 years ago

Multiple arguments should be allowed.

I don't understand the example, though. What are the types of the unbound variables a and b in the bottom two code blocks?

tinganho commented 9 years ago

a is of type Animal and b just a number. let's assume that isCat takes any as argument too.

mhegazy commented 9 years ago

Thanks @tinganho!

mintern commented 9 years ago

:clap: Thank you!

tinganho commented 9 years ago

:smiley:

DanielRosenwasser commented 9 years ago

:clap: Great job @tinganho!

JsonFreeman commented 9 years ago

@jbondc The scenario you are describing is covered by #1003, not this feature.

JsonFreeman commented 9 years ago

I see what you are saying, but it requires a way for the user to express the intended mapping from return values to narrowing types. I think this is not a useful capability in general. Certain special cases are useful though. Type predicates and property accesses with literals are indeed useful cases. They can use different mechanisms because they are different enough.

rotemdan commented 8 years ago

I'm aware the feature has already been finalized in 1.6, but just wanted to mention a possible alternative syntax using decorators (extended to functions [at least for this case] and evaluated at compile-time):

@inquiresType(Cat)
function isCat(a: Animal): boolean {
  return a.name === 'kitty';
}

var x: Animal;
if(isCat(x)) {
  x.meow(); // OK, x is Cat in this block
} 

class Node {
  @inquiresClassType(LeafNode)
  isLeafNode(): boolean { throw new Error('abstract'); }
}

class ParentNode extends Node {
  @inquiresClassType(LeafNode)
  isLeafNode(): boolean  { return false; }
}

class LeafNode extends Node {
  @inquiresClassType(LeafNode)
  isLeafNode(): boolean { return true; }
}

var someNode: LeafNode|ParentNode;
if(someNode.isLeafNode()) {
  // someNode: LeafNode in this block
}

I'm not saying the current syntax is bad at all. However, I would imagine that if the feature was proposed today, an approach like this might be seen as more conservative (would have a lower general impact on syntax, and probably be easier to implement in the compiler).

use-strict commented 8 years ago

@RyanCavanaugh , the first example says you can use this is LeafNode, but in reality only the a is Cat syntax works.

Maybe the example wasn't updated or it's a bug. The merged PR description however leads me to believe this was never implemented.

RyanCavanaugh commented 8 years ago

@use-strict you're correct that it wasn't implemented. Probably best to open a new issue if this is something you'd like to have (so we can gauge how many people are running into it).

mrcrowl commented 8 years ago

@RyanCavanaugh, did support for this is X make it into the 1.6 release?

class ParentNode extends Node {
  isLeafNode(): this is LeafNode { return false; }
}

I'm doing something along the lines of the following, but it appears to be unsupported?

abstract class Animal
{
    get isCat(): this is Dog;
}

class Dog extends Animal
{
    get isDog(): this is Dog { return true; }
}

EDIT: Whoops, I missed the post above which covered this. I have opened a new issue here: #5764

eggers commented 8 years ago

Show this have solved the error Index signature of object type implicitly has an 'any' type. for the example below?

var foo = {bar: 'bar'}
var prop = 'bar';

if (foo.hasOwnProperty(prop)) {
  console.log(foo[prop])
}
RyanCavanaugh commented 8 years ago

@eggers You'd have to rewrite it slightly, but it works:

function hasProp<T>(x: T, name: string): x is T & { [key: string]: any } {
  return x && x.hasOwnProperty(name);
}

var foo = {bar: 'bar'}
var prop = 'bar';

if (hasProp(foo, prop)) {
  console.log(foo[prop])
}
eggers commented 8 years ago

@RyanCavanaugh Not quite, because then the below doesn't throw an error like it should:

function hasProp<T>(x: T, name: string): x is T & { [key: string]: any } {
  return x && x.hasOwnProperty(name);
}

var foo = {bar: 'bar'}
var prop = 'bar';

if (hasProp(foo, prop)) {
  console.log(foo['abc'])
}
shelby3 commented 7 years ago

I don't understand why the proposed syntax is necessary, nor how it is even sound typing. The compiler isn't able to check the typing assumption.

Unless someone can justify it's existence in the version 1.6 of the compiler, I will never use it and wish there is a compiler flag to prevent its use. And I would prefer it be deprecated asap.

@RyanCavanaugh wrote:

In many cases, users will have their own functions that can provide _run-time type information_.

I suppose there are cases where the types can't be discriminated based on structural typing because the types don't differ in their member properties, but afaics instanceof would still always work with the prototype chain of class. In other words, instanceof is always nominal, not structural.

What other run-time type information scenarios would justify this syntax?

@RyanCavanaugh wrote:

Perhaps the second example was not clear in its intent. Consider something like this, where instanceof would not work:

The following works and there is no unchecked typing assumption:

abstract class Sortable {
  abstract sort(): void
}

class BaseCollection {
}

class List extends BaseCollection implements Sortable {
  sort() {}
}

var someNode : BaseCollection | List

if(someNode instanceof Sortable) {
  someNode.sort();
}

@mintern wrote:

Note that the *Entry classes are not easily accessible, so instanceof can't be used.

What do you mean by "not easily accessible"?

kitsonk commented 7 years ago

@shelby3 why are you commenting on issues that have been closed for 18 months including picking out comments made nearly two years ago? If you have an issue, it would be far more productive to state it against the current state of TypeScript instead of spending energy trying to debate things that have been dead a buried for extended periods of time.

shelby3 commented 7 years ago

@kitsonk wrote:

why are you commenting on issues that have been closed for 18 months including picking out comments made nearly two years ago?

Because the FAQ tells me to do precisely that:

Denial: It's healthy to believe that a suggestion might come back later. Do keep leaving feedback! We look at all comments on all issues - closed or otherwise. If you encounter a problem that would have been addressed by a suggestion, leave a comment explaning what you were doing and how you could have had a better experience. Having a record of these use cases helps us reprioritize.

@kitsonk wrote:

If you have an issue, it would be far more productive _to state it against the current state of TypeScript_ instead of spending energy trying to debate things that have been dead a buried for extended periods of time.

The bolded is exactly what I did. A bad design decision is never buried (click that link!). In particular, I am hoping Google SoundScript team is reading, so they will know which mistakes to consider avoiding.

And again, I asked for someone to provide to me justification for this feature change. Until I understand some counter logic to my points, I am going to continue to think it was a bad decision, hope it gets deprecated, and also try to influence it not being duplicated in other derivative works.

I am also open to being convinced via discussion that I am wrong, which I stated.

Btw, 1.6 was only released within the last year. The TypeScript development is highly accelerated at this time, and there is great danger of rushing design mistakes.

Btw, are you angry? I will quote from that same FAQ:

Anger: Don't be angry.

The way you worded your response to me was to entirely ignore the substance of my discussion and basically it seems you are trying to demoralize me by belittling my sincere and thoughtful contribution. A more productive (and less politically contentious) response from you would have actually address the substance of my comments. Any way, I will continue to do what I do, which is operate with a high standard of excellence and ethics and ignore those who try to turn factual discussions into political contests.

I also sense there will be some embarrassment if I am correct, which might be motivating the nature of your response. C'est la vie. All of us make mistakes, including myself. Being factual and objective is what matters. When the decision is inherently subjective, then I must acquiesce.

kitsonk commented 7 years ago

@shelby3 I am trying to point out that your thoughtful comments will simply be lost and ignored. Even though the FAQ states that, I think the context of which your comments were directed is for an issue that was to introduce user defined type guards. You seem to want to debate the nature of the nominal typing in TypeScript. I noticed you commented on #202 which is likely more aligned to the topic at hand, than some issue that has been resolved.

It was not politically motivated at all, nor am I angry. I saw someone who appears to be relatively new to TypeScript add significant comments to several older issues, some of which didn't seem to be aligned to the topic at hand. While maybe a bit direct, I was suggestion ways that individual might get more appropriate consideration to their thoughts.

shelby3 commented 7 years ago

@kitsonk thanks for clarifying that you are not waging any political campaign against me. To clarify the way I operate, I respond and document my own thoughts in context where they apply, for my own record of my thoughts and design decisions. If my sharing benefits anyone else and leads to any mutually beneficial discussion, then great. But driving action directly from my effort to study issues and comment is not the sole objective of the way I am working. For example, I will cross-reference in newer discussions, comments on older issues that I've read and commented on to try to bring myself up to speed on TypeScript. So just because a comment goes here in context, doesn't it mean it won't get tied into discussion impacting new discussion. Yes I am very interested in the nominal typing issue and in particular typeclasses and probably about to make a proposal on add them to model the prototype chain (but this is very complex). I did add an issue today suggesting if-else and switch as expressions, which I just noticed you are commenting negatively on. I am all over the place on TypeScript for the past 36 hours or so.

RyanCavanaugh commented 7 years ago

Just to set expectations here, there is absolutely no way we're going to remove an oft-requested and popular type system feature from the language because some people feel it can't be used correctly. You can set up a TSLint rule to disallow it in your codebase if you think it's somehow dangerous.

We use type guards internally all over the place, e.g. https://github.com/Microsoft/TypeScript/blob/master/src/compiler/utilities.ts#L894 . There are lots of situations where they're useful and we added this because sometimes the built-in type checking things are not good enough.

shelby3 commented 7 years ago

@RyanCavanaugh wrote:

You can set up a TSLint rule to disallow it in your codebase if you think it's somehow dangerous.

Thank you for making me aware of that tool. Kudos on providing such a configurable toolset.

there is absolutely no way we're going to remove an oft-requested and popular type system feature from the language because some people _feel it can't be used correctly_.

I appreciate your bluntness.

And it is good we air our respective mindsets, so we can see this isn't a personal battle, but somehow a difference of perspective.

There are no feelings involved in my objective analysis of the inability to the use the feature correctly. I would love to see someone refute my logic.

I explained that:

  1. Afaics, the compiler can't check the soundness of the declaration, thus (if my analysis is correct?) it is impossible to use the feature correctly. What is the point of a compile-time check on types, if type declarations can't be checked by the compiler to be consistent?

    It is unfathomable to me that you (and the majority here) would think that depending on users to manually check the consistency of their declarations is acceptable for compiler-enforced type checking. Whether it is popular and vested inertia, is irrelevant to discussion about correctness (since correctness and applicability to scaling large projects is the entire point of TypeScript ... well maybe correctness and permissiveness?[1]). For derivative projects (including forks), might want to correct this mistake if the TypeScript is unwilling to. It just boggles my mind how this feature ever got into the language. I must be missing something and I am hoping someone can point out my myopia? I hope I am wrong, because my current state-of-mind is bewilderment as to how a team can produce such a great project with many wise decisions and let this very bad one slip through the cracks.

  2. I can't find any use for the feature that can't be accurately checked by the compiler with an equally eloquent syntax and which doesn't require this compiler uncheckable feature.

It really boggles my objective mind. I am still hoping someone can point out how I am missing the point of this feature. I'd much prefer to find I am the one who is wrong, because it would increase my confidence in the TypeScript design team. Or at least admission of mistakes would be better than, "if it is popular, then correctness doesn't matter". Even some caveat in the documentation of the feature which explains how it is a potential soundness hole, would be more objective than covering our eyes and ears.

[1] Note I am preparing a rebuttal to those linked N4JS claims of being improvements over TypeScript and will link it here when I am done. And here it is.

aluanhaddad commented 7 years ago

@shelby3

The following works and there is no unchecked typing assumption:

abstract class Sortable { abstract sort(): void }

class BaseCollection { }

class List extends BaseCollection implements Sortable { sort() {} }

var someNode : BaseCollection | List

if(someNode instanceof Sortable) { someNode.sort(); }

It does not work. Checkout the generated JavaScript and the behavior of the code as demonstrated here http://www.typescriptlang.org/play/#src=abstract%20class%20Sortable%20%7B%0D%0A%20%20abstract%20sort()%3A%20void%0D%0A%7D%0D%0A%0D%0Aclass%20BaseCollection%20%7B%0D%0A%7D%0D%0A%0D%0Aclass%20List%20extends%20BaseCollection%20implements%20Sortable%20%7B%0D%0A%20%20sort()%20%7B%7D%0D%0A%7D%0D%0A%0D%0Avar%20someNode%20%3A%20BaseCollection%20%7C%20List%0D%0A%0D%0Aif(someNode%20instanceof%20Sortable)%20%7B%0D%0A%20%20alert('someNode%20has%20Sortable%20in%20its%20prototype%20chain')%3B%0D%0A%7D%20else%20%7B%0D%0A%09alert%20('someNode%20does%20NOT%20have%20Sortable%20in%20its%20prototype%20chain')%0D%0A%7D

This however, works quite well.

abstract class sortable {
    abstract sort(): void;
}
class BaseCollection {
}
class List extends BaseCollection implements Sortable {
  sort() {}
}

var someNode : BaseCollection | List

function isSortable(collection: BaseCollection): collection is Sortable {
    return typeof collection.sort === 'function';
}

if(isSortable(someNode)) {
    someNode.sort();
}

It is my strong feeling that this example demonstrates the usefulness of user defined type guards.

There are no feelings involved in my objective analysis of the inability to the use the feature correctly. I would love to see someone refute my logic.

You are begging the question.

shelby3 commented 7 years ago

@aluanhaddad I was missing the construction of an instance for someNode:

var someNode : BaseCollection | List   // someNode is `undefined`

if(someNode instanceof Sortable) {
     someNode.sort();
}

My corrected example shows that 100% abstract classes can be put into the prototype chain by employing the extends keyword, which was the intended point I was making.

The remaining problem is that TypeScript does not support multiple inheritance of subclass through linearization of the hierarchy, so we can't currently extend from both BaseCollection and Sortable simultaneously. But that shouldn't be necessarily...

(P.S. I have never coded in TypeScript and only started to learn it 48 hours ago. That will give you some indication of my experience level in general with programming and languages)

It appears to me to be an error that TypeScript is not putting the implemented interfaces in the prototype chain so that instanceof will work correctly for interfaces. Even though (even abstract) classes are treated nominally for the prototype chain with extend, they are still structurally typed otherwise. So the argument that interfaces are structurally typed doesn't seem to be a contradiction. And since interfaces have no implementation, they don't need linearization (they can be placed in the prototype chain in any random order). Note I need to search the issues and reread the FAQ to see if this has already been discussed.

https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#4.24

A type guard of the form x instanceof C, where x is not of type Any, C is of a subtype of the global type 'Function', and C has a property named 'prototype'

  • when true, narrows the type of x to the type of the 'prototype' property in C provided it is a subtype of the type of x

So I maintain my stance that the feature of this issue was not necessary. And now I add a bug report that TypeScript is failing to put interfaces into prototype chain. Not fixing a bug and making an unnecessary unsound typing feature to compensate for not doing so, is a design mistake. That is what open source is for, so that with enough eyeballs someone will catch the error.

It is my strong feeling that this example demonstrates the usefulness of user defined type guards.

There are no feelings involved in my objective analysis of the inability to the use the feature correctly. I would love to see someone refute my logic.

You are begging the question.

Again feelings have nothing to do with my posting activity. I am here on mission of production, documentation, sharing, and objectivity.

I realize you are a younger man well before the precipitous drop in testosterone (starting university in 2007 versus mine in 1983) and I notice you are upvoting spiteful comments against me in other issue threads where I am participating, but really man I am 51 years old. Ego is for little people. You'll earn your stripes eventually and realize the wisdom of the linked blog.

I didn't react strongly to this issue feature because of any desire to prove someone is wrong on the Internet, bravado, or offended by someone else's ego. I reacted strongly because the design of TypeScript is (potentially) a serious matter of great importance to work at hand.

Btw, I appreciate your discussion (even the humorous unnecessary bravado part). You are helping me (we are helping each other) to understand the solution. Isn't that wonderful.

shelby3 commented 7 years ago

@shelby3 wrote:

Even though (even abstract) classes are treated nominally for the prototype chain with extend, they are still structurally typed otherwise. So the argument that interfaces are structurally typed doesn't seem to be a contradiction.

I suppose the argument against my logic would be that putting interfaces in the prototype chain would be metadata inserted by the compiler and not an erasure of all elided typing hints.

The feature of this issue (which I argue is unsound) burdens the user with inserting superfluous adhoc metadata and eliminates the ability of the compiler to check it. Thus on balance, I think my logic stands. The compiler should insert the metadata and thus insure soundness.

mintern commented 7 years ago

Here's a perspective from a huge fan of TypeScript who is not involved in its development. We had a huge JavaScript codebase that we converted to TypeScript, and it's now much easier to work with. We now have easier and safer refactoring, auto-completion, semantic navigation, and some compiler checks on annotations that used to be part of comments instead of being part of the code.

@shelby3 wrote:

Afaics, the compiler can't check the soundness of the declaration, thus (if my analysis is correct?) it is impossible to use the feature correctly. What is the point of a compile-time check on types, if type declarations can't be checked by the compiler to be consistent?

Because of the incompleteness theorem, there is always a correct program that cannot be described by a rigid type system. Every language that I know of has an escape hatch: Rust has unsafe, Java has reflection and type-casting, and the list goes on and on. More to the point, raw JavaScript is completely dynamic, and it's up to the programmer to write everything correctly, with no help from a compiler.

To me, TypeScript is a tool that helps me write correct programs. Its gradual typing is a hint about its philosophy: start with whatever you have, and improve it incrementally by adding information to your program. As you continue to do that, TypeScript helps you more and more. I don't think its goal is to be perfect, just to be better, and it has succeeded for us at least: the TypeScript is significantly better than the JavaScript it replaced for us.

Bringing it back to this discussion, user-defined type guards are a perfect example. I can write a little function (like aluanhaddad's isSortable example) that tells the compiler, "Hey, I know that you can't tell this function is actually checking for a Sortable type, but trust me, I know what I'm doing here." And the compiler will believe me and allow me to use this new isSortable tool in the rest of my program.

Without that feature, I have to write the check and cast anywhere in my program that I need to check for a Sortable type, and that's much more error-prone than a user-defined type guard.

In other words, you're right that the compiler isn't verifying the correctness of a user-defined type guard function, but I'm pretty sure that's exactly the point of the feature.

shelby3 commented 7 years ago

And this response is from someone who is potentially going to become a huge fan of TypeScript.

Note in following I am not criticizing you. Not at all, I appreciate you raising this perspective so I can respond with my perspective on it. It isn't personal at all. We are trying to learn from each other. I am open to the possibility of learning that my perspective is incorrect.

@mintern wrote:

Every language that I know of has an escape hatch:

But my point is that the feature added from this issue is not an escape hatch from the compiler. A feature was added to the compiler to check something that it would not otherwise not check, and we are enabling a human to tell the compiler what the correct type of that check should be without enabling the compiler to have any way to verify that the human didn't make a mistake. A compiler is normally to enforce the consistency of invariants it derives from declarations (a declaration can never be a mistake because it is either consistent or a compiler error), not accepting mistakes from humans as a foundation for consistency. Rather just leave the checking off and have a true escape hatch, e.g. the type any, if the compiler can't check it.

I understand that any escape hatch makes the compiler's internal consistency potentially inconsistent externally, but the point of a compiler is never to be consistent with the entire universe, but to maintain an internal consistency. I am arguing we are destroying the internal consistency.

Also my point is that the compiler could indeed check the motivating cases properly but isn't doing, so instead we create an unnecessary feature which is more unsound than not having the feature at all and leaving it uncheck (an unverified check is worse than no check, because it is an assurance that be totally deceiving).

"Hey, I know that you can't tell this function is actually checking for a Sortable type, but trust me, I know what I'm doing here."

That is a slippery slope that I will run to hills away from TypeScript if that becomes the accepted community principle on compiler design. And I will actively deride TypeScript for that attitude if it is indeed prevalent (unless someone can teach/convince me it is a correct principle of design). That quoted English statement (not this issue feature) is perfectly okay to do within an escape hatch, i.e. that was the traditional theme of JavaScript (loose and free).

It is perfectly okay to create an escape hatch where we acknowledge that the compiler is not checking. IMO, it is not okay to start fooling the compiler to make assurances which rely on human error. AFAICS, that defeats the entire point of compile-time checking.

If we build a house on quicksand, we'll eventually end up with a clusterfuck. Let's not conflate escape hatches with houses-of-mirrors. The analogy to house-of-mirrors is very intentional. Soon we won't know what we are looking it, as the confluence and contagion of human error input to the compiler combines with other features in unfathomable Complexity of Whoops™. Due to the Second Law Of Thermodynamics (disorder always trending to maximum), we are most assuredly to end up with no typing assurance eventually any where in the code. Defending order requires a continual work to ward off erosion.

Perhaps @ahejlsberg has accumulated some experience which says otherwise? (Btw Anders do you remember Jeff Stock from Borland?)

Without that feature, I have to write the check and cast anywhere in my program that I need to check for a Sortable type, and that's much more error-prone than a user-defined type guard.

Let's bite the bullet and have the compiler add interfaces to the prototype chain, so that instanceof works as expected.

K.I.S.S. applies here.

Building up a clusterfuck of complexity just because of some purist design goal to never add metadata for erased typing hints is bogus, because we are adding metadata to the prototype chain for the base classes which may be erased if never instantiated.

aluanhaddad commented 7 years ago

@shelby3

I realize you are a younger man well before the precipitous drop in testosterone (starting university in 2007 versus mine in 1983) and I notice you are upvoting spiteful comments against me in other issue threads where I am participating, but really man I am 51 years old. Ego is for little people. You'll earn your stripes eventually and realize the wisdom of the linked blog.

These statements have no place in this discussion.

Btw, I appreciate your discussion (even the humorous unnecessary bravado part). You are helping me (we are helping each other) to understand the solution. Isn't that wonderful.

Indeed, I was attempting to be humorous, but I was also being serious. Stating that one is objective and not coming from any emotional place, especially after having already intimated that others are coming from an emotional place, makes one's further statements highly suspect. Yes, we are helping each other learn, and yes that is a wonderful thing.

So I maintain my stance that the feature of this issue was not necessary. And now I add a bug report that TypeScript is failing to put interfaces into prototype chain. Not fixing a bug and making an unnecessary unsound typing feature to compensate for not doing so, is a design mistake. That is what open source is for, so that with enough eyeballs someone will catch the error.

The remaining problem is that TypeScript does not support multiple inheritance of subclass through linearization of the hierarchy, so we can't currently extend from both BaseCollection and Sortable simultaneously.

In TypeScript interfaces have always been structural. They declare a compile time name for a certain shape. Not all TypeScript/JavaScript programs are class based. Consider the following code.

export function openModal(modalOptions: ModalOptions): void {
    const modal = createModalImpl(modalOptions.template, modalOptions.viewModel);
    modal.onOpen = modalOptions.onOpen;
    modal.onClose = modalOptions.onClose;
    modal.editable = modalOptions.readonly;
    modal.open();
}

export interface ModalOptions {
    template: string;
    viewModel: any;
    onOpen: (callback: () => void);
    onClose: (callback: () => void);
    readonly: boolean;
}

The purpose of the ModalOptions interface is to declare part of the interface of the function openModal so that the TypeScript compiler can check that the correct arguments are specified and so that the caller knows what to pass. It is used for both documentation and typechecking to formally describe the contract of the API, no classes are involved. Thus, the following two examples have the same behavior.

import { openModal, ModalOptions } from 'basic-modal-utilities';
import template from './template.html';

// ex 1.
const vm = {
    name: 'Bob',
    friends: ['Alice', 'Eve']
};
openModal({
    template: template,
    viewModel: vm,
    readonly: true,
    onOpen: {
        console.log(`modal opened with data ${json.stringify(vm)}.`);
    },
    onClose: () => {
        if (vm.name !== 'Bob' || 
            vm.friends[0] !== 'Alice' || 
            vm.friends[1] !== 'Eve' || 
            vm.length > 2) {
            console.error('Test failed: viewModel was modified');
        }
    }
});

// ex 2. 

class ReadonlyModalOptions implements ModalOptions {
    readonly = true;

    constructor(public viewModel, public template) { }

    onOpen() {
        console.log(`modal opened with data ${json.stringify(this.viewModel)}.`);   
    }

    onClose: () => {
        if (this.viewModel.name !== 'Bob' ||
            this.viewModel.friends[0] !== 'Alice' || 
            this.viewModel.friends[1] !== 'Eve' || 
            this.viewModel.length > 2) {
            console.error('Test failed: viewModel was modified');
        }
    }
}
const vm = {
    name: 'Bob',
    friends: ['Alice', 'Eve']
};
openModal(new ReadonlyModalOptions(vm, template));

Lots of APIs work this way. They simply require that a caller pass arguments that meets their requirements, but not how those requirements are defined. This is not only a common pattern but a useful one. If interfaces were added to the prototype chain, then it would be reasonable for someone to add the following line of code to the openModal function:

if (!modalOptions instanceof ModalOptions) {
    throw TypeError();
}

This introduces serious problems. Ex 1. no is longer valid. This is (annoying, but not fatal) for TypeScript consumers who do not want to create a class, but it is catastrophic for JavaScript consumers that can not implement the interface without manually wiring up the prototype chain themselves.

Another issue with linearization is that, as more interfaces are implemented, the depth of the synthetic class hierarchy grows exponentially. This becomes especially problematic in cases where interfaces extend other interfaces and classes reimplement interfaces. Consider

interface Sortable<T> {
    sort<U>(sortBy: (x: T) => U): this;
}
interface NumericallyIndexable<T> {
    [index: number]: T;
}
interface ArrayLike<T> extends Sortable<T>, NumericallyIndexable<T> {
    map<U>(projection: (x: T) => U): ArrayLike<U>;
    filter(predicate: (x: T) => boolean): ArrayLike<T>;
}
interface Grouping<K, V> {
   get(key: K): ArrayLike<V>;
}
class RichCollection implements ArrayLike<T>, Sortable<T> {
    constructor(private elements: T[] = []) { 
        elements.forEach((element, index) => {
            this[index] = element;
        });
    }
    sort<U>(sortBy: (x: T) => U) { 
        return new RichCollection(
            this.elements.sort((x, y) => x.toString().compare(y.toString())
        );
    }
    map<U>(projection: (x: T) => U) { 
        return new RichCollection(this.elements.map(projection)); 
    }
    filter(predicate: (x: T) => boolean) { 
        return new RichCollection(this.elements.filter(predicate)); 
    }
    groupBy<K>(keySelector: (x: T) => K): Grouping<K, T> { ... }
}
shelby3 commented 7 years ago

@aluanhaddad wrote:

These statements have no place in this discussion.

Neither do these provocateurs:

It is my strong feeling that this example demonstrates the usefulness of user defined type guards.

There are no feelings involved in my objective analysis of the inability to the use the feature correctly. I would love to see someone refute my logic.

You are begging the question.

When you intentionally start a fight and promote acrimony, then don't be surprised nor offended when you get punched. Sort of logical how that works eh? Reminds of George Carlin's statement about those who in Hawaii who build their homes next to an active volcano and then are surprised when they have lava flowing through the living room.

especially after having already intimated that others are coming from an emotional place

Listen son, I don't know what your problem is, but before you make a false accusation, please remember to quote the source of your false accusation. I made no such precedent statement about others. I quoted from the FAQ. Direct any such accusation to the source.

You are making trouble. Please stay on point and leave the personal noise out of the technical discussions.

I hope this is the end of this non-technical noise that nobody wants to have to wade through. (I know it won't be the end. I know you will carry this grudge around endlessly and in the future I'll have to deal with you taking out your repressed desire to spank me. I've been on the Internet long enough to know how this works.)

aluanhaddad commented 7 years ago

@shelby3

Listen son, I don't know what your problem is, but before you make a false accusation, please remember to quote the source of your false accusation. I made no such precedent statement about others. I quoted from the FAQ. Direct any such accusation to the source.

In https://github.com/Microsoft/TypeScript/issues/1007#issuecomment-246127188 you wrote

Btw, are you angry? I will quote from that same FAQ:

suggesting that @kitsonk was angry. There was no evidence for that. Even if there were, you were asking the question rhetorically, which weakens your argument by introducing an ad hominem fallacy.

spion commented 7 years ago

@shelby3 You are wrong. This sort of feature is precisely what makes TypeScript different from other typed languages. While they decide NOT to trust the user unless their input fits their narrow preconceptions of what is correctly modelled code, TypeScript takes a different approach. TypeScript mostly trusts the user. The user, in turn, needs to take special care in ensuring some of their code is correct, like type guards. But not all, as it would be with dynamic languages.

This is the biggest win of TypeScript: it lets you pick between "great power, great responsibility" (carefully hand-check the code, ensure that your type assertions are correct) and "lesser power, lesser responsibility" (once you hand-"prove" type assertions are correct, you don't have to do it for the rest of the code).

Its not a type system that ensures correctness. Its a type system that works with the user to ensure it. Its a "bring your own lemma" type system.

kitsonk commented 7 years ago

Its not a type system that ensures correctness. Its a type system that works with the user to ensure it does.

I agree, while acknowledging that the underlying language, JavaScript is permissive and loosely typed. Specifically quoting another TypeScript non-goal:

3) Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.

shelby3 commented 7 years ago

@aluanhaddad:

Btw, are you angry? I will quote from that same FAQ:

Do you not see question mark? Are you just determined to ignore the difference in the English language between a question and an implication or accusation or even insinuation.

Even if there were, you were asking the question rhetorically, which weakens your argument by introducing an ad hominem fallacy.

Where is your proof that I was asking the question rhetorically? You make an assumption that I am evil because obviously that is what you want to believe (since there is no hard evidence to base your accusation). Fantasy is nice and easy. Evidence and objectivity require one to be more circumspect.

There was no evidence for that.

Perhaps you should remember to read the entire comment:

@kitsonk wrote:

why are you commenting on issues that have been closed for 18 months including picking out comments made nearly two years ago?

...

Btw, are you angry? I will quote from that same FAQ:

Anger: Don't be angry.

And the follow-up:

@kitsonk thanks for clarifying that you are not waging any political campaign against me. To clarify the way I operate, I respond and document...

Why are you wasting precious time and carrying around baggage. Let it go and focus on the technical discussion. I will not respond again on this off-topic noise.

aluanhaddad commented 7 years ago

@shelby3 I do not think ageist or sexists remarks have any place in a technical discussion. I felt compelled to object.

Regardless, perhaps you would care to respond to the technical side of my comment, specifically the concerns I raised about linearization of interfaces into the class hierarchy.

shelby3 commented 7 years ago

@aluanhaddad:

In TypeScript interfaces have always been structural. ...

if (!modalOptions instanceof ModalOptions) {
    throw TypeError();
}

If the API consumes structural type, then it won't do that. If it does that, then it intends to consume nominally. The API designer hasn't lost any control. For maximum interoption with non-TypeScript transpiled consumers, the API design may choose to not to use nominal typing.

Edit: and pertaining to this thread's issue feature, if you are checking type structurally, then narrow the type using a guard via structural matching instead of nominally via instanceof or even with the property tag that this issue's feature requires.

Another issue with linearization is that, as more interfaces are implemented, the depth of the synthetic class hierarchy grows exponentially.

The entire prototype chain will only be searched for instanceof and missing properties (unless there are any Object properties needed, but this could be copied forward in the chain as an optimization). EMCAScript can choose to provide further optimizations in their standard if they realize that empty prototype objects that only exist to service the instanceof operator could be better served with a Map. And TypeScript could use a Map for optimization interim, while also populating the prototype chain for interoperability.

Inability to optimize is not usually a winning argument.


@aluanhaddad:

@shelby3 I do not think ageist or sexists remarks have any place in a technical discussion.

OMG! He pulls out the political correctness legal discrimination and State-monopoly-on-force weapon. I see you want to build a political and/or legal case for a ban or what ever. (I didn't expect it to escalate this far, so this is an eye opener for me when dealing with the latest crop of State indoctrinates)

I will have no more discussion nor interaction with you. You just earned a ban (block) from me, so will no longer see your comments. You may speak to my attorney.

Edit: unfortunately the Github block feature doesn't hide your comments from me. Nevertheless I am putting you on notice that I wish to stop all communication with you. Thank you.

shelby3 commented 7 years ago

@spion wrote:

@shelby3 You are wrong.

Wrong about what? Please quote my statement that is "wrong". Rather I think you mean to discuss and argue is your interpretation that TypeScript has design goals which don't match my concerns. That doesn't make me wrong. You know very well from other thread where you are participating, that I have stated that TypeScript's goals seem somewhat ambiguous to me.

The false accusations are flying in and now you @spion are downvoting my comments. So I see it has turned into a "shoot the messenger" politics instead of carefully thought out non-acriminous discussion of technology. That means it is digressing from technical discussion into a pissing match, and thus it is about time for me to leave unless the level of maturity and ethics rises pronto.

spion commented 7 years ago

No it hasn't. You started to veer off the technical rail by discussing age and testosterone, then responded with "Let it go and focus on the technical discussion. I will not respond again on this off-topic noise". I down-voted only these two comments because they are hypocritical.

You are inconsistent, rude and abrasive. Hell, you just sliced my comment at the "you are wrong" point and proceeded to ignore the rest of it, pretending it doesn't exist, and went back to your diatribe against this feature.

I suggest you use TypeScript for at a couple of weeks before commenting further on it.