Skip to content

Make the implicitly exported rule consistent between checker and binder #54659

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

Merged
merged 2 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ import {
getElementOrPropertyAccessName,
getEmitScriptTarget,
getEnclosingBlockScopeContainer,
getEnclosingContainer,
getErrorSpanForNode,
getEscapedTextOfIdentifierOrLiteral,
getEscapedTextOfJsxNamespacedName,
Expand Down Expand Up @@ -455,7 +456,8 @@ function getModuleInstanceStateForAliasTarget(specifier: ExportSpecifier, visite
return ModuleInstanceState.Instantiated; // Couldn't locate, assume could refer to a value
}

const enum ContainerFlags {
/** @internal */
export const enum ContainerFlags {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not experienced enough to know the right way to do this, but it does feel a little suspicious to export... I feel like I've seen a function that does something similar in the checker but I can't recall the name.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to make sure the container used to check the implicit export rules is precisely the same the binder uses, and the binder uses those flags for that... I saw a getContainerNode in services/utilities, and getEnclosingBlockScopeContainer in compiler/utilities, but those are different.

// The current node is not a container, and no container manipulation should happen before
// recursing into it.
None = 0,
Expand Down Expand Up @@ -2356,7 +2358,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
const saveCurrentFlow = currentFlow;
for (const typeAlias of delayedTypeAliases) {
const host = typeAlias.parent.parent;
container = (findAncestor(host.parent, n => !!(getContainerFlags(n) & ContainerFlags.IsContainer)) as IsContainer | undefined) || file;
container = (getEnclosingContainer(host) as IsContainer | undefined) || file;
blockScopeContainer = (getEnclosingBlockScopeContainer(host) as IsBlockScopedContainer | undefined) || file;
currentFlow = initFlowNode({ flags: FlowFlags.Start });
parent = typeAlias;
Expand Down Expand Up @@ -3768,7 +3770,8 @@ export function isExportsOrModuleExportsOrAlias(sourceFile: SourceFile, node: Ex
return false;
}

function getContainerFlags(node: Node): ContainerFlags {
/** @internal */
export function getContainerFlags(node: Node): ContainerFlags {
switch (node.kind) {
case SyntaxKind.ClassExpression:
case SyntaxKind.ClassDeclaration:
Expand Down
6 changes: 4 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ import {
getEmitModuleResolutionKind,
getEmitScriptTarget,
getEnclosingBlockScopeContainer,
getEnclosingContainer,
getEntityNameFromTypeNode,
getErrorSpanForNode,
getEscapedTextOfIdentifierOrLiteral,
Expand Down Expand Up @@ -39024,8 +39025,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
n.parent.kind !== SyntaxKind.ClassDeclaration &&
n.parent.kind !== SyntaxKind.ClassExpression &&
n.flags & NodeFlags.Ambient) {
if (!(flags & ModifierFlags.Ambient) && !(isModuleBlock(n.parent) && isModuleDeclaration(n.parent.parent) && isGlobalScopeAugmentation(n.parent.parent))) {
// It is nested in an ambient context, which means it is automatically exported
const container = getEnclosingContainer(n);
if ((container && container.flags & NodeFlags.ExportContext) && !(flags & ModifierFlags.Ambient) && !(isModuleBlock(n.parent) && isModuleDeclaration(n.parent.parent) && isGlobalScopeAugmentation(n.parent.parent))) {
// It is nested in an ambient export context, which means it is automatically exported
flags |= ModifierFlags.Export;
}
flags |= ModifierFlags.Ambient;
Expand Down
7 changes: 7 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ import {
ConditionalExpression,
ConstructorDeclaration,
ConstructSignatureDeclaration,
ContainerFlags,
contains,
containsPath,
createGetCanonicalFileName,
Expand Down Expand Up @@ -162,6 +163,7 @@ import {
GetCanonicalFileName,
getCombinedModifierFlags,
getCombinedNodeFlags,
getContainerFlags,
getDirectoryPath,
getJSDocAugmentsTag,
getJSDocDeprecatedTagNoCache,
Expand Down Expand Up @@ -2022,6 +2024,11 @@ export function isAnyImportOrReExport(node: Node): node is AnyImportOrReExport {
return isAnyImportSyntax(node) || isExportDeclaration(node);
}

/** @internal */
export function getEnclosingContainer(node: Node): Node | undefined {
return findAncestor(node.parent, n => !!(getContainerFlags(n) & ContainerFlags.IsContainer));
}

// Gets the nearest enclosing block scope container that has the provided node
// as a descendant, that is not the provided node.
/** @internal */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
replace-in-file/types/index.d.ts(4,19): error TS2395: Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.
replace-in-file/types/index.d.ts(7,13): error TS2395: Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.


==== replace-in-file/types/index.d.ts (2 errors) ====
// repro from https://github.com/microsoft/TypeScript/issues/54342

declare module 'replace-in-file' {
export function replaceInFile(config: unknown): Promise<unknown[]>;
~~~~~~~~~~~~~
!!! error TS2395: Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.
export default replaceInFile;

namespace replaceInFile {
~~~~~~~~~~~~~
!!! error TS2395: Individual declarations in merged declaration 'replaceInFile' must be all exported or all local.
export function sync(config: unknown): unknown[];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//// [tests/cases/compiler/namespaceNotMergedWithFunctionDefaultExport.ts] ////

=== replace-in-file/types/index.d.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54342

declare module 'replace-in-file' {
>'replace-in-file' : Symbol("replace-in-file", Decl(index.d.ts, 0, 0))

export function replaceInFile(config: unknown): Promise<unknown[]>;
>replaceInFile : Symbol(replaceInFile, Decl(index.d.ts, 2, 34))
>config : Symbol(config, Decl(index.d.ts, 3, 32))
>Promise : Symbol(Promise, Decl(lib.es5.d.ts, --, --))

export default replaceInFile;
>replaceInFile : Symbol(replaceInFile, Decl(index.d.ts, 2, 34), Decl(index.d.ts, 4, 31))

namespace replaceInFile {
>replaceInFile : Symbol(replaceInFile, Decl(index.d.ts, 2, 34), Decl(index.d.ts, 4, 31))

export function sync(config: unknown): unknown[];
>sync : Symbol(sync, Decl(index.d.ts, 6, 27))
>config : Symbol(config, Decl(index.d.ts, 7, 25))
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//// [tests/cases/compiler/namespaceNotMergedWithFunctionDefaultExport.ts] ////

=== replace-in-file/types/index.d.ts ===
// repro from https://github.com/microsoft/TypeScript/issues/54342

declare module 'replace-in-file' {
>'replace-in-file' : typeof import("replace-in-file")

export function replaceInFile(config: unknown): Promise<unknown[]>;
>replaceInFile : (config: unknown) => Promise<unknown[]>
>config : unknown

export default replaceInFile;
>replaceInFile : typeof replaceInFile

namespace replaceInFile {
>replaceInFile : typeof replaceInFile

export function sync(config: unknown): unknown[];
>sync : (config: unknown) => unknown[]
>config : unknown
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// @moduleResolution: node10
// @module: commonjs

// repro from https://github.com/microsoft/TypeScript/issues/54342

// @Filename: replace-in-file/types/index.d.ts
declare module 'replace-in-file' {
export function replaceInFile(config: unknown): Promise<unknown[]>;
export default replaceInFile;

namespace replaceInFile {
export function sync(config: unknown): unknown[];
}
}