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

Weak type checking for classes with unreferenced generic parameters #7797

Closed
malibuzios opened this issue Apr 4, 2016 · 14 comments
Closed
Labels
Duplicate An existing issue was already created

Comments

@malibuzios
Copy link

TypeScript Version:

1.8.9

Code

class Bag<T> {
    items = [];
}

function putStringsInTheBag(bag: Bag<string>) {
    bag.items.push("a", "b", "c", "d");
}

function putNumbersInTheBag(bag: Bag<number>) {
    bag.items.push(1, 2, 3, 4);
}

let bagOfStrings = new Bag<string>();
let bagOfNumbers = new Bag<number>();

putStringsInTheBag(bagOfNumbers); // No error!

Expected behavior:
Since Bag<number> is not assignable to Bag<string>, putStringsInTheBag(bagOfNumbers) should error.

Actual behavior:
No error

@jesseschalken
Copy link
Contributor

The typing is structural, not nominal. The typechecker doesn't have a Bag<number> and a Bag<string> per se, it has a {items:any[]} and a {items:any[]}.

Why didn't you declare items as items:T[] = []?

@malibuzios
Copy link
Author

@jesseschalken

That is exactly the programmer mistake the compiler did not catch here. Don't blame the programmer! They're only humans!

@jesseschalken
Copy link
Contributor

Ah, right. Does it still give the green light with --noImplicitAny?

@malibuzios
Copy link
Author

@jesseschalken

Maybe in the way it is formulated now, but that isn't really the point.. The programmer could have written:

class Bag<T> {
    items: any[] = [];
}

Would you recommend "blaming" them again for doing something that's not really optimal and "punish" them by hiding from them the simple fact they assigned Bag<number> to Bag<string> (which is practically always a mistake!) Is this how it works?

@jesseschalken
Copy link
Contributor

Would you recommend "blaming" them again for not doing something that's optimal and "punish" them by hiding from them the simple fact they assigned Bag<number> to Bag<string> (which is practically always an error!) Is this how it works?

any is how you tell the typechecker "leave me alone, I know what I'm doing". The only purpose of any is to bypass the typechecker. So yes, the developer gets the blame for any problems that result.

With Bag as you've defined it, Bag<number> is assigneable to Bag<string>, since there is no property of Bag which references T. The type system is structural, and they are structurally compatible.

@malibuzios
Copy link
Author

@jesseschalken

Now you blame the programmer again by telling them they should go and study how "structural typing" works!

In any case that is not a necessary property of structural typing! why would you think that way? It has to do with how [class] generics were applied on top of structural typing in this particular language.

@jesseschalken
Copy link
Contributor

The only thing I can get out of this is that "generic type not used in type definition" is probably a useful rule for tslint.

@jesseschalken
Copy link
Contributor

If I write a function:

function sum(xs:number[]) {
    return 4;
}

Should this be an error? A typechecker or static analysis tool can't tell what sum was supposed to do. For all it knows the programmer may have intended for it to ignore its parameter and return 4.

What you've done is the same thing, except with types rather than values. You've defined a generic type and not referenced it in the definition. The compiler can't tell that what you wrote wasn't what you intended. It can only work with the information it has.

The only useful thing it can do is warn that you have a type parameter which you didn't use, which was probably unintentional. That's not a hard rule though, so it belongs in TSLint.

That doesn't help you if you used T in another place though, but still have items:any[] = []. There is no way the compiler can know, in general, with the information it has, that when you wrote any you meant T.

@malibuzios
Copy link
Author

@jesseschalken

The scope of the problem I'm describing here, as of now, does not include interfaces (at least at this point). I'm only talking about identically defined and implemented classes or ones that are related by explicit inheritance. For interfaces it's a different story and I currently see that as less important.

When you have two entities that were defined and implemented exactly the same way with exactly the same code and logic, being assigned with incompatible signatures to one another is practically always a mistake. This is a very different story from interfaces.

For interfaces it may be that a linter approach may be sufficient, maybe not. I haven't looked at them that much at this point.

@jesseschalken
Copy link
Contributor

You've already had a response from the TS team regarding this exact topic in #7792 (comment) and #7792 (comment).

I'm not sure what else you're expecting by opening another issue.

@malibuzios
Copy link
Author

@jesseschalken

I don't see this as a trial, and they are not judges, and nobody should feel like they are being accused or need to become a lawyer to protect anyone "else".

This should be a mostly pragmatic discussion on the type checker allowing Bag<number> be assigned to Bag<string>. The code I provided is reasonable and appears type safe. I believe a very high percentage of programmers who would look at it would tell you it should error and surprised it doesn't. This is not different from any other bug reports that are submitted here.

The only thing is that incidentally it turns out is that fixing this touches on more fundamental issues of how generics were applied to classes and may "embarrass" the original designers of the language, people like @ahejlsberg etc., whom I personally invite to comment here. I believe the team members are simply trying to avoid the uncomfortable situation where they will have to re-think and re-discuss this, and prefer simply forgetting it and moving on.

@jesseschalken
Copy link
Contributor

Unless you can cite an actual problem besides "it's not what I expected" then there is no "pragmatic discussion" to be had and this issue can be closed.

@malibuzios
Copy link
Author

@jesseschalken

I don't understand what you want exactly and why do you care if someone reported an issue about something they felt was important enough to open a discussion for. The fact this is an issue that dates back to the original design doesn't mean it cannot be addressed.

It starts to feel like the only people who are in the position to consider this patiently and rationally are the original designers of the language. Participators and team members seem to tend to go with the "lawyer"/"diplomat" approach for the more fundamental aspects which are mostly out of their impact, which I feel is inappropriate.

I'm sorry but this type of negative attitude really discourages people from participating here.

@malibuzios
Copy link
Author

@jesseschalken

By a "pragmatic" attitude I meant looking at this design problem with an open mind and the goal of preventing programmer mistakes rather than strongly adhering to a rigid idea and justification of "what class generics in structural typing means and how it must be".

The only reason I closed the issue was because the atmosphere has become unreasonably hostile and negative. This surprises me for a forum that is supposed to attract people who are interested in language design, who I would expect would approach new suggestions openly and constructively.

@DanielRosenwasser DanielRosenwasser added the Duplicate An existing issue was already created label Apr 4, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants