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 guaranties for tuples #15656

Closed
kujon opened this issue May 8, 2017 · 6 comments
Closed

Weak type guaranties for tuples #15656

kujon opened this issue May 8, 2017 · 6 comments
Labels
Needs Investigation This issue needs a team member to investigate its status.

Comments

@kujon
Copy link
Contributor

kujon commented May 8, 2017

TypeScript Version: 2.3.2

I bumped into a strange behaviour wrt tuples by spotting a difference in the way type inference works with value and reference types.

Code

// Value types are inferred to their most specific type.
const foo = 42; // will infer to foo: 42 instead of foo: number

// Reference types infer to their most generic type.
const bar = [1, 2]; // will infer to bar: number[] instead of bar: [number, number]

This is understandable, because const/readonly work the same way as their JS equivalents (values of reference types are mutable), so casting to the most specific type without having a way of enforcing actual immutability would cause more harm than good.

At that point I expected that explicitly typing my tuples as [number, number] will not only make the compiler comply (no more number[] is not assignable to [number, number]), but also give me additional type guarantees. Then I discovered that I can do the following no problem:

const foo: [number, number] = [1, 2, 3];

const quux: [number, number] = [10, 11];
quux[3] = 12;

const bar: [number, number] = [4, 5];
bar.pop();

const baz: [number, number] = [6, 7];
baz.shift();

const qux: [number, number] = [8, 9];
qux.splice(0, 1);

Expected behaviour:
There are probably 2 things that could be done here:

  1. The exact length of tuples is enforced. Property accessor disallows accessing elements outside tuple's range (currently it uses the union type for everything outside the range). This is a documented behaviour, so probably harder to amend (unless implemented with a flag).
  2. Mutable Array.prototype methods are disallowed on tuples. Virtually all of them, when called on a tuple, guarantee that you're going to end up with a value incompatible with your tuple type. The exception would be calling them with arguments that do nothing (e.g. [1, 2].splice(0, 2)) and calling those which add and element (e.g. [1, 2].push(3)).

Actual behaviour:
Tuples behave like arrays with only minimum length being enforced.

@ghost
Copy link

ghost commented May 8, 2017

Related: #12308, #6229

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label May 24, 2017
@agentcooper
Copy link
Contributor

agentcooper commented May 26, 2017

One more example:

type Vector2d = [number, number]; 
 
const v: Vector2d = [1, 2, 3]; 
 
console.log(v[3].toString()); // throws an exception

TypeScript shows no errors.

Flow correctly shows all errors.

@KiaraGrouwstra
Copy link
Contributor

KiaraGrouwstra commented Jun 9, 2017

Also #16276, #16360. I'm under the impression the most elegant solution would be always inferring the most specific type.

@RyanCavanaugh
Copy link
Member

Tuples are now fixed length, but we've opted to leave the array methods in place for compatibility reasons

@RyanCavanaugh
Copy link
Member

Oh and as const exists now

@MicahZoltu
Copy link
Contributor

we've opted to leave the array methods in place for compatibility reasons

@RyanCavanaugh Is there somewhere I can read more discussion on this? This seems like an easy win for soundness (tuples not allowing .push or .pop. Historically legacy unsound behavior has not been given preference over soundness just because "people may be doing unsound things in their code" so this decision seems out of line with other decisions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
Development

No branches or pull requests

5 participants