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

Add a pedantic array variance option #40939

Closed
5 tasks done
cntkillme opened this issue Oct 5, 2020 · 4 comments
Closed
5 tasks done

Add a pedantic array variance option #40939

cntkillme opened this issue Oct 5, 2020 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@cntkillme
Copy link

Search Terms

variance array

Suggestion

Currently, TypeScript allows assignment from D[] to B[] when D is a subtype of B.
Providing a configurable option to control if arrays are covariant (current behavior) vs invariant per project would allow for the user to take advantage of a more sound type system. Additionally, the assignment of a D[] to a readonly B[] is sound and should also be permitted when "pedantic array variance" is specified.

Use Cases

Having the type-checker treat arrays as invariant will allow the compiler to catch a class of issues that are currently not caught. Allowing covariant arrays can be a source of subtle bugs that are hard to track, and many projects do not benefit from having covariant arrays.

By allowing this to be configurable and having the default behavior assume covariant arrays, backwards compatibility is maintained but the user may also opt in to a more sound type system.

Examples

interface B { x: number, y: number }
interface D extends B { z: number }
const arr = new Array<D>();

const readonlyRef: ReadonlyArray<B> = arr; // okay
const ref: Array<B> = arr; // allowing this allows the next line
ref.push({ x: 1, y: 2 }); // oops!
const z = arr[0].z; // undefined (but the type system thinks it is a number)

With pedantic array variance, assignment of arr to ref would produce a diagnostic.

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@cntkillme cntkillme changed the title Add a pedantic array variance option. Add a pedantic array variance option Oct 5, 2020
@andrewbranch
Copy link
Member

From a UX perspective, limiting this to arrays sounds like a reasonable idea, as they’re used everywhere and frequently mutated. From a type system perspective, there’s nothing special about arrays, and strict variance for them would arise naturally if we turned off bivariance for methods and/or calculated read-write properties and index signatures as invariant. This isn‘t to say that a special case couldn’t be carved out for arrays, but it would raise some weird design questions about types that are structurally array-like but not an instance of the global Array type. And logistically, if we did eventually create options for broader strict variance, that would make an array-specific carve-out obsolete, and could create weird invalid combinations of configuration options (--strictMethodVariance=true --pedanticArrayVariance=false?). Because of those things, it’s tempting to call this a duplicate of #18770, or more recently #39915.

@andrewbranch andrewbranch added the Duplicate An existing issue was already created label Oct 6, 2020
@cntkillme
Copy link
Author

I believe arrays are a special case, in checker.ts getVariance always returns arrayVariances (which is [VarianceFlags.Covariant]) for arrays and tuples.

@andrewbranch
Copy link
Member

They are a special case for performance reasons—because we control the shape of the Array interface, we can know in advance how the variance would compute structurally, so we skip it. If we were to change that special case, it would essentially mean that our shortcut returned the wrong result, which would have the weird effects I mentioned.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants