From 5999a521f6e852de92681acf8c91c01f102edab6 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Sat, 31 Dec 2016 19:12:57 -0800 Subject: [PATCH 01/14] Support for an external exportStar helper --- src/compiler/checker.ts | 8 ++- src/compiler/factory.ts | 52 ++++++++++++------- src/compiler/transformers/module/module.ts | 24 ++++----- src/compiler/types.ts | 3 +- tests/baselines/reference/importHelpersAmd.js | 8 ++- .../reference/importHelpersAmd.symbols | 19 ++++++- .../reference/importHelpersAmd.types | 17 ++++++ .../importHelpersInAmbientContext.js | 4 +- .../importHelpersInAmbientContext.symbols | 16 ++++++ .../importHelpersInAmbientContext.types | 16 ++++++ .../importHelpersNoHelpers.errors.txt | 21 +++++--- .../reference/importHelpersNoHelpers.js | 8 +++ .../reference/importHelpersSystem.js | 12 +++++ .../reference/importHelpersSystem.symbols | 3 +- .../reference/importHelpersSystem.types | 1 + tests/cases/compiler/importHelpersAmd.ts | 4 ++ .../compiler/importHelpersInAmbientContext.ts | 3 ++ .../cases/compiler/importHelpersNoHelpers.ts | 4 ++ tests/cases/compiler/importHelpersSystem.ts | 1 + 19 files changed, 178 insertions(+), 46 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 33b3505661a..2900932bc00 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -19160,6 +19160,10 @@ namespace ts { if (moduleSymbol && hasExportAssignmentSymbol(moduleSymbol)) { error(node.moduleSpecifier, Diagnostics.Module_0_uses_export_and_cannot_be_used_with_export_Asterisk, symbolToString(moduleSymbol)); } + + if (modulekind !== ModuleKind.System && modulekind !== ModuleKind.ES2015) { + checkExternalEmitHelpers(node, ExternalEmitHelpers.ExportStar); + } } } } @@ -20788,7 +20792,7 @@ namespace ts { function checkExternalEmitHelpers(location: Node, helpers: ExternalEmitHelpers) { if ((requestedExternalEmitHelpers & helpers) !== helpers && compilerOptions.importHelpers) { const sourceFile = getSourceFileOfNode(location); - if (isEffectiveExternalModule(sourceFile, compilerOptions)) { + if (!isDeclarationFile(sourceFile) && isEffectiveExternalModule(sourceFile, compilerOptions)) { const helpersModule = resolveHelpersModule(sourceFile, location); if (helpersModule !== unknownSymbol) { const uncheckedHelpers = helpers & ~requestedExternalEmitHelpers; @@ -20817,6 +20821,8 @@ namespace ts { case ExternalEmitHelpers.Param: return "__param"; case ExternalEmitHelpers.Awaiter: return "__awaiter"; case ExternalEmitHelpers.Generator: return "__generator"; + case ExternalEmitHelpers.ExportStar: return "__exportStar"; + default: Debug.fail("Unrecognized helper"); } } diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index ac1f52a991f..033d694ade2 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -2808,25 +2808,37 @@ namespace ts { return emitNode && emitNode.externalHelpersModuleName; } - export function getOrCreateExternalHelpersModuleNameIfNeeded(node: SourceFile, compilerOptions: CompilerOptions) { - if (compilerOptions.importHelpers && (isExternalModule(node) || compilerOptions.isolatedModules)) { + export function getOrCreateExternalHelpersModuleNameIfNeeded(node: SourceFile, compilerOptions: CompilerOptions, hasExportStarsToExportValues?: boolean) { + if (compilerOptions.importHelpers && isEffectiveExternalModule(node, compilerOptions)) { const externalHelpersModuleName = getExternalHelpersModuleName(node); if (externalHelpersModuleName) { return externalHelpersModuleName; } - const helpers = getEmitHelpers(node); - if (helpers) { - for (const helper of helpers) { - if (!helper.scoped) { - const parseNode = getOriginalNode(node, isSourceFile); - const emitNode = getOrCreateEmitNode(parseNode); - return emitNode.externalHelpersModuleName || (emitNode.externalHelpersModuleName = createUniqueName(externalHelpersModuleNameText)); + const moduleKind = getEmitModuleKind(compilerOptions); + let create = hasExportStarsToExportValues + && moduleKind !== ModuleKind.System + && moduleKind !== ModuleKind.ES2015; + if (!create) { + const helpers = getEmitHelpers(node); + if (helpers) { + for (const helper of helpers) { + if (!helper.scoped) { + create = true; + break; + } } } } + + if (create) { + const parseNode = getOriginalNode(node, isSourceFile); + const emitNode = getOrCreateEmitNode(parseNode); + return emitNode.externalHelpersModuleName || (emitNode.externalHelpersModuleName = createUniqueName(externalHelpersModuleNameText)); + } } } + /** * Adds an EmitHelper to a node. */ @@ -3293,17 +3305,6 @@ namespace ts { let exportEquals: ExportAssignment = undefined; let hasExportStarsToExportValues = false; - const externalHelpersModuleName = getOrCreateExternalHelpersModuleNameIfNeeded(sourceFile, compilerOptions); - const externalHelpersImportDeclaration = externalHelpersModuleName && createImportDeclaration( - /*decorators*/ undefined, - /*modifiers*/ undefined, - createImportClause(/*name*/ undefined, createNamespaceImport(externalHelpersModuleName)), - createLiteral(externalHelpersModuleNameText)); - - if (externalHelpersImportDeclaration) { - externalImports.push(externalHelpersImportDeclaration); - } - for (const node of sourceFile.statements) { switch (node.kind) { case SyntaxKind.ImportDeclaration: @@ -3414,6 +3415,17 @@ namespace ts { } } + const externalHelpersModuleName = getOrCreateExternalHelpersModuleNameIfNeeded(sourceFile, compilerOptions, hasExportStarsToExportValues); + const externalHelpersImportDeclaration = externalHelpersModuleName && createImportDeclaration( + /*decorators*/ undefined, + /*modifiers*/ undefined, + createImportClause(/*name*/ undefined, createNamespaceImport(externalHelpersModuleName)), + createLiteral(externalHelpersModuleNameText)); + + if (externalHelpersImportDeclaration) { + externalImports.unshift(externalHelpersImportDeclaration); + } + return { externalImports, exportSpecifiers, exportEquals, hasExportStarsToExportValues, exportedBindings, exportedNames, externalHelpersImportDeclaration }; } diff --git a/src/compiler/transformers/module/module.ts b/src/compiler/transformers/module/module.ts index 170af0778ca..da8a55ed3bc 100644 --- a/src/compiler/transformers/module/module.ts +++ b/src/compiler/transformers/module/module.ts @@ -87,7 +87,7 @@ namespace ts { addExportEqualsIfNeeded(statements, /*emitAsReturn*/ false); const updated = updateSourceFileNode(node, createNodeArray(statements, node.statements)); - if (currentModuleInfo.hasExportStarsToExportValues) { + if (currentModuleInfo.hasExportStarsToExportValues && !compilerOptions.importHelpers) { addEmitHelper(updated, exportStarHelper); } @@ -377,7 +377,7 @@ namespace ts { addExportEqualsIfNeeded(statements, /*emitAsReturn*/ true); const body = createBlock(statements, /*location*/ undefined, /*multiLine*/ true); - if (currentModuleInfo.hasExportStarsToExportValues) { + if (currentModuleInfo.hasExportStarsToExportValues && !compilerOptions.importHelpers) { // If we have any `export * from ...` declarations // we need to inform the emitter to add the __export helper. addEmitHelper(body, exportStarHelper); @@ -691,15 +691,7 @@ namespace ts { else { // export * from "mod"; return createStatement( - createCall( - createIdentifier("__export"), - /*typeArguments*/ undefined, - [ - moduleKind !== ModuleKind.AMD - ? createRequireCall(node) - : generatedName - ] - ), + createExportStarHelper(context, moduleKind !== ModuleKind.AMD ? createRequireCall(node) : generatedName), /*location*/ node ); } @@ -1441,6 +1433,14 @@ namespace ts { text: ` function __export(m) { for (var p in m) if (!exports.hasOwnProperty(p)) exports[p] = m[p]; - }` + } + ` }; + + function createExportStarHelper(context: TransformationContext, module: Expression) { + const compilerOptions = context.getCompilerOptions(); + return compilerOptions.importHelpers + ? createCall(getHelperName("__exportStar"), /*typeArguments*/ undefined, [module, createIdentifier("exports")]) + : createCall(createIdentifier("__export"), /*typeArguments*/ undefined, [module]); + } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 6eebb0b99d7..26045ba1688 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3733,9 +3733,10 @@ namespace ts { Param = 1 << 5, // __param (used by TypeScript decorators transformation) Awaiter = 1 << 6, // __awaiter (used by ES2017 async functions transformation) Generator = 1 << 7, // __generator (used by ES2015 generator transformation) + ExportStar = 1 << 8, // __exportStar (used by CommonJS/AMD/UMD module transformation) FirstEmitHelper = Extends, - LastEmitHelper = Generator + LastEmitHelper = ExportStar } /* @internal */ diff --git a/tests/baselines/reference/importHelpersAmd.js b/tests/baselines/reference/importHelpersAmd.js index fea93840fa9..96d398e05eb 100644 --- a/tests/baselines/reference/importHelpersAmd.js +++ b/tests/baselines/reference/importHelpersAmd.js @@ -5,16 +5,19 @@ export class A { } //// [b.ts] import { A } from "./a"; +export * from "./a"; export class B extends A { } //// [tslib.d.ts] export declare function __extends(d: Function, b: Function): void; export declare function __assign(t: any, ...sources: any[]): any; +export declare function __rest(t: any, propertyNames: string[]): any; export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; export declare function __param(paramIndex: number, decorator: Function): Function; export declare function __metadata(metadataKey: any, metadataValue: any): Function; export declare function __awaiter(thisArg: any, _arguments: any, P: Function, generator: Function): any; - +export declare function __generator(thisArg: any, body: Function): any; +export declare function __exportStar(m: any, exports: any): void; //// [a.js] define(["require", "exports"], function (require, exports) { @@ -27,8 +30,9 @@ define(["require", "exports"], function (require, exports) { exports.A = A; }); //// [b.js] -define(["require", "exports", "tslib", "./a"], function (require, exports, tslib_1, a_1) { +define(["require", "exports", "tslib", "./a", "./a"], function (require, exports, tslib_1, a_1, a_2) { "use strict"; + tslib_1.__exportStar(a_2, exports); var B = (function (_super) { tslib_1.__extends(B, _super); function B() { diff --git a/tests/baselines/reference/importHelpersAmd.symbols b/tests/baselines/reference/importHelpersAmd.symbols index 47528a8bba6..68506b4c353 100644 --- a/tests/baselines/reference/importHelpersAmd.symbols +++ b/tests/baselines/reference/importHelpersAmd.symbols @@ -6,8 +6,9 @@ export class A { } import { A } from "./a"; >A : Symbol(A, Decl(b.ts, 0, 8)) +export * from "./a"; export class B extends A { } ->B : Symbol(B, Decl(b.ts, 0, 24)) +>B : Symbol(B, Decl(b.ts, 1, 20)) >A : Symbol(A, Decl(b.ts, 0, 8)) === tests/cases/compiler/tslib.d.ts === @@ -23,6 +24,11 @@ export declare function __assign(t: any, ...sources: any[]): any; >t : Symbol(t, Decl(tslib.d.ts, --, --)) >sources : Symbol(sources, Decl(tslib.d.ts, --, --)) +export declare function __rest(t: any, propertyNames: string[]): any; +>__rest : Symbol(__rest, Decl(tslib.d.ts, --, --)) +>t : Symbol(t, Decl(tslib.d.ts, --, --)) +>propertyNames : Symbol(propertyNames, Decl(tslib.d.ts, --, --)) + export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; >__decorate : Symbol(__decorate, Decl(tslib.d.ts, --, --)) >decorators : Symbol(decorators, Decl(tslib.d.ts, --, --)) @@ -53,3 +59,14 @@ export declare function __awaiter(thisArg: any, _arguments: any, P: Function, ge >generator : Symbol(generator, Decl(tslib.d.ts, --, --)) >Function : Symbol(Function, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) +export declare function __generator(thisArg: any, body: Function): any; +>__generator : Symbol(__generator, Decl(tslib.d.ts, --, --)) +>thisArg : Symbol(thisArg, Decl(tslib.d.ts, --, --)) +>body : Symbol(body, Decl(tslib.d.ts, --, --)) +>Function : Symbol(Function, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) + +export declare function __exportStar(m: any, exports: any): void; +>__exportStar : Symbol(__exportStar, Decl(tslib.d.ts, --, --)) +>m : Symbol(m, Decl(tslib.d.ts, --, --)) +>exports : Symbol(exports, Decl(tslib.d.ts, --, --)) + diff --git a/tests/baselines/reference/importHelpersAmd.types b/tests/baselines/reference/importHelpersAmd.types index 9ff756ffaab..23456ae2333 100644 --- a/tests/baselines/reference/importHelpersAmd.types +++ b/tests/baselines/reference/importHelpersAmd.types @@ -6,6 +6,7 @@ export class A { } import { A } from "./a"; >A : typeof A +export * from "./a"; export class B extends A { } >B : B >A : A @@ -23,6 +24,11 @@ export declare function __assign(t: any, ...sources: any[]): any; >t : any >sources : any[] +export declare function __rest(t: any, propertyNames: string[]): any; +>__rest : (t: any, propertyNames: string[]) => any +>t : any +>propertyNames : string[] + export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; >__decorate : (decorators: Function[], target: any, key?: string | symbol, desc?: any) => any >decorators : Function[] @@ -53,3 +59,14 @@ export declare function __awaiter(thisArg: any, _arguments: any, P: Function, ge >generator : Function >Function : Function +export declare function __generator(thisArg: any, body: Function): any; +>__generator : (thisArg: any, body: Function) => any +>thisArg : any +>body : Function +>Function : Function + +export declare function __exportStar(m: any, exports: any): void; +>__exportStar : (m: any, exports: any) => void +>m : any +>exports : any + diff --git a/tests/baselines/reference/importHelpersInAmbientContext.js b/tests/baselines/reference/importHelpersInAmbientContext.js index b9db8c4751f..9433b41d117 100644 --- a/tests/baselines/reference/importHelpersInAmbientContext.js +++ b/tests/baselines/reference/importHelpersInAmbientContext.js @@ -49,11 +49,13 @@ declare namespace N { //// [tslib.d.ts] export declare function __extends(d: Function, b: Function): void; export declare function __assign(t: any, ...sources: any[]): any; +export declare function __rest(t: any, propertyNames: string[]): any; export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; export declare function __param(paramIndex: number, decorator: Function): Function; export declare function __metadata(metadataKey: any, metadataValue: any): Function; export declare function __awaiter(thisArg: any, _arguments: any, P: Function, generator: Function): any; - +export declare function __generator(thisArg: any, body: Function): any; +export declare function __exportStar(m: any, exports: any): void; //// [b.js] "use strict"; diff --git a/tests/baselines/reference/importHelpersInAmbientContext.symbols b/tests/baselines/reference/importHelpersInAmbientContext.symbols index 7da1ea95042..5b9b7e61e71 100644 --- a/tests/baselines/reference/importHelpersInAmbientContext.symbols +++ b/tests/baselines/reference/importHelpersInAmbientContext.symbols @@ -99,6 +99,11 @@ export declare function __assign(t: any, ...sources: any[]): any; >t : Symbol(t, Decl(tslib.d.ts, --, --)) >sources : Symbol(sources, Decl(tslib.d.ts, --, --)) +export declare function __rest(t: any, propertyNames: string[]): any; +>__rest : Symbol(__rest, Decl(tslib.d.ts, --, --)) +>t : Symbol(t, Decl(tslib.d.ts, --, --)) +>propertyNames : Symbol(propertyNames, Decl(tslib.d.ts, --, --)) + export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; >__decorate : Symbol(__decorate, Decl(tslib.d.ts, --, --)) >decorators : Symbol(decorators, Decl(tslib.d.ts, --, --)) @@ -129,3 +134,14 @@ export declare function __awaiter(thisArg: any, _arguments: any, P: Function, ge >generator : Symbol(generator, Decl(tslib.d.ts, --, --)) >Function : Symbol(Function, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) +export declare function __generator(thisArg: any, body: Function): any; +>__generator : Symbol(__generator, Decl(tslib.d.ts, --, --)) +>thisArg : Symbol(thisArg, Decl(tslib.d.ts, --, --)) +>body : Symbol(body, Decl(tslib.d.ts, --, --)) +>Function : Symbol(Function, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --)) + +export declare function __exportStar(m: any, exports: any): void; +>__exportStar : Symbol(__exportStar, Decl(tslib.d.ts, --, --)) +>m : Symbol(m, Decl(tslib.d.ts, --, --)) +>exports : Symbol(exports, Decl(tslib.d.ts, --, --)) + diff --git a/tests/baselines/reference/importHelpersInAmbientContext.types b/tests/baselines/reference/importHelpersInAmbientContext.types index 5f3102b68f5..ac0803e6bd7 100644 --- a/tests/baselines/reference/importHelpersInAmbientContext.types +++ b/tests/baselines/reference/importHelpersInAmbientContext.types @@ -99,6 +99,11 @@ export declare function __assign(t: any, ...sources: any[]): any; >t : any >sources : any[] +export declare function __rest(t: any, propertyNames: string[]): any; +>__rest : (t: any, propertyNames: string[]) => any +>t : any +>propertyNames : string[] + export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; >__decorate : (decorators: Function[], target: any, key?: string | symbol, desc?: any) => any >decorators : Function[] @@ -129,3 +134,14 @@ export declare function __awaiter(thisArg: any, _arguments: any, P: Function, ge >generator : Function >Function : Function +export declare function __generator(thisArg: any, body: Function): any; +>__generator : (thisArg: any, body: Function) => any +>thisArg : any +>body : Function +>Function : Function + +export declare function __exportStar(m: any, exports: any): void; +>__exportStar : (m: any, exports: any) => void +>m : any +>exports : any + diff --git a/tests/baselines/reference/importHelpersNoHelpers.errors.txt b/tests/baselines/reference/importHelpersNoHelpers.errors.txt index a0c089a9ddc..4034fbc649b 100644 --- a/tests/baselines/reference/importHelpersNoHelpers.errors.txt +++ b/tests/baselines/reference/importHelpersNoHelpers.errors.txt @@ -1,12 +1,16 @@ -tests/cases/compiler/external.ts(2,16): error TS2343: This syntax requires an imported helper named '__extends', but module 'tslib' has no exported member '__extends'. -tests/cases/compiler/external.ts(6,1): error TS2343: This syntax requires an imported helper named '__decorate', but module 'tslib' has no exported member '__decorate'. -tests/cases/compiler/external.ts(6,1): error TS2343: This syntax requires an imported helper named '__metadata', but module 'tslib' has no exported member '__metadata'. -tests/cases/compiler/external.ts(8,12): error TS2343: This syntax requires an imported helper named '__param', but module 'tslib' has no exported member '__param'. -tests/cases/compiler/external.ts(13,13): error TS2343: This syntax requires an imported helper named '__assign', but module 'tslib' has no exported member '__assign'. -tests/cases/compiler/external.ts(14,12): error TS2343: This syntax requires an imported helper named '__rest', but module 'tslib' has no exported member '__rest'. +tests/cases/compiler/external.ts(1,1): error TS2343: This syntax requires an imported helper named '__exportStar', but module 'tslib' has no exported member '__exportStar'. +tests/cases/compiler/external.ts(3,16): error TS2343: This syntax requires an imported helper named '__extends', but module 'tslib' has no exported member '__extends'. +tests/cases/compiler/external.ts(7,1): error TS2343: This syntax requires an imported helper named '__decorate', but module 'tslib' has no exported member '__decorate'. +tests/cases/compiler/external.ts(7,1): error TS2343: This syntax requires an imported helper named '__metadata', but module 'tslib' has no exported member '__metadata'. +tests/cases/compiler/external.ts(9,12): error TS2343: This syntax requires an imported helper named '__param', but module 'tslib' has no exported member '__param'. +tests/cases/compiler/external.ts(14,13): error TS2343: This syntax requires an imported helper named '__assign', but module 'tslib' has no exported member '__assign'. +tests/cases/compiler/external.ts(15,12): error TS2343: This syntax requires an imported helper named '__rest', but module 'tslib' has no exported member '__rest'. -==== tests/cases/compiler/external.ts (6 errors) ==== +==== tests/cases/compiler/external.ts (7 errors) ==== + export * from "./other"; + ~~~~~~~~~~~~~~~~~~~~~~~~ +!!! error TS2343: This syntax requires an imported helper named '__exportStar', but module 'tslib' has no exported member '__exportStar'. export class A { } export class B extends A { } ~~~~~~~~~ @@ -34,6 +38,9 @@ tests/cases/compiler/external.ts(14,12): error TS2343: This syntax requires an i ~ !!! error TS2343: This syntax requires an imported helper named '__rest', but module 'tslib' has no exported member '__rest'. +==== tests/cases/compiler/other.ts (0 errors) ==== + export const x = 1; + ==== tests/cases/compiler/script.ts (0 errors) ==== class A { } class B extends A { } diff --git a/tests/baselines/reference/importHelpersNoHelpers.js b/tests/baselines/reference/importHelpersNoHelpers.js index 560e16d1cca..ae709c086ba 100644 --- a/tests/baselines/reference/importHelpersNoHelpers.js +++ b/tests/baselines/reference/importHelpersNoHelpers.js @@ -1,6 +1,7 @@ //// [tests/cases/compiler/importHelpersNoHelpers.ts] //// //// [external.ts] +export * from "./other"; export class A { } export class B extends A { } @@ -16,6 +17,9 @@ const o = { a: 1 }; const y = { ...o }; const { ...x } = y; +//// [other.ts] +export const x = 1; + //// [script.ts] class A { } class B extends A { } @@ -32,9 +36,13 @@ class C { export {} +//// [other.js] +"use strict"; +exports.x = 1; //// [external.js] "use strict"; var tslib_1 = require("tslib"); +tslib_1.__exportStar(require("./other"), exports); var A = (function () { function A() { } diff --git a/tests/baselines/reference/importHelpersSystem.js b/tests/baselines/reference/importHelpersSystem.js index 20f2d299c52..43556ae6b76 100644 --- a/tests/baselines/reference/importHelpersSystem.js +++ b/tests/baselines/reference/importHelpersSystem.js @@ -5,6 +5,7 @@ export class A { } //// [b.ts] import { A } from "./a"; +export * from "./a"; export class B extends A { } //// [tslib.d.ts] @@ -38,6 +39,16 @@ System.register(["tslib", "./a"], function (exports_1, context_1) { "use strict"; var __moduleName = context_1 && context_1.id; var tslib_1, a_1, B; + var exportedNames_1 = { + "B": true + }; + function exportStar_1(m) { + var exports = {}; + for (var n in m) { + if (n !== "default" && !exportedNames_1.hasOwnProperty(n)) exports[n] = m[n]; + } + exports_1(exports); + } return { setters: [ function (tslib_1_1) { @@ -45,6 +56,7 @@ System.register(["tslib", "./a"], function (exports_1, context_1) { }, function (a_1_1) { a_1 = a_1_1; + exportStar_1(a_1_1); } ], execute: function () { diff --git a/tests/baselines/reference/importHelpersSystem.symbols b/tests/baselines/reference/importHelpersSystem.symbols index 47528a8bba6..d7d588a67ef 100644 --- a/tests/baselines/reference/importHelpersSystem.symbols +++ b/tests/baselines/reference/importHelpersSystem.symbols @@ -6,8 +6,9 @@ export class A { } import { A } from "./a"; >A : Symbol(A, Decl(b.ts, 0, 8)) +export * from "./a"; export class B extends A { } ->B : Symbol(B, Decl(b.ts, 0, 24)) +>B : Symbol(B, Decl(b.ts, 1, 20)) >A : Symbol(A, Decl(b.ts, 0, 8)) === tests/cases/compiler/tslib.d.ts === diff --git a/tests/baselines/reference/importHelpersSystem.types b/tests/baselines/reference/importHelpersSystem.types index 9ff756ffaab..de9ef3e9446 100644 --- a/tests/baselines/reference/importHelpersSystem.types +++ b/tests/baselines/reference/importHelpersSystem.types @@ -6,6 +6,7 @@ export class A { } import { A } from "./a"; >A : typeof A +export * from "./a"; export class B extends A { } >B : B >A : A diff --git a/tests/cases/compiler/importHelpersAmd.ts b/tests/cases/compiler/importHelpersAmd.ts index bef63328d52..38b02424b96 100644 --- a/tests/cases/compiler/importHelpersAmd.ts +++ b/tests/cases/compiler/importHelpersAmd.ts @@ -6,12 +6,16 @@ export class A { } // @filename: b.ts import { A } from "./a"; +export * from "./a"; export class B extends A { } // @filename: tslib.d.ts export declare function __extends(d: Function, b: Function): void; export declare function __assign(t: any, ...sources: any[]): any; +export declare function __rest(t: any, propertyNames: string[]): any; export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; export declare function __param(paramIndex: number, decorator: Function): Function; export declare function __metadata(metadataKey: any, metadataValue: any): Function; export declare function __awaiter(thisArg: any, _arguments: any, P: Function, generator: Function): any; +export declare function __generator(thisArg: any, body: Function): any; +export declare function __exportStar(m: any, exports: any): void; \ No newline at end of file diff --git a/tests/cases/compiler/importHelpersInAmbientContext.ts b/tests/cases/compiler/importHelpersInAmbientContext.ts index 8bfa5b25404..340961802a3 100644 --- a/tests/cases/compiler/importHelpersInAmbientContext.ts +++ b/tests/cases/compiler/importHelpersInAmbientContext.ts @@ -49,7 +49,10 @@ declare namespace N { // @filename: tslib.d.ts export declare function __extends(d: Function, b: Function): void; export declare function __assign(t: any, ...sources: any[]): any; +export declare function __rest(t: any, propertyNames: string[]): any; export declare function __decorate(decorators: Function[], target: any, key?: string | symbol, desc?: any): any; export declare function __param(paramIndex: number, decorator: Function): Function; export declare function __metadata(metadataKey: any, metadataValue: any): Function; export declare function __awaiter(thisArg: any, _arguments: any, P: Function, generator: Function): any; +export declare function __generator(thisArg: any, body: Function): any; +export declare function __exportStar(m: any, exports: any): void; \ No newline at end of file diff --git a/tests/cases/compiler/importHelpersNoHelpers.ts b/tests/cases/compiler/importHelpersNoHelpers.ts index 4ab48aba7df..787058890a2 100644 --- a/tests/cases/compiler/importHelpersNoHelpers.ts +++ b/tests/cases/compiler/importHelpersNoHelpers.ts @@ -5,6 +5,7 @@ // @experimentalDecorators: true // @emitDecoratorMetadata: true // @filename: external.ts +export * from "./other"; export class A { } export class B extends A { } @@ -20,6 +21,9 @@ const o = { a: 1 }; const y = { ...o }; const { ...x } = y; +// @filename: other.ts +export const x = 1; + // @filename: script.ts class A { } class B extends A { } diff --git a/tests/cases/compiler/importHelpersSystem.ts b/tests/cases/compiler/importHelpersSystem.ts index 5b6081f17f3..87e62785708 100644 --- a/tests/cases/compiler/importHelpersSystem.ts +++ b/tests/cases/compiler/importHelpersSystem.ts @@ -6,6 +6,7 @@ export class A { } // @filename: b.ts import { A } from "./a"; +export * from "./a"; export class B extends A { } // @filename: tslib.d.ts From 471e680ef087ce789698162e7c0ee74d585b8859 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 6 Jun 2017 18:10:00 -0700 Subject: [PATCH 02/14] Better types from jsdoc param tags --- src/compiler/checker.ts | 79 ++++--- .../typeFromParamTagForFunction.symbols | 184 +++++++++++++++ .../typeFromParamTagForFunction.types | 223 ++++++++++++++++++ .../salsa/typeFromParamTagForFunction.ts | 92 ++++++++ 4 files changed, 541 insertions(+), 37 deletions(-) create mode 100644 tests/baselines/reference/typeFromParamTagForFunction.symbols create mode 100644 tests/baselines/reference/typeFromParamTagForFunction.types create mode 100644 tests/cases/conformance/salsa/typeFromParamTagForFunction.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7427d921cf2..c7e70167997 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6814,37 +6814,53 @@ namespace ts { return undefined; } - function resolveTypeReferenceName(typeReferenceName: EntityNameExpression | EntityName) { + function resolveTypeReferenceName(node: TypeReferenceType, typeReferenceName: EntityNameExpression | EntityName) { if (!typeReferenceName) { return unknownSymbol; } - return resolveEntityName(typeReferenceName, SymbolFlags.Type) || unknownSymbol; + const meaning = node.kind === SyntaxKind.JSDocTypeReference + ? SymbolFlags.Type | SymbolFlags.Value + : SymbolFlags.Type; + + return resolveEntityName(typeReferenceName, meaning) || unknownSymbol; } function getTypeReferenceType(node: TypeReferenceType, symbol: Symbol) { const typeArguments = typeArgumentsFromTypeReferenceNode(node); // Do unconditionally so we mark type arguments as referenced. + let fallbackType: Type = unknownType; + while (true) { + if (symbol === unknownSymbol) { + return fallbackType; + } - if (symbol === unknownSymbol) { - return unknownType; + if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) { + return getTypeFromClassOrInterfaceReference(node, symbol, typeArguments); + } + + if (symbol.flags & SymbolFlags.TypeAlias) { + return getTypeFromTypeAliasReference(node, symbol, typeArguments); + } + + if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) { + // A JSDocTypeReference may have resolved to a value (as opposed to a type). If + // the value has a construct signature, we use the return type of the construct + // signature as the type; otherwise, the type of this reference is just the type + // of the value we resolved to. + if (symbol.flags & SymbolFlags.Function && (symbol.members || getJSDocClassTag(symbol.valueDeclaration))) { + return getInferredClassType(symbol); + } + + fallbackType = getTypeOfSymbol(symbol); + + // Try to use the symbol of the type (if present) to get a better type on the + // next pass. + symbol = fallbackType.symbol || unknownSymbol; + continue; + } + + return getTypeFromNonGenericTypeReference(node, symbol); } - - if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) { - return getTypeFromClassOrInterfaceReference(node, symbol, typeArguments); - } - - if (symbol.flags & SymbolFlags.TypeAlias) { - return getTypeFromTypeAliasReference(node, symbol, typeArguments); - } - - if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) { - // A JSDocTypeReference may have resolved to a value (as opposed to a type). In - // that case, the type of this reference is just the type of the value we resolved - // to. - return getTypeOfSymbol(symbol); - } - - return getTypeFromNonGenericTypeReference(node, symbol); } function getPrimitiveTypeFromJSDocTypeReference(node: JSDocTypeReference): Type { @@ -6888,21 +6904,10 @@ namespace ts { let symbol: Symbol; let type: Type; if (node.kind === SyntaxKind.JSDocTypeReference) { - type = getPrimitiveTypeFromJSDocTypeReference(node); - if (!type) { - const typeReferenceName = getTypeReferenceName(node); - symbol = resolveTypeReferenceName(typeReferenceName); - type = getTypeReferenceType(node, symbol); - } + type = getPrimitiveTypeFromJSDocTypeReference(node); } - else { - // We only support expressions that are simple qualified names. For other expressions this produces undefined. - const typeNameOrExpression: EntityNameOrEntityNameExpression = node.kind === SyntaxKind.TypeReference - ? (node).typeName - : isEntityNameExpression((node).expression) - ? (node).expression - : undefined; - symbol = typeNameOrExpression && resolveEntityName(typeNameOrExpression, SymbolFlags.Type) || unknownSymbol; + if (!type) { + symbol = resolveTypeReferenceName(node, getTypeReferenceName(node)); type = getTypeReferenceType(node, symbol); } // Cache both the resolved symbol and the resolved type. The resolved symbol is needed in when we check the @@ -19367,8 +19372,8 @@ namespace ts { function checkFunctionDeclaration(node: FunctionDeclaration): void { if (produceDiagnostics) { - checkFunctionOrMethodDeclaration(node) || checkGrammarForGenerator(node); - + checkFunctionOrMethodDeclaration(node); + checkGrammarForGenerator(node); checkCollisionWithCapturedSuperVariable(node, node.name); checkCollisionWithCapturedThisVariable(node, node.name); checkCollisionWithCapturedNewTargetVariable(node, node.name); diff --git a/tests/baselines/reference/typeFromParamTagForFunction.symbols b/tests/baselines/reference/typeFromParamTagForFunction.symbols new file mode 100644 index 00000000000..0df0dfdc206 --- /dev/null +++ b/tests/baselines/reference/typeFromParamTagForFunction.symbols @@ -0,0 +1,184 @@ +=== tests/cases/conformance/salsa/node.d.ts === +declare function require(id: string): any; +>require : Symbol(require, Decl(node.d.ts, 0, 0)) +>id : Symbol(id, Decl(node.d.ts, 0, 25)) + +declare var module: any, exports: any; +>module : Symbol(module, Decl(node.d.ts, 1, 11)) +>exports : Symbol(exports, Decl(node.d.ts, 1, 24)) + +=== tests/cases/conformance/salsa/a-ext.js === +exports.A = function () { +>exports : Symbol(A, Decl(a-ext.js, 0, 0)) +>A : Symbol(A, Decl(a-ext.js, 0, 0)) + + this.x = 1; +>x : Symbol((Anonymous function).x, Decl(a-ext.js, 0, 25)) + +}; + +=== tests/cases/conformance/salsa/a.js === +const { A } = require("./a-ext"); +>A : Symbol(A, Decl(a.js, 0, 7)) +>require : Symbol(require, Decl(node.d.ts, 0, 0)) +>"./a-ext" : Symbol("tests/cases/conformance/salsa/a-ext", Decl(a-ext.js, 0, 0)) + +/** @param {A} p */ +function a(p) { p.x; } +>a : Symbol(a, Decl(a.js, 0, 33)) +>p : Symbol(p, Decl(a.js, 3, 11)) +>p.x : Symbol((Anonymous function).x, Decl(a-ext.js, 0, 25)) +>p : Symbol(p, Decl(a.js, 3, 11)) +>x : Symbol((Anonymous function).x, Decl(a-ext.js, 0, 25)) + +=== tests/cases/conformance/salsa/b-ext.js === +exports.B = class { +>exports : Symbol(B, Decl(b-ext.js, 0, 0)) +>B : Symbol(B, Decl(b-ext.js, 0, 0)) + + constructor() { + this.x = 1; +>this.x : Symbol((Anonymous class).x, Decl(b-ext.js, 1, 19)) +>this : Symbol((Anonymous class), Decl(b-ext.js, 0, 11)) +>x : Symbol((Anonymous class).x, Decl(b-ext.js, 1, 19)) + } +}; + +=== tests/cases/conformance/salsa/b.js === +const { B } = require("./b-ext"); +>B : Symbol(B, Decl(b.js, 0, 7)) +>require : Symbol(require, Decl(node.d.ts, 0, 0)) +>"./b-ext" : Symbol("tests/cases/conformance/salsa/b-ext", Decl(b-ext.js, 0, 0)) + +/** @param {B} p */ +function b(p) { p.x; } +>b : Symbol(b, Decl(b.js, 0, 33)) +>p : Symbol(p, Decl(b.js, 3, 11)) +>p.x : Symbol((Anonymous class).x, Decl(b-ext.js, 1, 19)) +>p : Symbol(p, Decl(b.js, 3, 11)) +>x : Symbol((Anonymous class).x, Decl(b-ext.js, 1, 19)) + +=== tests/cases/conformance/salsa/c-ext.js === +export function C() { +>C : Symbol(C, Decl(c-ext.js, 0, 0)) + + this.x = 1; +>x : Symbol(C.x, Decl(c-ext.js, 0, 21)) +} + +=== tests/cases/conformance/salsa/c.js === +const { C } = require("./c-ext"); +>C : Symbol(C, Decl(c.js, 0, 7)) +>require : Symbol(require, Decl(node.d.ts, 0, 0)) +>"./c-ext" : Symbol("tests/cases/conformance/salsa/c-ext", Decl(c-ext.js, 0, 0)) + +/** @param {C} p */ +function c(p) { p.x; } +>c : Symbol(c, Decl(c.js, 0, 33)) +>p : Symbol(p, Decl(c.js, 3, 11)) +>p.x : Symbol(C.x, Decl(c-ext.js, 0, 21)) +>p : Symbol(p, Decl(c.js, 3, 11)) +>x : Symbol(C.x, Decl(c-ext.js, 0, 21)) + +=== tests/cases/conformance/salsa/d-ext.js === +export var D = function() { +>D : Symbol(D, Decl(d-ext.js, 0, 10)) + + this.x = 1; +>x : Symbol(D.x, Decl(d-ext.js, 0, 27)) + +}; + +=== tests/cases/conformance/salsa/d.js === +const { D } = require("./d-ext"); +>D : Symbol(D, Decl(d.js, 0, 7)) +>require : Symbol(require, Decl(node.d.ts, 0, 0)) +>"./d-ext" : Symbol("tests/cases/conformance/salsa/d-ext", Decl(d-ext.js, 0, 0)) + +/** @param {D} p */ +function d(p) { p.x; } +>d : Symbol(d, Decl(d.js, 0, 33)) +>p : Symbol(p, Decl(d.js, 3, 11)) +>p.x : Symbol(D.x, Decl(d-ext.js, 0, 27)) +>p : Symbol(p, Decl(d.js, 3, 11)) +>x : Symbol(D.x, Decl(d-ext.js, 0, 27)) + +=== tests/cases/conformance/salsa/e-ext.js === +export class E { +>E : Symbol(E, Decl(e-ext.js, 0, 0)) + + constructor() { + this.x = 1; +>this.x : Symbol(E.x, Decl(e-ext.js, 1, 19)) +>this : Symbol(E, Decl(e-ext.js, 0, 0)) +>x : Symbol(E.x, Decl(e-ext.js, 1, 19)) + } +} + +=== tests/cases/conformance/salsa/e.js === +const { E } = require("./e-ext"); +>E : Symbol(E, Decl(e.js, 0, 7)) +>require : Symbol(require, Decl(node.d.ts, 0, 0)) +>"./e-ext" : Symbol("tests/cases/conformance/salsa/e-ext", Decl(e-ext.js, 0, 0)) + +/** @param {E} p */ +function e(p) { p.x; } +>e : Symbol(e, Decl(e.js, 0, 33)) +>p : Symbol(p, Decl(e.js, 3, 11)) +>p.x : Symbol(E.x, Decl(e-ext.js, 1, 19)) +>p : Symbol(p, Decl(e.js, 3, 11)) +>x : Symbol(E.x, Decl(e-ext.js, 1, 19)) + +=== tests/cases/conformance/salsa/f.js === +var F = function () { +>F : Symbol(F, Decl(f.js, 0, 3)) + + this.x = 1; +>x : Symbol(F.x, Decl(f.js, 0, 21)) + +}; + +/** @param {F} p */ +function f(p) { p.x; } +>f : Symbol(f, Decl(f.js, 2, 2)) +>p : Symbol(p, Decl(f.js, 5, 11)) +>p.x : Symbol(F.x, Decl(f.js, 0, 21)) +>p : Symbol(p, Decl(f.js, 5, 11)) +>x : Symbol(F.x, Decl(f.js, 0, 21)) + +=== tests/cases/conformance/salsa/g.js === +function G() { +>G : Symbol(G, Decl(g.js, 0, 0)) + + this.x = 1; +>x : Symbol(G.x, Decl(g.js, 0, 14)) +} + +/** @param {G} p */ +function g(p) { p.x; } +>g : Symbol(g, Decl(g.js, 2, 1)) +>p : Symbol(p, Decl(g.js, 5, 11)) +>p.x : Symbol(G.x, Decl(g.js, 0, 14)) +>p : Symbol(p, Decl(g.js, 5, 11)) +>x : Symbol(G.x, Decl(g.js, 0, 14)) + +=== tests/cases/conformance/salsa/h.js === +class H { +>H : Symbol(H, Decl(h.js, 0, 0)) + + constructor() { + this.x = 1; +>this.x : Symbol(H.x, Decl(h.js, 1, 19)) +>this : Symbol(H, Decl(h.js, 0, 0)) +>x : Symbol(H.x, Decl(h.js, 1, 19)) + } +} + +/** @param {H} p */ +function h(p) { p.x; } +>h : Symbol(h, Decl(h.js, 4, 1)) +>p : Symbol(p, Decl(h.js, 7, 11)) +>p.x : Symbol(H.x, Decl(h.js, 1, 19)) +>p : Symbol(p, Decl(h.js, 7, 11)) +>x : Symbol(H.x, Decl(h.js, 1, 19)) + diff --git a/tests/baselines/reference/typeFromParamTagForFunction.types b/tests/baselines/reference/typeFromParamTagForFunction.types new file mode 100644 index 00000000000..c1e16ddb33d --- /dev/null +++ b/tests/baselines/reference/typeFromParamTagForFunction.types @@ -0,0 +1,223 @@ +=== tests/cases/conformance/salsa/node.d.ts === +declare function require(id: string): any; +>require : (id: string) => any +>id : string + +declare var module: any, exports: any; +>module : any +>exports : any + +=== tests/cases/conformance/salsa/a-ext.js === +exports.A = function () { +>exports.A = function () { this.x = 1;} : () => void +>exports.A : any +>exports : any +>A : any +>function () { this.x = 1;} : () => void + + this.x = 1; +>this.x = 1 : 1 +>this.x : any +>this : any +>x : any +>1 : 1 + +}; + +=== tests/cases/conformance/salsa/a.js === +const { A } = require("./a-ext"); +>A : () => void +>require("./a-ext") : typeof "tests/cases/conformance/salsa/a-ext" +>require : (id: string) => any +>"./a-ext" : "./a-ext" + +/** @param {A} p */ +function a(p) { p.x; } +>a : (p: { x: number; }) => void +>p : { x: number; } +>p.x : number +>p : { x: number; } +>x : number + +=== tests/cases/conformance/salsa/b-ext.js === +exports.B = class { +>exports.B = class { constructor() { this.x = 1; }} : typeof (Anonymous class) +>exports.B : any +>exports : any +>B : any +>class { constructor() { this.x = 1; }} : typeof (Anonymous class) + + constructor() { + this.x = 1; +>this.x = 1 : 1 +>this.x : number +>this : this +>x : number +>1 : 1 + } +}; + +=== tests/cases/conformance/salsa/b.js === +const { B } = require("./b-ext"); +>B : typeof (Anonymous class) +>require("./b-ext") : typeof "tests/cases/conformance/salsa/b-ext" +>require : (id: string) => any +>"./b-ext" : "./b-ext" + +/** @param {B} p */ +function b(p) { p.x; } +>b : (p: (Anonymous class)) => void +>p : (Anonymous class) +>p.x : number +>p : (Anonymous class) +>x : number + +=== tests/cases/conformance/salsa/c-ext.js === +export function C() { +>C : () => void + + this.x = 1; +>this.x = 1 : 1 +>this.x : any +>this : any +>x : any +>1 : 1 +} + +=== tests/cases/conformance/salsa/c.js === +const { C } = require("./c-ext"); +>C : () => void +>require("./c-ext") : typeof "tests/cases/conformance/salsa/c-ext" +>require : (id: string) => any +>"./c-ext" : "./c-ext" + +/** @param {C} p */ +function c(p) { p.x; } +>c : (p: { x: number; }) => void +>p : { x: number; } +>p.x : number +>p : { x: number; } +>x : number + +=== tests/cases/conformance/salsa/d-ext.js === +export var D = function() { +>D : () => void +>function() { this.x = 1;} : () => void + + this.x = 1; +>this.x = 1 : 1 +>this.x : any +>this : any +>x : any +>1 : 1 + +}; + +=== tests/cases/conformance/salsa/d.js === +const { D } = require("./d-ext"); +>D : () => void +>require("./d-ext") : typeof "tests/cases/conformance/salsa/d-ext" +>require : (id: string) => any +>"./d-ext" : "./d-ext" + +/** @param {D} p */ +function d(p) { p.x; } +>d : (p: { x: number; }) => void +>p : { x: number; } +>p.x : number +>p : { x: number; } +>x : number + +=== tests/cases/conformance/salsa/e-ext.js === +export class E { +>E : E + + constructor() { + this.x = 1; +>this.x = 1 : 1 +>this.x : number +>this : this +>x : number +>1 : 1 + } +} + +=== tests/cases/conformance/salsa/e.js === +const { E } = require("./e-ext"); +>E : typeof E +>require("./e-ext") : typeof "tests/cases/conformance/salsa/e-ext" +>require : (id: string) => any +>"./e-ext" : "./e-ext" + +/** @param {E} p */ +function e(p) { p.x; } +>e : (p: E) => void +>p : E +>p.x : number +>p : E +>x : number + +=== tests/cases/conformance/salsa/f.js === +var F = function () { +>F : () => void +>function () { this.x = 1;} : () => void + + this.x = 1; +>this.x = 1 : 1 +>this.x : any +>this : any +>x : any +>1 : 1 + +}; + +/** @param {F} p */ +function f(p) { p.x; } +>f : (p: { x: number; }) => void +>p : { x: number; } +>p.x : number +>p : { x: number; } +>x : number + +=== tests/cases/conformance/salsa/g.js === +function G() { +>G : () => void + + this.x = 1; +>this.x = 1 : 1 +>this.x : any +>this : any +>x : any +>1 : 1 +} + +/** @param {G} p */ +function g(p) { p.x; } +>g : (p: { x: number; }) => void +>p : { x: number; } +>p.x : number +>p : { x: number; } +>x : number + +=== tests/cases/conformance/salsa/h.js === +class H { +>H : H + + constructor() { + this.x = 1; +>this.x = 1 : 1 +>this.x : number +>this : this +>x : number +>1 : 1 + } +} + +/** @param {H} p */ +function h(p) { p.x; } +>h : (p: H) => void +>p : H +>p.x : number +>p : H +>x : number + diff --git a/tests/cases/conformance/salsa/typeFromParamTagForFunction.ts b/tests/cases/conformance/salsa/typeFromParamTagForFunction.ts new file mode 100644 index 00000000000..7adf236657f --- /dev/null +++ b/tests/cases/conformance/salsa/typeFromParamTagForFunction.ts @@ -0,0 +1,92 @@ +// @allowJs: true +// @checkJs: true +// @noEmit: true +// @module: commonjs +// @filename: node.d.ts +declare function require(id: string): any; +declare var module: any, exports: any; + +// @filename: a-ext.js +exports.A = function () { + this.x = 1; +}; + +// @filename: a.js +const { A } = require("./a-ext"); + +/** @param {A} p */ +function a(p) { p.x; } + +// @filename: b-ext.js +exports.B = class { + constructor() { + this.x = 1; + } +}; + +// @filename: b.js +const { B } = require("./b-ext"); + +/** @param {B} p */ +function b(p) { p.x; } + +// @filename: c-ext.js +export function C() { + this.x = 1; +} + +// @filename: c.js +const { C } = require("./c-ext"); + +/** @param {C} p */ +function c(p) { p.x; } + +// @filename: d-ext.js +export var D = function() { + this.x = 1; +}; + +// @filename: d.js +const { D } = require("./d-ext"); + +/** @param {D} p */ +function d(p) { p.x; } + +// @filename: e-ext.js +export class E { + constructor() { + this.x = 1; + } +} + +// @filename: e.js +const { E } = require("./e-ext"); + +/** @param {E} p */ +function e(p) { p.x; } + +// @filename: f.js +var F = function () { + this.x = 1; +}; + +/** @param {F} p */ +function f(p) { p.x; } + +// @filename: g.js +function G() { + this.x = 1; +} + +/** @param {G} p */ +function g(p) { p.x; } + +// @filename: h.js +class H { + constructor() { + this.x = 1; + } +} + +/** @param {H} p */ +function h(p) { p.x; } \ No newline at end of file From 9f9e20c5a354fc27e59fb5aa4f7adc4ac11a2163 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 6 Jun 2017 18:25:55 -0700 Subject: [PATCH 03/14] Limit getTypeReferenceType to two passes --- src/compiler/checker.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c7e70167997..c160b4f97ab 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6828,6 +6828,7 @@ namespace ts { function getTypeReferenceType(node: TypeReferenceType, symbol: Symbol) { const typeArguments = typeArgumentsFromTypeReferenceNode(node); // Do unconditionally so we mark type arguments as referenced. + let secondPass = true; let fallbackType: Type = unknownType; while (true) { if (symbol === unknownSymbol) { @@ -6844,18 +6845,22 @@ namespace ts { if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) { // A JSDocTypeReference may have resolved to a value (as opposed to a type). If - // the value has a construct signature, we use the return type of the construct - // signature as the type; otherwise, the type of this reference is just the type - // of the value we resolved to. + // the symbol is a constructor function, return the inferred class type; otherwise, + // the type of this reference is just the type of the value we resolved to. if (symbol.flags & SymbolFlags.Function && (symbol.members || getJSDocClassTag(symbol.valueDeclaration))) { return getInferredClassType(symbol); } - fallbackType = getTypeOfSymbol(symbol); + // Stop if this is the second pass + if (secondPass) { + return fallbackType; + } // Try to use the symbol of the type (if present) to get a better type on the - // next pass. + // second pass. + fallbackType = getTypeOfSymbol(symbol); symbol = fallbackType.symbol || unknownSymbol; + secondPass = true; continue; } From 7304a738a0564b6589888d8acec3b99941d447ec Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 7 Jun 2017 10:58:51 -0700 Subject: [PATCH 04/14] Fix typo in getTypeReferenceType --- src/compiler/checker.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c160b4f97ab..3b3551c1af8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6828,7 +6828,7 @@ namespace ts { function getTypeReferenceType(node: TypeReferenceType, symbol: Symbol) { const typeArguments = typeArgumentsFromTypeReferenceNode(node); // Do unconditionally so we mark type arguments as referenced. - let secondPass = true; + let secondPass = false; let fallbackType: Type = unknownType; while (true) { if (symbol === unknownSymbol) { From 4e927bdbd4486e8e5ca965aba94082ef2699c3d4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 11:24:19 -0700 Subject: [PATCH 05/14] Create js-inferred rest params in getSignatureOfDeclaration Previously they were created too late, in resolveCall, via mutation. The mutation of the signature caused bug #16139 because recursion detection in type checking didn't work. --- src/compiler/checker.ts | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7427d921cf2..464abee2842 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2825,6 +2825,17 @@ namespace ts { function symbolToParameterDeclaration(parameterSymbol: Symbol, context: NodeBuilderContext): ParameterDeclaration { const parameterDeclaration = getDeclarationOfKind(parameterSymbol, SyntaxKind.Parameter); + if (isTransientSymbol(parameterSymbol) && parameterSymbol.isRestParameter) { + // special-case synthetic rest parameters in JS files + return createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + parameterSymbol.isRestParameter ? createToken(SyntaxKind.DotDotDotToken) : undefined, + "args", + /*questionToken*/ undefined, + typeToTypeNodeHelper(anyArrayType, context), + /*initializer*/ undefined); + } const modifiers = parameterDeclaration.modifiers && parameterDeclaration.modifiers.map(getSynthesizedClone); const dotDotDotToken = isRestParameter(parameterDeclaration) ? createToken(SyntaxKind.DotDotDotToken) : undefined; const name = parameterDeclaration.name ? @@ -6384,8 +6395,17 @@ namespace ts { const typePredicate = declaration.type && declaration.type.kind === SyntaxKind.TypePredicate ? createTypePredicateFromTypePredicateNode(declaration.type as TypePredicateNode) : undefined; + // JS functions get a free rest parameter if they reference `arguments` + let hasRestLikeParameter = hasRestParameter(declaration); + if (!hasRestLikeParameter && isInJavaScriptFile(declaration) && !hasJSDocParameterTags(declaration) && containsArgumentsReference(declaration)) { + hasRestLikeParameter = true; + const syntheticArgsSymbol = createSymbol(SymbolFlags.Variable, "args"); + syntheticArgsSymbol.type = anyArrayType; + syntheticArgsSymbol.isRestParameter = true; + parameters.push(syntheticArgsSymbol); + } - links.resolvedSignature = createSignature(declaration, typeParameters, thisParameter, parameters, returnType, typePredicate, minArgumentCount, hasRestParameter(declaration), hasLiteralTypes); + links.resolvedSignature = createSignature(declaration, typeParameters, thisParameter, parameters, returnType, typePredicate, minArgumentCount, hasRestLikeParameter, hasLiteralTypes); } return links.resolvedSignature; } @@ -6420,14 +6440,14 @@ namespace ts { } } - function containsArgumentsReference(declaration: FunctionLikeDeclaration): boolean { + function containsArgumentsReference(declaration: SignatureDeclaration): boolean { const links = getNodeLinks(declaration); if (links.containsArgumentsReference === undefined) { if (links.flags & NodeCheckFlags.CaptureArguments) { links.containsArgumentsReference = true; } else { - links.containsArgumentsReference = traverse(declaration.body); + links.containsArgumentsReference = traverse((declaration as FunctionLikeDeclaration).body); } } return links.containsArgumentsReference; @@ -15482,21 +15502,6 @@ namespace ts { } } - if (signatures.length === 1) { - const declaration = signatures[0].declaration; - if (declaration && isInJavaScriptFile(declaration) && !hasJSDocParameterTags(declaration)) { - if (containsArgumentsReference(declaration)) { - const signatureWithRest = cloneSignature(signatures[0]); - const syntheticArgsSymbol = createSymbol(SymbolFlags.Variable, "args"); - syntheticArgsSymbol.type = anyArrayType; - syntheticArgsSymbol.isRestParameter = true; - signatureWithRest.parameters = concatenate(signatureWithRest.parameters, [syntheticArgsSymbol]); - signatureWithRest.hasRestParameter = true; - signatures = [signatureWithRest]; - } - } - } - const candidates = candidatesOutArray || []; // reorderCandidates fills up the candidates array directly reorderCandidates(signatures, candidates); From f5f2d243720a73c177c07d41cec3f82f99ce5cbc Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 11:28:26 -0700 Subject: [PATCH 06/14] Add tests for JS-inferred rest parameters --- .../argumentsObjectCreatesRestForJs.ts | 20 ++++++++++++++++++ .../jsSelfReferencingArgumentsFunction.ts | 8 +++++++ .../signatureHelpCallExpressionJs.ts | 21 ++++++++++++++----- 3 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 tests/cases/compiler/argumentsObjectCreatesRestForJs.ts create mode 100644 tests/cases/compiler/jsSelfReferencingArgumentsFunction.ts diff --git a/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts b/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts new file mode 100644 index 00000000000..4c3ca335d1d --- /dev/null +++ b/tests/cases/compiler/argumentsObjectCreatesRestForJs.ts @@ -0,0 +1,20 @@ +// @checkJs: true +// @allowJs: true +// @Filename: main.js +// @noemit: true +function allRest() { arguments; } +allRest(); +allRest(1, 2, 3); +function someRest(x, y) { arguments; } +someRest(); // x and y are still optional because they are in a JS file +someRest(1, 2, 3); + +/** + * @param {number} x - a thing + */ +function jsdocced(x) { arguments; } +jsdocced(1); + +function dontDoubleRest(x, ...y) { arguments; } +dontDoubleRest(1, 2, 3); + diff --git a/tests/cases/compiler/jsSelfReferencingArgumentsFunction.ts b/tests/cases/compiler/jsSelfReferencingArgumentsFunction.ts new file mode 100644 index 00000000000..386b88c9cd1 --- /dev/null +++ b/tests/cases/compiler/jsSelfReferencingArgumentsFunction.ts @@ -0,0 +1,8 @@ +// @Filename: foo.js +// @noEmit: true +// @allowJs: true +// Test #16139 +function Foo() { + arguments; + return new Foo(); +} diff --git a/tests/cases/fourslash/signatureHelpCallExpressionJs.ts b/tests/cases/fourslash/signatureHelpCallExpressionJs.ts index e425196bd07..17045a74717 100644 --- a/tests/cases/fourslash/signatureHelpCallExpressionJs.ts +++ b/tests/cases/fourslash/signatureHelpCallExpressionJs.ts @@ -4,14 +4,25 @@ // @allowJs: true // @Filename: main.js -////function fnTest() { arguments; } -////fnTest(/*1*/); -////fnTest(1, 2, 3); +////function allOptional() { arguments; } +////allOptional(/*1*/); +////allOptional(1, 2, 3); +////function someOptional(x, y) { arguments; } +////someOptional(/*2*/); +////someOptional(1, 2, 3); +////someOptional(); // no error here; x and y are optional in JS goTo.marker('1'); verify.signatureHelpCountIs(1); verify.currentSignatureParameterCountIs(1); -verify.currentSignatureHelpIs('fnTest(...args: any[]): void'); +verify.currentSignatureHelpIs('allOptional(...args: any[]): void'); verify.currentParameterHelpArgumentNameIs('args'); verify.currentParameterSpanIs("...args: any[]"); -verify.numberOfErrorsInCurrentFile(0); \ No newline at end of file + +goTo.marker('2'); +verify.signatureHelpCountIs(1); +verify.currentSignatureParameterCountIs(3); +verify.currentSignatureHelpIs('someOptional(x: any, y: any, ...args: any[]): void'); +verify.currentParameterHelpArgumentNameIs('x'); +verify.currentParameterSpanIs("x: any"); +verify.numberOfErrorsInCurrentFile(0); From 4b19a94856726e563c11c4da174421c34ab349f8 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 11:29:04 -0700 Subject: [PATCH 07/14] Update baselines --- .../argumentsObjectCreatesRestForJs.symbols | 44 ++++++++++++++ .../argumentsObjectCreatesRestForJs.types | 60 +++++++++++++++++++ ...jsSelfReferencingArgumentsFunction.symbols | 12 ++++ .../jsSelfReferencingArgumentsFunction.types | 13 ++++ 4 files changed, 129 insertions(+) create mode 100644 tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols create mode 100644 tests/baselines/reference/argumentsObjectCreatesRestForJs.types create mode 100644 tests/baselines/reference/jsSelfReferencingArgumentsFunction.symbols create mode 100644 tests/baselines/reference/jsSelfReferencingArgumentsFunction.types diff --git a/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols b/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols new file mode 100644 index 00000000000..6a95dbc86d8 --- /dev/null +++ b/tests/baselines/reference/argumentsObjectCreatesRestForJs.symbols @@ -0,0 +1,44 @@ +=== tests/cases/compiler/main.js === +function allRest() { arguments; } +>allRest : Symbol(allRest, Decl(main.js, 0, 0)) +>arguments : Symbol(arguments) + +allRest(); +>allRest : Symbol(allRest, Decl(main.js, 0, 0)) + +allRest(1, 2, 3); +>allRest : Symbol(allRest, Decl(main.js, 0, 0)) + +function someRest(x, y) { arguments; } +>someRest : Symbol(someRest, Decl(main.js, 2, 17)) +>x : Symbol(x, Decl(main.js, 3, 18)) +>y : Symbol(y, Decl(main.js, 3, 20)) +>arguments : Symbol(arguments) + +someRest(); // x and y are still optional because they are in a JS file +>someRest : Symbol(someRest, Decl(main.js, 2, 17)) + +someRest(1, 2, 3); +>someRest : Symbol(someRest, Decl(main.js, 2, 17)) + +/** + * @param {number} x - a thing + */ +function jsdocced(x) { arguments; } +>jsdocced : Symbol(jsdocced, Decl(main.js, 5, 18)) +>x : Symbol(x, Decl(main.js, 10, 18)) +>arguments : Symbol(arguments) + +jsdocced(1); +>jsdocced : Symbol(jsdocced, Decl(main.js, 5, 18)) + +function dontDoubleRest(x, ...y) { arguments; } +>dontDoubleRest : Symbol(dontDoubleRest, Decl(main.js, 11, 12)) +>x : Symbol(x, Decl(main.js, 13, 24)) +>y : Symbol(y, Decl(main.js, 13, 26)) +>arguments : Symbol(arguments) + +dontDoubleRest(1, 2, 3); +>dontDoubleRest : Symbol(dontDoubleRest, Decl(main.js, 11, 12)) + + diff --git a/tests/baselines/reference/argumentsObjectCreatesRestForJs.types b/tests/baselines/reference/argumentsObjectCreatesRestForJs.types new file mode 100644 index 00000000000..10596fb10c7 --- /dev/null +++ b/tests/baselines/reference/argumentsObjectCreatesRestForJs.types @@ -0,0 +1,60 @@ +=== tests/cases/compiler/main.js === +function allRest() { arguments; } +>allRest : (...args: any[]) => void +>arguments : IArguments + +allRest(); +>allRest() : void +>allRest : (...args: any[]) => void + +allRest(1, 2, 3); +>allRest(1, 2, 3) : void +>allRest : (...args: any[]) => void +>1 : 1 +>2 : 2 +>3 : 3 + +function someRest(x, y) { arguments; } +>someRest : (x: any, y: any, ...args: any[]) => void +>x : any +>y : any +>arguments : IArguments + +someRest(); // x and y are still optional because they are in a JS file +>someRest() : void +>someRest : (x: any, y: any, ...args: any[]) => void + +someRest(1, 2, 3); +>someRest(1, 2, 3) : void +>someRest : (x: any, y: any, ...args: any[]) => void +>1 : 1 +>2 : 2 +>3 : 3 + +/** + * @param {number} x - a thing + */ +function jsdocced(x) { arguments; } +>jsdocced : (x: number) => void +>x : number +>arguments : IArguments + +jsdocced(1); +>jsdocced(1) : void +>jsdocced : (x: number) => void +>1 : 1 + +function dontDoubleRest(x, ...y) { arguments; } +>dontDoubleRest : (x: any, ...y: any[]) => void +>x : any +>y : any[] +>arguments : IArguments + +dontDoubleRest(1, 2, 3); +>dontDoubleRest(1, 2, 3) : void +>dontDoubleRest : (x: any, ...y: any[]) => void +>1 : 1 +>2 : 2 +>3 : 3 + + diff --git a/tests/baselines/reference/jsSelfReferencingArgumentsFunction.symbols b/tests/baselines/reference/jsSelfReferencingArgumentsFunction.symbols new file mode 100644 index 00000000000..e4fe336575c --- /dev/null +++ b/tests/baselines/reference/jsSelfReferencingArgumentsFunction.symbols @@ -0,0 +1,12 @@ +=== tests/cases/compiler/foo.js === +// Test #16139 +function Foo() { +>Foo : Symbol(Foo, Decl(foo.js, 0, 0)) + + arguments; +>arguments : Symbol(arguments) + + return new Foo(); +>Foo : Symbol(Foo, Decl(foo.js, 0, 0)) +} + diff --git a/tests/baselines/reference/jsSelfReferencingArgumentsFunction.types b/tests/baselines/reference/jsSelfReferencingArgumentsFunction.types new file mode 100644 index 00000000000..cab51cbafc1 --- /dev/null +++ b/tests/baselines/reference/jsSelfReferencingArgumentsFunction.types @@ -0,0 +1,13 @@ +=== tests/cases/compiler/foo.js === +// Test #16139 +function Foo() { +>Foo : (...args: any[]) => any + + arguments; +>arguments : IArguments + + return new Foo(); +>new Foo() : any +>Foo : (...args: any[]) => any +} + From b57830f7f952b8858c44a65b48f157f46f0ac547 Mon Sep 17 00:00:00 2001 From: TravCav Date: Wed, 7 Jun 2017 14:58:25 -0400 Subject: [PATCH 08/14] enforcing curly braces (#16315) --- src/compiler/checker.ts | 3 +- src/compiler/commandLineParser.ts | 20 +++-- src/compiler/core.ts | 49 +++++++---- src/harness/fourslash.ts | 47 +++++----- src/harness/harness.ts | 3 +- src/server/session.ts | 12 ++- src/services/importTracker.ts | 141 ++++++++++++++++-------------- src/services/services.ts | 3 +- src/services/symbolDisplay.ts | 4 +- tslint.json | 1 + 10 files changed, 166 insertions(+), 117 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index bde5e050fb1..0478cbf6215 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -16831,8 +16831,9 @@ namespace ts { (expr as PropertyAccessExpression | ElementAccessExpression).expression.kind === SyntaxKind.ThisKeyword) { // Look for if this is the constructor for the class that `symbol` is a property of. const func = getContainingFunction(expr); - if (!(func && func.kind === SyntaxKind.Constructor)) + if (!(func && func.kind === SyntaxKind.Constructor)) { return true; + } // If func.parent is a class and symbol is a (readonly) property of that class, or // if func is a constructor and symbol is a (readonly) parameter property declared in it, // then symbol is writeable here. diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 23efd047e46..2488ad68101 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1626,10 +1626,12 @@ namespace ts { } // Remove any subpaths under an existing recursively watched directory. - for (const key in wildcardDirectories) if (hasProperty(wildcardDirectories, key)) { - for (const recursiveKey of recursiveKeys) { - if (key !== recursiveKey && containsPath(recursiveKey, key, path, !useCaseSensitiveFileNames)) { - delete wildcardDirectories[key]; + for (const key in wildcardDirectories) { + if (hasProperty(wildcardDirectories, key)) { + for (const recursiveKey of recursiveKeys) { + if (key !== recursiveKey && containsPath(recursiveKey, key, path, !useCaseSensitiveFileNames)) { + delete wildcardDirectories[key]; + } } } } @@ -1717,10 +1719,12 @@ namespace ts { /* @internal */ export function convertCompilerOptionsForTelemetry(opts: ts.CompilerOptions): ts.CompilerOptions { const out: ts.CompilerOptions = {}; - for (const key in opts) if (opts.hasOwnProperty(key)) { - const type = getOptionFromName(key); - if (type !== undefined) { // Ignore unknown options - out[key] = getOptionValueWithEmptyStrings(opts[key], type); + for (const key in opts) { + if (opts.hasOwnProperty(key)) { + const type = getOptionFromName(key); + if (type !== undefined) { // Ignore unknown options + out[key] = getOptionValueWithEmptyStrings(opts[key], type); + } } } return out; diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 747801f83bd..14da7dfa8f5 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -51,8 +51,10 @@ namespace ts { // Copies keys/values from template. Note that for..in will not throw if // template is undefined, and instead will just exit the loop. - for (const key in template) if (hasOwnProperty.call(template, key)) { - map.set(key, template[key]); + for (const key in template) { + if (hasOwnProperty.call(template, key)) { + map.set(key, template[key]); + } } return map; @@ -977,9 +979,12 @@ namespace ts { */ export function getOwnKeys(map: MapLike): string[] { const keys: string[] = []; - for (const key in map) if (hasOwnProperty.call(map, key)) { - keys.push(key); + for (const key in map) { + if (hasOwnProperty.call(map, key)) { + keys.push(key); + } } + return keys; } @@ -1042,8 +1047,10 @@ namespace ts { export function assign>(t: T1, ...args: any[]): any; export function assign>(t: T1, ...args: any[]) { for (const arg of args) { - for (const p in arg) if (hasProperty(arg, p)) { - t[p] = arg[p]; + for (const p in arg) { + if (hasProperty(arg, p)) { + t[p] = arg[p]; + } } } return t; @@ -1058,13 +1065,19 @@ namespace ts { export function equalOwnProperties(left: MapLike, right: MapLike, equalityComparer?: (left: T, right: T) => boolean) { if (left === right) return true; if (!left || !right) return false; - for (const key in left) if (hasOwnProperty.call(left, key)) { - if (!hasOwnProperty.call(right, key) === undefined) return false; - if (equalityComparer ? !equalityComparer(left[key], right[key]) : left[key] !== right[key]) return false; + for (const key in left) { + if (hasOwnProperty.call(left, key)) { + if (!hasOwnProperty.call(right, key) === undefined) return false; + if (equalityComparer ? !equalityComparer(left[key], right[key]) : left[key] !== right[key]) return false; + } } - for (const key in right) if (hasOwnProperty.call(right, key)) { - if (!hasOwnProperty.call(left, key)) return false; + + for (const key in right) { + if (hasOwnProperty.call(right, key)) { + if (!hasOwnProperty.call(left, key)) return false; + } } + return true; } @@ -1106,12 +1119,18 @@ namespace ts { export function extend(first: T1, second: T2): T1 & T2 { const result: T1 & T2 = {}; - for (const id in second) if (hasOwnProperty.call(second, id)) { - (result as any)[id] = (second as any)[id]; + for (const id in second) { + if (hasOwnProperty.call(second, id)) { + (result as any)[id] = (second as any)[id]; + } } - for (const id in first) if (hasOwnProperty.call(first, id)) { - (result as any)[id] = (first as any)[id]; + + for (const id in first) { + if (hasOwnProperty.call(first, id)) { + (result as any)[id] = (first as any)[id]; + } } + return result; } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index ec79d118156..2d2cb7f8453 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1036,21 +1036,27 @@ namespace FourSlash { fail(`Expected ${expected}, got ${actual}`); } - for (const key in actual) if (ts.hasProperty(actual as any, key)) { - const ak = actual[key], ek = expected[key]; - if (typeof ak === "object" && typeof ek === "object") { - recur(ak, ek, path ? path + "." + key : key); - } - else if (ak !== ek) { - fail(`Expected '${key}' to be '${ek}', got '${ak}'`); + for (const key in actual) { + if (ts.hasProperty(actual as any, key)) { + const ak = actual[key], ek = expected[key]; + if (typeof ak === "object" && typeof ek === "object") { + recur(ak, ek, path ? path + "." + key : key); + } + else if (ak !== ek) { + fail(`Expected '${key}' to be '${ek}', got '${ak}'`); + } } } - for (const key in expected) if (ts.hasProperty(expected as any, key)) { - if (!ts.hasProperty(actual as any, key)) { - fail(`${msgPrefix}Missing property '${key}'`); + + for (const key in expected) { + if (ts.hasProperty(expected as any, key)) { + if (!ts.hasProperty(actual as any, key)) { + fail(`${msgPrefix}Missing property '${key}'`); + } } } }; + if (fullActual === undefined || fullExpected === undefined) { if (fullActual === fullExpected) { return; @@ -1132,15 +1138,17 @@ namespace FourSlash { } public verifyQuickInfos(namesAndTexts: { [name: string]: string | [string, string] }) { - for (const name in namesAndTexts) if (ts.hasProperty(namesAndTexts, name)) { - const text = namesAndTexts[name]; - if (ts.isArray(text)) { - assert(text.length === 2); - const [expectedText, expectedDocumentation] = text; - this.verifyQuickInfoAt(name, expectedText, expectedDocumentation); - } - else { - this.verifyQuickInfoAt(name, text); + for (const name in namesAndTexts) { + if (ts.hasProperty(namesAndTexts, name)) { + const text = namesAndTexts[name]; + if (ts.isArray(text)) { + assert(text.length === 2); + const [expectedText, expectedDocumentation] = text; + this.verifyQuickInfoAt(name, expectedText, expectedDocumentation); + } + else { + this.verifyQuickInfoAt(name, text); + } } } } @@ -1149,7 +1157,6 @@ namespace FourSlash { if (expectedDocumentation === "") { throw new Error("Use 'undefined' instead"); } - const actualQuickInfo = this.languageService.getQuickInfoAtPosition(this.activeFile.fileName, this.currentCaretPosition); const actualQuickInfoText = actualQuickInfo ? ts.displayPartsToString(actualQuickInfo.displayParts) : ""; const actualQuickInfoDocumentation = actualQuickInfo ? ts.displayPartsToString(actualQuickInfo.documentation) : ""; diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 9bf4591112d..d61ba6e953b 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -259,8 +259,9 @@ namespace Utils { return true; } else if ((f & v) > 0) { - if (result.length) + if (result.length) { result += " | "; + } result += flags[v]; return false; } diff --git a/src/server/session.ts b/src/server/session.ts index 6db0999ee62..828bcdce1ac 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1563,18 +1563,22 @@ namespace ts.server { const normalizedFileName = toNormalizedPath(fileName); const project = this.projectService.getDefaultProjectForFile(normalizedFileName, /*refreshInferredProjects*/ true); for (const fileNameInProject of fileNamesInProject) { - if (this.getCanonicalFileName(fileNameInProject) === this.getCanonicalFileName(fileName)) + if (this.getCanonicalFileName(fileNameInProject) === this.getCanonicalFileName(fileName)) { highPriorityFiles.push(fileNameInProject); + } else { const info = this.projectService.getScriptInfo(fileNameInProject); if (!info.isScriptOpen()) { - if (fileNameInProject.indexOf(".d.ts") > 0) + if (fileNameInProject.indexOf(".d.ts") > 0) { veryLowPriorityFiles.push(fileNameInProject); - else + } + else { lowPriorityFiles.push(fileNameInProject); + } } - else + else { mediumPriorityFiles.push(fileNameInProject); + } } } diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index 6a0b9167997..5e8e7b8b3a7 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -73,54 +73,56 @@ namespace ts.FindAllReferences { function handleDirectImports(exportingModuleSymbol: Symbol): void { const theseDirectImports = getDirectImports(exportingModuleSymbol); - if (theseDirectImports) for (const direct of theseDirectImports) { - if (!markSeenDirectImport(direct)) { - continue; - } + if (theseDirectImports) { + for (const direct of theseDirectImports) { + if (!markSeenDirectImport(direct)) { + continue; + } - cancellationToken.throwIfCancellationRequested(); + cancellationToken.throwIfCancellationRequested(); - switch (direct.kind) { - case SyntaxKind.CallExpression: - if (!isAvailableThroughGlobal) { - const parent = direct.parent!; - if (exportKind === ExportKind.ExportEquals && parent.kind === SyntaxKind.VariableDeclaration) { - const { name } = parent as ts.VariableDeclaration; - if (name.kind === SyntaxKind.Identifier) { - directImports.push(name); - break; + switch (direct.kind) { + case SyntaxKind.CallExpression: + if (!isAvailableThroughGlobal) { + const parent = direct.parent!; + if (exportKind === ExportKind.ExportEquals && parent.kind === SyntaxKind.VariableDeclaration) { + const { name } = parent as ts.VariableDeclaration; + if (name.kind === SyntaxKind.Identifier) { + directImports.push(name); + break; + } } + + // Don't support re-exporting 'require()' calls, so just add a single indirect user. + addIndirectUser(direct.getSourceFile()); } + break; - // Don't support re-exporting 'require()' calls, so just add a single indirect user. - addIndirectUser(direct.getSourceFile()); - } - break; + case SyntaxKind.ImportEqualsDeclaration: + handleNamespaceImport(direct, direct.name, hasModifier(direct, ModifierFlags.Export)); + break; - case SyntaxKind.ImportEqualsDeclaration: - handleNamespaceImport(direct, direct.name, hasModifier(direct, ModifierFlags.Export)); - break; + case SyntaxKind.ImportDeclaration: + const namedBindings = direct.importClause && direct.importClause.namedBindings; + if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { + handleNamespaceImport(direct, namedBindings.name); + } + else { + directImports.push(direct); + } + break; - case SyntaxKind.ImportDeclaration: - const namedBindings = direct.importClause && direct.importClause.namedBindings; - if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { - handleNamespaceImport(direct, namedBindings.name); - } - else { - directImports.push(direct); - } - break; - - case SyntaxKind.ExportDeclaration: - if (!direct.exportClause) { - // This is `export * from "foo"`, so imports of this module may import the export too. - handleDirectImports(getContainingModuleSymbol(direct, checker)); - } - else { - // This is `export { foo } from "foo"` and creates an alias symbol, so recursive search will get handle re-exports. - directImports.push(direct); - } - break; + case SyntaxKind.ExportDeclaration: + if (!direct.exportClause) { + // This is `export * from "foo"`, so imports of this module may import the export too. + handleDirectImports(getContainingModuleSymbol(direct, checker)); + } + else { + // This is `export { foo } from "foo"` and creates an alias symbol, so recursive search will get handle re-exports. + directImports.push(direct); + } + break; + } } } } @@ -160,8 +162,10 @@ namespace ts.FindAllReferences { const moduleSymbol = checker.getMergedSymbol(sourceFileLike.symbol); Debug.assert(!!(moduleSymbol.flags & SymbolFlags.Module)); const directImports = getDirectImports(moduleSymbol); - if (directImports) for (const directImport of directImports) { - addIndirectUsers(getSourceFileLikeForImportDeclaration(directImport)); + if (directImports) { + for (const directImport of directImports) { + addIndirectUsers(getSourceFileLikeForImportDeclaration(directImport)); + } } } @@ -183,8 +187,10 @@ namespace ts.FindAllReferences { importSearches.push([location, symbol]); } - if (directImports) for (const decl of directImports) { - handleImport(decl); + if (directImports) { + for (const decl of directImports) { + handleImport(decl); + } } return { importSearches, singleReferences }; @@ -258,25 +264,27 @@ namespace ts.FindAllReferences { } function searchForNamedImport(namedBindings: NamedImportsOrExports | undefined): void { - if (namedBindings) for (const element of namedBindings.elements) { - const { name, propertyName } = element; - if ((propertyName || name).text !== exportName) { - continue; - } + if (namedBindings) { + for (const element of namedBindings.elements) { + const { name, propertyName } = element; + if ((propertyName || name).text !== exportName) { + continue; + } - if (propertyName) { - // This is `import { foo as bar } from "./a"` or `export { foo as bar } from "./a"`. `foo` isn't a local in the file, so just add it as a single reference. - singleReferences.push(propertyName); - if (!isForRename) { // If renaming `foo`, don't touch `bar`, just `foo`. - // Search locally for `bar`. - addSearch(name, checker.getSymbolAtLocation(name)); + if (propertyName) { + // This is `import { foo as bar } from "./a"` or `export { foo as bar } from "./a"`. `foo` isn't a local in the file, so just add it as a single reference. + singleReferences.push(propertyName); + if (!isForRename) { // If renaming `foo`, don't touch `bar`, just `foo`. + // Search locally for `bar`. + addSearch(name, checker.getSymbolAtLocation(name)); + } + } + else { + const localSymbol = element.kind === SyntaxKind.ExportSpecifier && element.propertyName + ? checker.getExportSpecifierLocalTargetSymbol(element) // For re-exporting under a different name, we want to get the re-exported symbol. + : checker.getSymbolAtLocation(name); + addSearch(name, localSymbol); } - } - else { - const localSymbol = element.kind === SyntaxKind.ExportSpecifier && element.propertyName - ? checker.getExportSpecifierLocalTargetSymbol(element) // For re-exporting under a different name, we want to get the re-exported symbol. - : checker.getSymbolAtLocation(name); - addSearch(name, localSymbol); } } } @@ -604,12 +612,13 @@ namespace ts.FindAllReferences { /** If at an export specifier, go to the symbol it refers to. */ function skipExportSpecifierSymbol(symbol: Symbol, checker: TypeChecker): Symbol { // For `export { foo } from './bar", there's nothing to skip, because it does not create a new alias. But `export { foo } does. - if (symbol.declarations) for (const declaration of symbol.declarations) { - if (isExportSpecifier(declaration) && !(declaration as ExportSpecifier).propertyName && !(declaration as ExportSpecifier).parent.parent.moduleSpecifier) { - return checker.getExportSpecifierLocalTargetSymbol(declaration); + if (symbol.declarations) { + for (const declaration of symbol.declarations) { + if (isExportSpecifier(declaration) && !(declaration as ExportSpecifier).propertyName && !(declaration as ExportSpecifier).parent.parent.moduleSpecifier) { + return checker.getExportSpecifierLocalTargetSymbol(declaration); + } } } - return symbol; } diff --git a/src/services/services.ts b/src/services/services.ts index c514df4660b..853803a2474 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -684,8 +684,9 @@ namespace ts { forEachChild(decl.name, visit); break; } - if (decl.initializer) + if (decl.initializer) { visit(decl.initializer); + } } // falls through case SyntaxKind.EnumMember: diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts index 910c31c36b3..345f0718fe3 100644 --- a/src/services/symbolDisplay.ts +++ b/src/services/symbolDisplay.ts @@ -4,8 +4,10 @@ namespace ts.SymbolDisplay { export function getSymbolKind(typeChecker: TypeChecker, symbol: Symbol, location: Node): ScriptElementKind { const { flags } = symbol; - if (flags & SymbolFlags.Class) return getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ? + if (flags & SymbolFlags.Class) { + return getDeclarationOfKind(symbol, SyntaxKind.ClassExpression) ? ScriptElementKind.localClassElement : ScriptElementKind.classElement; + } if (flags & SymbolFlags.Enum) return ScriptElementKind.enumElement; if (flags & SymbolFlags.TypeAlias) return ScriptElementKind.typeElement; if (flags & SymbolFlags.Interface) return ScriptElementKind.interfaceElement; diff --git a/tslint.json b/tslint.json index d3178915fe4..58106e52f0f 100644 --- a/tslint.json +++ b/tslint.json @@ -6,6 +6,7 @@ "comment-format": [true, "check-space" ], + "curly":[true, "ignore-same-line"], "indent": [true, "spaces" ], From abb9681248becf71eb035064e778c5ffb4ba6955 Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 7 Jun 2017 12:28:52 -0700 Subject: [PATCH 09/14] Support completions for JSDoc @param tag names (#16299) * Support completions for JSDoc @param tag names * Undo change to finishNode * Don't include trailing whitespace in @param range; instead, specialize getJsDocTagAtPosition --- src/compiler/core.ts | 4 +- src/compiler/parser.ts | 34 +++--- src/compiler/types.ts | 2 +- src/harness/fourslash.ts | 21 ++-- src/services/classifier.ts | 2 +- src/services/completions.ts | 101 +++++++++++++----- src/services/jsDoc.ts | 18 ++++ src/services/services.ts | 6 +- src/services/utilities.ts | 59 +++------- ...parsesCorrectly.argSynonymForParamTag.json | 1 + ...sCorrectly.argumentSynonymForParamTag.json | 1 + ...cComments.parsesCorrectly.oneParamTag.json | 1 + ...DocComments.parsesCorrectly.paramTag1.json | 1 + ...parsesCorrectly.paramTagNameThenType1.json | 1 + ...parsesCorrectly.paramTagNameThenType2.json | 1 + ...ents.parsesCorrectly.paramWithoutType.json | 1 + ...Comments.parsesCorrectly.twoParamTag2.json | 2 + ...parsesCorrectly.twoParamTagOnSameLine.json | 2 + ...sCorrectly.typedefTagWithChildrenTags.json | 6 +- .../cases/fourslash/commentsCommentParsing.ts | 27 ++--- .../fourslash/jsdocParameterNameCompletion.ts | 29 +++++ 21 files changed, 198 insertions(+), 122 deletions(-) create mode 100644 tests/cases/fourslash/jsdocParameterNameCompletion.ts diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 14da7dfa8f5..6ca99c2d086 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -523,8 +523,8 @@ namespace ts { return result || array; } - export function mapDefined(array: ReadonlyArray, mapFn: (x: T, i: number) => T | undefined): ReadonlyArray { - const result: T[] = []; + export function mapDefined(array: ReadonlyArray, mapFn: (x: T, i: number) => U | undefined): U[] { + const result: U[] = []; for (let i = 0; i < array.length; i++) { const item = array[i]; const mapped = mapFn(item, i); diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index b398ee66e0a..8002709f3f6 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -6663,14 +6663,12 @@ namespace ts { }); } - function parseBracketNameInPropertyAndParamTag() { - let name: Identifier; - let isBracketed: boolean; + function parseBracketNameInPropertyAndParamTag(): { name: Identifier, isBracketed: boolean } { // Looking for something like '[foo]' or 'foo' - if (parseOptionalToken(SyntaxKind.OpenBracketToken)) { - name = parseJSDocIdentifierName(); + const isBracketed = parseOptional(SyntaxKind.OpenBracketToken); + const name = parseJSDocIdentifierName(/*createIfMissing*/ true); + if (isBracketed) { skipWhitespace(); - isBracketed = true; // May have an optional default, e.g. '[foo = 42]' if (parseOptionalToken(SyntaxKind.EqualsToken)) { @@ -6679,9 +6677,7 @@ namespace ts { parseExpected(SyntaxKind.CloseBracketToken); } - else if (tokenIsIdentifierOrKeyword(token())) { - name = parseJSDocIdentifierName(); - } + return { name, isBracketed }; } @@ -6692,11 +6688,6 @@ namespace ts { const { name, isBracketed } = parseBracketNameInPropertyAndParamTag(); skipWhitespace(); - if (!name) { - parseErrorAtPosition(scanner.getStartPos(), 0, Diagnostics.Identifier_expected); - return undefined; - } - let preName: Identifier, postName: Identifier; if (typeExpression) { postName = name; @@ -6947,14 +6938,19 @@ namespace ts { return currentToken = scanner.scanJSDocToken(); } - function parseJSDocIdentifierName(): Identifier { - return createJSDocIdentifier(tokenIsIdentifierOrKeyword(token())); + function parseJSDocIdentifierName(createIfMissing = false): Identifier { + return createJSDocIdentifier(tokenIsIdentifierOrKeyword(token()), createIfMissing); } - function createJSDocIdentifier(isIdentifier: boolean): Identifier { + function createJSDocIdentifier(isIdentifier: boolean, createIfMissing: boolean): Identifier { if (!isIdentifier) { - parseErrorAtCurrentToken(Diagnostics.Identifier_expected); - return undefined; + if (createIfMissing) { + return createMissingNode(SyntaxKind.Identifier, /*reportAtCurrentPosition*/ true, Diagnostics.Identifier_expected); + } + else { + parseErrorAtCurrentToken(Diagnostics.Identifier_expected); + return undefined; + } } const pos = scanner.getTokenPos(); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 5efa10ef437..b9ecc52cee6 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -425,7 +425,7 @@ namespace ts { FirstNode = QualifiedName, FirstJSDocNode = JSDocTypeExpression, LastJSDocNode = JSDocLiteralType, - FirstJSDocTagNode = JSDocComment, + FirstJSDocTagNode = JSDocTag, LastJSDocTagNode = JSDocLiteralType } diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 2d2cb7f8453..f40d63709cb 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1602,16 +1602,19 @@ namespace FourSlash { } private printMembersOrCompletions(info: ts.CompletionInfo) { + if (info === undefined) { return "No completion info."; } + const { entries } = info; + function pad(s: string, length: number) { return s + new Array(length - s.length + 1).join(" "); } function max(arr: T[], selector: (x: T) => number): number { return arr.reduce((prev, x) => Math.max(prev, selector(x)), 0); } - const longestNameLength = max(info.entries, m => m.name.length); - const longestKindLength = max(info.entries, m => m.kind.length); - info.entries.sort((m, n) => m.sortText > n.sortText ? 1 : m.sortText < n.sortText ? -1 : m.name > n.name ? 1 : m.name < n.name ? -1 : 0); - const membersString = info.entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers}`).join("\n"); + const longestNameLength = max(entries, m => m.name.length); + const longestKindLength = max(entries, m => m.kind.length); + entries.sort((m, n) => m.sortText > n.sortText ? 1 : m.sortText < n.sortText ? -1 : m.name > n.name ? 1 : m.name < n.name ? -1 : 0); + const membersString = entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers}`).join("\n"); Harness.IO.log(membersString); } @@ -2163,7 +2166,7 @@ namespace FourSlash { Harness.IO.log(this.spanInfoToString(this.getNameOrDottedNameSpan(pos), "**")); } - private verifyClassifications(expected: { classificationType: string; text: string; textSpan?: TextSpan }[], actual: ts.ClassifiedSpan[]) { + private verifyClassifications(expected: { classificationType: string; text: string; textSpan?: TextSpan }[], actual: ts.ClassifiedSpan[], sourceFileText: string) { if (actual.length !== expected.length) { this.raiseError("verifyClassifications failed - expected total classifications to be " + expected.length + ", but was " + actual.length + @@ -2203,9 +2206,11 @@ namespace FourSlash { }); function jsonMismatchString() { + const showActual = actual.map(({ classificationType, textSpan }) => + ({ classificationType, text: sourceFileText.slice(textSpan.start, textSpan.start + textSpan.length) })); return Harness.IO.newLine() + "expected: '" + Harness.IO.newLine() + stringify(expected) + "'" + Harness.IO.newLine() + - "actual: '" + Harness.IO.newLine() + stringify(actual) + "'"; + "actual: '" + Harness.IO.newLine() + stringify(showActual) + "'"; } } @@ -2228,14 +2233,14 @@ namespace FourSlash { const actual = this.languageService.getSemanticClassifications(this.activeFile.fileName, ts.createTextSpan(0, this.activeFile.content.length)); - this.verifyClassifications(expected, actual); + this.verifyClassifications(expected, actual, this.activeFile.content); } public verifySyntacticClassifications(expected: { classificationType: string; text: string }[]) { const actual = this.languageService.getSyntacticClassifications(this.activeFile.fileName, ts.createTextSpan(0, this.activeFile.content.length)); - this.verifyClassifications(expected, actual); + this.verifyClassifications(expected, actual, this.activeFile.content); } public verifyOutliningSpans(spans: TextSpan[]) { diff --git a/src/services/classifier.ts b/src/services/classifier.ts index beeddda434e..ca8cf52a09b 100644 --- a/src/services/classifier.ts +++ b/src/services/classifier.ts @@ -814,7 +814,7 @@ namespace ts { * False will mean that node is not classified and traverse routine should recurse into node contents. */ function tryClassifyNode(node: Node): boolean { - if (isJSDocTag(node)) { + if (isJSDocNode(node)) { return true; } diff --git a/src/services/completions.ts b/src/services/completions.ts index 71fe4ce1c78..99560ffb4d8 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -18,7 +18,7 @@ namespace ts.Completions { return undefined; } - const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords } = completionData; + const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, hasFilteredClassMemberKeywords } = completionData; if (sourceFile.languageVariant === LanguageVariant.JSX && location && location.parent && location.parent.kind === SyntaxKind.JsxClosingElement) { @@ -36,14 +36,15 @@ namespace ts.Completions { }]}; } - if (requestJsDocTagName) { - // If the current position is a jsDoc tag name, only tag names should be provided for completion - return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries: JsDoc.getJSDocTagNameCompletions() }; - } - - if (requestJsDocTag) { - // If the current position is a jsDoc tag, only tags should be provided for completion - return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries: JsDoc.getJSDocTagCompletions() }; + if (request) { + const entries = request.kind === "JsDocTagName" + // If the current position is a jsDoc tag name, only tag names should be provided for completion + ? JsDoc.getJSDocTagNameCompletions() + : request.kind === "JsDocTag" + // If the current position is a jsDoc tag, only tags should be provided for completion + ? JsDoc.getJSDocTagCompletions() + : JsDoc.getJSDocParameterNameCompletions(request.tag); + return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries }; } const entries: CompletionEntry[] = []; @@ -66,7 +67,7 @@ namespace ts.Completions { addRange(entries, classMemberKeywordCompletions); } // Add keywords if this is not a member completion list - else if (!isMemberCompletion && !requestJsDocTag && !requestJsDocTagName) { + else if (!isMemberCompletion) { addRange(entries, keywordCompletions); } @@ -347,16 +348,27 @@ namespace ts.Completions { return undefined; } - function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number) { + interface CompletionData { + symbols: Symbol[]; + isGlobalCompletion: boolean; + isMemberCompletion: boolean; + isNewIdentifierLocation: boolean; + location: Node; + isRightOfDot: boolean; + request?: Request; + hasFilteredClassMemberKeywords: boolean; + } + type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; + + function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number): CompletionData { const isJavaScriptFile = isSourceFileJavaScript(sourceFile); - // JsDoc tag-name is just the name of the JSDoc tagname (exclude "@") - let requestJsDocTagName = false; - // JsDoc tag includes both "@" and tag-name - let requestJsDocTag = false; + let request: Request | undefined; let start = timestamp(); - const currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); // TODO: GH#15853 + const currentToken = getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); + // We will check for jsdoc comments with insideComment and getJsDocTagAtPosition. (TODO: that seems rather inefficient to check the same thing so many times.) + log("getCompletionData: Get current token: " + (timestamp() - start)); start = timestamp(); @@ -366,10 +378,10 @@ namespace ts.Completions { if (insideComment) { if (hasDocComment(sourceFile, position)) { - // The current position is next to the '@' sign, when no tag name being provided yet. - // Provide a full list of tag names if (sourceFile.text.charCodeAt(position - 1) === CharacterCodes.at) { - requestJsDocTagName = true; + // The current position is next to the '@' sign, when no tag name being provided yet. + // Provide a full list of tag names + request = { kind: "JsDocTagName" }; } else { // When completion is requested without "@", we will have check to make sure that @@ -389,7 +401,9 @@ namespace ts.Completions { // * |c| // */ const lineStart = getLineStartPositionForPosition(position, sourceFile); - requestJsDocTag = !(sourceFile.text.substring(lineStart, position).match(/[^\*|\s|(/\*\*)]/)); + if (!(sourceFile.text.substring(lineStart, position).match(/[^\*|\s|(/\*\*)]/))) { + request = { kind: "JsDocTag" }; + } } } @@ -397,10 +411,10 @@ namespace ts.Completions { // /** @type {number | string} */ // Completion should work in the brackets let insideJsDocTagExpression = false; - const tag = getJsDocTagAtPosition(sourceFile, position); + const tag = getJsDocTagAtPosition(currentToken, position); if (tag) { if (tag.tagName.pos <= position && position <= tag.tagName.end) { - requestJsDocTagName = true; + request = { kind: "JsDocTagName" }; } switch (tag.kind) { @@ -408,15 +422,18 @@ namespace ts.Completions { case SyntaxKind.JSDocParameterTag: case SyntaxKind.JSDocReturnTag: const tagWithExpression = tag; - if (tagWithExpression.typeExpression) { - insideJsDocTagExpression = tagWithExpression.typeExpression.pos < position && position < tagWithExpression.typeExpression.end; + if (tagWithExpression.typeExpression && tagWithExpression.typeExpression.pos < position && position < tagWithExpression.typeExpression.end) { + insideJsDocTagExpression = true; + } + else if (isJSDocParameterTag(tag) && (nodeIsMissing(tag.name) || tag.name.pos <= position && position <= tag.name.end)) { + request = { kind: "JsDocParameterName", tag }; } break; } } - if (requestJsDocTagName || requestJsDocTag) { - return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords: false }; + if (request) { + return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, hasFilteredClassMemberKeywords: false }; } if (!insideJsDocTagExpression) { @@ -553,7 +570,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); - return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), requestJsDocTagName, requestJsDocTag, hasFilteredClassMemberKeywords }; + return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, hasFilteredClassMemberKeywords }; function getTypeScriptMemberSymbols(): void { // Right of dot member completion list @@ -1518,4 +1535,34 @@ namespace ts.Completions { kind === SyntaxKind.EqualsEqualsEqualsToken || kind === SyntaxKind.ExclamationEqualsEqualsToken; } + + /** Get the corresponding JSDocTag node if the position is in a jsDoc comment */ + function getJsDocTagAtPosition(node: Node, position: number): JSDocTag | undefined { + const { jsDoc } = getJsDocHavingNode(node); + if (!jsDoc) return undefined; + + for (const { pos, end, tags } of jsDoc) { + if (!tags || position < pos || position > end) continue; + for (let i = tags.length - 1; i >= 0; i--) { + const tag = tags[i]; + if (position >= tag.pos) { + return tag; + } + } + } + } + + function getJsDocHavingNode(node: Node): Node { + if (!isToken(node)) return node; + + switch (node.kind) { + case SyntaxKind.VarKeyword: + case SyntaxKind.LetKeyword: + case SyntaxKind.ConstKeyword: + // if the current token is var, let or const, skip the VariableDeclarationList + return node.parent.parent; + default: + return node.parent; + } + } } diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts index 8ca55029603..baf9ae44b5f 100644 --- a/src/services/jsDoc.ts +++ b/src/services/jsDoc.ts @@ -132,6 +132,24 @@ namespace ts.JsDoc { })); } + export function getJSDocParameterNameCompletions(tag: JSDocParameterTag): CompletionEntry[] { + const nameThusFar = tag.name.text; + const jsdoc = tag.parent; + const fn = jsdoc.parent; + if (!ts.isFunctionLike(fn)) return []; + + return mapDefined(fn.parameters, param => { + if (!isIdentifier(param.name)) return undefined; + + const name = param.name.text; + if (jsdoc.tags.some(t => t !== tag && isJSDocParameterTag(t) && t.name.text === name) + || nameThusFar !== undefined && !startsWith(name, nameThusFar)) + return undefined; + + return { name, kind: ScriptElementKind.parameterElement, kindModifiers: "", sortText: "0" }; + }); + } + /** * Checks if position points to a valid position to add JSDoc comments, and if so, * returns the appropriate template. Otherwise returns an empty string. diff --git a/src/services/services.ts b/src/services/services.ts index 853803a2474..22fa67dd1c5 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -136,7 +136,7 @@ namespace ts { } private createChildren(sourceFile?: SourceFileLike) { - if (isJSDocTag(this)) { + if (this.kind === SyntaxKind.JSDocComment || isJSDocTag(this)) { /** Don't add trivia for "tokens" since this is in a comment. */ const children: Node[] = []; this.forEachChild(child => { children.push(child); }); @@ -146,9 +146,9 @@ namespace ts { const children: Node[] = []; scanner.setText((sourceFile || this.getSourceFile()).text); let pos = this.pos; - const useJSDocScanner = this.kind >= SyntaxKind.FirstJSDocTagNode && this.kind <= SyntaxKind.LastJSDocTagNode; + const useJSDocScanner = isJSDocNode(this); const processNode = (node: Node) => { - const isJSDocTagNode = isJSDocTag(node); + const isJSDocTagNode = isJSDocNode(node); if (!isJSDocTagNode && pos < node.pos) { pos = this.addSyntheticNodes(children, pos, node.pos, useJSDocScanner); } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 7a87c0939c7..ab958099c11 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -615,18 +615,21 @@ namespace ts { return getTouchingToken(sourceFile, position, includeJsDocComment, n => isPropertyName(n.kind)); } - /** Returns the token if position is in [start, end) or if position === end and includeItemAtEndPosition(token) === true */ - export function getTouchingToken(sourceFile: SourceFile, position: number, includeJsDocComment: boolean, includeItemAtEndPosition?: (n: Node) => boolean): Node { - return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ false, includeItemAtEndPosition, includeJsDocComment); + /** + * Returns the token if position is in [start, end). + * If position === end, returns the preceding token if includeItemAtEndPosition(previousToken) === true + */ + export function getTouchingToken(sourceFile: SourceFile, position: number, includeJsDocComment: boolean, includePrecedingTokenAtEndPosition?: (n: Node) => boolean): Node { + return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ false, includePrecedingTokenAtEndPosition, /*includeEndPosition*/ false, includeJsDocComment); } /** Returns a token if position is in [start-of-leading-trivia, end) */ - export function getTokenAtPosition(sourceFile: SourceFile, position: number, includeJsDocComment: boolean): Node { - return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ true, /*includeItemAtEndPosition*/ undefined, includeJsDocComment); + export function getTokenAtPosition(sourceFile: SourceFile, position: number, includeJsDocComment: boolean, includeEndPosition?: boolean): Node { + return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ true, /*includePrecedingTokenAtEndPosition*/ undefined, includeEndPosition, includeJsDocComment); } /** Get the token whose text contains the position */ - function getTokenAtPositionWorker(sourceFile: SourceFile, position: number, allowPositionInLeadingTrivia: boolean, includeItemAtEndPosition: (n: Node) => boolean, includeJsDocComment: boolean): Node { + function getTokenAtPositionWorker(sourceFile: SourceFile, position: number, allowPositionInLeadingTrivia: boolean, includePrecedingTokenAtEndPosition: (n: Node) => boolean, includeEndPosition: boolean, includeJsDocComment: boolean): Node { let current: Node = sourceFile; outer: while (true) { if (isToken(current)) { @@ -636,7 +639,7 @@ namespace ts { // find the child that contains 'position' for (const child of current.getChildren()) { - if (isJSDocNode(child) && !includeJsDocComment) { + if (!includeJsDocComment && isJSDocNode(child)) { continue; } @@ -646,13 +649,13 @@ namespace ts { } const end = child.getEnd(); - if (position < end || (position === end && child.kind === SyntaxKind.EndOfFileToken)) { + if (position < end || (position === end && (child.kind === SyntaxKind.EndOfFileToken || includeEndPosition))) { current = child; continue outer; } - else if (includeItemAtEndPosition && end === position) { + else if (includePrecedingTokenAtEndPosition && end === position) { const previousToken = findPrecedingToken(position, sourceFile, child); - if (previousToken && includeItemAtEndPosition(previousToken)) { + if (previousToken && includePrecedingTokenAtEndPosition(previousToken)) { return previousToken; } } @@ -901,42 +904,6 @@ namespace ts { } } - /** - * Get the corresponding JSDocTag node if the position is in a jsDoc comment - */ - export function getJsDocTagAtPosition(sourceFile: SourceFile, position: number): JSDocTag { - let node = ts.getTokenAtPosition(sourceFile, position, /*includeJsDocComment*/ false); - if (isToken(node)) { - switch (node.kind) { - case SyntaxKind.VarKeyword: - case SyntaxKind.LetKeyword: - case SyntaxKind.ConstKeyword: - // if the current token is var, let or const, skip the VariableDeclarationList - node = node.parent === undefined ? undefined : node.parent.parent; - break; - default: - node = node.parent; - break; - } - } - - if (node) { - if (node.jsDoc) { - for (const jsDoc of node.jsDoc) { - if (jsDoc.tags) { - for (const tag of jsDoc.tags) { - if (tag.pos <= position && position <= tag.end) { - return tag; - } - } - } - } - } - } - - return undefined; - } - function nodeHasTokens(n: Node): boolean { // If we have a token or node that has a non-zero width, it must have tokens. // Note, that getWidth() does not take trivia into account. diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argSynonymForParamTag.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argSynonymForParamTag.json index 92b9cb450e6..b47a07e8487 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argSynonymForParamTag.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argSynonymForParamTag.json @@ -40,6 +40,7 @@ "end": 27, "text": "name1" }, + "isBracketed": false, "comment": "Description" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argumentSynonymForParamTag.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argumentSynonymForParamTag.json index f398fb3af41..cf86fda872e 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argumentSynonymForParamTag.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.argumentSynonymForParamTag.json @@ -40,6 +40,7 @@ "end": 32, "text": "name1" }, + "isBracketed": false, "comment": "Description" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json index e70fc95367f..506487232e0 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.oneParamTag.json @@ -40,6 +40,7 @@ "end": 29, "text": "name1" }, + "isBracketed": false, "comment": "" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTag1.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTag1.json index 0dbdd83d2ba..abc116d7452 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTag1.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTag1.json @@ -40,6 +40,7 @@ "end": 29, "text": "name1" }, + "isBracketed": false, "comment": "Description text follows" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json index 204d94779b3..9d66ca3dd55 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType1.json @@ -40,6 +40,7 @@ "end": 20, "text": "name1" }, + "isBracketed": false, "comment": "" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType2.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType2.json index 7c79459768b..01d08a103c1 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType2.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramTagNameThenType2.json @@ -40,6 +40,7 @@ "end": 20, "text": "name1" }, + "isBracketed": false, "comment": "Description" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json index 2ff182483d9..ab15d24f618 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.paramWithoutType.json @@ -30,6 +30,7 @@ "end": 18, "text": "foo" }, + "isBracketed": false, "comment": "" }, "length": 1, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json index b51ab3598e6..5ba60030f1b 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTag2.json @@ -40,6 +40,7 @@ "end": 29, "text": "name1" }, + "isBracketed": false, "comment": "" }, "1": { @@ -79,6 +80,7 @@ "end": 55, "text": "name2" }, + "isBracketed": false, "comment": "" }, "length": 2, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTagOnSameLine.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTagOnSameLine.json index 3c20d5edcbc..7d26097bb34 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTagOnSameLine.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.twoParamTagOnSameLine.json @@ -40,6 +40,7 @@ "end": 29, "text": "name1" }, + "isBracketed": false, "comment": "" }, "1": { @@ -79,6 +80,7 @@ "end": 51, "text": "name2" }, + "isBracketed": false, "comment": "" }, "length": 2, diff --git a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json index 7a8f9c4bcc9..13826b63d9c 100644 --- a/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json +++ b/tests/baselines/reference/JSDocParsing/DocComments.parsesCorrectly.typedefTagWithChildrenTags.json @@ -103,7 +103,8 @@ "pos": 66, "end": 69, "text": "age" - } + }, + "isBracketed": false }, { "kind": "JSDocPropertyTag", @@ -141,7 +142,8 @@ "pos": 93, "end": 97, "text": "name" - } + }, + "isBracketed": false } ] }, diff --git a/tests/cases/fourslash/commentsCommentParsing.ts b/tests/cases/fourslash/commentsCommentParsing.ts index ce80fd519dd..f26d2f569f8 100644 --- a/tests/cases/fourslash/commentsCommentParsing.ts +++ b/tests/cases/fourslash/commentsCommentParsing.ts @@ -160,8 +160,8 @@ ////fo/*37q*/oBar(/*37*/"foo",/*38*/"bar"); /////** This is a comment */ ////var x; -/////** -//// * This is a comment +/////** +//// * This is a comment //// */ ////var y; /////** this is jsdoc style function with param tag as well as inline parameter help @@ -173,7 +173,7 @@ ////} /////*44*/jsD/*40q*/ocParamTest(/*40*/30, /*41*/40, /*42*/50, /*43*/60); /////** This is function comment -//// * And properly aligned comment +//// * And properly aligned comment //// */ ////function jsDocCommentAlignmentTest1() { ////} @@ -334,39 +334,40 @@ goTo.marker('27'); verify.completionListContains("multiply", "function multiply(a: number, b: number, c?: number, d?: any, e?: any): void", "This is multiplication function"); verify.completionListContains("f1", "function f1(a: number): any (+1 overload)", "fn f1 with number"); +const subtractDoc = "This is subtract function"; goTo.marker('28'); -verify.currentSignatureHelpDocCommentIs("This is subtract function{ () => string; } } f this is optional param f"); +verify.currentSignatureHelpDocCommentIs(subtractDoc); verify.currentParameterHelpArgumentDocCommentIs(""); verify.quickInfos({ "28q": [ "function subtract(a: number, b: number, c?: () => string, d?: () => string, e?: () => string, f?: () => string): void", - "This is subtract function{ () => string; } } f this is optional param f" + subtractDoc, ], "28aq": "(parameter) a: number" }); goTo.marker('29'); -verify.currentSignatureHelpDocCommentIs("This is subtract function{ () => string; } } f this is optional param f"); +verify.currentSignatureHelpDocCommentIs(subtractDoc); verify.currentParameterHelpArgumentDocCommentIs("this is about b"); verify.quickInfoAt("29aq", "(parameter) b: number", "this is about b"); goTo.marker('30'); -verify.currentSignatureHelpDocCommentIs("This is subtract function{ () => string; } } f this is optional param f"); +verify.currentSignatureHelpDocCommentIs(subtractDoc); verify.currentParameterHelpArgumentDocCommentIs("this is optional param c"); verify.quickInfoAt("30aq", "(parameter) c: () => string", "this is optional param c"); goTo.marker('31'); -verify.currentSignatureHelpDocCommentIs("This is subtract function{ () => string; } } f this is optional param f"); +verify.currentSignatureHelpDocCommentIs(subtractDoc); verify.currentParameterHelpArgumentDocCommentIs("this is optional param d"); verify.quickInfoAt("31aq", "(parameter) d: () => string", "this is optional param d"); goTo.marker('32'); -verify.currentSignatureHelpDocCommentIs("This is subtract function{ () => string; } } f this is optional param f"); +verify.currentSignatureHelpDocCommentIs(subtractDoc); verify.currentParameterHelpArgumentDocCommentIs("this is optional param e"); verify.quickInfoAt("32aq", "(parameter) e: () => string", "this is optional param e"); goTo.marker('33'); -verify.currentSignatureHelpDocCommentIs("This is subtract function{ () => string; } } f this is optional param f"); +verify.currentSignatureHelpDocCommentIs(subtractDoc); verify.currentParameterHelpArgumentDocCommentIs(""); verify.quickInfoAt("33aq", "(parameter) f: () => string"); @@ -454,11 +455,11 @@ verify.quickInfoAt("43aq", "(parameter) d: number"); goTo.marker('44'); verify.completionListContains("jsDocParamTest", "function jsDocParamTest(a: number, b: number, c: number, d: number): number", "this is jsdoc style function with param tag as well as inline parameter help"); verify.completionListContains("x", "var x: any", "This is a comment "); -verify.completionListContains("y", "var y: any", "This is a comment "); +verify.completionListContains("y", "var y: any", "This is a comment"); goTo.marker('45'); -verify.currentSignatureHelpDocCommentIs("This is function comment\nAnd properly aligned comment "); -verify.quickInfoAt("45q", "function jsDocCommentAlignmentTest1(): void", "This is function comment\nAnd properly aligned comment "); +verify.currentSignatureHelpDocCommentIs("This is function comment\nAnd properly aligned comment"); +verify.quickInfoAt("45q", "function jsDocCommentAlignmentTest1(): void", "This is function comment\nAnd properly aligned comment"); goTo.marker('46'); verify.currentSignatureHelpDocCommentIs("This is function comment\n And aligned with 4 space char margin"); diff --git a/tests/cases/fourslash/jsdocParameterNameCompletion.ts b/tests/cases/fourslash/jsdocParameterNameCompletion.ts new file mode 100644 index 00000000000..b379cb77135 --- /dev/null +++ b/tests/cases/fourslash/jsdocParameterNameCompletion.ts @@ -0,0 +1,29 @@ +/// + +/////** +//// * @param /*0*/ +//// */ +////function f(foo, bar) {} + +/////** +//// * @param foo +//// * @param /*1*/ +//// */ +////function g(foo, bar) {} + +/////** +//// * @param can/*2*/ +//// * @param cantaloupe +//// */ +////function h(cat, canary, canoodle, cantaloupe, zebra) {} + +/////** +//// * @param /*3*/ {string} /*4*/ +//// */ +////function i(foo, bar) {} + +verify.completionsAt("0", ["foo", "bar"]); +verify.completionsAt("1", ["bar"]); +verify.completionsAt("2", ["canary", "canoodle"]); +verify.completionsAt("3", ["foo", "bar"]); +verify.completionsAt("4", ["foo", "bar"]); From d3d917584162d9afa0e33397c8bd403e636f95ed Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 7 Jun 2017 14:13:30 -0700 Subject: [PATCH 10/14] PR Feedback --- src/compiler/checker.ts | 83 ++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3b3551c1af8..5868cd6dc3a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6814,57 +6814,56 @@ namespace ts { return undefined; } - function resolveTypeReferenceName(node: TypeReferenceType, typeReferenceName: EntityNameExpression | EntityName) { + function resolveTypeReferenceName(typeReferenceName: EntityNameExpression | EntityName, meaning: SymbolFlags) { if (!typeReferenceName) { return unknownSymbol; } - const meaning = node.kind === SyntaxKind.JSDocTypeReference - ? SymbolFlags.Type | SymbolFlags.Value - : SymbolFlags.Type; - return resolveEntityName(typeReferenceName, meaning) || unknownSymbol; } function getTypeReferenceType(node: TypeReferenceType, symbol: Symbol) { const typeArguments = typeArgumentsFromTypeReferenceNode(node); // Do unconditionally so we mark type arguments as referenced. - let secondPass = false; - let fallbackType: Type = unknownType; - while (true) { - if (symbol === unknownSymbol) { - return fallbackType; - } + if (symbol === unknownSymbol) { + return unknownType; + } - if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) { - return getTypeFromClassOrInterfaceReference(node, symbol, typeArguments); - } + const type = getTypeReferenceTypeWorker(node, symbol, typeArguments); + if (type) { + return type; + } - if (symbol.flags & SymbolFlags.TypeAlias) { - return getTypeFromTypeAliasReference(node, symbol, typeArguments); - } - - if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) { - // A JSDocTypeReference may have resolved to a value (as opposed to a type). If - // the symbol is a constructor function, return the inferred class type; otherwise, - // the type of this reference is just the type of the value we resolved to. - if (symbol.flags & SymbolFlags.Function && (symbol.members || getJSDocClassTag(symbol.valueDeclaration))) { - return getInferredClassType(symbol); + if (symbol.flags & SymbolFlags.Value && node.kind === SyntaxKind.JSDocTypeReference) { + // A JSDocTypeReference may have resolved to a value (as opposed to a type). If + // the symbol is a constructor function, return the inferred class type; otherwise, + // the type of this reference is just the type of the value we resolved to. + const valueType = getTypeOfSymbol(symbol); + if (valueType.symbol && !isInferredClassType(valueType)) { + const referenceType = getTypeReferenceTypeWorker(node, valueType.symbol, typeArguments); + if (referenceType) { + return referenceType; } - - // Stop if this is the second pass - if (secondPass) { - return fallbackType; - } - - // Try to use the symbol of the type (if present) to get a better type on the - // second pass. - fallbackType = getTypeOfSymbol(symbol); - symbol = fallbackType.symbol || unknownSymbol; - secondPass = true; - continue; } - return getTypeFromNonGenericTypeReference(node, symbol); + // Resolve the type reference as a Type for the purpose of reporting errors. + resolveTypeReferenceName(getTypeReferenceName(node), SymbolFlags.Type); + return valueType; + } + + return getTypeFromNonGenericTypeReference(node, symbol); + } + + function getTypeReferenceTypeWorker(node: TypeReferenceType, symbol: Symbol, typeArguments: Type[]): Type | undefined { + if (symbol.flags & (SymbolFlags.Class | SymbolFlags.Interface)) { + return getTypeFromClassOrInterfaceReference(node, symbol, typeArguments); + } + + if (symbol.flags & SymbolFlags.TypeAlias) { + return getTypeFromTypeAliasReference(node, symbol, typeArguments); + } + + if (symbol.flags & SymbolFlags.Function && node.kind === SyntaxKind.JSDocTypeReference && (symbol.members || getJSDocClassTag(symbol.valueDeclaration))) { + return getInferredClassType(symbol); } } @@ -6908,11 +6907,13 @@ namespace ts { if (!links.resolvedType) { let symbol: Symbol; let type: Type; + let meaning = SymbolFlags.Type; if (node.kind === SyntaxKind.JSDocTypeReference) { type = getPrimitiveTypeFromJSDocTypeReference(node); + meaning |= SymbolFlags.Value; } if (!type) { - symbol = resolveTypeReferenceName(node, getTypeReferenceName(node)); + symbol = resolveTypeReferenceName(getTypeReferenceName(node), meaning); type = getTypeReferenceType(node, symbol); } // Cache both the resolved symbol and the resolved type. The resolved symbol is needed in when we check the @@ -16155,6 +16156,12 @@ namespace ts { return links.inferredClassType; } + function isInferredClassType(type: Type) { + return type.symbol + && getObjectFlags(type) & ObjectFlags.Anonymous + && getSymbolLinks(type.symbol).inferredClassType === type; + } + /** * Syntactically and semantically checks a call or new expression. * @param node The call/new expression to be checked. From 00a926cc4e6de8a4cf2b910ea1db35ff3b15e496 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 7 Jun 2017 15:13:24 -0700 Subject: [PATCH 11/14] Fix parameter emit for synthetic function types --- src/compiler/emitter.ts | 26 ++- src/compiler/factory.ts | 2 +- src/harness/unittests/printer.ts | 175 +++++++++++++----- .../printsNodeCorrectly.functionTypes.js | 1 + 4 files changed, 146 insertions(+), 58 deletions(-) create mode 100644 tests/baselines/reference/printerApi/printsNodeCorrectly.functionTypes.js diff --git a/src/compiler/emitter.ts b/src/compiler/emitter.ts index 1e6cc3b12db..a788bf6a4d0 100644 --- a/src/compiler/emitter.ts +++ b/src/compiler/emitter.ts @@ -959,7 +959,7 @@ namespace ts { function emitConstructorType(node: ConstructorTypeNode) { write("new "); emitTypeParameters(node, node.typeParameters); - emitParametersForArrow(node, node.parameters); + emitParameters(node, node.parameters); write(" => "); emit(node.type); } @@ -2283,11 +2283,25 @@ namespace ts { emitList(parentNode, parameters, ListFormat.Parameters); } - function emitParametersForArrow(parentNode: Node, parameters: NodeArray) { - if (parameters && - parameters.length === 1 && - parameters[0].type === undefined && - parameters[0].pos === parentNode.pos) { + function canEmitSimpleArrowHead(parentNode: FunctionTypeNode | ArrowFunction, parameters: NodeArray) { + const parameter = singleOrUndefined(parameters); + return parameter + && parameter.pos === parentNode.pos // may not have parsed tokens between parent and parameter + && !(isArrowFunction(parentNode) && parentNode.type) // arrow function may not have return type annotation + && !some(parentNode.decorators) // parent may not have decorators + && !some(parentNode.modifiers) // parent may not have modifiers + && !some(parentNode.typeParameters) // parent may not have type parameters + && !some(parameter.decorators) // parameter may not have decorators + && !some(parameter.modifiers) // parameter may not have modifiers + && !parameter.dotDotDotToken // parameter may not be rest + && !parameter.questionToken // parameter may not be optional + && !parameter.type // parameter may not have a type annotation + && !parameter.initializer // parameter may not have an initializer + && isIdentifier(parameter.name); // parameter name must be identifier + } + + function emitParametersForArrow(parentNode: FunctionTypeNode | ArrowFunction, parameters: NodeArray) { + if (canEmitSimpleArrowHead(parentNode, parameters)) { emit(parameters[0]); } else { diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 1f9cf91efd0..32244a86792 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -228,7 +228,7 @@ namespace ts { // Signature elements - export function createTypeParameterDeclaration(name: string | Identifier, constraint: TypeNode | undefined, defaultType: TypeNode | undefined) { + export function createTypeParameterDeclaration(name: string | Identifier, constraint?: TypeNode, defaultType?: TypeNode) { const node = createSynthesizedNode(SyntaxKind.TypeParameter) as TypeParameterDeclaration; node.name = asName(name); node.constraint = constraint; diff --git a/src/harness/unittests/printer.ts b/src/harness/unittests/printer.ts index 4bdafe04ba1..23e301f3669 100644 --- a/src/harness/unittests/printer.ts +++ b/src/harness/unittests/printer.ts @@ -81,63 +81,136 @@ namespace ts { describe("printNode", () => { const printsCorrectly = makePrintsCorrectly("printsNodeCorrectly"); - let sourceFile: SourceFile; - before(() => sourceFile = createSourceFile("source.ts", "", ScriptTarget.ES2015)); - // tslint:disable boolean-trivia - const syntheticNode = createClassDeclaration( - undefined, - undefined, - /*name*/ createIdentifier("C"), - undefined, - undefined, - createNodeArray([ - createProperty( - undefined, + printsCorrectly("class", {}, printer => printer.printNode( + EmitHint.Unspecified, + createClassDeclaration( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*name*/ createIdentifier("C"), + /*typeParameters*/ undefined, + /*heritageClauses*/ undefined, + [createProperty( + /*decorators*/ undefined, createNodeArray([createToken(SyntaxKind.PublicKeyword)]), createIdentifier("prop"), - undefined, - undefined, - undefined - ) - ]) - ); + /*questionToken*/ undefined, + /*type*/ undefined, + /*initializer*/ undefined + )] + ), + createSourceFile("source.ts", "", ScriptTarget.ES2015) + )); + + printsCorrectly("namespaceExportDeclaration", {}, printer => printer.printNode( + EmitHint.Unspecified, + createNamespaceExportDeclaration("B"), + createSourceFile("source.ts", "", ScriptTarget.ES2015) + )); // https://github.com/Microsoft/TypeScript/issues/15971 - const classWithOptionalMethodAndProperty = createClassDeclaration( - undefined, - /* modifiers */ createNodeArray([createToken(SyntaxKind.DeclareKeyword)]), - /* name */ createIdentifier("X"), - undefined, - undefined, - createNodeArray([ - createMethod( - undefined, - undefined, - undefined, - /* name */ createIdentifier("method"), - /* questionToken */ createToken(SyntaxKind.QuestionToken), - undefined, - undefined, - /* type */ createKeywordTypeNode(SyntaxKind.VoidKeyword), - undefined + printsCorrectly("classWithOptionalMethodAndProperty", {}, printer => printer.printNode( + EmitHint.Unspecified, + createClassDeclaration( + /*decorators*/ undefined, + /*modifiers*/ [createToken(SyntaxKind.DeclareKeyword)], + /*name*/ createIdentifier("X"), + /*typeParameters*/ undefined, + /*heritageClauses*/ undefined, + [ + createMethod( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*asteriskToken*/ undefined, + /*name*/ createIdentifier("method"), + /*questionToken*/ createToken(SyntaxKind.QuestionToken), + /*typeParameters*/ undefined, + [], + /*type*/ createKeywordTypeNode(SyntaxKind.VoidKeyword), + /*body*/ undefined + ), + createProperty( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*name*/ createIdentifier("property"), + /*questionToken*/ createToken(SyntaxKind.QuestionToken), + /*type*/ createKeywordTypeNode(SyntaxKind.StringKeyword), + /*initializer*/ undefined + ), + ] + ), + createSourceFile("source.ts", "", ScriptTarget.ES2015) + )); + + // https://github.com/Microsoft/TypeScript/issues/15651 + printsCorrectly("functionTypes", {}, printer => printer.printNode( + EmitHint.Unspecified, + createTupleTypeNode([ + createFunctionTypeNode( + /*typeArguments*/ undefined, + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + createIdentifier("args") + )], + createKeywordTypeNode(SyntaxKind.AnyKeyword) ), - createProperty( - undefined, - undefined, - /* name */ createIdentifier("property"), - /* questionToken */ createToken(SyntaxKind.QuestionToken), - /* type */ createKeywordTypeNode(SyntaxKind.StringKeyword), - undefined + createFunctionTypeNode( + [createTypeParameterDeclaration("T")], + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + createIdentifier("args") + )], + createKeywordTypeNode(SyntaxKind.AnyKeyword) ), - ]) - ); - - // tslint:enable boolean-trivia - printsCorrectly("class", {}, printer => printer.printNode(EmitHint.Unspecified, syntheticNode, sourceFile)); - - printsCorrectly("namespaceExportDeclaration", {}, printer => printer.printNode(EmitHint.Unspecified, createNamespaceExportDeclaration("B"), sourceFile)); - - printsCorrectly("classWithOptionalMethodAndProperty", {}, printer => printer.printNode(EmitHint.Unspecified, classWithOptionalMethodAndProperty, sourceFile)); + createFunctionTypeNode( + /*typeArguments*/ undefined, + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + createToken(SyntaxKind.DotDotDotToken), + createIdentifier("args") + )], + createKeywordTypeNode(SyntaxKind.AnyKeyword) + ), + createFunctionTypeNode( + /*typeArguments*/ undefined, + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + createIdentifier("args"), + createToken(SyntaxKind.QuestionToken) + )], + createKeywordTypeNode(SyntaxKind.AnyKeyword) + ), + createFunctionTypeNode( + /*typeArguments*/ undefined, + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + createIdentifier("args"), + /*questionToken*/ undefined, + createKeywordTypeNode(SyntaxKind.AnyKeyword) + )], + createKeywordTypeNode(SyntaxKind.AnyKeyword) + ), + createFunctionTypeNode( + /*typeArguments*/ undefined, + [createParameter( + /*decorators*/ undefined, + /*modifiers*/ undefined, + /*dotDotDotToken*/ undefined, + createObjectBindingPattern([]) + )], + createKeywordTypeNode(SyntaxKind.AnyKeyword) + ), + ]), + createSourceFile("source.ts", "", ScriptTarget.ES2015) + )); }); }); } diff --git a/tests/baselines/reference/printerApi/printsNodeCorrectly.functionTypes.js b/tests/baselines/reference/printerApi/printsNodeCorrectly.functionTypes.js new file mode 100644 index 00000000000..5bfda3ba7c9 --- /dev/null +++ b/tests/baselines/reference/printerApi/printsNodeCorrectly.functionTypes.js @@ -0,0 +1 @@ +[args => any, (args) => any, (...args) => any, (args?) => any, (args: any) => any, ({}) => any] \ No newline at end of file From 43e3d60f09cf88d6fdd72745a0dfc802567a4e18 Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 7 Jun 2017 15:50:26 -0700 Subject: [PATCH 12/14] Fix lint failure (#16338) * Fix lint failure * Use curly braces --- src/services/jsDoc.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/jsDoc.ts b/src/services/jsDoc.ts index baf9ae44b5f..5d03ad03b0c 100644 --- a/src/services/jsDoc.ts +++ b/src/services/jsDoc.ts @@ -143,8 +143,9 @@ namespace ts.JsDoc { const name = param.name.text; if (jsdoc.tags.some(t => t !== tag && isJSDocParameterTag(t) && t.name.text === name) - || nameThusFar !== undefined && !startsWith(name, nameThusFar)) + || nameThusFar !== undefined && !startsWith(name, nameThusFar)) { return undefined; + } return { name, kind: ScriptElementKind.parameterElement, kindModifiers: "", sortText: "0" }; }); From c8d33bc38ec61e923d6ff62c2e7b004bc56c6e5b Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 7 Jun 2017 22:17:40 -0700 Subject: [PATCH 13/14] Update generated files --- src/lib/dom.generated.d.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/dom.generated.d.ts b/src/lib/dom.generated.d.ts index 3aa813c0a8a..da81bba5d78 100644 --- a/src/lib/dom.generated.d.ts +++ b/src/lib/dom.generated.d.ts @@ -3483,7 +3483,7 @@ interface DragEvent extends MouseEvent { declare var DragEvent: { prototype: DragEvent; - new(): DragEvent; + new(type: "drag" | "dragend" | "dragenter" | "dragexit" | "dragleave" | "dragover" | "dragstart" | "drop", dragEventInit?: { dataTransfer?: DataTransfer }): DragEvent; }; interface DynamicsCompressorNode extends AudioNode { @@ -8224,6 +8224,7 @@ interface Navigator extends Object, NavigatorID, NavigatorOnLine, NavigatorConte readonly serviceWorker: ServiceWorkerContainer; readonly webdriver: boolean; readonly hardwareConcurrency: number; + readonly languages: string[]; getGamepads(): Gamepad[]; javaEnabled(): boolean; msLaunchUri(uri: string, successCallback?: MSLaunchUriCallback, noHandlerCallback?: MSLaunchUriCallback): void; From 158a6371e934a173a65d7524d99d22cf48799308 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 7 Jun 2017 22:32:18 -0700 Subject: [PATCH 14/14] Update authors for TS 2.4 --- .mailmap | 21 ++++++++++++++++++++- AUTHORS.md | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap index 73ebed0cc7c..97941bcdf5a 100644 --- a/.mailmap +++ b/.mailmap @@ -248,4 +248,23 @@ rdosanjh # Raj Dosanjh gdh1995 # Dahan Gong cedvdb # @cedvdb kpreisser # K. Preißer -e-cloud # @e-cloud \ No newline at end of file +e-cloud # @e-cloud +Andrew Casey Andrew Casey +Andrew Stegmaier +Benny Neugebauer +Blaine Bublitz +Charles Pierce +Daniel Król +Diogo Franco (Kovensky) +Donald Pipowitch +Halasi Tamás +Ika +Joe Chung +Kate Miháliková +Mohsen Azimi +Noel Varanda +Reiner Dolp +t_ # @t_ +TravCav # @TravCav +Vladimir Kurchatkin +William Orr \ No newline at end of file diff --git a/AUTHORS.md b/AUTHORS.md index 4772d5371fa..824c696ce4f 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -15,7 +15,9 @@ TypeScript is authored by: * Anders Hejlsberg * Andreas Martin * Andrej Baran +* Andrew Casey * Andrew Ochsner +* Andrew Stegmaier * Andrew Z Allen * András Parditka * Andy Hanson @@ -31,7 +33,9 @@ TypeScript is authored by: * Ben Duffield * Ben Mosher * Benjamin Bock +* Benny Neugebauer * Bill Ticehurst +* Blaine Bublitz * Blake Embrey * @bootstraponline * Bowden Kelly @@ -39,6 +43,7 @@ TypeScript is authored by: * Bryan Forbes * Caitlin Potter * @cedvdb +* Charles Pierce * Charly POLY * Chris Bubernak * Christophe Vidal @@ -52,6 +57,7 @@ TypeScript is authored by: * Dan Corder * Dan Quirk * Daniel Hollocher +* Daniel Król * Daniel Lehenbauer * Daniel Rosenwasser * David Kmenta @@ -60,9 +66,11 @@ TypeScript is authored by: * David Souther * Denis Nedelyaev * Dick van den Brink +* Diogo Franco (Kovensky) * Dirk Bäumer * Dirk Holtwick * Dom Chen +* Donald Pipowitch * Doug Ilijev * @e-cloud * Elisée Maurer @@ -89,12 +97,14 @@ TypeScript is authored by: * Guilherme Oenning * Guillaume Salles * Guy Bedford +* Halasi Tamás * Harald Niesche * Hendrik Liebau * Herrington Darkholme * Homa Wong * Iain Monro * Igor Novozhilov +* Ika * Ingvar Stepanyan * Isiah Meadows * Ivo Gabe de Wolff @@ -111,6 +121,7 @@ TypeScript is authored by: * Jeffrey Morlan * Jesse Schalken * Jiri Tobisek +* Joe Chung * Joel Day * Joey Wilson * Johannes Rieken @@ -131,6 +142,7 @@ TypeScript is authored by: * K. Preißer * Kagami Sascha Rosylight * Kanchalai Tanglertsampan +* Kate Miháliková * Keith Mashinter * Ken Howard * Kenji Imamula @@ -159,6 +171,7 @@ TypeScript is authored by: * Mike Busyrev * Mine Starks * Mohamed Hegazy +* Mohsen Azimi * Myles Megyesi * Natalie Coley * Nathan Shively-Sanders @@ -166,6 +179,7 @@ TypeScript is authored by: * Nicolas Henry * Nima Zahedi * Noah Chen +* Noel Varanda * Noj Vek * Oleg Mihailik * Oleksandr Chekhovskyi @@ -186,6 +200,7 @@ TypeScript is authored by: * Punya Biswal * Rado Kirov * Raj Dosanjh +* Reiner Dolp * Richard Karmazín * Richard Knoll * Richard Sentino @@ -213,6 +228,7 @@ TypeScript is authored by: * Sudheesh Singanamalla * Sébastien Arod * @T18970237136 +* @t_ * Tarik Ozket * Tetsuharu Ohzeki * Thomas Loubiou @@ -225,13 +241,16 @@ TypeScript is authored by: * togru * Tomas Grubliauskas * Torben Fitschen +* @TravCav * TruongSinh Tran-Nguyen * Vadi Taslim * Vidar Tonaas Fauske * Viktor Zozulyak * Vilic Vane +* Vladimir Kurchatkin * Vladimir Matveev * Wesley Wigham +* William Orr * York Yao * @yortus * Yuichi Nukiyama