Skip to content

Fix indexing and destructuring of unions of tuple types #27587

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Oct 11, 2018
Merged

Fix indexing and destructuring of unions of tuple types #27587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
4d9a202
Properly widen initializer types in binding elements
ahejlsberg Oct 5, 2018
c9ea6c3
Accept new baselines
ahejlsberg Oct 6, 2018
7f3f98e
Properly handle unions of tuple types
ahejlsberg Oct 6, 2018
1299c93
Remove unused (and incorrect) code in sys.ts
ahejlsberg Oct 6, 2018
919fce9
Accept new baselines
ahejlsberg Oct 6, 2018
d4f480c
Add tests
ahejlsberg Oct 7, 2018
86704e5
Accept new baselines
ahejlsberg Oct 7, 2018
48f2dd9
Fix linting issue
ahejlsberg Oct 7, 2018
62aeead
Merge branch 'master' into fixUnionOfTupleIndexing
ahejlsberg Oct 7, 2018
2e5a39a
Error when indexing out of bounds in a tuple or union of tuples
ahejlsberg Oct 8, 2018
209f30c
Update test
ahejlsberg Oct 8, 2018
840214f
Accept new baselines
ahejlsberg Oct 8, 2018
dd63656
Address CR feedback
ahejlsberg Oct 11, 2018
b040ea6
One more CR fix
ahejlsberg Oct 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 32 additions & 33 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4683,14 +4683,14 @@ namespace ts {
// If the parent is a tuple type, the rest element has a tuple type of the
// remaining tuple element types. Otherwise, the rest element has an array type with same
// element type as the parent type.
type = isTupleType(parentType) ?
sliceTupleType(parentType, index) :
type = everyType(parentType, isTupleType) ?
mapType(parentType, t => sliceTupleType(<TupleTypeReference>t, index)) :
createArrayType(elementType);
}
else {
// Use specific property type when parent is a tuple or numeric index type when parent is an array
const index = pattern.elements.indexOf(declaration);
type = isTupleLikeType(parentType) ?
type = everyType(parentType, isTupleLikeType) ?
getTupleElementType(parentType, index) || declaration.initializer && checkDeclarationInitializer(declaration) :
elementType;
if (!type) {
Expand All @@ -4706,11 +4706,11 @@ namespace ts {
}
// In strict null checking mode, if a default value of a non-undefined type is specified, remove
// undefined from the final type.
if (strictNullChecks && declaration.initializer && !(getFalsyFlags(checkExpressionCached(declaration.initializer)) & TypeFlags.Undefined)) {
if (strictNullChecks && declaration.initializer && !(getFalsyFlags(checkDeclarationInitializer(declaration)) & TypeFlags.Undefined)) {
type = getTypeWithFacts(type, TypeFacts.NEUndefined);
}
return declaration.initializer && !getEffectiveTypeAnnotationNode(walkUpBindingElementsAndPatterns(declaration)) ?
getUnionType([type, checkExpressionCached(declaration.initializer)], UnionReduction.Subtype) :
getUnionType([type, checkDeclarationInitializer(declaration)], UnionReduction.Subtype) :
type;
}

Expand Down Expand Up @@ -7309,10 +7309,10 @@ namespace ts {
}
}
else if (isUnion) {
const index = !isLateBoundName(name) && ((isNumericLiteralName(name) && getIndexInfoOfType(type, IndexKind.Number)) || getIndexInfoOfType(type, IndexKind.String));
if (index) {
checkFlags |= index.isReadonly ? CheckFlags.Readonly : 0;
indexTypes = append(indexTypes, index.type);
const indexInfo = !isLateBoundName(name) && (isNumericLiteralName(name) && getIndexInfoOfType(type, IndexKind.Number) || getIndexInfoOfType(type, IndexKind.String));
if (indexInfo) {
checkFlags |= indexInfo.isReadonly ? CheckFlags.Readonly : 0;
indexTypes = append(indexTypes, isTupleType(type) ? getRestTypeOfTupleType(type) || undefinedType : indexInfo.type);
}
else {
checkFlags |= CheckFlags.Partial;
Expand Down Expand Up @@ -9311,11 +9311,12 @@ namespace ts {
getFlowTypeOfReference(accessExpression, propType) :
propType;
}
if (isTupleType(objectType)) {
const restType = getRestTypeOfTupleType(objectType);
if (restType && isNumericLiteralName(propName) && +propName >= 0) {
return restType;
if (everyType(objectType, isTupleType) && isNumericLiteralName(propName) && +propName >= 0) {
if (accessNode && everyType(objectType, t => !(<TupleTypeReference>t).target.hasRestElement)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this isn't using a undefinedWideningType which means outside of strict mode, the new types are going to be strictly less permissive than they were before. Are we OK with that?

Copy link
Member Author

@ahejlsberg ahejlsberg Oct 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're going to be less permissive no matter which undefined we use because previously, when the object type was a union type, we'd just fall back to the numeric index signature. For example:

type T = [boolean] | [string, number];
type T0 = T[0];  // Unchanged, string | boolean
type T1 = T[1];  // Now number | undefined, previously number | boolean
type T2 = T[2];  // Now undefined, previously string | number | boolean

I would argue that the non-widening undefined is the right choice above. It only matters in non-strict mode and the only time you'd ever observe a difference is when it is not in a union with something else, i.e. T2 above. I don't think we'd help anybody if T2 was magically going to widen to any.

const indexNode = accessNode.kind === SyntaxKind.ElementAccessExpression ? accessNode.argumentExpression : accessNode.indexType;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

type UndefinedTuple = [string, ...undefined[]]; // ???

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getTupleElementType still handles undefinedType incorrectly.

error(indexNode, Diagnostics.Property_0_does_not_exist_on_type_1, unescapeLeadingUnderscores(propName), typeToString(objectType));
}
return mapType(objectType, t => getRestTypeOfTupleType(<TupleTypeReference>t) || undefinedType);
}
}
if (!(indexType.flags & TypeFlags.Nullable) && isTypeAssignableToKind(indexType, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.ESSymbolLike)) {
Expand Down Expand Up @@ -12877,9 +12878,14 @@ namespace ts {
}

function getTupleElementType(type: Type, index: number) {
return isTupleType(type) ?
index < getLengthOfTupleType(type) ? type.typeArguments![index] : getRestTypeOfTupleType(type) :
getTypeOfPropertyOfType(type, "" + index as __String);
const propType = getTypeOfPropertyOfType(type, "" + index as __String);
if (propType) {
return propType;
}
if (everyType(type, isTupleType) && !everyType(type, t => !(<TupleTypeReference>t).target.hasRestElement)) {
return mapType(type, t => getRestTypeOfTupleType(<TupleTypeReference>t) || undefinedType);
}
return undefined;
Copy link
Member

@weswigham weswigham Oct 10, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, same remark - this needs to use a unique marker and not the undefinedType to signal the lack of a restType return, as it prevents a legitimate undefined rest type from functioning.

}

function isNeitherUnitTypeNorNever(type: Type): boolean {
Expand Down Expand Up @@ -14371,7 +14377,7 @@ namespace ts {
}

function getTypeOfDestructuredArrayElement(type: Type, index: number) {
return isTupleLikeType(type) && getTupleElementType(type, index) ||
return everyType(type, isTupleLikeType) && getTupleElementType(type, index) ||
checkIteratedTypeOrElementType(type, /*errorNode*/ undefined, /*allowStringInput*/ false, /*allowAsyncIterables*/ false) ||
errorType;
}
Expand Down Expand Up @@ -14569,6 +14575,10 @@ namespace ts {
return type.flags & TypeFlags.Union ? forEach((<UnionType>type).types, f) : f(type);
}

function everyType(type: Type, f: (t: Type) => boolean): boolean {
return type.flags & TypeFlags.Union ? every((<UnionType>type).types, f) : f(type);
}

function filterType(type: Type, f: (t: Type) => boolean): Type {
if (type.flags & TypeFlags.Union) {
const types = (<UnionType>type).types;
Expand Down Expand Up @@ -16673,11 +16683,6 @@ namespace ts {
return mapType(type, t => getIndexTypeOfStructuredType(t, kind), /*noReductions*/ true);
}

// Return true if the given contextual type is a tuple-like type
function contextualTypeIsTupleLikeType(type: Type): boolean {
return !!(type.flags & TypeFlags.Union ? forEach((<UnionType>type).types, isTupleLikeType) : isTupleLikeType(type));
}

// In an object literal contextually typed by a type T, the contextual type of a property assignment is the type of
// the matching property in T, if one exists. Otherwise, it is the type of the numeric index signature in T, if one
// exists. Otherwise, it is the type of the string index signature in T, if one exists.
Expand Down Expand Up @@ -17229,7 +17234,8 @@ namespace ts {
}

function getArrayLiteralTupleTypeIfApplicable(elementTypes: Type[], contextualType: Type | undefined, hasRestElement: boolean, elementCount = elementTypes.length) {
if (contextualType && contextualTypeIsTupleLikeType(contextualType)) {
// Infer a tuple type when the contextual type is or contains a tuple-like type
if (contextualType && forEachType(contextualType, isTupleLikeType)) {
const minLength = elementCount - (hasRestElement ? 1 : 0);
const pattern = contextualType.pattern;
// If array literal is contextually typed by a binding pattern or an assignment pattern, pad the resulting
Expand Down Expand Up @@ -18959,13 +18965,6 @@ namespace ts {
error(indexExpression, Diagnostics.A_const_enum_member_can_only_be_accessed_using_a_string_literal);
return errorType;
}
if (isTupleType(objectType) && !objectType.target.hasRestElement && isNumericLiteral(indexExpression)) {
const index = +indexExpression.text;
const maximumIndex = length(objectType.target.typeParameters);
if (index >= maximumIndex) {
error(indexExpression, Diagnostics.Index_0_is_out_of_bounds_in_tuple_of_length_1, index, maximumIndex);
}
}

return checkIndexedAccessIndexType(getIndexedAccessType(objectType, isForInVariableForNumericPropertyNames(indexExpression) ? numberType : indexType, node), node);
}
Expand Down Expand Up @@ -21704,7 +21703,7 @@ namespace ts {
if (element.kind !== SyntaxKind.SpreadElement) {
const propName = "" + elementIndex as __String;
const type = isTypeAny(sourceType) ? sourceType :
isTupleLikeType(sourceType) ? getTupleElementType(sourceType, elementIndex) :
everyType(sourceType, isTupleLikeType) ? getTupleElementType(sourceType, elementIndex) :
elementType;
if (type) {
return checkDestructuringAssignment(element, type, checkMode);
Expand All @@ -21730,8 +21729,8 @@ namespace ts {
}
else {
checkGrammarForDisallowedTrailingComma(node.elements, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
const type = isTupleType(sourceType) ?
sliceTupleType(sourceType, elementIndex) :
const type = everyType(sourceType, isTupleType) ?
mapType(sourceType, t => sliceTupleType(<TupleTypeReference>t, elementIndex)) :
createArrayType(elementType);
return checkDestructuringAssignment(restExpression, type, checkMode);
}
Expand Down
4 changes: 0 additions & 4 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2481,10 +2481,6 @@
"category": "Error",
"code": 2732
},
"Index '{0}' is out-of-bounds in tuple of length {1}.": {
"category": "Error",
"code": 2733
},
"It is highly likely that you are missing a semicolon.": {
"category": "Error",
"code": 2734
Expand Down
17 changes: 0 additions & 17 deletions src/compiler/sys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,6 @@ namespace ts {
Low = 250
}

function getPriorityValues(highPriorityValue: number): [number, number, number] {
const mediumPriorityValue = highPriorityValue * 2;
const lowPriorityValue = mediumPriorityValue * 4;
return [highPriorityValue, mediumPriorityValue, lowPriorityValue];
}

function pollingInterval(watchPriority: PollingInterval): number {
return pollingIntervalsForPriority[watchPriority];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line above causes an error with this PR because we now correctly include undefined in the type because watchPriority is one of the values 250, 500, or 2000, all of which are out of range of the tuple type. This code is basically bogus, but also never used.

}

const pollingIntervalsForPriority = getPriorityValues(250);

/* @internal */
export function watchFileUsingPriorityPollingInterval(host: System, fileName: string, callback: FileWatcherCallback, watchPriority: PollingInterval): FileWatcher {
return host.watchFile!(fileName, callback, pollingInterval(watchPriority));
}

/* @internal */
export type HostWatchFile = (fileName: string, callback: FileWatcherCallback, pollingInterval: PollingInterval | undefined) => FileWatcher;
/* @internal */
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/bestCommonTypeOfTuple.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(22,13): error TS2733: Index '2' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(23,13): error TS2733: Index '2' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(24,13): error TS2733: Index '2' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(25,13): error TS2733: Index '3' is out-of-bounds in tuple of length 3.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(22,13): error TS2339: Property '2' does not exist on type '[(x: number) => string, (x: number) => number]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(23,13): error TS2339: Property '2' does not exist on type '[E1, E2]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(24,13): error TS2339: Property '2' does not exist on type '[number, any]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts(25,13): error TS2339: Property '3' does not exist on type '[E1, E2, number]'.


==== tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple.ts (4 errors) ====
Expand All @@ -28,13 +28,13 @@ tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfT
t4 = [E1.one, E2.two, 20];
var e1 = t1[2]; // {}
~
!!! error TS2733: Index '2' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '2' does not exist on type '[(x: number) => string, (x: number) => number]'.
var e2 = t2[2]; // {}
~
!!! error TS2733: Index '2' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '2' does not exist on type '[E1, E2]'.
var e3 = t3[2]; // any
~
!!! error TS2733: Index '2' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '2' does not exist on type '[number, any]'.
var e4 = t4[3]; // number
~
!!! error TS2733: Index '3' is out-of-bounds in tuple of length 3.
!!! error TS2339: Property '3' does not exist on type '[E1, E2, number]'.
16 changes: 8 additions & 8 deletions tests/baselines/reference/bestCommonTypeOfTuple.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,26 +76,26 @@ t4 = [E1.one, E2.two, 20];
>20 : 20

var e1 = t1[2]; // {}
>e1 : ((x: number) => string) | ((x: number) => number)
>t1[2] : ((x: number) => string) | ((x: number) => number)
>e1 : undefined
>t1[2] : undefined
>t1 : [(x: number) => string, (x: number) => number]
>2 : 2

var e2 = t2[2]; // {}
>e2 : E1 | E2
>t2[2] : E1 | E2
>e2 : undefined
>t2[2] : undefined
>t2 : [E1, E2]
>2 : 2

var e3 = t3[2]; // any
>e3 : any
>t3[2] : any
>e3 : undefined
>t3[2] : undefined
>t3 : [number, any]
>2 : 2

var e4 = t4[3]; // number
>e4 : number
>t4[3] : number
>e4 : undefined
>t4[3] : undefined
>t4 : [E1, E2, number]
>3 : 3

20 changes: 10 additions & 10 deletions tests/baselines/reference/bestCommonTypeOfTuple2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(17,14): error TS2733: Index '4' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(18,14): error TS2733: Index '4' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(19,14): error TS2733: Index '4' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(20,14): error TS2733: Index '2' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(21,14): error TS2733: Index '2' is out-of-bounds in tuple of length 2.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(17,14): error TS2339: Property '4' does not exist on type '[C, base]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(18,14): error TS2339: Property '4' does not exist on type '[C, D]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(19,14): error TS2339: Property '4' does not exist on type '[C1, D1]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(20,14): error TS2339: Property '2' does not exist on type '[base1, C1]'.
tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts(21,14): error TS2339: Property '2' does not exist on type '[C1, F]'.


==== tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfTuple2.ts (5 errors) ====
Expand All @@ -24,17 +24,17 @@ tests/cases/conformance/types/typeRelationships/bestCommonType/bestCommonTypeOfT

var e11 = t1[4]; // base
~
!!! error TS2733: Index '4' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '4' does not exist on type '[C, base]'.
var e21 = t2[4]; // {}
~
!!! error TS2733: Index '4' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '4' does not exist on type '[C, D]'.
var e31 = t3[4]; // C1
~
!!! error TS2733: Index '4' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '4' does not exist on type '[C1, D1]'.
var e41 = t4[2]; // base1
~
!!! error TS2733: Index '2' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '2' does not exist on type '[base1, C1]'.
var e51 = t5[2]; // {}
~
!!! error TS2733: Index '2' is out-of-bounds in tuple of length 2.
!!! error TS2339: Property '2' does not exist on type '[C1, F]'.

20 changes: 10 additions & 10 deletions tests/baselines/reference/bestCommonTypeOfTuple2.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,32 +49,32 @@ var t5: [C1, F]
>t5 : [C1, F]

var e11 = t1[4]; // base
>e11 : base | C
>t1[4] : base | C
>e11 : undefined
>t1[4] : undefined
>t1 : [C, base]
>4 : 4

var e21 = t2[4]; // {}
>e21 : C | D
>t2[4] : C | D
>e21 : undefined
>t2[4] : undefined
>t2 : [C, D]
>4 : 4

var e31 = t3[4]; // C1
>e31 : C1 | D1
>t3[4] : C1 | D1
>e31 : undefined
>t3[4] : undefined
>t3 : [C1, D1]
>4 : 4

var e41 = t4[2]; // base1
>e41 : base1 | C1
>t4[2] : base1 | C1
>e41 : undefined
>t4[2] : undefined
>t4 : [base1, C1]
>2 : 2

var e51 = t5[2]; // {}
>e51 : F | C1
>t5[2] : F | C1
>e51 : undefined
>t5[2] : undefined
>t5 : [C1, F]
>2 : 2

Loading