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.

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

Normative: Arbitrary module namespace identifier names #49297

Closed
wants to merge 6 commits into from
Closed
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
6 changes: 4 additions & 2 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ import {
ModifierFlags,
ModuleBlock,
ModuleDeclaration,
moduleExportNameText,
moduleExportNameTextEscaped,
Mutable,
NamespaceExportDeclaration,
Node,
Expand Down Expand Up @@ -432,7 +434,7 @@ function getModuleInstanceStateForAliasTarget(specifier: ExportSpecifier, visite
const statements = p.statements;
let found: ModuleInstanceState | undefined;
for (const statement of statements) {
if (nodeHasName(statement, name)) {
if (nodeHasName(statement, moduleExportNameText(name))) {
if (!statement.parent) {
setParent(statement, p);
setParentRecursive(statement, /*incremental*/ false);
Expand Down Expand Up @@ -738,7 +740,7 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
function declareSymbol(symbolTable: SymbolTable, parent: Symbol | undefined, node: Declaration, includes: SymbolFlags, excludes: SymbolFlags, isReplaceableByMethod?: boolean, isComputedName?: boolean): Symbol {
Debug.assert(isComputedName || !hasDynamicName(node));

const isDefaultExport = hasSyntacticModifier(node, ModifierFlags.Default) || isExportSpecifier(node) && node.name.escapedText === "default";
const isDefaultExport = hasSyntacticModifier(node, ModifierFlags.Default) || isExportSpecifier(node) && moduleExportNameTextEscaped(node.name) === "default";

// The exported symbol for an export default function/class node is always named "default"
const name = isComputedName ? InternalSymbolName.Computed
Expand Down
145 changes: 98 additions & 47 deletions src/compiler/checker.ts

Large diffs are not rendered by default.

12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1601,6 +1601,10 @@
"category": "Error",
"code": 1490
},
"String literal module export names are not allowed when the 'module' option is set to 'es2020' or lower.": {
"category": "Error",
"code": 1491
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down Expand Up @@ -1659,6 +1663,14 @@
"category": "Message",
"code": 2212
},
"Imported name can only be an Identifier": {
"category": "Error",
"code": 2213
},
"String literal module export names must be followed by a 'from' clause.": {
"category": "Error",
"code": 2214
},

"Duplicate identifier '{0}'.": {
"category": "Error",
Expand Down
59 changes: 41 additions & 18 deletions src/compiler/factory/nodeFactory.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
__String,
AccessExpression,
addRange,
append,
appendIfUnique,
Expand Down Expand Up @@ -61,6 +62,7 @@ import {
containsObjectRestOrSpread,
ContinueStatement,
createBaseNodeFactory,
createElementAccessOrPropertyAccessExpression,
createNodeConverters,
createParenthesizerRules,
createScanner,
Expand Down Expand Up @@ -175,6 +177,7 @@ import {
isHoistedFunction,
isHoistedVariableStatement,
isIdentifier,
isIdentifierText,
isImportDeclaration,
isImportEqualsDeclaration,
isImportKeyword,
Expand All @@ -187,6 +190,7 @@ import {
isMethodDeclaration,
isMethodSignature,
isModuleDeclaration,
isModuleExportName,
isNamedDeclaration,
isNodeArray,
isNodeKind,
Expand Down Expand Up @@ -313,6 +317,7 @@ import {
ModuleBlock,
ModuleBody,
ModuleDeclaration,
ModuleExportName,
ModuleKind,
ModuleName,
ModuleReference,
Expand Down Expand Up @@ -983,6 +988,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
get createLogicalNot() { return getPrefixUnaryCreateFunction(SyntaxKind.ExclamationToken); },
get createPostfixIncrement() { return getPostfixUnaryCreateFunction(SyntaxKind.PlusPlusToken); },
get createPostfixDecrement() { return getPostfixUnaryCreateFunction(SyntaxKind.MinusMinusToken); },
createModuleExportName,

// Compound nodes
createImmediatelyInvokedFunctionExpression,
Expand Down Expand Up @@ -4713,7 +4719,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function createNamespaceExport(name: Identifier): NamespaceExport {
function createNamespaceExport(name: ModuleExportName): NamespaceExport {
const node = createBaseDeclaration<NamespaceExport>(SyntaxKind.NamespaceExport);
node.name = name;
node.transformFlags |=
Expand All @@ -4724,7 +4730,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function updateNamespaceExport(node: NamespaceExport, name: Identifier) {
function updateNamespaceExport(node: NamespaceExport, name: ModuleExportName) {
return node.name !== name
? update(createNamespaceExport(name), node)
: node;
Expand All @@ -4747,7 +4753,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function createImportSpecifier(isTypeOnly: boolean, propertyName: Identifier | undefined, name: Identifier) {
function createImportSpecifier(isTypeOnly: boolean, propertyName: ModuleExportName | undefined, name: Identifier) {
const node = createBaseDeclaration<ImportSpecifier>(SyntaxKind.ImportSpecifier);
node.isTypeOnly = isTypeOnly;
node.propertyName = propertyName;
Expand All @@ -4760,7 +4766,15 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function updateImportSpecifier(node: ImportSpecifier, isTypeOnly: boolean, propertyName: Identifier | undefined, name: Identifier) {
function createModuleExportName(name: string, languageVersion: ScriptTarget): ModuleExportName;
function createModuleExportName(name: string | undefined, languageVersion: ScriptTarget): ModuleExportName | undefined;
function createModuleExportName(name: string | undefined, languageVersion: ScriptTarget): ModuleExportName | undefined {
if (name === undefined) return undefined;
return isIdentifierText(name, languageVersion) ? createIdentifier(name) : createStringLiteral(name);
}

// @api
function updateImportSpecifier(node: ImportSpecifier, isTypeOnly: boolean, propertyName: ModuleExportName | undefined, name: Identifier) {
return node.isTypeOnly !== isTypeOnly
|| node.propertyName !== propertyName
|| node.name !== name
Expand Down Expand Up @@ -4868,11 +4882,11 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function createExportSpecifier(isTypeOnly: boolean, propertyName: string | Identifier | undefined, name: string | Identifier) {
function createExportSpecifier(isTypeOnly: boolean, propertyName: ModuleExportName | undefined, name: ModuleExportName) {
const node = createBaseNode<ExportSpecifier>(SyntaxKind.ExportSpecifier);
node.isTypeOnly = isTypeOnly;
node.propertyName = asName(propertyName);
node.name = asName(name);
node.propertyName = propertyName;
node.name = name;
node.transformFlags |=
propagateChildFlags(node.propertyName) |
propagateChildFlags(node.name);
Expand All @@ -4883,7 +4897,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
}

// @api
function updateExportSpecifier(node: ExportSpecifier, isTypeOnly: boolean, propertyName: Identifier | undefined, name: Identifier) {
function updateExportSpecifier(node: ExportSpecifier, isTypeOnly: boolean, propertyName: ModuleExportName | undefined, name: ModuleExportName) {
return node.isTypeOnly !== isTypeOnly
|| node.propertyName !== propertyName
|| node.name !== name
Expand Down Expand Up @@ -6647,9 +6661,10 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
: reduceLeft(expressions, factory.createComma)!;
}

function getName(node: Declaration | undefined, allowComments?: boolean, allowSourceMaps?: boolean, emitFlags: EmitFlags = 0, ignoreAssignedName?: boolean) {
function getName(node: Declaration | undefined, allowComments?: boolean, allowSourceMaps?: boolean, emitFlags: EmitFlags = 0, ignoreAssignedName?: boolean): Identifier | ModuleExportName {
const nodeName = ignoreAssignedName ? node && getNonAssignedNameOfDeclaration(node) : getNameOfDeclaration(node);
if (nodeName && isIdentifier(nodeName) && !isGeneratedIdentifier(nodeName)) {
// ModuleExportName includes Identifier
if (nodeName && isModuleExportName(nodeName) && !isGeneratedIdentifier(nodeName)) {
// TODO(rbuckton): Does this need to be parented?
const name = setParent(setTextRange(cloneNode(nodeName), nodeName), nodeName.parent);
emitFlags |= getEmitFlags(nodeName);
Expand All @@ -6673,7 +6688,9 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
function getInternalName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean) {
return getName(node, allowComments, allowSourceMaps, EmitFlags.LocalName | EmitFlags.InternalName);
const name = getName(node, allowComments, allowSourceMaps, EmitFlags.LocalName | EmitFlags.InternalName);
Debug.assertNode(name, isIdentifier);
return name;
}

/**
Expand All @@ -6688,7 +6705,9 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
* @param ignoreAssignedName Indicates that the assigned name of a declaration shouldn't be considered.
*/
function getLocalName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean, ignoreAssignedName?: boolean) {
return getName(node, allowComments, allowSourceMaps, EmitFlags.LocalName, ignoreAssignedName);
const name = getName(node, allowComments, allowSourceMaps, EmitFlags.LocalName, ignoreAssignedName);
Debug.assertNode(name, isIdentifier);
return name;
}

/**
Expand All @@ -6701,7 +6720,7 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
* @param allowComments A value indicating whether comments may be emitted for the name.
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
function getExportName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean): Identifier {
function getExportName(node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean): Identifier | ModuleExportName {
return getName(node, allowComments, allowSourceMaps, EmitFlags.ExportName);
}

Expand All @@ -6713,7 +6732,9 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
function getDeclarationName(node: Declaration | undefined, allowComments?: boolean, allowSourceMaps?: boolean) {
return getName(node, allowComments, allowSourceMaps);
const name = getName(node, allowComments, allowSourceMaps);
Debug.assertNode(name, isIdentifier);
return name;
}

/**
Expand All @@ -6724,8 +6745,8 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
* @param allowComments A value indicating whether comments may be emitted for the name.
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
function getNamespaceMemberName(ns: Identifier, name: Identifier, allowComments?: boolean, allowSourceMaps?: boolean): PropertyAccessExpression {
const qualifiedName = createPropertyAccessExpression(ns, nodeIsSynthesized(name) ? name : cloneNode(name));
function getNamespaceMemberName(ns: Identifier, name: ModuleExportName, allowComments?: boolean, allowSourceMaps?: boolean): AccessExpression {
const qualifiedName = createElementAccessOrPropertyAccessExpression(ns, nodeIsSynthesized(name) ? name : cloneNode(name));
setTextRange(qualifiedName, name);
let emitFlags: EmitFlags = 0;
if (!allowSourceMaps) emitFlags |= EmitFlags.NoSourceMap;
Expand All @@ -6745,11 +6766,13 @@ export function createNodeFactory(flags: NodeFactoryFlags, baseFactory: BaseNode
* @param allowComments A value indicating whether comments may be emitted for the name.
* @param allowSourceMaps A value indicating whether source maps may be emitted for the name.
*/
function getExternalModuleOrNamespaceExportName(ns: Identifier | undefined, node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean): Identifier | PropertyAccessExpression {
function getExternalModuleOrNamespaceExportName(ns: Identifier | undefined, node: Declaration, allowComments?: boolean, allowSourceMaps?: boolean): Identifier | AccessExpression {
if (ns && hasSyntacticModifier(node, ModifierFlags.Export)) {
return getNamespaceMemberName(ns, getName(node), allowComments, allowSourceMaps);
}
return getExportName(node, allowComments, allowSourceMaps);
const name = getExportName(node, allowComments, allowSourceMaps);
Debug.assertNode(name, isIdentifier);
return name;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/factory/nodeTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ import {
MissingDeclaration,
ModuleBlock,
ModuleDeclaration,
ModuleExportName,
NamedExports,
NamedImports,
NamedTupleMember,
Expand Down Expand Up @@ -321,6 +322,10 @@ export function isPrivateIdentifier(node: Node): node is PrivateIdentifier {
return node.kind === SyntaxKind.PrivateIdentifier;
}

export function isModuleExportName(node: Node): node is ModuleExportName {
return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.StringLiteral;
}

// Reserved Words

/** @internal */
Expand Down
8 changes: 6 additions & 2 deletions src/compiler/factory/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -792,15 +792,19 @@ export function getOrCreateExternalHelpersModuleNameIfNeeded(factory: NodeFactor
}

/**
* Get the name of that target module from an import or export declaration
* Get the name of that target module from an import or export declaration.
*
* This is only used in AMD and SystemJS emit.
*
* @internal
*/
export function getLocalNameForExternalImport(factory: NodeFactory, node: ImportDeclaration | ExportDeclaration | ImportEqualsDeclaration, sourceFile: SourceFile): Identifier | undefined {
const namespaceDeclaration = getNamespaceDeclarationNode(node);
if (namespaceDeclaration && !isDefaultImport(node) && !isExportNamespaceAsDefaultDeclaration(node)) {
const name = namespaceDeclaration.name;
return isGeneratedIdentifier(name) ? name : factory.createIdentifier(getSourceTextOfNodeFromSourceFile(sourceFile, name) || idText(name));
if (isGeneratedIdentifier(name)) return name;
if (name.kind === SyntaxKind.StringLiteral) return factory.getGeneratedNameForNode(name);
return factory.createIdentifier(getSourceTextOfNodeFromSourceFile(sourceFile, name) || idText(name));
}
if (node.kind === SyntaxKind.ImportDeclaration && node.importClause) {
return factory.getGeneratedNameForNode(node);
Expand Down
Loading