Skip to content

Module or import types #22592

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

Merged
merged 23 commits into from
Apr 2, 2018
Merged

Module or import types #22592

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9c69227
Type side of import types
weswigham Mar 14, 2018
e9d01ce
Value side of import types
weswigham Mar 14, 2018
31afd58
Accept library changes
weswigham Mar 14, 2018
6ceaee0
Refined implementation, more tests
weswigham Mar 14, 2018
fa9fe50
Allow resolutions to be performed late if the resolution still result…
weswigham Mar 15, 2018
5d2715c
Add another test case
weswigham Mar 15, 2018
79c4729
Add some jsdoc usages
weswigham Mar 15, 2018
9e77045
Merge branch 'master' into import-types
weswigham Mar 23, 2018
080ba6f
Allow nodebuilder to use import types where appropriate
weswigham Mar 23, 2018
0be6ce4
Parse & check generic instantiations
weswigham Mar 23, 2018
73075dd
use import types in nodebuilder for typeof module symbols
weswigham Mar 23, 2018
fefa9da
Wire up go to definition for import types
weswigham Mar 24, 2018
b07fe67
Accept updated type/symbol baselines now that symbols are wired in
weswigham Mar 24, 2018
20950c5
PR feedback
weswigham Mar 30, 2018
8a8efc9
Merge branch 'master' into import-types
weswigham Mar 30, 2018
7521c63
Fix changes from merge
weswigham Mar 30, 2018
257b848
Walk back late import handling
weswigham Mar 30, 2018
a06607a
Remove unused diagnostic
weswigham Mar 30, 2018
87b102e
Remove unrelated changes
weswigham Mar 30, 2018
7770788
Use recursive function over loop
weswigham Mar 30, 2018
d24ae2e
Emit type arguments
weswigham Mar 30, 2018
fbd6ce8
undo unrelated change
weswigham Mar 30, 2018
0c143a0
Test for and support import type nodes in bundled declaration emit
weswigham Mar 30, 2018
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
86 changes: 82 additions & 4 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,17 @@ namespace ts {
if (ambientModule) {
return ambientModule;
}
const resolvedModule = getResolvedModule(getSourceFileOfNode(location), moduleReference);
const currentSourceFile = getSourceFileOfNode(location);
const resolvedModuleState = getResolvedModule(currentSourceFile, moduleReference);
let resolvedModule: ResolvedModuleFull;
if (currentSourceFile && resolvedModuleState === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure i understand why we need to do the resolution on two phases, given that we only support string literals as arguments to import

Copy link
Member Author

Choose a reason for hiding this comment

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

given that we only support string literals as arguments to import

String literal types. Right now it's not an error to say

type Alias = "module";
const x: import(Alias);

the modules in question just only get loaded if they're already included in the build, otherwise produce an error. (see importTypeNested test.) I can restrain this further, but I don't much see the point in limiting it? I could also handle unions pretty easily as it turns out, too (by bypassing normal typequery logic I've given myself a place to return union types and do multiple namespace lookups).

Copy link
Contributor

Choose a reason for hiding this comment

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

As we have chatted offline. i think the real value of such feature is to enable higher order types, since we have decided not to do that, i would say no need for the complexity. I would just remove all the deferred module resolution stuff, and keep it simple.

Copy link
Contributor

Choose a reason for hiding this comment

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

parsing a type to be future proof is fine. but just make it such that the argument has to be string literal node.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhegazy Done. It now issues an error if not a string literal, and the late-resolution code has been removed.

// Fallback to uncached lookup for late-bound module names which have not been resolved
const resolutions = host.resolveModuleName([moduleReference], currentSourceFile.fileName);
resolvedModule = firstOrUndefined(resolutions);
}
else {
resolvedModule = getResolvedModuleFromState(resolvedModuleState);
}
const resolutionDiagnostic = resolvedModule && getResolutionDiagnostic(compilerOptions, resolvedModule);
const sourceFile = resolvedModule && !resolutionDiagnostic && host.getSourceFile(resolvedModule.resolvedFileName);
if (sourceFile) {
Expand Down Expand Up @@ -2182,15 +2192,15 @@ namespace ts {
// An external module with an 'export =' declaration may be referenced as an ES6 module provided the 'export ='
// references a symbol that is at least declared as a module or a variable. The target of the 'export =' may
// combine other declarations with the module or variable (e.g. a class/module, function/module, interface/variable).
function resolveESModuleSymbol(moduleSymbol: Symbol, moduleReferenceExpression: Expression, dontResolveAlias: boolean): Symbol {
function resolveESModuleSymbol(moduleSymbol: Symbol, referencingLocation: Node, dontResolveAlias: boolean): Symbol {
const symbol = resolveExternalModuleSymbol(moduleSymbol, dontResolveAlias);
if (!dontResolveAlias && symbol) {
if (!(symbol.flags & (SymbolFlags.Module | SymbolFlags.Variable))) {
error(moduleReferenceExpression, Diagnostics.Module_0_resolves_to_a_non_module_entity_and_cannot_be_imported_using_this_construct, symbolToString(moduleSymbol));
error(referencingLocation, Diagnostics.Module_0_resolves_to_a_non_module_entity_and_cannot_be_imported_using_this_construct, symbolToString(moduleSymbol));
return symbol;
}
if (compilerOptions.esModuleInterop) {
const referenceParent = moduleReferenceExpression.parent;
const referenceParent = referencingLocation.parent;
if (
(isImportDeclaration(referenceParent) && getNamespaceDeclarationNode(referenceParent)) ||
isImportCall(referenceParent)
Expand Down Expand Up @@ -8411,6 +8421,65 @@ namespace ts {
return links.resolvedType;
}

function getTypeFromImportTypeNode(node: ImportTypeNode): Type {
const links = getNodeLinks(node);
if (!links.resolvedType) {
const argumentType = getTypeFromTypeNode(node.argument);
const targetMeaning = node.isTypeOf ? SymbolFlags.Value : SymbolFlags.Type;
// TODO: Future work: support unions/generics/whatever via a deferred import-type
if (!argumentType || !(argumentType.flags & TypeFlags.StringLiteral)) {
error(node.argument, Diagnostics.Import_specifier_must_be_a_string_literal_type_but_here_is_0, argumentType ? typeToString(argumentType) : "undefined");
return links.resolvedType = anyType;
}
const moduleName = (argumentType as StringLiteralType).value;
const innerModuleSymbol = resolveExternalModule(node, moduleName, Diagnostics.Cannot_find_module_0, node, /*isForAugmentation*/ false);
if (!innerModuleSymbol) {
error(node, Diagnostics.Cannot_find_module_0, moduleName);
return links.resolvedType = anyType;
}
const moduleSymbol = resolveExternalModuleSymbol(innerModuleSymbol, /*dontResolveAlias*/ false);
if (!moduleSymbol) {
error(node, Diagnostics.Cannot_find_module_0, moduleName);
return links.resolvedType = anyType;
}
if (node.qualifier) {
const nameStack: Identifier[] = [];
let currentNamespace = moduleSymbol;
let current = node.qualifier;
while (true) {
if (current.kind === SyntaxKind.Identifier) {
nameStack.push(current);
break;
}
else {
nameStack.push(current.right);
current = current.left;
Copy link
Contributor

Choose a reason for hiding this comment

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

that error message does not seem right, you probably want something like typeof can not b sued on a type or type arguments can not be used on a value or just type arguments can not be used here

Copy link
Member Author

Choose a reason for hiding this comment

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

'Doh, I put this one here as a placeholder and forgot to replace it with a new message. Thanks for the catch!

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

unknowType?

while (current = nameStack.pop()) {
const meaning = nameStack.length ? SymbolFlags.Namespace : targetMeaning;
const next = getSymbol(getExportsOfSymbol(getMergedSymbol(resolveSymbol(currentNamespace))), current.escapedText, meaning);
if (!next) {
error(current, Diagnostics.Namespace_0_has_no_exported_member_1, getFullyQualifiedName(currentNamespace), declarationNameToString(current));
Copy link
Contributor

Choose a reason for hiding this comment

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

i would just check the node to be a string literal.

return links.resolvedType = anyType;
}
currentNamespace = next;
Copy link
Contributor

Choose a reason for hiding this comment

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

unknownType?

}
links.resolvedType = targetMeaning === SymbolFlags.Value ? getTypeOfSymbol(currentNamespace) : getDeclaredTypeOfSymbol(currentNamespace);
}
else {
if (moduleSymbol.flags & targetMeaning) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should not resolveExternalModule already create an error?

links.resolvedType = targetMeaning === SymbolFlags.Value ? getTypeOfSymbol(moduleSymbol) : getDeclaredTypeOfSymbol(moduleSymbol);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

unknownType?

else {
error(node, targetMeaning === SymbolFlags.Value ? Diagnostics.Module_0_does_not_refer_to_a_value_but_is_used_as_a_value_here : Diagnostics.Module_0_does_not_refer_to_a_type_but_is_used_as_a_type_here, moduleName);
links.resolvedType = anyType;
Copy link
Contributor

Choose a reason for hiding this comment

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

how would this fail? should not always return a symbol?

Copy link
Member Author

@weswigham weswigham Mar 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it can, was probably just being a bit too defensive (the function clearly can return undefined, but probably only when the input symbol is such).

}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe extract to a recursive function to avoid the complexity.

return links.resolvedType;
}

function getTypeFromTypeLiteralOrFunctionOrConstructorTypeNode(node: TypeNode): Type {
const links = getNodeLinks(node);
if (!links.resolvedType) {
Expand Down Expand Up @@ -8704,6 +8773,8 @@ namespace ts {
return getTypeFromConditionalTypeNode(<ConditionalTypeNode>node);
case SyntaxKind.InferType:
return getTypeFromInferTypeNode(<InferTypeNode>node);
case SyntaxKind.ImportTypeNode:
return getTypeFromImportTypeNode(<ImportTypeNode>node);
// This function assumes that an identifier or qualified name is a type expression
// Callers should first ensure this by calling isTypeNode
case SyntaxKind.Identifier:
Expand Down Expand Up @@ -20609,6 +20680,11 @@ namespace ts {
checkSourceElement(node.typeParameter);
}

function checkImportType(node: ImportTypeNode) {
checkSourceElement(node.argument);
getTypeFromTypeNode(node);
}

function isPrivateWithinAmbient(node: Node): boolean {
return hasModifier(node, ModifierFlags.Private) && !!(node.flags & NodeFlags.Ambient);
}
Expand Down Expand Up @@ -24356,6 +24432,8 @@ namespace ts {
return checkConditionalType(<ConditionalTypeNode>node);
case SyntaxKind.InferType:
return checkInferType(<InferTypeNode>node);
case SyntaxKind.ImportTypeNode:
return checkImportType(<ImportTypeNode>node);
case SyntaxKind.JSDocAugmentsTag:
return checkJSDocAugmentsTag(node as JSDocAugmentsTag);
case SyntaxKind.JSDocTypedefTag:
Expand Down
15 changes: 15 additions & 0 deletions src/compiler/declarationEmitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,8 @@ namespace ts {
return emitConditionalType(<ConditionalTypeNode>type);
case SyntaxKind.InferType:
return emitInferType(<InferTypeNode>type);
case SyntaxKind.ImportTypeNode:
return emitImportType(<ImportTypeNode>type);
case SyntaxKind.ParenthesizedType:
return emitParenType(<ParenthesizedTypeNode>type);
case SyntaxKind.TypeOperator:
Expand Down Expand Up @@ -567,6 +569,19 @@ namespace ts {
writeTextOfNode(currentText, node.typeParameter.name);
}

function emitImportType(node: ImportTypeNode) {
if (node.isTypeOf) {
write("typeof ");
}
write("import(");
emitType(node.argument);
write(")");
if (node.qualifier) {
write(".");
writeEntityName(node.qualifier); // Don't do visibility checking; this entity name is _not_ looked up like a type query is
}
}

function emitParenType(type: ParenthesizedTypeNode) {
write("(");
emitType(type.type);
Expand Down
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,6 +951,18 @@
"category": "Error",
"code": 1338
},
"Module '{0}' does not refer to a value, but is used as a value here.": {
"category": "Error",
"code": 1339
},
"Module '{0}' does not refer to a type, but is used as a type here.": {
"category": "Error",
"code": 1340
},
"Import specifier must be a string literal type, but here is '{0}'.": {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, not sure the type name here gives you much value.. we only allow string literals, not even strings.. so i would just say only string literals are valid import specifiers.

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll parse any type there to be permissive, though; this is more of a grammar error.

Copy link
Contributor

Choose a reason for hiding this comment

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

I know. i am just saying whatever {0} is, the error message is just saying you cannot have it.. in other words, the type name is extraneous

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 find including the type in the error message useful as frequently an error only exists because I believe that the type is correct at a given location; so seeing where reality differs from my expectation is useful.

"category": "Error",
"code": 1341
},

"Duplicate identifier '{0}'.": {
"category": "Error",
Expand Down
17 changes: 17 additions & 0 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,8 @@ namespace ts {
return emitMappedType(<MappedTypeNode>node);
case SyntaxKind.LiteralType:
return emitLiteralType(<LiteralTypeNode>node);
case SyntaxKind.ImportTypeNode:
return emitImportTypeNode(<ImportTypeNode>node);
case SyntaxKind.JSDocAllType:
write("*");
return;
Expand Down Expand Up @@ -1285,6 +1287,21 @@ namespace ts {
emitExpression(node.literal);
}

function emitImportTypeNode(node: ImportTypeNode) {
if (node.isTypeOf) {
writeKeyword("typeof");
writeSpace();
}
writeKeyword("import");
writePunctuation("(");
emit(node.argument);
writePunctuation(")");
if (node.qualifier) {
writePunctuation(".");
emit(node.qualifier);
}
}

//
// Binding patterns
//
Expand Down
14 changes: 14 additions & 0 deletions src/compiler/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,20 @@ namespace ts {
: node;
}

export function createImportTypeNode(argument: TypeNode, qualifier?: EntityName) {
const node = <ImportTypeNode>createSynthesizedNode(SyntaxKind.ImportTypeNode);
node.argument = argument;
node.qualifier = qualifier;
return node;
}

export function updateImportTypeNode(node: ImportTypeNode, argument: TypeNode, qualifier?: EntityName) {
return node.argument !== argument
&& node.qualifier !== qualifier
? updateNode(createImportTypeNode(argument, qualifier), node)
: node;
}

export function createParenthesizedType(type: TypeNode) {
const node = <ParenthesizedTypeNode>createSynthesizedNode(SyntaxKind.ParenthesizedType);
node.type = type;
Expand Down
29 changes: 28 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ namespace ts {
visitNode(cbNode, (<ConditionalTypeNode>node).falseType);
case SyntaxKind.InferType:
return visitNode(cbNode, (<InferTypeNode>node).typeParameter);
case SyntaxKind.ImportTypeNode:
return visitNode(cbNode, (<ImportTypeNode>node).argument) ||
visitNode(cbNode, (<ImportTypeNode>node).qualifier);
case SyntaxKind.ParenthesizedType:
case SyntaxKind.TypeOperator:
return visitNode(cbNode, (<ParenthesizedTypeNode | TypeOperatorNode>node).type);
Expand Down Expand Up @@ -2702,6 +2705,27 @@ namespace ts {
return finishNode(node);
}

function isStartOfTypeOfImportType() {
nextToken();
return token() === SyntaxKind.ImportKeyword;
}

function parseImportType(): ImportTypeNode {
sourceFile.flags |= NodeFlags.PossiblyContainsDynamicImport;
const node = createNode(SyntaxKind.ImportTypeNode) as ImportTypeNode;
if (parseOptional(SyntaxKind.TypeOfKeyword)) {
node.isTypeOf = true;
}
parseExpected(SyntaxKind.ImportKeyword);
parseExpected(SyntaxKind.OpenParenToken);
node.argument = parseType();
parseExpected(SyntaxKind.CloseParenToken);
if (parseOptional(SyntaxKind.DotToken)) {
node.qualifier = parseEntityName(/*allowReservedWords*/ true, Diagnostics.Type_expected);
}
return finishNode(node);
}

function nextTokenIsNumericLiteral() {
return nextToken() === SyntaxKind.NumericLiteral;
}
Expand Down Expand Up @@ -2747,13 +2771,15 @@ namespace ts {
}
}
case SyntaxKind.TypeOfKeyword:
return parseTypeQuery();
return lookAhead(isStartOfTypeOfImportType) ? parseImportType() : parseTypeQuery();
case SyntaxKind.OpenBraceToken:
return lookAhead(isStartOfMappedType) ? parseMappedType() : parseTypeLiteral();
case SyntaxKind.OpenBracketToken:
return parseTupleType();
case SyntaxKind.OpenParenToken:
return parseParenthesizedType();
case SyntaxKind.ImportKeyword:
return parseImportType();
default:
return parseTypeReference();
}
Expand Down Expand Up @@ -2789,6 +2815,7 @@ namespace ts {
case SyntaxKind.ExclamationToken:
case SyntaxKind.DotDotDotToken:
case SyntaxKind.InferKeyword:
case SyntaxKind.ImportKeyword:
return true;
case SyntaxKind.MinusToken:
return !inStartOfParameter && lookAhead(nextTokenIsNumericLiteral);
Expand Down
16 changes: 10 additions & 6 deletions src/compiler/program.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,8 @@ namespace ts {
getSourceFileFromReference,
sourceFileToPackageName,
redirectTargetsSet,
isEmittedFile
isEmittedFile,
resolveModuleName: resolveModuleNamesWorker
};

verifyCompilerOptions();
Expand Down Expand Up @@ -739,7 +740,7 @@ namespace ts {
const result: ResolvedModuleFull[] = [];
for (const moduleName of moduleNames) {
const resolvedModule = file.resolvedModules.get(moduleName);
result.push(resolvedModule);
result.push(getResolvedModuleFromState(resolvedModule));
}
return result;
}
Expand Down Expand Up @@ -768,7 +769,7 @@ namespace ts {
const moduleName = moduleNames[i];
// If the source file is unchanged and doesnt have invalidated resolution, reuse the module resolutions
if (file === oldSourceFile && !hasInvalidatedResolution(oldSourceFile.path)) {
const oldResolvedModule = oldSourceFile && oldSourceFile.resolvedModules.get(moduleName);
const oldResolvedModule = getResolvedModuleFromState(oldSourceFile && oldSourceFile.resolvedModules.get(moduleName));
if (oldResolvedModule) {
if (isTraceEnabled(options, host)) {
trace(host, Diagnostics.Reusing_resolution_of_module_0_to_file_1_from_old_program, moduleName, containingFile);
Expand Down Expand Up @@ -834,7 +835,7 @@ namespace ts {
// If we change our policy of rechecking failed lookups on each program create,
// we should adjust the value returned here.
function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean {
const resolutionToFile = getResolvedModule(oldProgramState.oldSourceFile, moduleName);
const resolutionToFile = getResolvedModuleFromState(getResolvedModule(oldProgramState.oldSourceFile, moduleName));
const resolvedFile = resolutionToFile && oldProgramState.program && oldProgramState.program.getSourceFile(resolutionToFile.resolvedFileName);
if (resolutionToFile && resolvedFile && !resolvedFile.externalModuleIndicator) {
// In the old program, we resolved to an ambient module that was in the same
Expand Down Expand Up @@ -1017,10 +1018,10 @@ namespace ts {
const oldProgramState: OldProgramState = { program: oldProgram, oldSourceFile, modifiedFilePaths };
const resolutions = resolveModuleNamesReusingOldState(moduleNames, newSourceFilePath, newSourceFile, oldProgramState);
// ensure that module resolution results are still correct
const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo);
const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo, getResolvedModuleFromState);
if (resolutionsChanged) {
oldProgram.structureIsReused = StructureIsReused.SafeModules;
newSourceFile.resolvedModules = zipToMap(moduleNames, resolutions);
newSourceFile.resolvedModules = zipToMap(moduleNames, map(resolutions, r => ({ tag: "success" as "success", data: r })));
}
else {
newSourceFile.resolvedModules = oldSourceFile.resolvedModules;
Expand Down Expand Up @@ -1682,6 +1683,9 @@ namespace ts {
else if (isImportCall(node) && node.arguments.length === 1 && node.arguments[0].kind === SyntaxKind.StringLiteral) {
(imports || (imports = [])).push(<StringLiteral>(<CallExpression>node).arguments[0]);
}
else if (isLiteralImportTypeNode(node)) {
(imports || (imports = [])).push(node.argument.literal);
}
else {
forEachChild(node, collectDynamicImportOrRequireCalls);
}
Expand Down
Loading