Skip to content

Commit 8cc216a

Browse files
jakebaileyjakebailey
authored
Ensure parameters are in scope when converting parameter/return types to type nodes (#49627)
1 parent 088fdf6 commit 8cc216a

15 files changed

+1406
-15
lines changed

‎src/compiler/checker.ts

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7276,6 +7276,86 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
72767276
}
72777277

72787278
const expandedParams = getExpandedParameters(signature, /*skipUnionExpanding*/ true)[0];
7279+
7280+
// For regular function/method declarations, the enclosing declaration will already be signature.declaration,
7281+
// so this is a no-op, but for arrow functions and function expressions, the enclosing declaration will be
7282+
// the declaration that the arrow function / function expression is assigned to.
7283+
//
7284+
// If the parameters or return type include "typeof globalThis.paramName", using the wrong scope will lead
7285+
// us to believe that we can emit "typeof paramName" instead, even though that would refer to the parameter,
7286+
// not the global. Make sure we are in the right scope by changing the enclosingDeclaration to the function.
7287+
//
7288+
// We can't use the declaration directly; it may be in another file and so we may lose access to symbols
7289+
// accessible to the current enclosing declaration, or gain access to symbols not accessible to the current
7290+
// enclosing declaration. To keep this chain accurate, insert a fake scope into the chain which makes the
7291+
// function's parameters visible.
7292+
//
7293+
// If the declaration is in a JS file, then we don't need to do this at all, as there are no annotations besides
7294+
// JSDoc, which are always outside the function declaration, so are not in the parameter scope.
7295+
let cleanup: (() => void) | undefined;
7296+
if (
7297+
context.enclosingDeclaration
7298+
&& signature.declaration
7299+
&& signature.declaration !== context.enclosingDeclaration
7300+
&& !isInJSFile(signature.declaration)
7301+
&& some(expandedParams)
7302+
) {
7303+
// As a performance optimization, reuse the same fake scope within this chain.
7304+
// This is especially needed when we are working on an excessively deep type;
7305+
// if we don't do this, then we spend all of our time adding more and more
7306+
// scopes that need to be searched in isSymbolAccessible later. Since all we
7307+
// really want to do is to mark certain names as unavailable, we can just keep
7308+
// all of the names we're introducing in one large table and push/pop from it as
7309+
// needed; isSymbolAccessible will walk upward and find the closest "fake" scope,
7310+
// which will conveniently report on any and all faked scopes in the chain.
7311+
//
7312+
// It'd likely be better to store this somewhere else for isSymbolAccessible, but
7313+
// since that API _only_ uses the enclosing declaration (and its parents), this is
7314+
// seems like the best way to inject names into that search process.
7315+
//
7316+
// Note that we only check the most immediate enclosingDeclaration; the only place we
7317+
// could potentially add another fake scope into the chain is right here, so we don't
7318+
// traverse all ancestors.
7319+
const existingFakeScope = getNodeLinks(context.enclosingDeclaration).fakeScopeForSignatureDeclaration ? context.enclosingDeclaration : undefined;
7320+
Debug.assertOptionalNode(existingFakeScope, isBlock);
7321+
7322+
const locals = existingFakeScope?.locals ?? createSymbolTable();
7323+
7324+
let newLocals: __String[] | undefined;
7325+
for (const param of expandedParams) {
7326+
if (!locals.has(param.escapedName)) {
7327+
newLocals = append(newLocals, param.escapedName);
7328+
locals.set(param.escapedName, param);
7329+
}
7330+
}
7331+
7332+
if (newLocals) {
7333+
function removeNewLocals() {
7334+
forEach(newLocals, s => locals.delete(s));
7335+
}
7336+
7337+
if (existingFakeScope) {
7338+
cleanup = removeNewLocals;
7339+
}
7340+
else {
7341+
// Use a Block for this; the type of the node doesn't matter so long as it
7342+
// has locals, and this is cheaper/easier than using a function-ish Node.
7343+
const fakeScope = parseNodeFactory.createBlock(emptyArray);
7344+
getNodeLinks(fakeScope).fakeScopeForSignatureDeclaration = true;
7345+
fakeScope.locals = locals;
7346+
7347+
const saveEnclosingDeclaration = context.enclosingDeclaration;
7348+
setParent(fakeScope, saveEnclosingDeclaration);
7349+
context.enclosingDeclaration = fakeScope;
7350+
7351+
cleanup = () => {
7352+
context.enclosingDeclaration = saveEnclosingDeclaration;
7353+
removeNewLocals();
7354+
};
7355+
}
7356+
}
7357+
}
7358+
72797359
// If the expanded parameter list had a variadic in a non-trailing position, don't expand it
72807360
const parameters = (some(expandedParams, p => p !== expandedParams[expandedParams.length - 1] && !!(getCheckFlags(p) & CheckFlags.RestParameter)) ? signature.parameters : expandedParams).map(parameter => symbolToParameterDeclaration(parameter, context, kind === SyntaxKind.Constructor, options?.privateSymbolVisitor, options?.bundledImports));
72817361
const thisParameter = context.flags & NodeBuilderFlags.OmitThisParameter ? undefined : tryGetThisParameterDeclaration(signature, context);
@@ -7331,6 +7411,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
73317411
node.typeArguments = factory.createNodeArray(typeArguments);
73327412
}
73337413

