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.

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

Jump to bottom

Fix leaked alias symbol independent type params in printback #42211

Closed
wants to merge 4 commits into from
Closed

Fix leaked alias symbol independent type params in printback #42211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1b52d23
First, track recursive expansion on union comparisons so alias varian…
weswigham Jan 4, 2021
db9529d
Use that fixed alias variance finding to omit unreachable independent…
weswigham Jan 4, 2021
3fe794d
Fix perf issue in recursiveTypeReferences2 by broadening recursion gu…
weswigham Jan 4, 2021
7879343
Merge branch 'master' into fix-leaked-independent-type-params
weswigham Jan 5, 2021
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
40 changes: 28 additions & 12 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4536,7 +4536,24 @@ namespace ts {
}

if (!inTypeAlias && type.aliasSymbol && (context.flags & NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope || isTypeSymbolAccessible(type.aliasSymbol, context.enclosingDeclaration))) {
const typeArgumentNodes = mapToTypeNodes(type.aliasTypeArguments, context);
// An `Independent` type variable is unused - so it won't factor into the cache key of the type being aliased, meaning no matter how the
// type argument changes, the same underlying type is used. This means that for aliases, the _first_ type argument we encounter gets
// cached as the alias type argument, then reused forevermore. This means an uninstantiated type parameter can leak out of its
// intended context, since completely unrelated locations will end up pointing to the same instantiation. To entirely avoid needing
// to check for such a scenario (and then issuing a very odd visbility error), we simply replace all `Independent` type argument locations
// whose cached value is unreachable with `unknown`, as their actual value has no bearing on the constructed type. This can cause some visual oddness,
// like `fn<T>(arg: T): PublicWrap<T>` at a use-site becoming `fn<{x: number}>(arg: {x: number}): PublicWrap<unknown>` when `T` is independent,
// but has no bearing on relationships, (as the underlying types are the still just the one type) and always produces a functioning declaration file.
const variances = getAliasVariances(type.aliasSymbol);
const typeNodeOverrides = map(variances, (v, i) => {
const typeArg = type.aliasTypeArguments?.[i];
if ((v & VarianceFlags.VarianceMask) === VarianceFlags.Independent && !!typeArg && !!(typeArg.flags & TypeFlags.TypeParameter) && !isTypeSymbolAccessible(typeArg.symbol, context.enclosingDeclaration)) {
context.approximateLength += 7;
return factory.createKeywordTypeNode(SyntaxKind.UnknownKeyword);
}
return undefined;
});
const typeArgumentNodes = mapToTypeNodes(type.aliasTypeArguments, context, /*isBareList*/ undefined, typeNodeOverrides);
if (isReservedMemberName(type.aliasSymbol.escapedName) && !(type.aliasSymbol.flags & SymbolFlags.Class)) return factory.createTypeReferenceNode(factory.createIdentifier(""), typeArgumentNodes);
return symbolToTypeNode(type.aliasSymbol, context, SymbolFlags.Type, typeArgumentNodes);
}
Expand Down Expand Up @@ -5077,7 +5094,7 @@ namespace ts {
}
}

