Merge pull request #124 from bloomberg/isolated-declarations-safety-improvements

Improve declaration emit type safety.

Author: dragomirtitian

src/compiler/_namespaces/ts.ts

@@ -61,7 +61,6 @@ export * from "../transformers/declarations/emitBinder";
 export * from "../transformers/declarations/emitResolver";
 export * from "../transformers/declarations/transpileDeclaration";
 export * from "../transformers/declarations/localInferenceResolver";
-export * from "../transformers/declarations/types";
 export * from "../transformers/declarations";
 export * from "../transformer";
 export * from "../emitter";

src/compiler/transformer.ts

@@ -40,6 +40,7 @@ import {
     NodeFlags,
     noop,
     notImplemented,
+    NullTransformationContext,
     returnUndefined,
     ScriptTarget,
     setEmitFlags,
@@ -49,6 +50,7 @@ import {
     SyntaxKind,
     tracing,
     TransformationContext,
+    TransformationContextKind,
     TransformationResult,
     transformClassFields,
     transformDeclarations,
@@ -267,6 +269,7 @@ export function transformNodes<T extends Node>(resolver: EmitResolver | undefine
     // The transformation context is provided to each transformer as part of transformer
     // initialization.
     const context: TransformationContext = {
+        kind: TransformationContextKind.FullContext,
         factory,
         getCompilerOptions: () => options,
         getEmitResolver: () => resolver!, // TODO: GH#18217
@@ -664,7 +667,8 @@ export function transformNodes<T extends Node>(resolver: EmitResolver | undefine
 }
 
 /** @internal */
-export const nullTransformationContext: TransformationContext = {
+export const nullTransformationContext: NullTransformationContext = {
+    kind: TransformationContextKind.NullContext,
     factory: factory, // eslint-disable-line object-shorthand
     getCompilerOptions: () => ({}),
     getEmitResolver: notImplemented,

src/compiler/transformers/declarations.ts

@@ -23,6 +23,7 @@ import {
     ConstructorTypeNode,
     ConstructSignatureDeclaration,
     contains,
+    CoreEmitResolver,
     createDiagnosticForNode,
     createDiagnosticForRange,
     createEmptyExports,
@@ -46,7 +47,6 @@ import {
     EnumDeclaration,
     ExportAssignment,
     ExportDeclaration,
-    Expression,
     ExpressionWithTypeArguments,
     factory,
     FileReference,
@@ -139,7 +139,7 @@ import {
     isMethodSignature,
     isModifier,
     isModuleDeclaration,
-    IsolatedEmitResolver,
+    IsolatedTransformationContext,
     isOmittedExpression,
     isPrivateIdentifier,
     isPropertySignature,
@@ -167,7 +167,6 @@ import {
     LateBoundDeclaration,
     LateVisibilityPaintedStatement,
     length,
-    LocalInferenceResolver,
     map,
     mapDefined,
     MethodDeclaration,
@@ -221,6 +220,7 @@ import {
     SyntaxKind,
     toFileNameLowerCase,
     TransformationContext,
+    TransformationContextKind,
     transformNodes,
     tryCast,
     tryGetModuleSpecifierFromDeclaration,
@@ -293,7 +293,7 @@ const declarationEmitNodeBuilderFlags = NodeBuilderFlags.MultilineObjectLiterals
  *
  * @internal
  */
-export function transformDeclarations(context: TransformationContext, _useTscEmit = true) {
+export function transformDeclarations(context: TransformationContext | IsolatedTransformationContext) {
     const throwDiagnostic = () => Debug.fail("Diagnostic emitted without context");
     let getSymbolAccessibilityDiagnostic: GetSymbolAccessibilityDiagnostic = throwDiagnostic;
     let needsDeclare = true;
@@ -318,108 +318,80 @@ export function transformDeclarations(context: TransformationContext, _useTscEmi
     let refs: Map<NodeId, SourceFile>;
     let libs: Map<string, boolean>;
     let emittedImports: readonly AnyImportSyntax[] | undefined; // must be declared in container so it can be `undefined` while transformer's first pass
-    const { localInferenceResolver, isolatedDeclarations, host, resolver, symbolTracker, ensureNoInitializer, useTscEmit } = createTransformerServices();
+    const { localInferenceResolver, isolatedDeclarations, host, resolver, symbolTracker, ensureNoInitializer, useLocalInferenceTypePrint } = createTransformerServices();
     const options = context.getCompilerOptions();
     const { noResolve, stripInternal } = options;
     return transformRoot;
 
-    function createTransformerServices(): {
-        isolatedDeclarations: true;
-        useTscEmit: false;
-        resolver: IsolatedEmitResolver;
-        localInferenceResolver: LocalInferenceResolver;
-        host: undefined;
-        symbolTracker: undefined;
-        ensureNoInitializer: (node: CanHaveLiteralInitializer) => Expression | undefined;
-    } | {
-        isolatedDeclarations: true;
-        useTscEmit: true;
-        resolver: EmitResolver;
-        localInferenceResolver: LocalInferenceResolver;
-        host: EmitHost;
-        symbolTracker: SymbolTracker;
-        ensureNoInitializer: (node: CanHaveLiteralInitializer) => Expression | undefined;
-    } | {
-        isolatedDeclarations: false;
-        useTscEmit: false;
-        resolver: EmitResolver;
-        localInferenceResolver: undefined;
-        host: EmitHost;
-        symbolTracker: SymbolTracker;
-        ensureNoInitializer: (node: CanHaveLiteralInitializer) => Expression | undefined;
-    } {
-        const { isolatedDeclarations, resolver: localInferenceResolver } = createLocalInferenceResolver({
-            ensureParameter,
-            context,
-            visitDeclarationSubtree,
-            setEnclosingDeclarations(node) {
-                const oldNode = enclosingDeclaration;
-                enclosingDeclaration = node;
-                return oldNode;
-            },
-            checkEntityNameVisibility(name, container) {
-                return checkEntityNameVisibility(name, container ?? enclosingDeclaration);
-            },
-        });
+    function createTransformerServices() {
+        const isolatedDeclarations = context.getCompilerOptions().isolatedDeclarations;
 
         if (isolatedDeclarations) {
-            if (!_useTscEmit) {
-                const resolver: IsolatedEmitResolver = context.getEmitResolver();
+            const localInferenceResolver = createLocalInferenceResolver({
+                ensureParameter,
+                context,
+                visitDeclarationSubtree,
+                setEnclosingDeclarations(node) {
+                    const oldNode = enclosingDeclaration;
+                    enclosingDeclaration = node;
+                    return oldNode;
+                },
+                checkEntityNameVisibility(name, container) {
+                    return checkEntityNameVisibility(name, container ?? enclosingDeclaration);
+                },
+            });
+            if (context.kind === TransformationContextKind.IsolatedContext) {
+                const resolver: CoreEmitResolver = context.getEmitResolver();
                 // Ideally nothing should require the symbol tracker in isolated declarations mode.
                 // createLiteralConstValue is the one exception
                 const emptySymbolTracker = {};
                 return {
                     isolatedDeclarations,
-                    useTscEmit: false,
+                    useLocalInferenceTypePrint: true,
                     resolver,
                     localInferenceResolver,
                     symbolTracker: undefined,
                     host: undefined,
-                    ensureNoInitializer: (node: CanHaveLiteralInitializer) => {
-                        if (shouldPrintWithInitializer(node)) {
-                            return resolver.createLiteralConstValue(getParseTreeNode(node) as CanHaveLiteralInitializer, emptySymbolTracker); // TODO: Make safe
-                        }
-                        return undefined;
-                    },
-                };
+                    ensureNoInitializer: createEnsureNoInitializer(emptySymbolTracker),
+                } as const;
             }
             else {
                 const host = context.getEmitHost();
                 const resolver: EmitResolver = context.getEmitResolver();
                 const symbolTracker = createSymbolTracker(resolver, host);
                 return {
                     isolatedDeclarations,
-                    useTscEmit: true,
+                    useLocalInferenceTypePrint: false,
                     resolver,
                     localInferenceResolver,
                     symbolTracker,
                     host,
-                    ensureNoInitializer: (node: CanHaveLiteralInitializer) => {
-                        if (shouldPrintWithInitializer(node)) {
-                            return resolver.createLiteralConstValue(getParseTreeNode(node) as CanHaveLiteralInitializer, symbolTracker); // TODO: Make safe
-                        }
-                        return undefined;
-                    },
-                };
+                    ensureNoInitializer: createEnsureNoInitializer(symbolTracker),
+                } as const;
             }
         }
         else {
+            Debug.assert(context.kind === TransformationContextKind.FullContext);
             const host = context.getEmitHost();
             const resolver = context.getEmitResolver();
             const symbolTracker: SymbolTracker = createSymbolTracker(resolver, host);
             return {
-                isolatedDeclarations,
-                useTscEmit: false,
-                localInferenceResolver,
+                isolatedDeclarations: false,
+                useLocalInferenceTypePrint: false,
+                localInferenceResolver: undefined,
                 resolver,
                 symbolTracker,
                 host,
-                ensureNoInitializer: (node: CanHaveLiteralInitializer) => {
-                    if (shouldPrintWithInitializer(node)) {
-                        return resolver.createLiteralConstValue(getParseTreeNode(node) as CanHaveLiteralInitializer, symbolTracker); // TODO: Make safe
-                    }
-                    return undefined;
-                },
+                ensureNoInitializer: createEnsureNoInitializer(symbolTracker),
+            } as const;
+        }
+
+        function createEnsureNoInitializer(symbolTracker: SymbolTracker) {
+            return function ensureNoInitializer(node: CanHaveLiteralInitializer) {
+                if (shouldPrintWithInitializer(node)) {
+                    return resolver.createLiteralConstValue(getParseTreeNode(node) as CanHaveLiteralInitializer, symbolTracker); // TODO: Make safe
+                }
+                return undefined;
             };
         }
     }
@@ -659,6 +631,7 @@ export function transformDeclarations(context: TransformationContext, _useTscEmi
             libs = new Map();
             existingTypeReferencesSources = node.sourceFiles;
             let hasNoDefaultLib = false;
+            Debug.assert(!isolatedDeclarations, "Bundles are not supported in isolated declarations");
             const bundle = factory.createBundle(
                 map(node.sourceFiles, sourceFile => {
                     if (sourceFile.isDeclarationFile) return undefined!; // Omit declaration files from bundle results, too // TODO: GH#18217
@@ -681,7 +654,7 @@ export function transformDeclarations(context: TransformationContext, _useTscEmi
                             sourceFile,
                             [factory.createModuleDeclaration(
                                 [factory.createModifier(SyntaxKind.DeclareKeyword)],
-                                factory.createStringLiteral(getResolvedExternalModuleName(context.getEmitHost(), sourceFile)),
+                                factory.createStringLiteral(getResolvedExternalModuleName(host, sourceFile)),
                                 factory.createModuleBlock(setTextRange(factory.createNodeArray(transformAndReplaceLatePaintedStatements(statements)), sourceFile.statements)),
                             )],
                             /*isDeclarationFile*/ true,
@@ -952,7 +925,7 @@ export function transformDeclarations(context: TransformationContext, _useTscEmi
         }
         if (isolatedDeclarations) {
             const { typeNode, isInvalid } = localInferenceResolver.fromInitializer(node, type, currentSourceFile);
-            if (!useTscEmit || isInvalid) {
+            if (useLocalInferenceTypePrint || isInvalid) {
                 return typeNode;
             }
         }
@@ -1121,7 +1094,7 @@ export function transformDeclarations(context: TransformationContext, _useTscEmi
             if (isBundledEmit) {
                 // Bundle emit not supported for isolatedDeclarations
                 if (!isolatedDeclarations) {
-                    const newName = getExternalModuleNameFromDeclaration(context.getEmitHost(), resolver, parent);
+                    const newName = getExternalModuleNameFromDeclaration(host, resolver, parent);
                     if (newName) {
                         return factory.createStringLiteral(newName);
                     }

src/compiler/transformers/declarations/emitBinder.ts

@@ -62,7 +62,6 @@ import {
     isVarConst,
     isVariableDeclaration,
     MappedTypeNode,
-    MemberKey,
     MethodDeclaration,
     MethodSignature,
     ModifierFlags,
@@ -816,6 +815,11 @@ export function bindSourceFileForDeclarationEmit(file: SourceFile) {
     }
 }
 
+/** @internal */
+export type MemberKey = string & {
+    __memberKey: void;
+};
+
 /**
  * Gets the symbolic name for a member from its type.
  * @internal

src/compiler/transformers/declarations/emitResolver.ts

@@ -1,6 +1,7 @@
 import {
     bindSourceFileForDeclarationEmit,
     ComputedPropertyName,
+    CoreEmitResolver,
     createEntityVisibilityChecker,
     createEvaluator,
     Debug,
@@ -39,7 +40,6 @@ import {
     isIdentifier,
     isInfinityOrNaNString,
     isNumericLiteral,
-    IsolatedEmitResolver,
     isPrefixUnaryExpression,
     isPrimitiveLiteralValue,
     isPropertyAccessExpression,
@@ -70,7 +70,7 @@ import {
 } from "../../_namespaces/ts";
 
 /** @internal */
-export function createEmitDeclarationResolver(file: SourceFile): IsolatedEmitResolver {
+export function createEmitDeclarationResolver(file: SourceFile): CoreEmitResolver {
     const { getNodeLinks, resolveMemberKey, resolveName, resolveEntityName } = bindSourceFileForDeclarationEmit(file);
 
     const { isEntityNameVisible } = createEntityVisibilityChecker({
@@ -394,4 +394,4 @@ export function createEmitDeclarationResolver(file: SourceFile): IsolatedEmitRes
 
         return false;
     }
-}
+        }

src/compiler/transformers/declarations/localInferenceResolver.ts

@@ -31,6 +31,7 @@ import {
     isMethodOrAccessor,
     isNoSubstitutionTemplateLiteral,
     isNumericLiteral,
+    IsolatedTransformationContext,
     isOmittedExpression,
     isOptionalDeclaration,
     isParameter,
@@ -120,44 +121,39 @@ export function createLocalInferenceResolver({
     visitDeclarationSubtree(input: Node): VisitResult<Node | undefined>;
     checkEntityNameVisibility(name: EntityNameOrEntityNameExpression, container?: Node): void;
     ensureParameter(p: ParameterDeclaration): ParameterDeclaration;
-    context: TransformationContext;
-}): { resolver: LocalInferenceResolver; isolatedDeclarations: true; } | { resolver: undefined; isolatedDeclarations: false; } {
+    context: IsolatedTransformationContext | TransformationContext;
+}): LocalInferenceResolver {
     let currentSourceFile: SourceFile;
     const options = context.getCompilerOptions();
     const resolver = context.getEmitResolver();
-    if (!options.isolatedDeclarations) {
-        return { resolver: undefined, isolatedDeclarations: false };
-    }
+    Debug.assert(options.isolatedDeclarations, "createLocalInferenceResolver can only be called when isolatedDeclarations is true");
     const { factory } = context;
     let inferenceContext: { isInvalid: boolean; disableErrors: boolean; } = undefined!;
     const strictNullChecks = !!options.strict || !!options.strictNullChecks;
 
     return {
-        resolver: {
-            fromInitializer(node: HasInferredType | ExportAssignment, type: TypeNode | undefined, sourceFile: SourceFile) {
-                const oldSourceFile = currentSourceFile;
-                const hasExistingContext = inferenceContext !== undefined;
-                if (!hasExistingContext) {
-                    inferenceContext = { isInvalid: false, disableErrors: false };
-                }
-                currentSourceFile = sourceFile;
-                try {
-                    const typeNode = localInferenceFromInitializer(node, type);
-                    if (typeNode !== undefined) {
-                        return { isInvalid: inferenceContext.isInvalid, typeNode };
-                    }
-                    return { isInvalid: true, typeNode: invalid(node) };
+        fromInitializer(node: HasInferredType | ExportAssignment, type: TypeNode | undefined, sourceFile: SourceFile) {
+            const oldSourceFile = currentSourceFile;
+            const hasExistingContext = inferenceContext !== undefined;
+            if (!hasExistingContext) {
+                inferenceContext = { isInvalid: false, disableErrors: false };
+            }
+            currentSourceFile = sourceFile;
+            try {
+                const typeNode = localInferenceFromInitializer(node, type);
+                if (typeNode !== undefined) {
+                    return { isInvalid: inferenceContext.isInvalid, typeNode };
                 }
-                finally {
-                    currentSourceFile = oldSourceFile;
-                    if (!hasExistingContext) {
-                        inferenceContext = undefined!;
-                    }
+                return { isInvalid: true, typeNode: invalid(node) };
+            }
+            finally {
+                currentSourceFile = oldSourceFile;
+                if (!hasExistingContext) {
+                    inferenceContext = undefined!;
                 }
-            },
-            makeInvalidType,
+            }
         },
-        isolatedDeclarations: options.isolatedDeclarations,
+        makeInvalidType,
     };
     function hasParseError(node: Node) {
         return !!(node.flags & NodeFlags.ThisNodeHasError);