7414+
cleanup?.();
73347415
return node;
73357416
}
73367417

@@ -7996,13 +8077,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
79968077
return !(getObjectFlags(type) & ObjectFlags.Reference) || !isTypeReferenceNode(existing) || length(existing.typeArguments) >= getMinTypeArgumentCount((type as TypeReference).target.typeParameters);
79978078
}
79988079

8080+
function getEnclosingDeclarationIgnoringFakeScope(enclosingDeclaration: Node) {
8081+
return getNodeLinks(enclosingDeclaration).fakeScopeForSignatureDeclaration ? enclosingDeclaration.parent : enclosingDeclaration;
8082+
}
8083+
79998084
/**
80008085
* Unlike `typeToTypeNodeHelper`, this handles setting up the `AllowUniqueESSymbolType` flag
80018086
* so a `unique symbol` is returned when appropriate for the input symbol, rather than `typeof sym`
80028087
*/
80038088
function serializeTypeForDeclaration(context: NodeBuilderContext, type: Type, symbol: Symbol, enclosingDeclaration: Node | undefined, includePrivateSymbol?: (s: Symbol) => void, bundled?: boolean) {
80048089
if (!isErrorType(type) && enclosingDeclaration) {
8005-
const declWithExistingAnnotation = getDeclarationWithTypeAnnotation(symbol, enclosingDeclaration);
8090+
const declWithExistingAnnotation = getDeclarationWithTypeAnnotation(symbol, getEnclosingDeclarationIgnoringFakeScope(enclosingDeclaration));
80068091
if (declWithExistingAnnotation && !isFunctionLikeDeclaration(declWithExistingAnnotation) && !isGetAccessorDeclaration(declWithExistingAnnotation)) {
80078092
// try to reuse the existing annotation
80088093
const existing = getEffectiveTypeAnnotationNode(declWithExistingAnnotation)!;
@@ -8038,7 +8123,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
80388123
function serializeReturnTypeForSignature(context: NodeBuilderContext, type: Type, signature: Signature, includePrivateSymbol?: (s: Symbol) => void, bundled?: boolean) {
80398124
if (!isErrorType(type) && context.enclosingDeclaration) {
80408125
const annotation = signature.declaration && getEffectiveReturnTypeNode(signature.declaration);
8041-
if (!!findAncestor(annotation, n => n === context.enclosingDeclaration) && annotation) {
8126+
const enclosingDeclarationIgnoringFakeScope = getEnclosingDeclarationIgnoringFakeScope(context.enclosingDeclaration);
8127+
if (!!findAncestor(annotation, n => n === enclosingDeclarationIgnoringFakeScope) && annotation) {
80428128
const annotated = getTypeFromTypeNode(annotation);
80438129
const thisInstantiated = annotated.flags & TypeFlags.TypeParameter && (annotated as TypeParameter).isThisType ? instantiateType(annotated, signature.mapper) : annotated;
80448130
if (thisInstantiated === type && existingTypeNodeIsNotReferenceOrIsReferenceWithCompatibleTypeArgumentCount(annotation, type)) {

‎src/compiler/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5997,6 +5997,7 @@ export interface NodeLinks {
59975997
serializedTypes?: Map<string, SerializedTypeEntry>; // Collection of types serialized at this location
59985998
decoratorSignature?: Signature; // Signature for decorator as if invoked by the runtime.
59995999
parameterInitializerContainsUndefined?: boolean; // True if this is a parameter declaration whose type annotation contains "undefined".
6000+
fakeScopeForSignatureDeclaration?: boolean; // True if this is a fake scope injected into an enclosing declaration chain.
60006001
}
60016002

60026003
/** @internal */

‎tests/baselines/reference/declarationEmitGlobalThisPreserved.js

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
//// [declarationEmitGlobalThisPreserved.ts]
2+
// Adding this makes tooltips fail too.
3+
// declare global {
4+
// namespace isNaN {
5+
// const prop: number;
6+
// }
7+
// }
8+
9+
// Broken inference cases.
10+
11+
export const a1 = (isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN => isNaN;
12+
export const a2 = (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN): typeof globalThis.isNaN => bar ?? isNaN;
13+
export const a3 = (isNaN: number, bar: typeof globalThis.isNaN): typeof globalThis.isNaN => bar;
14+
export const a4 = (isNaN: number): typeof globalThis.isNaN => globalThis.isNaN;
15+
16+
export const aObj = {
17+
a1: (isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN => isNaN,
18+
a2: (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN): typeof globalThis.isNaN => bar ?? isNaN,
19+
a3: (isNaN: number, bar: typeof globalThis.isNaN): typeof globalThis.isNaN => bar,
20+
a4: (isNaN: number): typeof globalThis.isNaN => globalThis.isNaN,
21+
}
22+
23+
export type a4Return = ReturnType<ReturnType<typeof a4>>;
24+
export type a4oReturn = ReturnType<ReturnType<typeof aObj['a4']>>;
25+
26+
export const b1 = (isNaN: typeof globalThis.isNaN) => isNaN;
27+
export const b2 = (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => bar ?? isNaN;
28+
export const b3 = (isNaN: number, bar: typeof globalThis.isNaN) => bar;
29+
export const b4 = (isNaN: number) => globalThis.isNaN;
30+
31+
export const bObj = {
32+
b1: (isNaN: typeof globalThis.isNaN) => isNaN,
33+
b2: (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => bar ?? isNaN,
34+
b3: (isNaN: number, bar: typeof globalThis.isNaN) => bar,
35+
b4: (isNaN: number) => globalThis.isNaN,
36+
}
37+
38+
export type b4Return = ReturnType<ReturnType<typeof b4>>;
39+
export type b4oReturn = ReturnType<ReturnType<typeof bObj['b4']>>;
40+
41+
export function c1(isNaN: typeof globalThis.isNaN) { return isNaN }
42+
export function c2(isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) { return bar ?? isNaN }
43+
export function c3(isNaN: number, bar: typeof globalThis.isNaN) { return bar }
44+
export function c4(isNaN: number) { return globalThis.isNaN; }
45+
46+
export const cObj = {
47+
c1(isNaN: typeof globalThis.isNaN) { return isNaN },
48+
c2(isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) { return bar ?? isNaN },
49+
c3(isNaN: number, bar: typeof globalThis.isNaN) { return bar },
50+
c4(isNaN: number) { return globalThis.isNaN; },
51+
}
52+
53+
export type c4Return = ReturnType<ReturnType<typeof c4>>;
54+
export type c4oReturn = ReturnType<ReturnType<typeof cObj['c4']>>;
55+
56+
export function d1() {
57+
const fn = (isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN => isNaN;
58+
return function() { return fn };
59+
}
60+
61+
export function d2() {
62+
const fn = (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN): typeof globalThis.isNaN => bar ?? isNaN;
63+
return function() { return fn };
64+
}
65+
66+
export function d3() {
67+
const fn = (isNaN: number, bar: typeof globalThis.isNaN): typeof globalThis.isNaN => bar;
68+
return function() { return fn };
69+
}
70+
71+
export function d4() {
72+
const fn = (isNaN: number): typeof globalThis.isNaN => globalThis.isNaN;
73+
return function() { return fn };
74+
}
75+
76+
export type d4Return = ReturnType<ReturnType<ReturnType<ReturnType<typeof d4>>>>;
77+
78+
export class A {
79+
method1(isNaN: typeof globalThis.isNaN) { return isNaN }
80+
method2(isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) { return bar ?? isNaN }
81+
method3(isNaN: number, bar: typeof globalThis.isNaN) { return bar }
82+
method4(isNaN: number) { return globalThis.isNaN; }
83+
}
84+
85+
export function fromParameter(isNaN: number, bar: typeof globalThis.isNaN) {
86+
return function() { return { bar } };
87+
}
88+
89+
// Non-inference cases.
90+
91+
export const explicitlyTypedVariable: (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN = (isNaN) => isNaN;
92+
93+
export function explicitlyTypedFunction(isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN {
94+
return isNaN;
95+
};
96+
97+
export type AsObjectProperty = {
98+
isNaN: typeof globalThis.isNaN;
99+
}
100+
101+
export class AsClassProperty {
102+
isNaN?: typeof globalThis.isNaN;
103+
}
104+
105+
export type AsFunctionType = (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
106+
107+
108+
109+
110+
111+
//// [declarationEmitGlobalThisPreserved.d.ts]
112+
export declare const a1: (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
113+
export declare const a2: (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => typeof globalThis.isNaN;
114+
export declare const a3: (isNaN: number, bar: typeof globalThis.isNaN) => typeof globalThis.isNaN;
115+
export declare const a4: (isNaN: number) => typeof globalThis.isNaN;
116+
export declare const aObj: {
117+
a1: (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
118+
a2: (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => typeof globalThis.isNaN;
119+
a3: (isNaN: number, bar: typeof globalThis.isNaN) => typeof globalThis.isNaN;
120+
a4: (isNaN: number) => typeof globalThis.isNaN;
121+
};
122+
export type a4Return = ReturnType<ReturnType<typeof a4>>;
123+
export type a4oReturn = ReturnType<ReturnType<typeof aObj['a4']>>;
124+
export declare const b1: (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
125+
export declare const b2: (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => typeof globalThis.isNaN;
126+
export declare const b3: (isNaN: number, bar: typeof globalThis.isNaN) => typeof globalThis.isNaN;
127+
export declare const b4: (isNaN: number) => typeof globalThis.isNaN;
128+
export declare const bObj: {
129+
b1: (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
130+
b2: (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => typeof globalThis.isNaN;
131+
b3: (isNaN: number, bar: typeof globalThis.isNaN) => typeof globalThis.isNaN;
132+
b4: (isNaN: number) => typeof globalThis.isNaN;
133+
};
134+
export type b4Return = ReturnType<ReturnType<typeof b4>>;
135+
export type b4oReturn = ReturnType<ReturnType<typeof bObj['b4']>>;
136+
export declare function c1(isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN;
137+
export declare function c2(isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN): typeof globalThis.isNaN;
138+
export declare function c3(isNaN: number, bar: typeof globalThis.isNaN): typeof globalThis.isNaN;
139+
export declare function c4(isNaN: number): typeof globalThis.isNaN;
140+
export declare const cObj: {
141+
c1(isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN;
142+
c2(isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN): typeof globalThis.isNaN;
143+
c3(isNaN: number, bar: typeof globalThis.isNaN): typeof globalThis.isNaN;
144+
c4(isNaN: number): typeof globalThis.isNaN;
145+
};
146+
export type c4Return = ReturnType<ReturnType<typeof c4>>;
147+
export type c4oReturn = ReturnType<ReturnType<typeof cObj['c4']>>;
148+
export declare function d1(): () => (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
149+
export declare function d2(): () => (isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN) => typeof globalThis.isNaN;
150+
export declare function d3(): () => (isNaN: number, bar: typeof globalThis.isNaN) => typeof globalThis.isNaN;
151+
export declare function d4(): () => (isNaN: number) => typeof globalThis.isNaN;
152+
export type d4Return = ReturnType<ReturnType<ReturnType<ReturnType<typeof d4>>>>;
153+
export declare class A {
154+
method1(isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN;
155+
method2(isNaN: typeof globalThis.isNaN, bar?: typeof globalThis.isNaN): typeof globalThis.isNaN;
156+
method3(isNaN: number, bar: typeof globalThis.isNaN): typeof globalThis.isNaN;
157+
method4(isNaN: number): typeof globalThis.isNaN;
158+
}
159+
export declare function fromParameter(isNaN: number, bar: typeof globalThis.isNaN): () => {
160+
bar: typeof globalThis.isNaN;
161+
};
162+
export declare const explicitlyTypedVariable: (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;
163+
export declare function explicitlyTypedFunction(isNaN: typeof globalThis.isNaN): typeof globalThis.isNaN;
164+
export type AsObjectProperty = {
165+
isNaN: typeof globalThis.isNaN;
166+
};
167+
export declare class AsClassProperty {
168+
isNaN?: typeof globalThis.isNaN;
169+
}
170+
export type AsFunctionType = (isNaN: typeof globalThis.isNaN) => typeof globalThis.isNaN;

0 commit comments

Comments
 (0)