function mapToTypeNodes(types: readonly Type[] | undefined, context: NodeBuilderContext, isBareList?: boolean): TypeNode[] | undefined {
function mapToTypeNodes(types: readonly Type[] | undefined, context: NodeBuilderContext, isBareList?: boolean, typeNodeOverrides?: (TypeNode | undefined)[]): TypeNode[] | undefined {
if (some(types)) {
if (checkTruncationLength(context)) {
if (!isBareList) {
Expand Down Expand Up @@ -5107,7 +5124,7 @@ namespace ts {
break;
}
context.approximateLength += 2; // Account for whitespace + separator
const typeNode = typeToTypeNodeHelper(type, context);
const typeNode = typeNodeOverrides?.[i - 1] || typeToTypeNodeHelper(type, context);
if (typeNode) {
result.push(typeNode);
if (seenNames && isIdentifierTypeReference(typeNode)) {
Expand Down Expand Up @@ -17022,9 +17039,7 @@ namespace ts {
// we need to deconstruct unions before intersections (because unions are always at the top),
// and we need to handle "each" relations before "some" relations for the same kind of type.
if (source.flags & TypeFlags.UnionOrIntersection || target.flags & TypeFlags.UnionOrIntersection) {
result = getConstituentCount(source) * getConstituentCount(target) >= 4 ?
recursiveTypeRelatedTo(source, target, reportErrors, intersectionState | IntersectionState.UnionIntersectionCheck) :
structuredTypeRelatedTo(source, target, reportErrors, intersectionState | IntersectionState.UnionIntersectionCheck);
result = recursiveTypeRelatedTo(source, target, reportErrors, intersectionState | IntersectionState.UnionIntersectionCheck);
}
if (!result && !(source.flags & TypeFlags.Union) && (source.flags & (TypeFlags.StructuredOrInstantiable) || target.flags & TypeFlags.StructuredOrInstantiable)) {
if (result = recursiveTypeRelatedTo(source, target, reportErrors, intersectionState)) {
Expand Down Expand Up @@ -17427,7 +17442,7 @@ namespace ts {
// Third, check if both types are part of deeply nested chains of generic type instantiations and if so assume the types are
// equal and infinitely expanding. Fourth, if we have reached a depth of 100 nested comparisons, assume we have runaway recursion
// and issue an error. Otherwise, actually compare the structure of the two types.
function recursiveTypeRelatedTo(source: Type, target: Type, reportErrors: boolean, intersectionState: IntersectionState): Ternary {
function recursiveTypeRelatedTo(source: Type, target: Type, reportErrors: boolean, intersectionState: IntersectionState, recurRelation = structuredTypeRelatedTo): Ternary {
if (overflow) {
return Ternary.False;
}
Expand Down Expand Up @@ -17498,7 +17513,7 @@ namespace ts {
});
}

const result = expandingFlags !== ExpandingFlags.Both ? structuredTypeRelatedTo(source, target, reportErrors, intersectionState) : Ternary.Maybe;
const result = expandingFlags !== ExpandingFlags.Both ? recurRelation(source, target, reportErrors, intersectionState) : Ternary.Maybe;
if (outofbandVarianceMarkerHandler) {
outofbandVarianceMarkerHandler = originalHandler;
}
Expand Down Expand Up @@ -19054,6 +19069,11 @@ namespace ts {
// The root object represents the origin of the conditional type
return (type as ConditionalType).root;
}
if (type.flags & TypeFlags.UnionOrIntersection && type.aliasSymbol && type.aliasTypeArguments) {
// Unions and intersections are difficult to make recusive, but they can be, if they have an associated alias
// (which is then later used to make a recursive type reference). In such cases, use the alias symbol as the identity.
return type.aliasSymbol;
}
return undefined;
}

Expand Down Expand Up @@ -21528,10 +21548,6 @@ namespace ts {
return mappedTypes && getUnionType(mappedTypes, noReductions ? UnionReduction.None : UnionReduction.Literal);
}

function getConstituentCount(type: Type) {
return type.flags & TypeFlags.UnionOrIntersection ? (<UnionOrIntersectionType>type).types.length : 1;
}

function extractTypesOfKind(type: Type, kind: TypeFlags) {
return filterType(type, t => (t.flags & kind) !== 0);
}
Expand Down
6 changes: 0 additions & 6 deletions tests/baselines/reference/baseClassImprovedMismatchErrors.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,15 @@ tests/cases/compiler/baseClassImprovedMismatchErrors.ts(8,5): error TS2416: Prop
tests/cases/compiler/baseClassImprovedMismatchErrors.ts(9,5): error TS2416: Property 'fn' in type 'Derived' is not assignable to the same property in base type 'Base'.
Type '() => string | number' is not assignable to type '() => number'.
Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/compiler/baseClassImprovedMismatchErrors.ts(14,5): error TS2416: Property 'n' in type 'DerivedInterface' is not assignable to the same property in base type 'Base'.
Type 'string | DerivedInterface' is not assignable to type 'string | Base'.
Type 'DerivedInterface' is not assignable to type 'string | Base'.
Type 'DerivedInterface' is not assignable to type 'Base'.
The types returned by 'fn()' are incompatible between these types.
Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/compiler/baseClassImprovedMismatchErrors.ts(15,5): error TS2416: Property 'fn' in type 'DerivedInterface' is not assignable to the same property in base type 'Base'.
Type '() => string | number' is not assignable to type '() => number'.
Type 'string | number' is not assignable to type 'number'.
Type 'string' is not assignable to type 'number'.


==== tests/cases/compiler/baseClassImprovedMismatchErrors.ts (4 errors) ====
Expand All @@ -44,7 +41,6 @@ tests/cases/compiler/baseClassImprovedMismatchErrors.ts(15,5): error TS2416: Pro
!!! error TS2416: Property 'fn' in type 'Derived' is not assignable to the same property in base type 'Base'.
!!! error TS2416: Type '() => string | number' is not assignable to type '() => number'.
!!! error TS2416: Type 'string | number' is not assignable to type 'number'.
!!! error TS2416: Type 'string' is not assignable to type 'number'.
return 10 as number | string;
}
}
Expand All @@ -57,13 +53,11 @@ tests/cases/compiler/baseClassImprovedMismatchErrors.ts(15,5): error TS2416: Pro
!!! error TS2416: Type 'DerivedInterface' is not assignable to type 'Base'.
!!! error TS2416: The types returned by 'fn()' are incompatible between these types.
!!! error TS2416: Type 'string | number' is not assignable to type 'number'.
!!! error TS2416: Type 'string' is not assignable to type 'number'.
fn() {
~~
!!! error TS2416: Property 'fn' in type 'DerivedInterface' is not assignable to the same property in base type 'Base'.
!!! error TS2416: Type '() => string | number' is not assignable to type '() => number'.
!!! error TS2416: Type 'string | number' is not assignable to type 'number'.
!!! error TS2416: Type 'string' is not assignable to type 'number'.
return 10 as number | string;
}
}
2 changes: 0 additions & 2 deletions tests/baselines/reference/bigintWithLib.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ tests/cases/compiler/bigintWithLib.ts(31,35): error TS2769: No overload matches
Argument of type 'number[]' is not assignable to parameter of type 'Iterable<bigint>'.
Overload 3 of 3, '(buffer: ArrayBufferLike, byteOffset?: number, length?: number): BigUint64Array', gave the following error.
Argument of type 'number[]' is not assignable to parameter of type 'ArrayBufferLike'.
Type 'number[]' is not assignable to type 'SharedArrayBuffer'.
tests/cases/compiler/bigintWithLib.ts(36,13): error TS2540: Cannot assign to 'length' because it is a read-only property.
tests/cases/compiler/bigintWithLib.ts(43,25): error TS2345: Argument of type 'number' is not assignable to parameter of type 'bigint'.
tests/cases/compiler/bigintWithLib.ts(46,26): error TS2345: Argument of type 'number' is not assignable to parameter of type 'bigint'.
Expand Down Expand Up @@ -84,7 +83,6 @@ tests/cases/compiler/bigintWithLib.ts(46,26): error TS2345: Argument of type 'nu
!!! error TS2769: Argument of type 'number[]' is not assignable to parameter of type 'Iterable<bigint>'.
!!! error TS2769: Overload 3 of 3, '(buffer: ArrayBufferLike, byteOffset?: number, length?: number): BigUint64Array', gave the following error.
!!! error TS2769: Argument of type 'number[]' is not assignable to parameter of type 'ArrayBufferLike'.
!!! error TS2769: Type 'number[]' is not assignable to type 'SharedArrayBuffer'.
bigUintArray = new BigUint64Array(new ArrayBuffer(80));
bigUintArray = new BigUint64Array(new ArrayBuffer(80), 8);
bigUintArray = new BigUint64Array(new ArrayBuffer(80), 8, 3);
Expand Down
2 changes: 0 additions & 2 deletions tests/baselines/reference/callChain.3.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts(3,7): error TS2322: Type 'number | undefined' is not assignable to type 'number'.
Type 'undefined' is not assignable to type 'number'.
tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts(4,7): error TS2322: Type 'number | undefined' is not assignable to type 'number'.
Type 'undefined' is not assignable to type 'number'.


==== tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts (2 errors) ====
Expand All @@ -14,7 +13,6 @@ tests/cases/conformance/expressions/optionalChaining/callChain/callChain.3.ts(4,
const n2: number = a?.m?.({x: absorb()}); // likewise
~~
!!! error TS2322: Type 'number | undefined' is not assignable to type 'number'.
!!! error TS2322: Type 'undefined' is not assignable to type 'number'.
const n3: number | undefined = a?.m?.({x: 12}); // should be ok
const n4: number | undefined = a?.m?.({x: absorb()}); // likewise

Expand Down
4 changes: 0 additions & 4 deletions tests/baselines/reference/callWithSpread2.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(27,5): erro
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(28,5): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(29,13): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(30,13): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(31,11): error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
Type 'string' is not assignable to type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(32,11): error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(33,8): error TS2556: Expected 1-3 arguments, but got 0 or more.
tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(34,8): error TS2556: Expected 1-3 arguments, but got 0 or more.
Expand Down Expand Up @@ -56,14 +54,12 @@ tests/cases/conformance/expressions/functionCalls/callWithSpread2.ts(36,14): err
prefix("b", ...mixed)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
prefix("c", ...tuple)
~~~~~~~~
!!! error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
rest("e", ...mixed)
~~~~~~~~
!!! error TS2345: Argument of type 'string | number' is not assignable to parameter of type 'number'.
!!! error TS2345: Type 'string' is not assignable to type 'number'.
rest("f", ...tuple)
~~~~~~~~
!!! error TS2345: Argument of type 'string' is not assignable to parameter of type 'number'.
Expand Down
Loading