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

Pedantic: deny mutable type generalization #39915

Open
awerlogus opened this issue Aug 5, 2020 · 3 comments
Open

Pedantic: deny mutable type generalization #39915

awerlogus opened this issue Aug 5, 2020 · 3 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@awerlogus
Copy link

Search Terms

type generalization
type expanding

Suggestion

Currently we have a vulnerability in type safety of mutable variables. You can generalize the type of some variable and change its value by passing there something not compatible with the old type. Examples:

const a: Array<number> = [1, 2]

const b: Array<number | string> = a

b.push('a is not array of numbers anymore')
const a = { data: 0 }

function b (param: { data: string | number }) {
  param.data = 'a is broken now'
}

b(a)

So I suggest adding a new compiler option under 'pedantic' label which will deny such type conversions:

const a: Array<number> = [1, 2]

// Error: type 'Array<number>' is not assignable to 'Array<number | string>'
const b: Array<number | string> = a
const a: Array<number> = [1, 2]

// OK: 'ReadonlyArray' won't let us mutate the state of 'a'
const b: ReadonlyArray<number | string> = a
const a = { foo: 0, bar: 'string' }

function b (param: { readonly foo: number | string, bar: string }) {
  param.bar = String(param.foo)
}

// OK
// property 'foo: number' is assignable to 'readonly foo: number | string'
// property 'bar: string' is assignable to 'bar: string'
b(a) 

function c (param: { readonly foo: number | string, bar: number | string }) {
  param.bar = String(param.foo)
}

// Error: property 'bar: string' is not assignable to 'bar: number | string'
c(a)
@DanielRosenwasser
Copy link
Member

I'm pretty sure we have experimented with strictly applying variance checks for methods (and mutable properties), but lib.d.ts would no longer pass a type-check. @RyanCavanaugh might remember better.

@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Aug 6, 2020
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 6, 2020

The DOM wouldn't typecheck under strict method variance because of how the event listener methods on DOM objects are defined (flow types these as "Function" to get around this). This would really be the least of your problems though; there's a reason C++ has const as a type and member modifier -- without those primitives, every declaration file would really need two versions (if not more) of every type.

@awerlogus
Copy link
Author

@RyanCavanaugh

The DOM wouldn't typecheck under strict method variance because of how the event listener methods on DOM objects are defined

Can you give an example of the code that will be broken after this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

3 participants