From 5999a521f6e852de92681acf8c91c01f102edab6 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Sat, 31 Dec 2016 19:12:57 -0800 Subject: [PATCH 01/23] 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 1f3ef7df7a7754530b98c867007744329c592d77 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 6 Jun 2017 14:58:18 -0700 Subject: [PATCH 02/23] Refactor refactor --- scripts/buildProtocol.ts | 10 +- src/harness/fourslash.ts | 11 +- src/harness/harnessLanguageService.ts | 2 +- src/harness/unittests/session.ts | 4 +- src/server/client.ts | 38 ++++-- src/server/protocol.ts | 109 +++++++++++++----- src/server/session.ts | 47 +++++--- src/services/refactorProvider.ts | 16 +-- .../refactors/convertFunctionToEs6Class.ts | 41 +++++-- src/services/services.ts | 13 ++- src/services/types.ts | 52 ++++++++- .../fourslash/convertFunctionToEs6Class1.ts | 2 +- .../fourslash/convertFunctionToEs6Class2.ts | 2 +- .../fourslash/convertFunctionToEs6Class3.ts | 2 +- tests/cases/fourslash/fourslash.ts | 4 +- .../convertFunctionToEs6Class-server.ts | 2 +- 16 files changed, 260 insertions(+), 95 deletions(-) diff --git a/scripts/buildProtocol.ts b/scripts/buildProtocol.ts index 37ebd0105ae..63da8bbece7 100644 --- a/scripts/buildProtocol.ts +++ b/scripts/buildProtocol.ts @@ -113,7 +113,7 @@ class DeclarationsWalker { } } -function generateProtocolFile(protocolTs: string, typeScriptServicesDts: string): string { +function generateProtocolFile(outputFile: string, protocolTs: string, typeScriptServicesDts: string) { const options = { target: ts.ScriptTarget.ES5, declaration: true, noResolve: true, types: [], stripInternal: true }; /** @@ -163,14 +163,17 @@ function generateProtocolFile(protocolTs: string, typeScriptServicesDts: string) protocolDts += "\nimport protocol = ts.server.protocol;"; protocolDts += "\nexport = protocol;"; protocolDts += "\nexport as namespace protocol;"; + // do sanity check and try to compile generated text as standalone program const sanityCheckProgram = getProgramWithProtocolText(protocolDts, /*includeTypeScriptServices*/ false); const diagnostics = [...sanityCheckProgram.getSyntacticDiagnostics(), ...sanityCheckProgram.getSemanticDiagnostics(), ...sanityCheckProgram.getGlobalDiagnostics()]; + + ts.sys.writeFile(outputFile, protocolDts); + if (diagnostics.length) { const flattenedDiagnostics = diagnostics.map(d => `${ts.flattenDiagnosticMessageText(d.messageText, "\n")} at ${d.file.fileName} line ${d.start}`).join("\n"); throw new Error(`Unexpected errors during sanity check: ${flattenedDiagnostics}`); } - return protocolDts; } if (process.argv.length < 5) { @@ -181,5 +184,4 @@ if (process.argv.length < 5) { const protocolTs = process.argv[2]; const typeScriptServicesDts = process.argv[3]; const outputFile = process.argv[4]; -const generatedProtocolDts = generateProtocolFile(protocolTs, typeScriptServicesDts); -ts.sys.writeFile(outputFile, generatedProtocolDts); +generateProtocolFile(outputFile, protocolTs, typeScriptServicesDts); diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 07aacf3c8a7..ec79d118156 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2741,6 +2741,7 @@ namespace FourSlash { markerName: string, expectedContent: string, refactorNameToApply: string, + actionName: string, formattingOptions?: ts.FormatCodeSettings) { formattingOptions = formattingOptions || this.formatCodeSettings; @@ -2753,9 +2754,11 @@ namespace FourSlash { this.raiseError(`The expected refactor: ${refactorNameToApply} is not available at the marker location.`); } - const codeActions = this.languageService.getRefactorCodeActions(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply); + const editInfo = this.languageService.getEditsForRefactor(this.activeFile.fileName, formattingOptions, markerPos, refactorNameToApply, actionName); - this.applyCodeActions(codeActions); + for (const edit of editInfo.edits) { + this.applyEdits(edit.fileName, edit.textChanges); + } const actualContent = this.getFileContent(this.activeFile.fileName); if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) { @@ -3798,8 +3801,8 @@ namespace FourSlashInterface { this.state.verifyRangeAfterCodeFix(expectedText, includeWhiteSpace, errorCode, index); } - public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: ts.FormatCodeSettings): void { - this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, formattingOptions); + public fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: ts.FormatCodeSettings): void { + this.state.verifyFileAfterApplyingRefactorAtMarker(markerName, expectedContent, refactorNameToApply, actionName, formattingOptions); } public rangeIs(expectedText: string, includeWhiteSpace?: boolean): void { diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 7aefb0f3a1f..132db1e53eb 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -492,7 +492,7 @@ namespace Harness.LanguageService { getCodeFixDiagnostics(): ts.Diagnostic[] { throw new Error("Not supported on the shim."); } - getRefactorCodeActions(): ts.CodeAction[] { + getEditsForRefactor(): ts.RefactorEditInfo { throw new Error("Not supported on the shim."); } getApplicableRefactors(): ts.ApplicableRefactorInfo[] { diff --git a/src/harness/unittests/session.ts b/src/harness/unittests/session.ts index db33d87f087..efc769efeca 100644 --- a/src/harness/unittests/session.ts +++ b/src/harness/unittests/session.ts @@ -240,8 +240,8 @@ namespace ts.server { CommandNames.GetCodeFixesFull, CommandNames.GetSupportedCodeFixes, CommandNames.GetApplicableRefactors, - CommandNames.GetRefactorCodeActions, - CommandNames.GetRefactorCodeActionsFull, + CommandNames.GetEditsForRefactor, + CommandNames.GetEditsForRefactorFull, ]; it("should not throw when commands are executed with invalid arguments", () => { diff --git a/src/server/client.ts b/src/server/client.ts index aca52422e44..e8142367f14 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -719,20 +719,42 @@ namespace ts.server { return response.body; } - getRefactorCodeActions( + getEditsForRefactor( fileName: string, _formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, - refactorName: string) { + refactorName: string, + actionName: string): RefactorEditInfo { - const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetRefactorCodeActionsRequestArgs; - args.refactorName = refactorName; + const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs; + args.refactor = refactorName; + args.action = actionName; - const request = this.processRequest(CommandNames.GetRefactorCodeActions, args); - const response = this.processResponse(request); - const codeActions = response.body.actions; + const request = this.processRequest(CommandNames.GetEditsForRefactor, args); + const response = this.processResponse(request); + const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits); - return map(codeActions, codeAction => this.convertCodeActions(codeAction, fileName)); + const renameFilename: string | undefined = response.body.renameFilename; + let renameLocation: number | undefined = undefined; + if (renameFilename !== undefined) { + renameLocation = this.lineOffsetToPosition(renameFilename, response.body.renameLocation); + } + + return { + edits, + renameFilename, + renameLocation + }; + } + + private convertCodeEditsToTextChanges(edits: ts.server.protocol.FileCodeEdits[]): FileTextChanges[] { + return edits.map(edit => { + const fileName = edit.fileName; + return { + fileName, + textChanges: edit.textChanges.map(t => this.convertTextChangeToCodeEdit(t, fileName)) + }; + }); } convertCodeActions(entry: protocol.CodeAction, fileName: string): CodeAction { diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 4e474edd0c4..e22569e4654 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -98,8 +98,11 @@ namespace ts.server.protocol { GetSupportedCodeFixes = "getSupportedCodeFixes", GetApplicableRefactors = "getApplicableRefactors", - GetRefactorCodeActions = "getRefactorCodeActions", - GetRefactorCodeActionsFull = "getRefactorCodeActions-full", + GetEditsForRefactor = "getEditsForRefactor", + /* @internal */ + GetEditsForRefactorFull = "getEditsForRefactor-full", + + // NOTE: If updating this, be sure to also update `allCommandNames` in `harness/unittests/session.ts`. } /** @@ -401,52 +404,98 @@ namespace ts.server.protocol { export type FileLocationOrRangeRequestArgs = FileLocationRequestArgs | FileRangeRequestArgs; + /** + * Request refactorings at a given position or selection area. + */ export interface GetApplicableRefactorsRequest extends Request { command: CommandTypes.GetApplicableRefactors; arguments: GetApplicableRefactorsRequestArgs; } - export type GetApplicableRefactorsRequestArgs = FileLocationOrRangeRequestArgs; - export interface ApplicableRefactorInfo { - name: string; - description: string; - } - + /** + * Response is a list of available refactorings. + * Each refactoring exposes 1 or more "Actions"; a user selects one action to invoke a refactoring + */ export interface GetApplicableRefactorsResponse extends Response { body?: ApplicableRefactorInfo[]; } - export interface GetRefactorCodeActionsRequest extends Request { - command: CommandTypes.GetRefactorCodeActions; - arguments: GetRefactorCodeActionsRequestArgs; + /** + * A set of one or more available refactoring actions, grouped under a parent refactoring. + */ + export interface ApplicableRefactorInfo { + /** + * The programmatic name of the refactoring + */ + name: string; + /** + * A description of this refactoring category to show to the user. + * If the refactoring gets inlined (see below), this text will not be visible. + */ + description: string; + /** + * Inlineable refactorings can have their actions hoisted out to the top level + * of a context menu. Non-inlineanable refactorings should always be shown inside + * their parent grouping. + * + * If not specified, this value is assumed to be 'true' + */ + inlineable?: boolean; + + actions: RefactorActionInfo[]; } - export type GetRefactorCodeActionsRequestArgs = FileLocationOrRangeRequestArgs & { - /* The kind of the applicable refactor */ - refactorName: string; + /** + * Represents a single refactoring action - for example, the "Extract Method..." refactor might + * offer several actions, each corresponding to a surround class or closure to extract into. + */ + export type RefactorActionInfo = { + /** + * The programmatic name of the refactoring action + */ + name: string; + + /** + * A description of this refactoring action to show to the user. + * If the parent refactoring is inlined away, this will be the only text shown, + * so this description should make sense by itself if the parent is inlineable=true + */ + description: string; }; - export type RefactorCodeActions = { - actions: protocol.CodeAction[]; - renameLocation?: number - }; - - /* @internal */ - export type RefactorCodeActionsFull = { - actions: ts.CodeAction[]; - renameLocation?: number - }; - - export interface GetRefactorCodeActionsResponse extends Response { - body: RefactorCodeActions; + export interface GetEditsForRefactorRequest extends Request { + command: CommandTypes.GetEditsForRefactor; + arguments: GetEditsForRefactorRequestArgs; } - /* @internal */ - export interface GetRefactorCodeActionsFullResponse extends Response { - body: RefactorCodeActionsFull; + /** + * Request the edits that a particular refactoring action produces. + * Callers must specify the name of the refactor and the name of the action. + */ + export type GetEditsForRefactorRequestArgs = FileLocationOrRangeRequestArgs & { + /* The 'name' property from the refactoring that offered this action */ + refactor: string; + /* The 'name' property from the refactoring action */ + action: string; + }; + + + export interface GetEditsForRefactorResponse extends Response { + body?: RefactorEditInfo; } + export type RefactorEditInfo = { + edits: FileCodeEdits[]; + + /** + * An optional location where the editor should start a rename operation once + * the refactoring edits have been applied + */ + renameLocation?: Location; + renameFilename?: string; + }; + /** * Request for the available codefixes at a specific position. */ diff --git a/src/server/session.ts b/src/server/session.ts index 6ec234952db..6db0999ee62 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1425,29 +1425,40 @@ namespace ts.server { return project.getLanguageService().getApplicableRefactors(file, position || textRange); } - private getRefactorCodeActions(args: protocol.GetRefactorCodeActionsRequestArgs, simplifiedResult: boolean): protocol.RefactorCodeActions | protocol.RefactorCodeActionsFull { + private getEditsForRefactor(args: protocol.GetEditsForRefactorRequestArgs, simplifiedResult: boolean): ts.RefactorEditInfo | protocol.RefactorEditInfo { const { file, project } = this.getFileAndProjectWithoutRefreshingInferredProjects(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const { position, textRange } = this.extractPositionAndRange(args, scriptInfo); - const result: ts.CodeAction[] = project.getLanguageService().getRefactorCodeActions( + const result = project.getLanguageService().getEditsForRefactor( file, this.projectService.getFormatCodeOptions(), position || textRange, - args.refactorName + args.refactor, + args.action ); - if (simplifiedResult) { - // Not full + if (result === undefined) { return { - actions: result.map(action => this.mapCodeAction(action, scriptInfo)) + edits: [] + }; + } + + if (simplifiedResult) { + const file = result.renameFilename; + let location: ILineInfo | undefined = undefined; + if (file !== undefined && result.renameLocation !== undefined) { + const renameScriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(file)); + location = renameScriptInfo.positionToLineOffset(result.renameLocation); + } + return { + renameLocation: location, + renameFilename: file, + edits: result.edits.map(change => this.mapTextChangesToCodeEdits(project, change)) }; } else { - // Full - return { - actions: result - }; + return result; } } @@ -1505,6 +1516,14 @@ namespace ts.server { }; } + private mapTextChangesToCodeEdits(project: Project, textChanges: FileTextChanges): protocol.FileCodeEdits { + const scriptInfo = project.getScriptInfoForNormalizedPath(toNormalizedPath(textChanges.fileName)); + return { + fileName: textChanges.fileName, + textChanges: textChanges.textChanges.map(textChange => this.convertTextChangeToCodeEdit(textChange, scriptInfo)) + }; + } + private convertTextChangeToCodeEdit(change: ts.TextChange, scriptInfo: ScriptInfo): protocol.CodeEdit { return { start: scriptInfo.positionToLineOffset(change.span.start), @@ -1833,11 +1852,11 @@ namespace ts.server { [CommandNames.GetApplicableRefactors]: (request: protocol.GetApplicableRefactorsRequest) => { return this.requiredResponse(this.getApplicableRefactors(request.arguments)); }, - [CommandNames.GetRefactorCodeActions]: (request: protocol.GetRefactorCodeActionsRequest) => { - return this.requiredResponse(this.getRefactorCodeActions(request.arguments, /*simplifiedResult*/ true)); + [CommandNames.GetEditsForRefactor]: (request: protocol.GetEditsForRefactorRequest) => { + return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ true)); }, - [CommandNames.GetRefactorCodeActionsFull]: (request: protocol.GetRefactorCodeActionsRequest) => { - return this.requiredResponse(this.getRefactorCodeActions(request.arguments, /*simplifiedResult*/ false)); + [CommandNames.GetEditsForRefactorFull]: (request: protocol.GetEditsForRefactorRequest) => { + return this.requiredResponse(this.getEditsForRefactor(request.arguments, /*simplifiedResult*/ false)); } }); diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index 1058f9c2ca6..3c02ddf671c 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -8,10 +8,10 @@ namespace ts { description: string; /** Compute the associated code actions */ - getCodeActions(context: RefactorContext): CodeAction[]; + getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined; - /** A fast syntactic check to see if the refactor is applicable at given position. */ - isApplicable(context: RefactorContext): boolean; + /** Compute (quickly) which actions are available here */ + getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined; } export interface RefactorContext { @@ -34,7 +34,6 @@ namespace ts { } export function getApplicableRefactors(context: RefactorContext): ApplicableRefactorInfo[] | undefined { - let results: ApplicableRefactorInfo[]; const refactorList: Refactor[] = []; refactors.forEach(refactor => { @@ -44,16 +43,17 @@ namespace ts { if (context.cancellationToken && context.cancellationToken.isCancellationRequested()) { return results; } - if (refactor.isApplicable(context)) { - (results || (results = [])).push({ name: refactor.name, description: refactor.description }); + const infos = refactor.getAvailableActions(context); + if (infos && infos.length) { + (results || (results = [])).push(...infos); } } return results; } - export function getRefactorCodeActions(context: RefactorContext, refactorName: string): CodeAction[] | undefined { + export function getEditsForRefactor(context: RefactorContext, refactorName: string, actionName: string): RefactorEditInfo | undefined { const refactor = refactors.get(refactorName); - return refactor && refactor.getCodeActions(context); + return refactor && refactor.getEditsForAction(context, actionName); } } } diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/refactors/convertFunctionToEs6Class.ts index a34e0ccca2f..3c549463ec5 100644 --- a/src/services/refactors/convertFunctionToEs6Class.ts +++ b/src/services/refactors/convertFunctionToEs6Class.ts @@ -1,16 +1,18 @@ /* @internal */ namespace ts.refactor { + const actionName = "convert"; + const convertFunctionToES6Class: Refactor = { name: "Convert to ES2015 class", description: Diagnostics.Convert_function_to_an_ES2015_class.message, - getCodeActions, - isApplicable + getEditsForAction, + getAvailableActions }; registerRefactor(convertFunctionToES6Class); - function isApplicable(context: RefactorContext): boolean { + function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] { const start = context.startPosition; const node = getTokenAtPosition(context.file, start, /*includeJsDocComment*/ false); const checker = context.program.getTypeChecker(); @@ -20,10 +22,28 @@ namespace ts.refactor { symbol = (symbol.valueDeclaration as VariableDeclaration).initializer.symbol; } - return symbol && symbol.flags & SymbolFlags.Function && symbol.members && symbol.members.size > 0; + if (symbol && symbol.flags & SymbolFlags.Function && symbol.members && symbol.members.size > 0) { + return [ + { + name: convertFunctionToES6Class.name, + description: convertFunctionToES6Class.description, + actions: [ + { + description: convertFunctionToES6Class.description, + name: actionName + } + ] + } + ]; + } } - function getCodeActions(context: RefactorContext): CodeAction[] | undefined { + function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined { + // Somehow wrong action got invoked? + if (actionName !== action) { + return undefined; + } + const start = context.startPosition; const sourceFile = context.file; const checker = context.program.getTypeChecker(); @@ -35,7 +55,7 @@ namespace ts.refactor { const deletes: (() => any)[] = []; if (!(ctorSymbol.flags & (SymbolFlags.Function | SymbolFlags.Variable))) { - return []; + return undefined; } const ctorDeclaration = ctorSymbol.valueDeclaration; @@ -63,7 +83,7 @@ namespace ts.refactor { } if (!newClassDeclaration) { - return []; + return undefined; } // Because the preceding node could be touched, we need to insert nodes before delete nodes. @@ -72,10 +92,9 @@ namespace ts.refactor { deleteCallback(); } - return [{ - description: formatStringFromArgs(Diagnostics.Convert_function_0_to_class.message, [ctorSymbol.name]), - changes: changeTracker.getChanges() - }]; + return { + edits: changeTracker.getChanges() + }; function deleteNode(node: Node, inList = false) { if (deletedNodes.some(n => isNodeDescendantOf(node, n))) { diff --git a/src/services/services.ts b/src/services/services.ts index 21c756a9acc..876ef69dd72 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1989,15 +1989,16 @@ namespace ts { return refactor.getApplicableRefactors(getRefactorContext(file, positionOrRange)); } - function getRefactorCodeActions( + function getEditsForRefactor( fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, - refactorName: string): CodeAction[] | undefined { + refactorName: string, + actionName: string): RefactorEditInfo { synchronizeHostData(); const file = getValidSourceFile(fileName); - return refactor.getRefactorCodeActions(getRefactorContext(file, positionOrRange, formatOptions), refactorName); + return refactor.getEditsForRefactor(getRefactorContext(file, positionOrRange, formatOptions), refactorName, actionName); } return { @@ -2005,8 +2006,6 @@ namespace ts { cleanupSemanticCache, getSyntacticDiagnostics, getSemanticDiagnostics, - getApplicableRefactors, - getRefactorCodeActions, getCompilerOptionsDiagnostics, getSyntacticClassifications, getSemanticClassifications, @@ -2044,7 +2043,9 @@ namespace ts { getEmitOutput, getNonBoundSourceFile, getSourceFile, - getProgram + getProgram, + getApplicableRefactors, + getEditsForRefactor, }; } diff --git a/src/services/types.ts b/src/services/types.ts index e042276e650..0e5af40c57a 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -261,8 +261,9 @@ namespace ts { isValidBraceCompletionAtPosition(fileName: string, position: number, openingBrace: number): boolean; getCodeFixesAtPosition(fileName: string, start: number, end: number, errorCodes: number[], formatOptions: FormatCodeSettings): CodeAction[]; + getApplicableRefactors(fileName: string, positionOrRaneg: number | TextRange): ApplicableRefactorInfo[]; - getRefactorCodeActions(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string): CodeAction[] | undefined; + getEditsForRefactor(fileName: string, formatOptions: FormatCodeSettings, positionOrRange: number | TextRange, refactorName: string, actionName: string): RefactorEditInfo | undefined; getEmitOutput(fileName: string, emitOnlyDtsFiles?: boolean): EmitOutput; @@ -353,11 +354,60 @@ namespace ts { changes: FileTextChanges[]; } + /** + * A set of one or more available refactoring actions, grouped under a parent refactoring. + */ export interface ApplicableRefactorInfo { + /** + * The programmatic name of the refactoring + */ name: string; + /** + * A description of this refactoring category to show to the user. + * If the refactoring gets inlined (see below), this text will not be visible. + */ description: string; + /** + * Inlineable refactorings can have their actions hoisted out to the top level + * of a context menu. Non-inlineanable refactorings should always be shown inside + * their parent grouping. + * + * If not specified, this value is assumed to be 'true' + */ + inlineable?: boolean; + + actions: RefactorActionInfo[]; } + /** + * Represents a single refactoring action - for example, the "Extract Method..." refactor might + * offer several actions, each corresponding to a surround class or closure to extract into. + */ + export type RefactorActionInfo = { + /** + * The programmatic name of the refactoring action + */ + name: string; + + /** + * A description of this refactoring action to show to the user. + * If the parent refactoring is inlined away, this will be the only text shown, + * so this description should make sense by itself if the parent is inlineable=true + */ + description: string; + }; + + /** + * A set of edits to make in response to a refactor action, plus an optional + * location where renaming should be invoked from + */ + export type RefactorEditInfo = { + edits: FileTextChanges[]; + renameFilename?: string; + renameLocation?: number; + }; + + export interface TextInsertion { newText: string; /** The position in newText the caret should point to after the insertion. */ diff --git a/tests/cases/fourslash/convertFunctionToEs6Class1.ts b/tests/cases/fourslash/convertFunctionToEs6Class1.ts index 6275e48e627..51cc73fd64a 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class1.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class1.ts @@ -23,4 +23,4 @@ verify.fileAfterApplyingRefactorAtMarker('1', foo.prototype.instanceProp1 = "hello"; foo.prototype.instanceProp2 = undefined; foo.staticProp = "world"; -`, 'Convert to ES2015 class'); \ No newline at end of file +`, 'Convert to ES2015 class', 'convert'); \ No newline at end of file diff --git a/tests/cases/fourslash/convertFunctionToEs6Class2.ts b/tests/cases/fourslash/convertFunctionToEs6Class2.ts index 5d9a9f32585..d5ed277b452 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class2.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class2.ts @@ -24,4 +24,4 @@ verify.fileAfterApplyingRefactorAtMarker('4', foo.instanceProp1 = "hello"; foo.instanceProp2 = undefined; foo.staticProp = "world"; -`, 'Convert to ES2015 class'); +`, 'Convert to ES2015 class', 'convert'); diff --git a/tests/cases/fourslash/convertFunctionToEs6Class3.ts b/tests/cases/fourslash/convertFunctionToEs6Class3.ts index af8955dcb71..bb48ffadec2 100644 --- a/tests/cases/fourslash/convertFunctionToEs6Class3.ts +++ b/tests/cases/fourslash/convertFunctionToEs6Class3.ts @@ -25,4 +25,4 @@ class foo { foo.prototype.instanceProp1 = "hello"; foo.prototype.instanceProp2 = undefined; foo.staticProp = "world"; -`, 'Convert to ES2015 class'); \ No newline at end of file +`, 'Convert to ES2015 class', 'convert'); \ No newline at end of file diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 3e80b005625..928a9f0a0cd 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -236,8 +236,8 @@ declare namespace FourSlashInterface { noMatchingBracePositionInCurrentFile(bracePosition: number): void; DocCommentTemplate(expectedText: string, expectedOffset: number, empty?: boolean): void; noDocCommentTemplate(): void; - rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void - getAndApplyCodeFix(errorCode?: number, index?: number): void; + rangeAfterCodeFix(expectedText: string, includeWhiteSpace?: boolean, errorCode?: number, index?: number): void; + fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, actionName: string, formattingOptions?: FormatCodeOptions): void; rangeIs(expectedText: string, includeWhiteSpace?: boolean): void; fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: FormatCodeOptions): void; importFixAtPosition(expectedTextArray: string[], errorCode?: number): void; diff --git a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts index 9ab47086f20..5782b4a1f41 100644 --- a/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts +++ b/tests/cases/fourslash/server/convertFunctionToEs6Class-server.ts @@ -22,4 +22,4 @@ class fn { console.log('hello world'); } } -`, 'Convert to ES2015 class'); +`, 'Convert to ES2015 class', 'convert'); From f395bc1d831222b912d9d5db9e653ba5d17e9f4a Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 6 Jun 2017 16:01:53 -0700 Subject: [PATCH 03/23] Handle missing body case --- src/server/client.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/server/client.ts b/src/server/client.ts index e8142367f14..2cfe32f521a 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -732,6 +732,13 @@ namespace ts.server { const request = this.processRequest(CommandNames.GetEditsForRefactor, args); const response = this.processResponse(request); + + if (!response.body) { + return { + edits: [] + }; + } + const edits: FileTextChanges[] = this.convertCodeEditsToTextChanges(response.body.edits); const renameFilename: string | undefined = response.body.renameFilename; From f739f68231f59eee768b87ec3dcc179fa9fbcee0 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 6 Jun 2017 16:08:24 -0700 Subject: [PATCH 04/23] Tidy up --- src/server/protocol.ts | 2 +- src/services/refactors/convertFunctionToEs6Class.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index e22569e4654..f79774abf20 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -415,7 +415,7 @@ namespace ts.server.protocol { /** * Response is a list of available refactorings. - * Each refactoring exposes 1 or more "Actions"; a user selects one action to invoke a refactoring + * Each refactoring exposes one or more "Actions"; a user selects one action to invoke a refactoring */ export interface GetApplicableRefactorsResponse extends Response { body?: ApplicableRefactorInfo[]; diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/refactors/convertFunctionToEs6Class.ts index 3c549463ec5..d275924b76a 100644 --- a/src/services/refactors/convertFunctionToEs6Class.ts +++ b/src/services/refactors/convertFunctionToEs6Class.ts @@ -22,7 +22,7 @@ namespace ts.refactor { symbol = (symbol.valueDeclaration as VariableDeclaration).initializer.symbol; } - if (symbol && symbol.flags & SymbolFlags.Function && symbol.members && symbol.members.size > 0) { + if (symbol && (symbol.flags & SymbolFlags.Function) && symbol.members && (symbol.members.size > 0)) { return [ { name: convertFunctionToES6Class.name, From e3c4a7031d0ef7fd035367a7a46e55b3392f8ee0 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Tue, 6 Jun 2017 16:35:11 -0700 Subject: [PATCH 05/23] Reuse already-computed modification time --- src/server/server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/server.ts b/src/server/server.ts index 7e1ee683c8c..fa90e8df88d 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -526,7 +526,7 @@ namespace ts.server { watchedFile.callback(watchedFile.fileName); } else if (watchedFile.mtime.getTime() !== stats.mtime.getTime()) { - watchedFile.mtime = getModifiedTime(watchedFile.fileName); + watchedFile.mtime = stats.mtime; watchedFile.callback(watchedFile.fileName, watchedFile.mtime.getTime() === 0); } }); From f725d7de5f1709352ee712494b25ca5ec071ff41 Mon Sep 17 00:00:00 2001 From: Ryan Cavanaugh Date: Tue, 6 Jun 2017 17:43:30 -0700 Subject: [PATCH 06/23] Rename function --- scripts/buildProtocol.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/buildProtocol.ts b/scripts/buildProtocol.ts index 63da8bbece7..e03338bf60d 100644 --- a/scripts/buildProtocol.ts +++ b/scripts/buildProtocol.ts @@ -113,7 +113,7 @@ class DeclarationsWalker { } } -function generateProtocolFile(outputFile: string, protocolTs: string, typeScriptServicesDts: string) { +function writeProtocolFile(outputFile: string, protocolTs: string, typeScriptServicesDts: string) { const options = { target: ts.ScriptTarget.ES5, declaration: true, noResolve: true, types: [], stripInternal: true }; /** @@ -184,4 +184,4 @@ if (process.argv.length < 5) { const protocolTs = process.argv[2]; const typeScriptServicesDts = process.argv[3]; const outputFile = process.argv[4]; -generateProtocolFile(outputFile, protocolTs, typeScriptServicesDts); +writeProtocolFile(outputFile, protocolTs, typeScriptServicesDts); From ce1d1c81ef65907d9d70aa930b3bb0f6fbe53167 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Thu, 18 May 2017 11:08:07 -0700 Subject: [PATCH 07/23] Add support for external file references in source maps --- src/compiler/core.ts | 10 ++++++++- src/compiler/factory.ts | 15 ++++++++++--- src/compiler/scanner.ts | 2 +- src/compiler/sourcemap.ts | 47 +++++++++++++++++++++++++++------------ src/compiler/types.ts | 29 ++++++++++++++++-------- src/services/services.ts | 10 +++++++++ src/services/types.ts | 4 ++++ 7 files changed, 89 insertions(+), 28 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 1792a393234..747801f83bd 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2249,6 +2249,7 @@ namespace ts { getSymbolConstructor(): new (flags: SymbolFlags, name: string) => Symbol; getTypeConstructor(): new (checker: TypeChecker, flags: TypeFlags) => Type; getSignatureConstructor(): new (checker: TypeChecker) => Signature; + getSourceMapSourceConstructor(): new (fileName: string, text: string, skipTrivia?: (pos: number) => number) => SourceMapSource; } function Symbol(this: Symbol, flags: SymbolFlags, name: string) { @@ -2279,6 +2280,12 @@ namespace ts { this.original = undefined; } + function SourceMapSource(this: SourceMapSource, fileName: string, text: string, skipTrivia?: (pos: number) => number) { + this.fileName = fileName; + this.text = text; + this.skipTrivia = skipTrivia || (pos => pos); + } + export let objectAllocator: ObjectAllocator = { getNodeConstructor: () => Node, getTokenConstructor: () => Node, @@ -2286,7 +2293,8 @@ namespace ts { getSourceFileConstructor: () => Node, getSymbolConstructor: () => Symbol, getTypeConstructor: () => Type, - getSignatureConstructor: () => Signature + getSignatureConstructor: () => Signature, + getSourceMapSourceConstructor: () => SourceMapSource, }; export const enum AssertionLevel { diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 12e8110e7be..aaf7d958072 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -2314,15 +2314,24 @@ namespace ts { /** * Sets a custom text range to use when emitting source maps. */ - export function setSourceMapRange(node: T, range: TextRange | undefined) { + export function setSourceMapRange(node: T, range: SourceMapRange | undefined) { getOrCreateEmitNode(node).sourceMapRange = range; return node; } + let SourceMapSource: new (fileName: string, text: string, skipTrivia?: (pos: number) => number) => SourceMapSource; + + /** + * Create an external source map source file reference + */ + export function createSourceMapSource(fileName: string, text: string, skipTrivia?: (pos: number) => number): SourceMapSource { + return new (SourceMapSource || (SourceMapSource = objectAllocator.getSourceMapSourceConstructor()))(fileName, text, skipTrivia); + } + /** * Gets the TextRange to use for source maps for a token of a node. */ - export function getTokenSourceMapRange(node: Node, token: SyntaxKind): TextRange | undefined { + export function getTokenSourceMapRange(node: Node, token: SyntaxKind): SourceMapRange | undefined { const emitNode = node.emitNode; const tokenSourceMapRanges = emitNode && emitNode.tokenSourceMapRanges; return tokenSourceMapRanges && tokenSourceMapRanges[token]; @@ -2331,7 +2340,7 @@ namespace ts { /** * Sets the TextRange to use for source maps for a token of a node. */ - export function setTokenSourceMapRange(node: T, token: SyntaxKind, range: TextRange | undefined) { + export function setTokenSourceMapRange(node: T, token: SyntaxKind, range: SourceMapRange | undefined) { const emitNode = getOrCreateEmitNode(node); const tokenSourceMapRanges = emitNode.tokenSourceMapRanges || (emitNode.tokenSourceMapRanges = []); tokenSourceMapRanges[token] = range; diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 68a36c40a80..dd43b3c8263 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -363,7 +363,7 @@ namespace ts { }; } - export function getLineAndCharacterOfPosition(sourceFile: SourceFile, position: number): LineAndCharacter { + export function getLineAndCharacterOfPosition(sourceFile: SourceFileLike, position: number): LineAndCharacter { return computeLineAndCharacterOfPosition(getLineStarts(sourceFile), position); } diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index d743f488e75..ffef485581b 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -22,7 +22,7 @@ namespace ts { * * @param sourceFile The source file. */ - setSourceFile(sourceFile: SourceFile): void; + setSourceFile(sourceFile: SourceMapSource): void; /** * Emits a mapping. @@ -81,7 +81,7 @@ namespace ts { export function createSourceMapWriter(host: EmitHost, writer: EmitTextWriter): SourceMapWriter { const compilerOptions = host.getCompilerOptions(); const extendedDiagnostics = compilerOptions.extendedDiagnostics; - let currentSourceFile: SourceFile; + let currentSource: SourceMapSource; let currentSourceText: string; let sourceMapDir: string; // The directory in which sourcemap will be @@ -109,6 +109,13 @@ namespace ts { getSourceMappingURL, }; + /** + * Skips trivia such as comments and white-space that can optionally overriden by the source map source + */ + function skipSourceTrivia(pos: number): number { + return currentSource.skipTrivia ? currentSource.skipTrivia(pos) : skipTrivia(currentSourceText, pos); + } + /** * Initialize the SourceMapWriter for a new output file. * @@ -125,7 +132,7 @@ namespace ts { reset(); } - currentSourceFile = undefined; + currentSource = undefined; currentSourceText = undefined; // Current source map file and its index in the sources list @@ -192,7 +199,7 @@ namespace ts { return; } - currentSourceFile = undefined; + currentSource = undefined; sourceMapDir = undefined; sourceMapSourceIndex = undefined; lastRecordedSourceMapSpan = undefined; @@ -263,7 +270,7 @@ namespace ts { performance.mark("beforeSourcemap"); } - const sourceLinePos = getLineAndCharacterOfPosition(currentSourceFile, pos); + const sourceLinePos = getLineAndCharacterOfPosition(currentSource, pos); // Convert the location to be one-based. sourceLinePos.line++; @@ -320,14 +327,22 @@ namespace ts { if (node) { const emitNode = node.emitNode; const emitFlags = emitNode && emitNode.flags; - const { pos, end } = emitNode && emitNode.sourceMapRange || node; + const range = emitNode && emitNode.sourceMapRange; + const { pos, end } = range || node; + let source = range && range.source; + const oldSource = currentSource; + if (source === oldSource) source = undefined; + + if (source) setSourceFile(source); if (node.kind !== SyntaxKind.NotEmittedStatement && (emitFlags & EmitFlags.NoLeadingSourceMap) === 0 && pos >= 0) { - emitPos(skipTrivia(currentSourceText, pos)); + emitPos(skipSourceTrivia(pos)); } + if (source) setSourceFile(oldSource); + if (emitFlags & EmitFlags.NoNestedSourceMaps) { disabled = true; emitCallback(hint, node); @@ -337,11 +352,15 @@ namespace ts { emitCallback(hint, node); } + if (source) setSourceFile(source); + if (node.kind !== SyntaxKind.NotEmittedStatement && (emitFlags & EmitFlags.NoTrailingSourceMap) === 0 && end >= 0) { emitPos(end); } + + if (source) setSourceFile(oldSource); } } @@ -362,7 +381,7 @@ namespace ts { const emitFlags = emitNode && emitNode.flags; const range = emitNode && emitNode.tokenSourceMapRanges && emitNode.tokenSourceMapRanges[token]; - tokenPos = skipTrivia(currentSourceText, range ? range.pos : tokenPos); + tokenPos = skipSourceTrivia(range ? range.pos : tokenPos); if ((emitFlags & EmitFlags.NoTokenLeadingSourceMaps) === 0 && tokenPos >= 0) { emitPos(tokenPos); } @@ -382,13 +401,13 @@ namespace ts { * * @param sourceFile The source file. */ - function setSourceFile(sourceFile: SourceFile) { + function setSourceFile(sourceFile: SourceMapSource) { if (disabled) { return; } - currentSourceFile = sourceFile; - currentSourceText = currentSourceFile.text; + currentSource = sourceFile; + currentSourceText = currentSource.text; // Add the file to tsFilePaths // If sourceroot option: Use the relative path corresponding to the common directory path @@ -396,7 +415,7 @@ namespace ts { const sourcesDirectoryPath = compilerOptions.sourceRoot ? host.getCommonSourceDirectory() : sourceMapDir; const source = getRelativePathToDirectoryOrUrl(sourcesDirectoryPath, - currentSourceFile.fileName, + currentSource.fileName, host.getCurrentDirectory(), host.getCanonicalFileName, /*isAbsolutePathAnUrl*/ true); @@ -407,10 +426,10 @@ namespace ts { sourceMapData.sourceMapSources.push(source); // The one that can be used from program to get the actual source file - sourceMapData.inputSourceFileNames.push(currentSourceFile.fileName); + sourceMapData.inputSourceFileNames.push(currentSource.fileName); if (compilerOptions.inlineSources) { - sourceMapData.sourceMapSourcesContent.push(currentSourceFile.text); + sourceMapData.sourceMapSourcesContent.push(currentSource.text); } } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 1c7f36bb27b..40b3662e336 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4008,18 +4008,29 @@ namespace ts { ES2015FunctionSyntaxMask = ContainsCapturedLexicalThis | ContainsDefaultValueAssignments, } + export interface SourceMapRange extends TextRange { + source?: SourceMapSource; + } + + export interface SourceMapSource { + fileName: string; + text: string; + /* @internal */ lineMap: number[]; + skipTrivia?: (pos: number) => number; + } + /* @internal */ export interface EmitNode { - annotatedNodes?: Node[]; // Tracks Parse-tree nodes with EmitNodes for eventual cleanup. - flags?: EmitFlags; // Flags that customize emit - leadingComments?: SynthesizedComment[]; // Synthesized leading comments + annotatedNodes?: Node[]; // Tracks Parse-tree nodes with EmitNodes for eventual cleanup. + flags?: EmitFlags; // Flags that customize emit + leadingComments?: SynthesizedComment[]; // Synthesized leading comments trailingComments?: SynthesizedComment[]; // Synthesized trailing comments - commentRange?: TextRange; // The text range to use when emitting leading or trailing comments - sourceMapRange?: TextRange; // The text range to use when emitting leading or trailing source mappings - tokenSourceMapRanges?: TextRange[]; // The text range to use when emitting source mappings for tokens - constantValue?: string | number; // The constant value of an expression - externalHelpersModuleName?: Identifier; // The local name for an imported helpers module - helpers?: EmitHelper[]; // Emit helpers for the node + commentRange?: TextRange; // The text range to use when emitting leading or trailing comments + sourceMapRange?: SourceMapRange; // The text range to use when emitting leading or trailing source mappings + tokenSourceMapRanges?: SourceMapRange[]; // The text range to use when emitting source mappings for tokens + constantValue?: string | number; // The constant value of an expression + externalHelpersModuleName?: Identifier; // The local name for an imported helpers module + helpers?: EmitHelper[]; // Emit helpers for the node } export const enum EmitFlags { diff --git a/src/services/services.ts b/src/services/services.ts index 21c756a9acc..baba64586e9 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -732,6 +732,15 @@ namespace ts { } } + class SourceMapSourceObject implements SourceMapSource { + lineMap: number[]; + constructor (public fileName: string, public text: string, public skipTrivia?: (pos: number) => number) {} + + public getLineAndCharacterOfPosition(pos: number): LineAndCharacter { + return ts.getLineAndCharacterOfPosition(this, pos); + } + } + function getServicesObjectAllocator(): ObjectAllocator { return { getNodeConstructor: () => NodeObject, @@ -742,6 +751,7 @@ namespace ts { getSymbolConstructor: () => SymbolObject, getTypeConstructor: () => TypeObject, getSignatureConstructor: () => SignatureObject, + getSourceMapSourceConstructor: () => SourceMapSourceObject, }; } diff --git a/src/services/types.ts b/src/services/types.ts index e042276e650..49fe2262605 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -71,6 +71,10 @@ namespace ts { getLineAndCharacterOfPosition(pos: number): LineAndCharacter; } + export interface SourceMapSource { + getLineAndCharacterOfPosition(pos: number): LineAndCharacter; + } + /** * Represents an immutable snapshot of a script at a specified time.Once acquired, the * snapshot is observably immutable. i.e. the same calls with the same parameters will return From 471e680ef087ce789698162e7c0ee74d585b8859 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Tue, 6 Jun 2017 18:10:00 -0700 Subject: [PATCH 08/23] 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 09/23] 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 10/23] 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 68122ea4cc9a16dc39276dc5c5a30033aeb327e0 Mon Sep 17 00:00:00 2001 From: Andy Date: Wed, 7 Jun 2017 11:16:12 -0700 Subject: [PATCH 11/23] Support find-all-references for `export =` of class expression (#16327) * Support find-all-references for `export =` of class expression * Add comments --- src/services/findAllReferences.ts | 9 +-- src/services/importTracker.ts | 74 +++++++++++++------ .../fourslash/findAllRefsClassExpression0.ts | 16 ++++ .../fourslash/findAllRefsClassExpression1.ts | 17 +++++ .../fourslash/findAllRefsClassExpression2.ts | 16 ++++ 5 files changed, 103 insertions(+), 29 deletions(-) create mode 100644 tests/cases/fourslash/findAllRefsClassExpression0.ts create mode 100644 tests/cases/fourslash/findAllRefsClassExpression1.ts create mode 100644 tests/cases/fourslash/findAllRefsClassExpression2.ts diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index c1a7408fae7..67a967ec8b9 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -496,11 +496,10 @@ namespace ts.FindAllReferences.Core { const { text = stripQuotes(getDeclaredName(this.checker, symbol, location)), allSearchSymbols = undefined } = searchOptions; const escapedText = escapeIdentifier(text); const parents = this.options.implementations && getParentSymbolsOfPropertyAccess(location, symbol, this.checker); - return { location, symbol, comingFrom, text, escapedText, parents, includes }; - - function includes(referenceSymbol: Symbol): boolean { - return allSearchSymbols ? contains(allSearchSymbols, referenceSymbol) : referenceSymbol === symbol; - } + return { + location, symbol, comingFrom, text, escapedText, parents, + includes: referenceSymbol => allSearchSymbols ? contains(allSearchSymbols, referenceSymbol) : referenceSymbol === symbol, + }; } private readonly symbolIdToReferences: Entry[][] = []; diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index 585af7ebd31..6a0b9167997 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -436,8 +436,8 @@ namespace ts.FindAllReferences { if (parent.kind === SyntaxKind.PropertyAccessExpression) { // When accessing an export of a JS module, there's no alias. The symbol will still be flagged as an export even though we're at the use. // So check that we are at the declaration. - return symbol.declarations.some(d => d === parent) && parent.parent.kind === ts.SyntaxKind.BinaryExpression - ? getSpecialPropertyExport(parent.parent as ts.BinaryExpression, /*useLhsSymbol*/ false) + return symbol.declarations.some(d => d === parent) && isBinaryExpression(parent.parent) + ? getSpecialPropertyExport(parent.parent, /*useLhsSymbol*/ false) : undefined; } else { @@ -449,31 +449,41 @@ namespace ts.FindAllReferences { else { const exportNode = getExportNode(parent); if (exportNode && hasModifier(exportNode, ModifierFlags.Export)) { - if (exportNode.kind === SyntaxKind.ImportEqualsDeclaration && (exportNode as ImportEqualsDeclaration).moduleReference === node) { + if (isImportEqualsDeclaration(exportNode) && exportNode.moduleReference === node) { // We're at `Y` in `export import X = Y`. This is not the exported symbol, the left-hand-side is. So treat this as an import statement. if (comingFromExport) { return undefined; } - const lhsSymbol = checker.getSymbolAtLocation((exportNode as ImportEqualsDeclaration).name); + const lhsSymbol = checker.getSymbolAtLocation(exportNode.name); return { kind: ImportExport.Import, symbol: lhsSymbol, isNamedImport: false }; } else { return exportInfo(symbol, getExportKindForDeclaration(exportNode)); } } - else if (parent.kind === SyntaxKind.ExportAssignment) { - // Get the symbol for the `export =` node; its parent is the module it's the export of. - const exportingModuleSymbol = parent.symbol.parent; - Debug.assert(!!exportingModuleSymbol); - return { kind: ImportExport.Export, symbol, exportInfo: { exportingModuleSymbol, exportKind: ExportKind.ExportEquals } }; + // If we are in `export = a;`, `parent` is the export assignment. + else if (isExportAssignment(parent)) { + return getExportAssignmentExport(parent); } - else if (parent.kind === ts.SyntaxKind.BinaryExpression) { - return getSpecialPropertyExport(parent as ts.BinaryExpression, /*useLhsSymbol*/ true); + // If we are in `export = class A {};` at `A`, `parent.parent` is the export assignment. + else if (isExportAssignment(parent.parent)) { + return getExportAssignmentExport(parent.parent); } - else if (parent.parent.kind === SyntaxKind.BinaryExpression) { - return getSpecialPropertyExport(parent.parent as ts.BinaryExpression, /*useLhsSymbol*/ true); + // Similar for `module.exports =` and `exports.A =`. + else if (isBinaryExpression(parent)) { + return getSpecialPropertyExport(parent, /*useLhsSymbol*/ true); } + else if (isBinaryExpression(parent.parent)) { + return getSpecialPropertyExport(parent.parent, /*useLhsSymbol*/ true); + } + } + + function getExportAssignmentExport(ex: ExportAssignment): ExportedSymbol { + // Get the symbol for the `export =` node; its parent is the module it's the export of. + const exportingModuleSymbol = ex.symbol.parent; + Debug.assert(!!exportingModuleSymbol); + return { kind: ImportExport.Export, symbol, exportInfo: { exportingModuleSymbol, exportKind: ExportKind.ExportEquals } }; } function getSpecialPropertyExport(node: ts.BinaryExpression, useLhsSymbol: boolean): ExportedSymbol | undefined { @@ -496,21 +506,21 @@ namespace ts.FindAllReferences { function getImport(): ImportedSymbol | undefined { const isImport = isNodeImport(node); - if (!isImport) return; + if (!isImport) return undefined; // A symbol being imported is always an alias. So get what that aliases to find the local symbol. let importedSymbol = checker.getImmediateAliasedSymbol(symbol); - if (importedSymbol) { - // Search on the local symbol in the exporting module, not the exported symbol. - importedSymbol = skipExportSpecifierSymbol(importedSymbol, checker); - // Similarly, skip past the symbol for 'export =' - if (importedSymbol.name === "export=") { - importedSymbol = checker.getImmediateAliasedSymbol(importedSymbol); - } + if (!importedSymbol) return undefined; - if (symbolName(importedSymbol) === symbol.name) { // If this is a rename import, do not continue searching. - return { kind: ImportExport.Import, symbol: importedSymbol, ...isImport }; - } + // Search on the local symbol in the exporting module, not the exported symbol. + importedSymbol = skipExportSpecifierSymbol(importedSymbol, checker); + // Similarly, skip past the symbol for 'export =' + if (importedSymbol.name === "export=") { + importedSymbol = getExportEqualsLocalSymbol(importedSymbol, checker); + } + + if (symbolName(importedSymbol) === symbol.name) { // If this is a rename import, do not continue searching. + return { kind: ImportExport.Import, symbol: importedSymbol, ...isImport }; } } @@ -525,6 +535,22 @@ namespace ts.FindAllReferences { } } + function getExportEqualsLocalSymbol(importedSymbol: Symbol, checker: TypeChecker): Symbol { + if (importedSymbol.flags & SymbolFlags.Alias) { + return checker.getImmediateAliasedSymbol(importedSymbol); + } + + const decl = importedSymbol.valueDeclaration; + if (isExportAssignment(decl)) { // `export = class {}` + return decl.expression.symbol; + } + else if (isBinaryExpression(decl)) { // `module.exports = class {}` + return decl.right.symbol; + } + Debug.fail(); + } + + // If a reference is a class expression, the exported node would be its parent. // If a reference is a variable declaration, the exported node would be the variable statement. function getExportNode(parent: Node): Node | undefined { if (parent.kind === SyntaxKind.VariableDeclaration) { diff --git a/tests/cases/fourslash/findAllRefsClassExpression0.ts b/tests/cases/fourslash/findAllRefsClassExpression0.ts new file mode 100644 index 00000000000..50abfae0230 --- /dev/null +++ b/tests/cases/fourslash/findAllRefsClassExpression0.ts @@ -0,0 +1,16 @@ +/// + +// @Filename: /a.ts +////export = class [|{| "isWriteAccess": true, "isDefinition": true |}A|] { +//// m() { [|A|]; } +////}; + +// @Filename: /b.ts +////import [|{| "isWriteAccess": true, "isDefinition": true |}A|] = require("./a"); +////[|A|]; + +const [r0, r1, r2, r3] = test.ranges(); +const defs = { definition: "(local class) A", ranges: [r0, r1] }; +const imports = { definition: 'import A = require("./a")', ranges: [r2, r3] }; +verify.referenceGroups([r0, r1], [defs, imports]); +verify.referenceGroups([r2, r3], [imports, defs]); diff --git a/tests/cases/fourslash/findAllRefsClassExpression1.ts b/tests/cases/fourslash/findAllRefsClassExpression1.ts new file mode 100644 index 00000000000..bd581871842 --- /dev/null +++ b/tests/cases/fourslash/findAllRefsClassExpression1.ts @@ -0,0 +1,17 @@ +/// + +// @allowJs: true + +// @Filename: /a.js +////module.exports = class [|{| "isWriteAccess": true, "isDefinition": true |}A|] {}; + +// @Filename: /b.js +////import [|{| "isWriteAccess": true, "isDefinition": true |}A|] = require("./a"); +////[|A|]; + +const [r0, r1, r2] = test.ranges(); +const defs = { definition: "(local class) A", ranges: [r0] }; +const imports = { definition: 'import A = require("./a")', ranges: [r1, r2] }; +verify.referenceGroups([r0], [defs, imports]); +verify.referenceGroups([r1, r2], [imports, defs]); + diff --git a/tests/cases/fourslash/findAllRefsClassExpression2.ts b/tests/cases/fourslash/findAllRefsClassExpression2.ts new file mode 100644 index 00000000000..ce2fc8bbf3d --- /dev/null +++ b/tests/cases/fourslash/findAllRefsClassExpression2.ts @@ -0,0 +1,16 @@ +/// + +// @allowJs: true + +// @Filename: /a.js +////exports.[|{| "isWriteAccess": true, "isDefinition": true |}A|] = class {}; + +// @Filename: /b.js +////import { [|{| "isWriteAccess": true, "isDefinition": true |}A|] } from "./a"; +////[|A|]; + +const [r0, r1, r2] = test.ranges(); +const defs = { definition: "(property) A: typeof (Anonymous class)", ranges: [r0] }; +const imports = { definition: "import A", ranges: [r1, r2] }; +verify.referenceGroups([r0], [defs, imports]); +verify.referenceGroups([r1, r2], [imports, defs]); From 4e927bdbd4486e8e5ca965aba94082ef2699c3d4 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 11:24:19 -0700 Subject: [PATCH 12/23] 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 13/23] 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 14/23] 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 15/23] 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 16/23] 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 10beac2c1cc849348421cf70b74a81b2b38e1126 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 12:58:36 -0700 Subject: [PATCH 17/23] Delay instantiation of signature return type `getReturnTypeOfSignature` correctly handles an un-instantiated signature, but `instantiateSignature` used to eagerly instantiate the return type. This caused an infinite recursion in #16233. Now `instantiateSignature` doesn't instantiate the return type, but relies on `getReturnTypeOfSignature` to do it. --- 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 bde5e050fb1..4df03b95d48 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8038,7 +8038,7 @@ namespace ts { const result = createSignature(signature.declaration, freshTypeParameters, signature.thisParameter && instantiateSymbol(signature.thisParameter, mapper), instantiateList(signature.parameters, mapper, instantiateSymbol), - instantiateType(signature.resolvedReturnType, mapper), + /*resolvedReturnType*/ undefined, freshTypePredicate, signature.minArgumentCount, signature.hasRestParameter, signature.hasLiteralTypes); result.target = signature; From c0b8c217b1da47d552f9f04c79e7c89584d8c848 Mon Sep 17 00:00:00 2001 From: Nathan Shively-Sanders Date: Wed, 7 Jun 2017 13:27:31 -0700 Subject: [PATCH 18/23] Test returning an infinite type in an intersection --- .../reference/returnInfiniteIntersection.js | 15 +++++++++++ .../returnInfiniteIntersection.symbols | 21 +++++++++++++++ .../returnInfiniteIntersection.types | 27 +++++++++++++++++++ .../compiler/returnInfiniteIntersection.ts | 6 +++++ 4 files changed, 69 insertions(+) create mode 100644 tests/baselines/reference/returnInfiniteIntersection.js create mode 100644 tests/baselines/reference/returnInfiniteIntersection.symbols create mode 100644 tests/baselines/reference/returnInfiniteIntersection.types create mode 100644 tests/cases/compiler/returnInfiniteIntersection.ts diff --git a/tests/baselines/reference/returnInfiniteIntersection.js b/tests/baselines/reference/returnInfiniteIntersection.js new file mode 100644 index 00000000000..db77d21f32d --- /dev/null +++ b/tests/baselines/reference/returnInfiniteIntersection.js @@ -0,0 +1,15 @@ +//// [returnInfiniteIntersection.ts] +function recursive() { + let x = (subkey: T) => recursive(); + return x as typeof x & { p }; +} + +let result = recursive()(1) + + +//// [returnInfiniteIntersection.js] +function recursive() { + var x = function (subkey) { return recursive(); }; + return x; +} +var result = recursive()(1); diff --git a/tests/baselines/reference/returnInfiniteIntersection.symbols b/tests/baselines/reference/returnInfiniteIntersection.symbols new file mode 100644 index 00000000000..125861ac78d --- /dev/null +++ b/tests/baselines/reference/returnInfiniteIntersection.symbols @@ -0,0 +1,21 @@ +=== tests/cases/compiler/returnInfiniteIntersection.ts === +function recursive() { +>recursive : Symbol(recursive, Decl(returnInfiniteIntersection.ts, 0, 0)) + + let x = (subkey: T) => recursive(); +>x : Symbol(x, Decl(returnInfiniteIntersection.ts, 1, 7)) +>T : Symbol(T, Decl(returnInfiniteIntersection.ts, 1, 13)) +>subkey : Symbol(subkey, Decl(returnInfiniteIntersection.ts, 1, 16)) +>T : Symbol(T, Decl(returnInfiniteIntersection.ts, 1, 13)) +>recursive : Symbol(recursive, Decl(returnInfiniteIntersection.ts, 0, 0)) + + return x as typeof x & { p }; +>x : Symbol(x, Decl(returnInfiniteIntersection.ts, 1, 7)) +>x : Symbol(x, Decl(returnInfiniteIntersection.ts, 1, 7)) +>p : Symbol(p, Decl(returnInfiniteIntersection.ts, 2, 28)) +} + +let result = recursive()(1) +>result : Symbol(result, Decl(returnInfiniteIntersection.ts, 5, 3)) +>recursive : Symbol(recursive, Decl(returnInfiniteIntersection.ts, 0, 0)) + diff --git a/tests/baselines/reference/returnInfiniteIntersection.types b/tests/baselines/reference/returnInfiniteIntersection.types new file mode 100644 index 00000000000..e7ca48a69d1 --- /dev/null +++ b/tests/baselines/reference/returnInfiniteIntersection.types @@ -0,0 +1,27 @@ +=== tests/cases/compiler/returnInfiniteIntersection.ts === +function recursive() { +>recursive : () => ((subkey: T) => any & { p: any; }) & { p: any; } + + let x = (subkey: T) => recursive(); +>x : (subkey: T) => any & { p: any; } +>(subkey: T) => recursive() : (subkey: T) => any & { p: any; } +>T : T +>subkey : T +>T : T +>recursive() : ((subkey: T) => any & { p: any; }) & { p: any; } +>recursive : () => ((subkey: T) => any & { p: any; }) & { p: any; } + + return x as typeof x & { p }; +>x as typeof x & { p } : ((subkey: T) => any & { p: any; }) & { p: any; } +>x : (subkey: T) => any & { p: any; } +>x : (subkey: T) => any & { p: any; } +>p : any +} + +let result = recursive()(1) +>result : ((subkey: T) => any & { p: any; }) & { p: any; } +>recursive()(1) : ((subkey: T) => any & { p: any; }) & { p: any; } +>recursive() : ((subkey: T) => any & { p: any; }) & { p: any; } +>recursive : () => ((subkey: T) => any & { p: any; }) & { p: any; } +>1 : 1 + diff --git a/tests/cases/compiler/returnInfiniteIntersection.ts b/tests/cases/compiler/returnInfiniteIntersection.ts new file mode 100644 index 00000000000..3bb0d1fcc79 --- /dev/null +++ b/tests/cases/compiler/returnInfiniteIntersection.ts @@ -0,0 +1,6 @@ +function recursive() { + let x = (subkey: T) => recursive(); + return x as typeof x & { p }; +} + +let result = recursive()(1) From d3d917584162d9afa0e33397c8bd403e636f95ed Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Wed, 7 Jun 2017 14:13:30 -0700 Subject: [PATCH 19/23] 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 20/23] 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 21/23] 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 22/23] 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 23/23] 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