From d5f246fd9919072338e0d31331e1fd7276eebd7c Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 18 Apr 2017 15:48:01 -0700 Subject: [PATCH 01/28] Reuse module resolutions in unmodified files --- src/compiler/program.ts | 180 +++++++++++++++++++++++++--------------- 1 file changed, 114 insertions(+), 66 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index f2d44adb1f1..8081775b079 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -6,6 +6,23 @@ namespace ts { const emptyArray: any[] = []; const ignoreDiagnosticCommentRegEx = /(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?)/; + + const enum StructuralChangesFromOldProgram { + None = 0, + NoOldProgram = 1, + ModuleResolutionOptions = 2, + RootNames = 3, + TypesOptions = 4, + SourceFileRemoved = 5, + HasNoDefaultLib = 6, + TripleSlashReferences = 7, + Imports = 8, + ModuleAugmentations = 9, + TripleSlashTypesReferences = 10, + + CannotReuseResolution = NoOldProgram | ModuleResolutionOptions | RootNames | TypesOptions | SourceFileRemoved + } + export function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName = "tsconfig.json"): string { while (true) { const fileName = combinePaths(searchPath, configName); @@ -298,6 +315,7 @@ namespace ts { let diagnosticsProducingTypeChecker: TypeChecker; let noDiagnosticsTypeChecker: TypeChecker; let classifiableNames: Map; + let modifiedFilePaths: Path[] | undefined; const cachedSemanticDiagnosticsForFile: DiagnosticCache = {}; const cachedDeclarationDiagnosticsForFile: DiagnosticCache = {}; @@ -367,7 +385,11 @@ namespace ts { // used to track cases when two file names differ only in casing const filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap(fileName => fileName.toLowerCase()) : undefined; - if (!tryReuseStructureFromOldProgram()) { + const structuralChanges = tryReuseStructureFromOldProgram(); + if (structuralChanges === StructuralChangesFromOldProgram.None) { + Debug.assert(oldProgram.structureIsReused === true); + } + else { forEach(rootNames, name => processRootFile(name, /*isDefaultLib*/ false)); // load type declarations specified via 'types' argument or implicitly from types/ and node_modules/@types folders @@ -476,18 +498,40 @@ namespace ts { } interface OldProgramState { - program: Program; + program: Program | undefined; file: SourceFile; modifiedFilePaths: Path[]; } - function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile, oldProgramState?: OldProgramState) { - if (!oldProgramState && !file.ambientModuleNames.length) { - // if old program state is not supplied and file does not contain locally defined ambient modules - // then the best we can do is fallback to the default logic + function createOldProgramState( + program: Program | undefined, + file: SourceFile, + modifiedFilePaths: Path[]): OldProgramState { + return { program, file, modifiedFilePaths }; + } + + function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile, oldProgramState: OldProgramState) { + if (structuralChanges & StructuralChangesFromOldProgram.CannotReuseResolution && !file.ambientModuleNames.length) { + // If the old program state does not permit reusing resolutions and `file` does not contain locally defined ambient modules, + // the best we can do is fallback to the default logic. return resolveModuleNamesWorker(moduleNames, containingFile); } + + const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); + if (oldSourceFile === file) { + const oldSourceFileResult: ResolvedModuleFull[] = []; + for (const moduleName of moduleNames) { + const resolvedModule = oldSourceFile.resolvedModules.get(moduleName); + oldSourceFileResult.push(resolvedModule); + } + return oldSourceFileResult; + } + + // if options didn't change but there were changes in module names/triple slash/etc., + // rty to get file with smae name form old program, if equal, then we can just copy resolutions verbatim. + // else the file was edited. Then we just redo the reoslutions for the whole file (could go further, but kiss for now). + // at this point we know that either // - file has local declarations for ambient modules // OR @@ -535,7 +579,7 @@ namespace ts { // since they are known to be unknown unknownModuleNames = moduleNames.slice(0, i); } - // mark prediced resolution in the result list + // Mark predicted resolution in the result list. result[i] = predictedToResolveToAmbientModuleMarker; } else if (unknownModuleNames) { @@ -568,16 +612,13 @@ namespace ts { Debug.assert(j === resolutions.length); return result; - function checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState?: OldProgramState): boolean { - if (!oldProgramState) { - return false; - } + function checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean { const resolutionToFile = getResolvedModule(oldProgramState.file, moduleName); if (resolutionToFile) { // module used to be resolved to file - ignore it return false; } - const ambientModule = oldProgram.getTypeChecker().tryFindAmbientModuleWithoutAugmentations(moduleName); + const ambientModule = oldProgramState.program && oldProgramState.program.getTypeChecker().tryFindAmbientModuleWithoutAugmentations(moduleName); if (!(ambientModule && ambientModule.declarations)) { return false; } @@ -599,16 +640,16 @@ namespace ts { } } - function tryReuseStructureFromOldProgram(): boolean { + function tryReuseStructureFromOldProgram(): StructuralChangesFromOldProgram { if (!oldProgram) { - return false; + return StructuralChangesFromOldProgram.NoOldProgram; } // check properties that can affect structure of the program or module resolution strategy // if any of these properties has changed - structure cannot be reused const oldOptions = oldProgram.getCompilerOptions(); if (changesAffectModuleResolution(oldOptions, options)) { - return false; + return StructuralChangesFromOldProgram.ModuleResolutionOptions; } Debug.assert(!oldProgram.structureIsReused); @@ -616,17 +657,18 @@ namespace ts { // there is an old program, check if we can reuse its structure const oldRootNames = oldProgram.getRootFileNames(); if (!arrayIsEqualTo(oldRootNames, rootNames)) { - return false; + return StructuralChangesFromOldProgram.RootNames; } if (!arrayIsEqualTo(options.types, oldOptions.types)) { - return false; + return StructuralChangesFromOldProgram.TypesOptions; } // check if program source files has changed in the way that can affect structure of the program const newSourceFiles: SourceFile[] = []; const filePaths: Path[] = []; const modifiedSourceFiles: { oldFile: SourceFile, newFile: SourceFile }[] = []; + let structuralChanges = StructuralChangesFromOldProgram.None; for (const oldSourceFile of oldProgram.getSourceFiles()) { let newSourceFile = host.getSourceFileByPath @@ -634,64 +676,70 @@ namespace ts { : host.getSourceFile(oldSourceFile.fileName, options.target); if (!newSourceFile) { - return false; - } - - newSourceFile.path = oldSourceFile.path; - filePaths.push(newSourceFile.path); - - if (oldSourceFile !== newSourceFile) { - if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { - // value of no-default-lib has changed - // this will affect if default library is injected into the list of files - return false; - } - - // check tripleslash references - if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) { - // tripleslash references has changed - return false; - } - - // check imports and module augmentations - collectExternalModuleReferences(newSourceFile); - if (!arrayIsEqualTo(oldSourceFile.imports, newSourceFile.imports, moduleNameIsEqualTo)) { - // imports has changed - return false; - } - if (!arrayIsEqualTo(oldSourceFile.moduleAugmentations, newSourceFile.moduleAugmentations, moduleNameIsEqualTo)) { - // moduleAugmentations has changed - return false; - } - - if (!arrayIsEqualTo(oldSourceFile.typeReferenceDirectives, newSourceFile.typeReferenceDirectives, fileReferenceIsEqualTo)) { - // 'types' references has changed - return false; - } - - // tentatively approve the file - modifiedSourceFiles.push({ oldFile: oldSourceFile, newFile: newSourceFile }); + structuralChanges |= StructuralChangesFromOldProgram.SourceFileRemoved; } else { - // file has no changes - use it as is - newSourceFile = oldSourceFile; - } + newSourceFile.path = oldSourceFile.path; + filePaths.push(newSourceFile.path); - // if file has passed all checks it should be safe to reuse it - newSourceFiles.push(newSourceFile); + if (oldSourceFile !== newSourceFile) { + if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { + // value of no-default-lib has changed + // this will affect if default library is injected into the list of files + structuralChanges |= StructuralChangesFromOldProgram.HasNoDefaultLib; + } + + // check tripleslash references + if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) { + // tripleslash references has changed + structuralChanges |= StructuralChangesFromOldProgram.TripleSlashReferences; + } + + // check imports and module augmentations + collectExternalModuleReferences(newSourceFile); + if (!arrayIsEqualTo(oldSourceFile.imports, newSourceFile.imports, moduleNameIsEqualTo)) { + // imports has changed + structuralChanges |= StructuralChangesFromOldProgram.Imports; + } + if (!arrayIsEqualTo(oldSourceFile.moduleAugmentations, newSourceFile.moduleAugmentations, moduleNameIsEqualTo)) { + // moduleAugmentations has changed + structuralChanges |= StructuralChangesFromOldProgram.ModuleAugmentations; + } + + if (!arrayIsEqualTo(oldSourceFile.typeReferenceDirectives, newSourceFile.typeReferenceDirectives, fileReferenceIsEqualTo)) { + // 'types' references has changed + structuralChanges |= StructuralChangesFromOldProgram.TripleSlashTypesReferences; + } + + // tentatively approve the file + modifiedSourceFiles.push({ oldFile: oldSourceFile, newFile: newSourceFile }); + } + else { + // file has no changes - use it as is + newSourceFile = oldSourceFile; + } + + // if file has passed all checks it should be safe to reuse it + newSourceFiles.push(newSourceFile); + } } - const modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path); + if (structuralChanges !== StructuralChangesFromOldProgram.None) { + return structuralChanges; + } + + modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path); // try to verify results of module resolution for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory); if (resolveModuleNamesWorker) { const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral); - const resolutions = resolveModuleNamesReusingOldState(moduleNames, newSourceFilePath, newSourceFile, { file: oldSourceFile, program: oldProgram, modifiedFilePaths }); + const oldProgramState = createOldProgramState(oldProgram, oldSourceFile, modifiedFilePaths); + const resolutions = resolveModuleNamesReusingOldState(moduleNames, newSourceFilePath, newSourceFile, oldProgramState); // ensure that module resolution results are still correct const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo); if (resolutionsChanged) { - return false; + return StructuralChangesFromOldProgram.Imports | StructuralChangesFromOldProgram.ModuleAugmentations; } } if (resolveTypeReferenceDirectiveNamesWorker) { @@ -700,7 +748,7 @@ namespace ts { // ensure that types resolutions are still correct const resolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, resolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, typeDirectiveIsEqualTo); if (resolutionsChanged) { - return false; + return StructuralChangesFromOldProgram.TripleSlashTypesReferences; } } // pass the cache of module/types resolutions from the old source file @@ -722,7 +770,7 @@ namespace ts { resolvedTypeReferenceDirectives = oldProgram.getResolvedTypeReferenceDirectives(); oldProgram.structureIsReused = true; - return true; + return StructuralChangesFromOldProgram.None; } function getEmitHost(writeFileCallback?: WriteFileCallback): EmitHost { @@ -1519,11 +1567,11 @@ namespace ts { function processImportedModules(file: SourceFile) { collectExternalModuleReferences(file); if (file.imports.length || file.moduleAugmentations.length) { - file.resolvedModules = createMap(); // Because global augmentation doesn't have string literal name, we can check for global augmentation as such. const nonGlobalAugmentation = filter(file.moduleAugmentations, (moduleAugmentation) => moduleAugmentation.kind === SyntaxKind.StringLiteral); const moduleNames = map(concatenate(file.imports, nonGlobalAugmentation), getTextOfLiteral); - const resolutions = resolveModuleNamesReusingOldState(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory), file); + const oldProgramState = createOldProgramState(oldProgram, file, modifiedFilePaths); + const resolutions = resolveModuleNamesReusingOldState(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory), file, oldProgramState); Debug.assert(resolutions.length === moduleNames.length); for (let i = 0; i < moduleNames.length; i++) { const resolution = resolutions[i]; From 77f9099423fd2ca36ac5908cfabed1d20ef8ffbb Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 18 Apr 2017 16:15:39 -0700 Subject: [PATCH 02/28] Remove WIP comment --- src/compiler/program.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 8081775b079..d2e257867f4 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -527,11 +527,6 @@ namespace ts { } return oldSourceFileResult; } - - // if options didn't change but there were changes in module names/triple slash/etc., - // rty to get file with smae name form old program, if equal, then we can just copy resolutions verbatim. - // else the file was edited. Then we just redo the reoslutions for the whole file (could go further, but kiss for now). - // at this point we know that either // - file has local declarations for ambient modules // OR From c3582d2443f6133c88f46596236b47f301c39568 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 18 Apr 2017 16:23:01 -0700 Subject: [PATCH 03/28] cleanup --- src/compiler/program.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index d2e257867f4..ed9c08d6d96 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -517,9 +517,9 @@ namespace ts { return resolveModuleNamesWorker(moduleNames, containingFile); } - const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); if (oldSourceFile === file) { + // `file` is unchanged from the old program, so we can reuse old module resolutions. const oldSourceFileResult: ResolvedModuleFull[] = []; for (const moduleName of moduleNames) { const resolvedModule = oldSourceFile.resolvedModules.get(moduleName); @@ -527,6 +527,7 @@ namespace ts { } return oldSourceFileResult; } + // at this point we know that either // - file has local declarations for ambient modules // OR From 7966c08cc462b9f9fbf12dcbfa60e690a65d99c2 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 20 Apr 2017 11:02:35 -0700 Subject: [PATCH 04/28] make enum bitflag and add trace message --- src/compiler/diagnosticMessages.json | 7 +- src/compiler/program.ts | 29 ++++---- .../unittests/reuseProgramStructure.ts | 73 +++++++++++++++++-- 3 files changed, 87 insertions(+), 22 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index a1200b6a072..ee379787768 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3199,9 +3199,12 @@ }, "Scoped package detected, looking in '{0}'": { "category": "Message", - "code": "6182" + "code": 6182 + }, + "Reusing module resolutions originating in '{0}' since this file was not modified.": { + "category": "Message", + "code": 6183 }, - "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", "code": 7005 diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 373ccb08d4d..2a9891e9d64 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -8,17 +8,17 @@ namespace ts { const enum StructuralChangesFromOldProgram { - None = 0, - NoOldProgram = 1, - ModuleResolutionOptions = 2, - RootNames = 3, - TypesOptions = 4, - SourceFileRemoved = 5, - HasNoDefaultLib = 6, - TripleSlashReferences = 7, - Imports = 8, - ModuleAugmentations = 9, - TripleSlashTypesReferences = 10, + None = 0, + NoOldProgram = 1 << 1, + ModuleResolutionOptions = 1 << 2, + RootNames = 1 << 3, + TypesOptions = 1 << 4, + SourceFileRemoved = 1 << 5, + HasNoDefaultLib = 1 << 6, + TripleSlashReferences = 1 << 7, + Imports = 1 << 8, + ModuleAugmentations = 1 << 9, + TripleSlashTypesReferences = 1 << 10, CannotReuseResolution = NoOldProgram | ModuleResolutionOptions | RootNames | TypesOptions | SourceFileRemoved } @@ -520,6 +520,9 @@ namespace ts { const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); if (oldSourceFile === file) { // `file` is unchanged from the old program, so we can reuse old module resolutions. + if (isTraceEnabled(options, host)) { + trace(host, Diagnostics.Reusing_module_resolutions_originating_in_0_since_this_file_was_not_modified, file.path); + } const oldSourceFileResult: ResolvedModuleFull[] = []; for (const moduleName of moduleNames) { const resolvedModule = oldSourceFile.resolvedModules.get(moduleName); @@ -710,10 +713,6 @@ namespace ts { // tentatively approve the file modifiedSourceFiles.push({ oldFile: oldSourceFile, newFile: newSourceFile }); } - else { - // file has no changes - use it as is - newSourceFile = oldSourceFile; - } // if file has passed all checks it should be safe to reuse it newSourceFiles.push(newSourceFile); diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index 43014f25afb..e53aa0b4524 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -140,9 +140,7 @@ namespace ts { useCaseSensitiveFileNames(): boolean { return sys && sys.useCaseSensitiveFileNames; }, - getNewLine(): string { - return sys ? sys.newLine : newLine; - }, + getNewLine, fileExists: fileName => files.has(fileName), readFile: fileName => { const file = files.get(fileName); @@ -217,7 +215,7 @@ namespace ts { describe("Reuse program structure", () => { const target = ScriptTarget.Latest; - const files = [ + const files: NamedSourceText[] = [ { name: "a.ts", text: SourceText.New( ` /// @@ -468,8 +466,73 @@ namespace ts { "File '/fs.jsx' does not exist.", "======== Module name 'fs' was not resolved. ========", ], "should look for 'fs' again since node.d.ts was changed"); - }); + + it("can reuse module resolutions from non-modified files", () => { + const files = [ + { name: "a1.ts", text: SourceText.New("", "", "let x = 1;") }, + { name: "a2.ts", text: SourceText.New("", "", "let x = 1;") }, + { name: "b1.ts", text: SourceText.New("", "export class B { x: number; }", "") }, + { name: "b2.ts", text: SourceText.New("", "export class B { x: number; }", "") }, + { name: "node_modules/@types/typerefs1/index.d.ts", text: SourceText.New("", "", "declare let z: string;") }, + { name: "node_modules/@types/typerefs2/index.d.ts", text: SourceText.New("", "", "declare let z: string;") }, + { + name: "f1.ts", + text: SourceText.New( + `/// ${newLine}/// ${newLine}/// `, + `import { B } from './b1';${newLine}export let BB = B;`, + "declare module './b1' { interface B { y: string; } }") + }, + { + name: "f2.ts", + text: SourceText.New( + `/// ${newLine}/// `, + `import { B } from './b2';${newLine}import { BB } from './f1';`, + "(new BB).x; (new BB).y;") + }, + ]; + + const options = { target: ScriptTarget.ES2015, traceResolution: true }; + const program_1 = newProgram(files, files.map(f => f.name), options); + assert.deepEqual(program_1.host.getTrace(), + [ + "======== Resolving type reference directive 'typerefs1', containing file 'f1.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs1/package.json' does not exist.", + "File 'node_modules/@types/typerefs1/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs1' was successfully resolved to 'node_modules/@types/typerefs1/index.d.ts', primary: true. ========", + "======== Resolving module './b1' from 'f1.ts'. ========", + "Module resolution kind is not specified, using 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "======== Resolving module './b2' from 'f2.ts'. ========", + "Module resolution kind is not specified, using 'Classic'.", + "File 'b2.ts' exist - use it as a name resolution result.", + "======== Module name './b2' was successfully resolved to 'b2.ts'. ========", + "======== Resolving module './f1' from 'f2.ts'. ========", + "Module resolution kind is not specified, using 'Classic'.", + "File 'f1.ts' exist - use it as a name resolution result.", + "======== Module name './f1' was successfully resolved to 'f1.ts'. ========" + ], + "First program should execute module reoslution normally."); + + // Create project. Edit file-exists to track disk accesses. Edit f1.ts, update project. + // check that updating project "inherits" the fileexists implementation. + const indexOfF1 = 6; + function updateProgram_1(files: NamedSourceText[]): void { + files[indexOfF1].text = SourceText.New("","",""); + } + + const program_2 = updateProgram(program_1, program_1.getRootFileNames(), options, updateProgram_1); + assert.deepEqual(program_2.host.getTrace(), [ + "Module 'fs' was resolved as ambient module declared in '/a/b/node.d.ts' since this file was not modified." + ], "should reuse module resolutions in f2 since it is unchanged"); + }); }); describe("host is optional", () => { From 0ea1b82a8570d7ffadcad583defa07354184969f Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 20 Apr 2017 18:55:15 -0700 Subject: [PATCH 05/28] test module reuse --- src/compiler/program.ts | 12 +- src/compiler/types.ts | 8 +- .../unittests/cachingInServerLSHost.ts | 5 +- src/harness/unittests/moduleResolution.ts | 2 +- .../unittests/reuseProgramStructure.ts | 181 ++++++++++-------- src/server/project.ts | 2 +- 6 files changed, 118 insertions(+), 92 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 2a9891e9d64..199788d47b0 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -387,7 +387,7 @@ namespace ts { const structuralChanges = tryReuseStructureFromOldProgram(); if (structuralChanges === StructuralChangesFromOldProgram.None) { - Debug.assert(oldProgram.structureIsReused === true); + Debug.assert(oldProgram.structureIsReused === StructureIsReused.Completely); } else { forEach(rootNames, name => processRootFile(name, /*isDefaultLib*/ false)); @@ -648,18 +648,21 @@ namespace ts { // if any of these properties has changed - structure cannot be reused const oldOptions = oldProgram.getCompilerOptions(); if (changesAffectModuleResolution(oldOptions, options)) { + oldProgram.structureIsReused = StructureIsReused.Not; return StructuralChangesFromOldProgram.ModuleResolutionOptions; } - Debug.assert(!oldProgram.structureIsReused); + Debug.assert(!(oldProgram.structureIsReused & (StructureIsReused.Completely | StructureIsReused.ModulesInUneditedFiles))); // there is an old program, check if we can reuse its structure const oldRootNames = oldProgram.getRootFileNames(); if (!arrayIsEqualTo(oldRootNames, rootNames)) { + oldProgram.structureIsReused = StructureIsReused.Not; return StructuralChangesFromOldProgram.RootNames; } if (!arrayIsEqualTo(options.types, oldOptions.types)) { + oldProgram.structureIsReused = StructureIsReused.Not; return StructuralChangesFromOldProgram.TypesOptions; } @@ -670,7 +673,7 @@ namespace ts { let structuralChanges = StructuralChangesFromOldProgram.None; for (const oldSourceFile of oldProgram.getSourceFiles()) { - let newSourceFile = host.getSourceFileByPath + const newSourceFile = host.getSourceFileByPath ? host.getSourceFileByPath(oldSourceFile.fileName, oldSourceFile.path, options.target) : host.getSourceFile(oldSourceFile.fileName, options.target); @@ -720,6 +723,7 @@ namespace ts { } if (structuralChanges !== StructuralChangesFromOldProgram.None) { + oldProgram.structureIsReused = structuralChanges & StructuralChangesFromOldProgram.CannotReuseResolution ? StructureIsReused.Not : StructureIsReused.ModulesInUneditedFiles; return structuralChanges; } @@ -763,8 +767,8 @@ namespace ts { fileProcessingDiagnostics.reattachFileDiagnostics(modifiedFile.newFile); } resolvedTypeReferenceDirectives = oldProgram.getResolvedTypeReferenceDirectives(); - oldProgram.structureIsReused = true; + oldProgram.structureIsReused = StructureIsReused.Completely; return StructuralChangesFromOldProgram.None; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 8874fd1a798..ec722a9309e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2406,7 +2406,13 @@ namespace ts { /* @internal */ getResolvedTypeReferenceDirectives(): Map; /* @internal */ isSourceFileFromExternalLibrary(file: SourceFile): boolean; // For testing purposes only. - /* @internal */ structureIsReused?: boolean; + /* @internal */ structureIsReused?: StructureIsReused; + } + + export const enum StructureIsReused { + Completely, + ModulesInUneditedFiles, + Not } export interface CustomTransformers { diff --git a/src/harness/unittests/cachingInServerLSHost.ts b/src/harness/unittests/cachingInServerLSHost.ts index 0725bf36a3b..1b87fc848b5 100644 --- a/src/harness/unittests/cachingInServerLSHost.ts +++ b/src/harness/unittests/cachingInServerLSHost.ts @@ -201,14 +201,13 @@ namespace ts { assert.isTrue(diags.length === 1, "one diagnostic expected"); assert.isTrue(typeof diags[0].messageText === "string" && ((diags[0].messageText).indexOf("Cannot find module") === 0), "should be 'cannot find module' message"); - // assert that import will success once file appear on disk fileMap.set(imported.name, imported); fileExistsCalledForBar = false; rootScriptInfo.editContent(0, root.content.length, `import {y} from "bar"`); diags = project.getLanguageService().getSemanticDiagnostics(root.name); - assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called"); - assert.isTrue(diags.length === 0); + assert.isTrue(fileExistsCalledForBar, "'fileExists' should be called."); + assert.isTrue(diags.length === 0, "The import should succeed once the imported file appears on disk."); }); }); } diff --git a/src/harness/unittests/moduleResolution.ts b/src/harness/unittests/moduleResolution.ts index 14e13b60457..a4220f50321 100644 --- a/src/harness/unittests/moduleResolution.ts +++ b/src/harness/unittests/moduleResolution.ts @@ -1042,7 +1042,7 @@ import b = require("./moduleB"); assert.equal(diagnostics1.length, 1, "expected one diagnostic"); createProgram(names, {}, compilerHost, program1); - assert.isTrue(program1.structureIsReused); + assert.isTrue(program1.structureIsReused === StructureIsReused.Completely); const diagnostics2 = program1.getFileProcessingDiagnostics().getDiagnostics(); assert.equal(diagnostics2.length, 1, "expected one diagnostic"); assert.equal(diagnostics1[0].messageText, diagnostics2[0].messageText, "expected one diagnostic"); diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index e53aa0b4524..8810d8a6401 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -140,7 +140,9 @@ namespace ts { useCaseSensitiveFileNames(): boolean { return sys && sys.useCaseSensitiveFileNames; }, - getNewLine, + getNewLine(): string { + return sys ? sys.newLine : newLine; + }, fileExists: fileName => files.has(fileName), readFile: fileName => { const file = files.get(fileName); @@ -216,12 +218,14 @@ namespace ts { describe("Reuse program structure", () => { const target = ScriptTarget.Latest; const files: NamedSourceText[] = [ - { name: "a.ts", text: SourceText.New( - ` + { + name: "a.ts", text: SourceText.New( + ` /// /// /// -`, "", `var x = 1`) }, +`, "", `var x = 1`) + }, { name: "b.ts", text: SourceText.New(`/// `, "", `var y = 2`) }, { name: "c.ts", text: SourceText.New("", "", `var z = 1;`) }, { name: "types/typerefs/index.d.ts", text: SourceText.New("", "", `declare let z: number;`) }, @@ -232,7 +236,7 @@ namespace ts { const program_2 = updateProgram(program_1, ["a.ts"], { target }, files => { files[0].text = files[0].text.updateProgram("var x = 100"); }); - assert.isTrue(program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Completely); const program1Diagnostics = program_1.getSemanticDiagnostics(program_1.getSourceFile("a.ts")); const program2Diagnostics = program_2.getSemanticDiagnostics(program_1.getSourceFile("a.ts")); assert.equal(program1Diagnostics.length, program2Diagnostics.length); @@ -243,7 +247,7 @@ namespace ts { const program_2 = updateProgram(program_1, ["a.ts"], { target }, files => { files[0].text = files[0].text.updateProgram("var x = 100"); }); - assert.isTrue(program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Completely); const program1Diagnostics = program_1.getSemanticDiagnostics(program_1.getSourceFile("a.ts")); const program2Diagnostics = program_2.getSemanticDiagnostics(program_1.getSourceFile("a.ts")); assert.equal(program1Diagnostics.length, program2Diagnostics.length); @@ -257,19 +261,19 @@ namespace ts { `; files[0].text = files[0].text.updateReferences(newReferences); }); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.ModulesInUneditedFiles); }); it("fails if change affects type references", () => { const program_1 = newProgram(files, ["a.ts"], { types: ["a"] }); updateProgram(program_1, ["a.ts"], { types: ["b"] }, noop); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Not); }); it("succeeds if change doesn't affect type references", () => { const program_1 = newProgram(files, ["a.ts"], { types: ["a"] }); updateProgram(program_1, ["a.ts"], { types: ["a"] }, noop); - assert.isTrue(program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Completely); }); it("fails if change affects imports", () => { @@ -277,7 +281,7 @@ namespace ts { updateProgram(program_1, ["a.ts"], { target }, files => { files[2].text = files[2].text.updateImportsAndExports("import x from 'b'"); }); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.ModulesInUneditedFiles); }); it("fails if change affects type directives", () => { @@ -289,25 +293,25 @@ namespace ts { /// `; files[0].text = files[0].text.updateReferences(newReferences); }); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.ModulesInUneditedFiles); }); it("fails if module kind changes", () => { const program_1 = newProgram(files, ["a.ts"], { target, module: ModuleKind.CommonJS }); updateProgram(program_1, ["a.ts"], { target, module: ModuleKind.AMD }, noop); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Not); }); it("fails if rootdir changes", () => { const program_1 = newProgram(files, ["a.ts"], { target, module: ModuleKind.CommonJS, rootDir: "/a/b" }); updateProgram(program_1, ["a.ts"], { target, module: ModuleKind.CommonJS, rootDir: "/a/c" }, noop); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Not); }); it("fails if config path changes", () => { const program_1 = newProgram(files, ["a.ts"], { target, module: ModuleKind.CommonJS, configFilePath: "/a/b/tsconfig.json" }); updateProgram(program_1, ["a.ts"], { target, module: ModuleKind.CommonJS, configFilePath: "/a/c/tsconfig.json" }, noop); - assert.isTrue(!program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Not); }); it("resolution cache follows imports", () => { @@ -326,7 +330,7 @@ namespace ts { const program_2 = updateProgram(program_1, ["a.ts"], options, files => { files[0].text = files[0].text.updateProgram("var x = 2"); }); - assert.isTrue(program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Completely); // content of resolution cache should not change checkResolvedModulesCache(program_1, "a.ts", createMapFromTemplate({ "b": createResolvedModule("b.ts") })); @@ -336,7 +340,7 @@ namespace ts { const program_3 = updateProgram(program_2, ["a.ts"], options, files => { files[0].text = files[0].text.updateImportsAndExports(""); }); - assert.isTrue(!program_2.structureIsReused); + assert.isTrue(program_2.structureIsReused === StructureIsReused.ModulesInUneditedFiles); checkResolvedModulesCache(program_3, "a.ts", /*expectedContent*/ undefined); const program_4 = updateProgram(program_3, ["a.ts"], options, files => { @@ -345,7 +349,7 @@ namespace ts { `; files[0].text = files[0].text.updateImportsAndExports(newImports); }); - assert.isTrue(!program_3.structureIsReused); + assert.isTrue(program_3.structureIsReused === StructureIsReused.ModulesInUneditedFiles); checkResolvedModulesCache(program_4, "a.ts", createMapFromTemplate({ "b": createResolvedModule("b.ts"), "c": undefined })); }); @@ -363,7 +367,7 @@ namespace ts { const program_2 = updateProgram(program_1, ["/a.ts"], options, files => { files[0].text = files[0].text.updateProgram("var x = 2"); }); - assert.isTrue(program_1.structureIsReused); + assert.isTrue(program_1.structureIsReused === StructureIsReused.Completely); // content of resolution cache should not change checkResolvedTypeDirectivesCache(program_1, "/a.ts", createMapFromTemplate({ "typedefs": { resolvedFileName: "/types/typedefs/index.d.ts", primary: true } })); @@ -374,7 +378,7 @@ namespace ts { files[0].text = files[0].text.updateReferences(""); }); - assert.isTrue(!program_2.structureIsReused); + assert.isTrue(program_2.structureIsReused === StructureIsReused.ModulesInUneditedFiles); checkResolvedTypeDirectivesCache(program_3, "/a.ts", /*expectedContent*/ undefined); updateProgram(program_3, ["/a.ts"], options, files => { @@ -383,7 +387,7 @@ namespace ts { `; files[0].text = files[0].text.updateReferences(newReferences); }); - assert.isTrue(!program_3.structureIsReused); + assert.isTrue(program_3.structureIsReused === StructureIsReused.ModulesInUneditedFiles); checkResolvedTypeDirectivesCache(program_1, "/a.ts", createMapFromTemplate({ "typedefs": { resolvedFileName: "/types/typedefs/index.d.ts", primary: true } })); }); @@ -468,71 +472,84 @@ namespace ts { ], "should look for 'fs' again since node.d.ts was changed"); }); - it("can reuse module resolutions from non-modified files", () => { - const files = [ - { name: "a1.ts", text: SourceText.New("", "", "let x = 1;") }, - { name: "a2.ts", text: SourceText.New("", "", "let x = 1;") }, - { name: "b1.ts", text: SourceText.New("", "export class B { x: number; }", "") }, - { name: "b2.ts", text: SourceText.New("", "export class B { x: number; }", "") }, - { name: "node_modules/@types/typerefs1/index.d.ts", text: SourceText.New("", "", "declare let z: string;") }, - { name: "node_modules/@types/typerefs2/index.d.ts", text: SourceText.New("", "", "declare let z: string;") }, - { - name: "f1.ts", - text: SourceText.New( - `/// ${newLine}/// ${newLine}/// `, - `import { B } from './b1';${newLine}export let BB = B;`, - "declare module './b1' { interface B { y: string; } }") - }, - { - name: "f2.ts", - text: SourceText.New( - `/// ${newLine}/// `, - `import { B } from './b2';${newLine}import { BB } from './f1';`, - "(new BB).x; (new BB).y;") - }, - ]; + it("can reuse module resolutions from non-modified files", () => { + const files = [ + { name: "a1.ts", text: SourceText.New("", "", "let x = 1;") }, + { name: "a2.ts", text: SourceText.New("", "", "let x = 1;") }, + { name: "b1.ts", text: SourceText.New("", "export class B { x: number; }", "") }, + { name: "b2.ts", text: SourceText.New("", "export class B { x: number; }", "") }, + { name: "node_modules/@types/typerefs1/index.d.ts", text: SourceText.New("", "", "declare let z: string;") }, + { name: "node_modules/@types/typerefs2/index.d.ts", text: SourceText.New("", "", "declare let z: string;") }, + { + name: "f1.ts", + text: + SourceText.New( + `/// ${newLine}/// ${newLine}/// `, + `import { B } from './b1';${newLine}export let BB = B;`, + "declare module './b1' { interface B { y: string; } }") + }, + { + name: "f2.ts", + text: SourceText.New( + `/// ${newLine}/// `, + `import { B } from './b2';${newLine}import { BB } from './f1';`, + "(new BB).x; (new BB).y;") + }, + ]; - const options = { target: ScriptTarget.ES2015, traceResolution: true }; - const program_1 = newProgram(files, files.map(f => f.name), options); - assert.deepEqual(program_1.host.getTrace(), - [ - "======== Resolving type reference directive 'typerefs1', containing file 'f1.ts', root directory 'node_modules/@types'. ========", - "Resolving with primary search path 'node_modules/@types'.", - "File 'node_modules/@types/typerefs1/package.json' does not exist.", - "File 'node_modules/@types/typerefs1/index.d.ts' exist - use it as a name resolution result.", - "======== Type reference directive 'typerefs1' was successfully resolved to 'node_modules/@types/typerefs1/index.d.ts', primary: true. ========", - "======== Resolving module './b1' from 'f1.ts'. ========", - "Module resolution kind is not specified, using 'Classic'.", - "File 'b1.ts' exist - use it as a name resolution result.", - "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", - "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", - "Resolving with primary search path 'node_modules/@types'.", - "File 'node_modules/@types/typerefs2/package.json' does not exist.", - "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", - "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", - "======== Resolving module './b2' from 'f2.ts'. ========", - "Module resolution kind is not specified, using 'Classic'.", - "File 'b2.ts' exist - use it as a name resolution result.", - "======== Module name './b2' was successfully resolved to 'b2.ts'. ========", - "======== Resolving module './f1' from 'f2.ts'. ========", - "Module resolution kind is not specified, using 'Classic'.", - "File 'f1.ts' exist - use it as a name resolution result.", - "======== Module name './f1' was successfully resolved to 'f1.ts'. ========" - ], - "First program should execute module reoslution normally."); + const options: CompilerOptions = { target: ScriptTarget.ES2015, traceResolution: true, moduleResolution: ModuleResolutionKind.Classic }; + const program_1 = newProgram(files, files.map(f => f.name), options); + assert.deepEqual(program_1.host.getTrace(), + [ + "======== Resolving type reference directive 'typerefs1', containing file 'f1.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs1/package.json' does not exist.", + "File 'node_modules/@types/typerefs1/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs1' was successfully resolved to 'node_modules/@types/typerefs1/index.d.ts', primary: true. ========", + "======== Resolving module './b1' from 'f1.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "======== Resolving module './b2' from 'f2.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b2.ts' exist - use it as a name resolution result.", + "======== Module name './b2' was successfully resolved to 'b2.ts'. ========", + "======== Resolving module './f1' from 'f2.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'f1.ts' exist - use it as a name resolution result.", + "======== Module name './f1' was successfully resolved to 'f1.ts'. ========" + ], + "First program should execute module reoslution normally."); - // Create project. Edit file-exists to track disk accesses. Edit f1.ts, update project. - // check that updating project "inherits" the fileexists implementation. - const indexOfF1 = 6; - function updateProgram_1(files: NamedSourceText[]): void { - files[indexOfF1].text = SourceText.New("","",""); - } + const program_1Diagnostics = program_1.getSemanticDiagnostics(program_1.getSourceFile("f2.ts")); + assert(program_1Diagnostics.length === 0, `initial program should be well-formed`); - const program_2 = updateProgram(program_1, program_1.getRootFileNames(), options, updateProgram_1); - assert.deepEqual(program_2.host.getTrace(), [ - "Module 'fs' was resolved as ambient module declared in '/a/b/node.d.ts' since this file was not modified." - ], "should reuse module resolutions in f2 since it is unchanged"); - }); + const indexOfF1 = 6; + const program_2 = updateProgram(program_1, program_1.getRootFileNames(), options, f => { + let newSourceText = f[indexOfF1].text.updateProgram(""); + newSourceText = newSourceText.updateImportsAndExports(""); + newSourceText = newSourceText.updateReferences(""); + f[indexOfF1] = { name: "f1.ts", text: newSourceText }; + }); + + + const program_2Diagnostics = program_2.getSemanticDiagnostics(program_2.getSourceFile("f2.ts")); + assert(program_2Diagnostics.length === 1, `f1 no longer augments BB, should get one error.`); + + assert.deepEqual(program_2.host.getTrace(), [ + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "Reusing module resolutions originating in 'f2.ts' since this file was not modified." + ], "should reuse module resolutions in f2 since it is unchanged"); + }); }); describe("host is optional", () => { diff --git a/src/server/project.ts b/src/server/project.ts index 09c8d74c8c7..56c394379a0 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -552,7 +552,7 @@ namespace ts.server { // bump up the version if // - oldProgram is not set - this is a first time updateGraph is called // - newProgram is different from the old program and structure of the old program was not reused. - if (!oldProgram || (this.program !== oldProgram && !oldProgram.structureIsReused)) { + if (!oldProgram || (this.program !== oldProgram && !(oldProgram.structureIsReused & StructureIsReused.Completely))) { hasChanges = true; if (oldProgram) { for (const f of oldProgram.getSourceFiles()) { From ac20fc2d260a8363edf6f921dc0e4cd93566c865 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 21 Apr 2017 16:54:42 -0700 Subject: [PATCH 06/28] consolidate program-structure reuse flags --- src/compiler/program.ts | 122 ++++++++++++++++------------------------ 1 file changed, 49 insertions(+), 73 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 199788d47b0..8bd7bbbc765 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -6,23 +6,6 @@ namespace ts { const emptyArray: any[] = []; const ignoreDiagnosticCommentRegEx = /(^\s*$)|(^\s*\/\/\/?\s*(@ts-ignore)?)/; - - const enum StructuralChangesFromOldProgram { - None = 0, - NoOldProgram = 1 << 1, - ModuleResolutionOptions = 1 << 2, - RootNames = 1 << 3, - TypesOptions = 1 << 4, - SourceFileRemoved = 1 << 5, - HasNoDefaultLib = 1 << 6, - TripleSlashReferences = 1 << 7, - Imports = 1 << 8, - ModuleAugmentations = 1 << 9, - TripleSlashTypesReferences = 1 << 10, - - CannotReuseResolution = NoOldProgram | ModuleResolutionOptions | RootNames | TypesOptions | SourceFileRemoved - } - export function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName = "tsconfig.json"): string { while (true) { const fileName = combinePaths(searchPath, configName); @@ -385,11 +368,8 @@ namespace ts { // used to track cases when two file names differ only in casing const filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? createFileMap(fileName => fileName.toLowerCase()) : undefined; - const structuralChanges = tryReuseStructureFromOldProgram(); - if (structuralChanges === StructuralChangesFromOldProgram.None) { - Debug.assert(oldProgram.structureIsReused === StructureIsReused.Completely); - } - else { + const structuralIsReused = tryReuseStructureFromOldProgram(); + if (structuralIsReused !== StructureIsReused.Completely) { forEach(rootNames, name => processRootFile(name, /*isDefaultLib*/ false)); // load type declarations specified via 'types' argument or implicitly from types/ and node_modules/@types folders @@ -511,7 +491,7 @@ namespace ts { } function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile, oldProgramState: OldProgramState) { - if (structuralChanges & StructuralChangesFromOldProgram.CannotReuseResolution && !file.ambientModuleNames.length) { + if (structuralIsReused === StructureIsReused.Not && !file.ambientModuleNames.length) { // If the old program state does not permit reusing resolutions and `file` does not contain locally defined ambient modules, // the best we can do is fallback to the default logic. return resolveModuleNamesWorker(moduleNames, containingFile); @@ -639,17 +619,16 @@ namespace ts { } } - function tryReuseStructureFromOldProgram(): StructuralChangesFromOldProgram { + function tryReuseStructureFromOldProgram(): StructureIsReused { if (!oldProgram) { - return StructuralChangesFromOldProgram.NoOldProgram; + return StructureIsReused.Not; } // check properties that can affect structure of the program or module resolution strategy // if any of these properties has changed - structure cannot be reused const oldOptions = oldProgram.getCompilerOptions(); if (changesAffectModuleResolution(oldOptions, options)) { - oldProgram.structureIsReused = StructureIsReused.Not; - return StructuralChangesFromOldProgram.ModuleResolutionOptions; + return oldProgram.structureIsReused = StructureIsReused.Not; } Debug.assert(!(oldProgram.structureIsReused & (StructureIsReused.Completely | StructureIsReused.ModulesInUneditedFiles))); @@ -658,19 +637,19 @@ namespace ts { const oldRootNames = oldProgram.getRootFileNames(); if (!arrayIsEqualTo(oldRootNames, rootNames)) { oldProgram.structureIsReused = StructureIsReused.Not; - return StructuralChangesFromOldProgram.RootNames; + return StructureIsReused.Not; } if (!arrayIsEqualTo(options.types, oldOptions.types)) { oldProgram.structureIsReused = StructureIsReused.Not; - return StructuralChangesFromOldProgram.TypesOptions; + return StructureIsReused.Not; } // check if program source files has changed in the way that can affect structure of the program const newSourceFiles: SourceFile[] = []; const filePaths: Path[] = []; const modifiedSourceFiles: { oldFile: SourceFile, newFile: SourceFile }[] = []; - let structuralChanges = StructuralChangesFromOldProgram.None; + oldProgram.structureIsReused = StructureIsReused.Completely; for (const oldSourceFile of oldProgram.getSourceFiles()) { const newSourceFile = host.getSourceFileByPath @@ -678,53 +657,51 @@ namespace ts { : host.getSourceFile(oldSourceFile.fileName, options.target); if (!newSourceFile) { - structuralChanges |= StructuralChangesFromOldProgram.SourceFileRemoved; + return oldProgram.structureIsReused = StructureIsReused.Not; } - else { - newSourceFile.path = oldSourceFile.path; - filePaths.push(newSourceFile.path); - if (oldSourceFile !== newSourceFile) { - if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { - // value of no-default-lib has changed - // this will affect if default library is injected into the list of files - structuralChanges |= StructuralChangesFromOldProgram.HasNoDefaultLib; - } + newSourceFile.path = oldSourceFile.path; + filePaths.push(newSourceFile.path); - // check tripleslash references - if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) { - // tripleslash references has changed - structuralChanges |= StructuralChangesFromOldProgram.TripleSlashReferences; - } - - // check imports and module augmentations - collectExternalModuleReferences(newSourceFile); - if (!arrayIsEqualTo(oldSourceFile.imports, newSourceFile.imports, moduleNameIsEqualTo)) { - // imports has changed - structuralChanges |= StructuralChangesFromOldProgram.Imports; - } - if (!arrayIsEqualTo(oldSourceFile.moduleAugmentations, newSourceFile.moduleAugmentations, moduleNameIsEqualTo)) { - // moduleAugmentations has changed - structuralChanges |= StructuralChangesFromOldProgram.ModuleAugmentations; - } - - if (!arrayIsEqualTo(oldSourceFile.typeReferenceDirectives, newSourceFile.typeReferenceDirectives, fileReferenceIsEqualTo)) { - // 'types' references has changed - structuralChanges |= StructuralChangesFromOldProgram.TripleSlashTypesReferences; - } - - // tentatively approve the file - modifiedSourceFiles.push({ oldFile: oldSourceFile, newFile: newSourceFile }); + if (oldSourceFile !== newSourceFile) { + if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { + // value of no-default-lib has changed + // this will affect if default library is injected into the list of files + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; } - // if file has passed all checks it should be safe to reuse it - newSourceFiles.push(newSourceFile); + // check tripleslash references + if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) { + // tripleslash references has changed + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + } + + // check imports and module augmentations + collectExternalModuleReferences(newSourceFile); + if (!arrayIsEqualTo(oldSourceFile.imports, newSourceFile.imports, moduleNameIsEqualTo)) { + // imports has changed + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + } + if (!arrayIsEqualTo(oldSourceFile.moduleAugmentations, newSourceFile.moduleAugmentations, moduleNameIsEqualTo)) { + // moduleAugmentations has changed + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + } + + if (!arrayIsEqualTo(oldSourceFile.typeReferenceDirectives, newSourceFile.typeReferenceDirectives, fileReferenceIsEqualTo)) { + // 'types' references has changed + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + } + + // tentatively approve the file + modifiedSourceFiles.push({ oldFile: oldSourceFile, newFile: newSourceFile }); } + + // if file has passed all checks it should be safe to reuse it + newSourceFiles.push(newSourceFile); } - if (structuralChanges !== StructuralChangesFromOldProgram.None) { - oldProgram.structureIsReused = structuralChanges & StructuralChangesFromOldProgram.CannotReuseResolution ? StructureIsReused.Not : StructureIsReused.ModulesInUneditedFiles; - return structuralChanges; + if (oldProgram.structureIsReused !== StructureIsReused.Completely) { + return oldProgram.structureIsReused; } modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path); @@ -738,7 +715,7 @@ namespace ts { // ensure that module resolution results are still correct const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo); if (resolutionsChanged) { - return StructuralChangesFromOldProgram.Imports | StructuralChangesFromOldProgram.ModuleAugmentations; + return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; } } if (resolveTypeReferenceDirectiveNamesWorker) { @@ -747,7 +724,7 @@ namespace ts { // ensure that types resolutions are still correct const resolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, resolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, typeDirectiveIsEqualTo); if (resolutionsChanged) { - return StructuralChangesFromOldProgram.TripleSlashTypesReferences; + return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; } } // pass the cache of module/types resolutions from the old source file @@ -768,8 +745,7 @@ namespace ts { } resolvedTypeReferenceDirectives = oldProgram.getResolvedTypeReferenceDirectives(); - oldProgram.structureIsReused = StructureIsReused.Completely; - return StructuralChangesFromOldProgram.None; + return oldProgram.structureIsReused = StructureIsReused.Completely; } function getEmitHost(writeFileCallback?: WriteFileCallback): EmitHost { From 6547c1816f23ddcc42eb7f8ec3fd6026893eafd1 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Thu, 27 Apr 2017 20:27:48 -0700 Subject: [PATCH 07/28] check ambient modules before reusing old state --- src/compiler/diagnosticMessages.json | 8 +- src/compiler/program.ts | 126 +++++++++--------- src/compiler/types.ts | 1 + src/compiler/utilities.ts | 2 +- .../unittests/reuseProgramStructure.ts | 3 +- 5 files changed, 74 insertions(+), 66 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index ee379787768..b045def34e7 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3201,9 +3201,13 @@ "category": "Message", "code": 6182 }, - "Reusing module resolutions originating in '{0}' since this file was not modified.": { + "Reusing resolution of module '{0}' to file '{1}' from old program.": { "category": "Message", - "code": 6183 + "code": 618 + }, + "Reusing module resolutions originating in '{0}' since resolutions are unchanged from old program.": { + "category": "Message", + "code": 6184 }, "Variable '{0}' implicitly has an '{1}' type.": { "category": "Error", diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 8bd7bbbc765..15dd8a097d4 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -480,6 +480,7 @@ namespace ts { interface OldProgramState { program: Program | undefined; file: SourceFile; + /** The collection of paths modified *since* the old program. */ modifiedFilePaths: Path[]; } @@ -497,91 +498,83 @@ namespace ts { return resolveModuleNamesWorker(moduleNames, containingFile); } - const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); - if (oldSourceFile === file) { - // `file` is unchanged from the old program, so we can reuse old module resolutions. - if (isTraceEnabled(options, host)) { - trace(host, Diagnostics.Reusing_module_resolutions_originating_in_0_since_this_file_was_not_modified, file.path); - } - const oldSourceFileResult: ResolvedModuleFull[] = []; - for (const moduleName of moduleNames) { - const resolvedModule = oldSourceFile.resolvedModules.get(moduleName); - oldSourceFileResult.push(resolvedModule); - } - return oldSourceFileResult; - } - - // at this point we know that either + // at this point we know at least one of the following hold: // - file has local declarations for ambient modules - // OR // - old program state is available - // OR - // - both of items above - // With this it is possible that we can tell how some module names from the initial list will be resolved - // without doing actual resolution (in particular if some name was resolved to ambient module). - // Such names should be excluded from the list of module names that will be provided to `resolveModuleNamesWorker` - // since we don't want to resolve them again. + // With this information, we can infer some module resolutions without performing resolution. - // this is a list of modules for which we cannot predict resolution so they should be actually resolved + /** An ordered list of module names for which we cannot recover the resolution. */ let unknownModuleNames: string[]; - // this is a list of combined results assembles from predicted and resolved results. - // Order in this list matches the order in the original list of module names `moduleNames` which is important - // so later we can split results to resolutions of modules and resolutions of module augmentations. + /** + * The indexing of elements in this list matches that of `moduleNames`. + * + * Before combining results, result[i] is in one of the following states: + * * undefined: needs to be recomputed, + * * predictedToResolveToAmbientModuleMarker: known to be an ambient module. + * Needs to be reset to undefined before returning, + * * ResolvedModuleFull instance: can be reused. + */ let result: ResolvedModuleFull[]; - // a transient placeholder that is used to mark predicted resolution in the result list + /** A transient placeholder used to mark predicted resolution in the result list. */ const predictedToResolveToAmbientModuleMarker: ResolvedModuleFull = {}; + const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); + for (let i = 0; i < moduleNames.length; i++) { const moduleName = moduleNames[i]; - // module name is known to be resolved to ambient module if - // - module name is contained in the list of ambient modules that are locally declared in the file - // - in the old program module name was resolved to ambient module whose declaration is in non-modified file + // TODO: if we want to reuse resolutions more aggressively, refine this to check for whether the + // text of the corresponding modulenames has changed. + if (file === oldSourceFile) { + const oldResolvedModule = oldSourceFile && oldSourceFile.resolvedModules.get(moduleName); + if (oldResolvedModule) { + if (isTraceEnabled(options, host)) { + trace(host, Diagnostics.Reusing_resolution_of_module_0_to_file_1_from_old_program, moduleName, containingFile); + } + (result || (result = new Array(moduleNames.length)))[i] = oldResolvedModule; + continue; + } + } + // We know moduleName resolves to an ambient module provided that moduleName: + // - is in the list of ambient modules locally declared in the current source file. + // - resolved to an ambient module in the old program whose declaration is in an unmodified file // (so the same module declaration will land in the new program) - let isKnownToResolveToAmbientModule = false; + let resolvesToAmbientModuleInNonModifiedFile = false; if (contains(file.ambientModuleNames, moduleName)) { - isKnownToResolveToAmbientModule = true; + resolvesToAmbientModuleInNonModifiedFile = true; if (isTraceEnabled(options, host)) { trace(host, Diagnostics.Module_0_was_resolved_as_locally_declared_ambient_module_in_file_1, moduleName, containingFile); } } else { - isKnownToResolveToAmbientModule = checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName, oldProgramState); + resolvesToAmbientModuleInNonModifiedFile = moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName, oldProgramState); } - if (isKnownToResolveToAmbientModule) { - if (!unknownModuleNames) { - // found a first module name for which result can be prediced - // this means that this module name should not be passed to `resolveModuleNamesWorker`. - // We'll use a separate list for module names that are definitely unknown. - result = new Array(moduleNames.length); - // copy all module names that appear before the current one in the list - // since they are known to be unknown - unknownModuleNames = moduleNames.slice(0, i); - } - // Mark predicted resolution in the result list. - result[i] = predictedToResolveToAmbientModuleMarker; + if (resolvesToAmbientModuleInNonModifiedFile) { + (result || (result = new Array(moduleNames.length)))[i] = predictedToResolveToAmbientModuleMarker; } - else if (unknownModuleNames) { - // found unknown module name and we are already using separate list for those - add it to the list - unknownModuleNames.push(moduleName); + else { + // Resolution failed in the old program, or resolved to an ambient module for which we can't reuse the result. + (unknownModuleNames || (unknownModuleNames = [])).push(moduleName); } } - if (!unknownModuleNames) { - // we've looked throught the list but have not seen any predicted resolution - // use default logic - return resolveModuleNamesWorker(moduleNames, containingFile); - } - - const resolutions = unknownModuleNames.length + const resolutions = unknownModuleNames && unknownModuleNames.length ? resolveModuleNamesWorker(unknownModuleNames, containingFile) : emptyArray; - // combine results of resolutions and predicted results + // Combine results of resolutions and predicted results + if (!result) { + // There were no unresolved/ambient resolutions. + Debug.assert(resolutions.length === moduleNames.length); + return resolutions; + } + let j = 0; for (let i = 0; i < result.length; i++) { - if (result[i] === predictedToResolveToAmbientModuleMarker) { - result[i] = undefined; + if (result[i]) { + if (result[i] === predictedToResolveToAmbientModuleMarker) { + result[i] = undefined; + } } else { result[i] = resolutions[j]; @@ -589,9 +582,12 @@ namespace ts { } } Debug.assert(j === resolutions.length); + return result; - function checkModuleNameResolvedToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean { + // TODO: if we change our policy of rechecking failed lookups on each program create, + // we should adjust the value returned here. + function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean { const resolutionToFile = getResolvedModule(oldProgramState.file, moduleName); if (resolutionToFile) { // module used to be resolved to file - ignore it @@ -706,7 +702,7 @@ namespace ts { modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path); // try to verify results of module resolution - for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { + modifiedSourceFilesLoop: for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory); if (resolveModuleNamesWorker) { const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral); @@ -715,7 +711,8 @@ namespace ts { // ensure that module resolution results are still correct const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo); if (resolutionsChanged) { - return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + continue modifiedSourceFilesLoop; } } if (resolveTypeReferenceDirectiveNamesWorker) { @@ -724,7 +721,8 @@ namespace ts { // ensure that types resolutions are still correct const resolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, resolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, typeDirectiveIsEqualTo); if (resolutionsChanged) { - return oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + continue modifiedSourceFilesLoop; } } // pass the cache of module/types resolutions from the old source file @@ -732,6 +730,10 @@ namespace ts { newSourceFile.resolvedTypeReferenceDirectiveNames = oldSourceFile.resolvedTypeReferenceDirectiveNames; } + if (oldProgram.structureIsReused !== StructureIsReused.Completely) { + return oldProgram.structureIsReused; + } + // update fileName -> file mapping for (let i = 0; i < newSourceFiles.length; i++) { filesByName.set(filePaths[i], newSourceFiles[i]); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index ec722a9309e..4c850d673a9 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2409,6 +2409,7 @@ namespace ts { /* @internal */ structureIsReused?: StructureIsReused; } + /* @internal */ export const enum StructureIsReused { Completely, ModulesInUneditedFiles, diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index f34c15ee346..4381682c9f4 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -123,7 +123,7 @@ namespace ts { /* @internal */ export function hasChangesInResolutions(names: string[], newResolutions: T[], oldResolutions: Map, comparer: (oldResolution: T, newResolution: T) => boolean): boolean { if (names.length !== newResolutions.length) { - return false; + return true; } for (let i = 0; i < names.length; i++) { const newResolution = newResolutions[i]; diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index 8810d8a6401..d7140cfc8b2 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -547,7 +547,8 @@ namespace ts { "File 'node_modules/@types/typerefs2/package.json' does not exist.", "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", - "Reusing module resolutions originating in 'f2.ts' since this file was not modified." + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." ], "should reuse module resolutions in f2 since it is unchanged"); }); }); From 86d7031932952164e1463514ce3aad39099165de Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 28 Apr 2017 10:40:31 -0700 Subject: [PATCH 08/28] inline `createOldProgramState` --- src/compiler/program.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 15dd8a097d4..63d257c359a 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -484,13 +484,6 @@ namespace ts { modifiedFilePaths: Path[]; } - function createOldProgramState( - program: Program | undefined, - file: SourceFile, - modifiedFilePaths: Path[]): OldProgramState { - return { program, file, modifiedFilePaths }; - } - function resolveModuleNamesReusingOldState(moduleNames: string[], containingFile: string, file: SourceFile, oldProgramState: OldProgramState) { if (structuralIsReused === StructureIsReused.Not && !file.ambientModuleNames.length) { // If the old program state does not permit reusing resolutions and `file` does not contain locally defined ambient modules, @@ -706,7 +699,7 @@ namespace ts { const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory); if (resolveModuleNamesWorker) { const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral); - const oldProgramState = createOldProgramState(oldProgram, oldSourceFile, modifiedFilePaths); + const oldProgramState = { program: oldProgram, file: oldSourceFile, modifiedFilePaths }; const resolutions = resolveModuleNamesReusingOldState(moduleNames, newSourceFilePath, newSourceFile, oldProgramState); // ensure that module resolution results are still correct const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo); @@ -1549,7 +1542,7 @@ namespace ts { // Because global augmentation doesn't have string literal name, we can check for global augmentation as such. const nonGlobalAugmentation = filter(file.moduleAugmentations, (moduleAugmentation) => moduleAugmentation.kind === SyntaxKind.StringLiteral); const moduleNames = map(concatenate(file.imports, nonGlobalAugmentation), getTextOfLiteral); - const oldProgramState = createOldProgramState(oldProgram, file, modifiedFilePaths); + const oldProgramState = { program: oldProgram, file, modifiedFilePaths }; const resolutions = resolveModuleNamesReusingOldState(moduleNames, getNormalizedAbsolutePath(file.fileName, currentDirectory), file, oldProgramState); Debug.assert(resolutions.length === moduleNames.length); for (let i = 0; i < moduleNames.length; i++) { From c63d6d145af92d95ec478d40aa9281c101f67041 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Fri, 28 Apr 2017 16:27:41 -0700 Subject: [PATCH 09/28] Avoid double-resolving modified files --- src/compiler/core.ts | 9 ++++ src/compiler/program.ts | 53 +++++++++++++------ src/compiler/types.ts | 2 +- .../unittests/reuseProgramStructure.ts | 14 ++--- 4 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 3c867024485..a024a574a93 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -251,6 +251,15 @@ namespace ts { } } + export function zipToMap(keys: string[], values: T[]): Map { + Debug.assert(keys.length === values.length); + const map = createMap(); + for (let i = 0; i < keys.length; ++i) { + map.set(keys[i], values[i]); + } + return map; + } + /** * Iterates through `array` by index and performs the callback on each element of array until the callback * returns a falsey value, then returns false. diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 63d257c359a..c44fdce391e 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -491,7 +491,24 @@ namespace ts { return resolveModuleNamesWorker(moduleNames, containingFile); } - // at this point we know at least one of the following hold: + const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); + if (oldSourceFile !== file && file.resolvedModules) { + // `file` was created for the new program. + // + // We only set `file.resolvedModules` via work from the current function, + // so it is defined iff we already called the current function on `file`. + // That call happened no later than the creation of the `file` object, + // which per above occured during the current program creation. + // Since we model program creation as an atomic operation w/r/t IO, + // it is safe to reuse resolutions from the earlier call. + const result: ResolvedModuleFull[] = []; + for (const moduleName of moduleNames) { + const resolvedModule = file.resolvedModules.get(moduleName); + result.push(resolvedModule); + } + return result; + } + // At this point, we know at least one of the following hold: // - file has local declarations for ambient modules // - old program state is available // With this information, we can infer some module resolutions without performing resolution. @@ -511,7 +528,6 @@ namespace ts { /** A transient placeholder used to mark predicted resolution in the result list. */ const predictedToResolveToAmbientModuleMarker: ResolvedModuleFull = {}; - const oldSourceFile = oldProgramState.program && oldProgramState.program.getSourceFile(containingFile); for (let i = 0; i < moduleNames.length; i++) { const moduleName = moduleNames[i]; @@ -620,7 +636,7 @@ namespace ts { return oldProgram.structureIsReused = StructureIsReused.Not; } - Debug.assert(!(oldProgram.structureIsReused & (StructureIsReused.Completely | StructureIsReused.ModulesInUneditedFiles))); + Debug.assert(!(oldProgram.structureIsReused & (StructureIsReused.Completely | StructureIsReused.SafeModules))); // there is an old program, check if we can reuse its structure const oldRootNames = oldProgram.getRootFileNames(); @@ -653,32 +669,34 @@ namespace ts { filePaths.push(newSourceFile.path); if (oldSourceFile !== newSourceFile) { + // The `newSourceFile` object was created for the new program. + if (oldSourceFile.hasNoDefaultLib !== newSourceFile.hasNoDefaultLib) { // value of no-default-lib has changed // this will affect if default library is injected into the list of files - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.SafeModules; } // check tripleslash references if (!arrayIsEqualTo(oldSourceFile.referencedFiles, newSourceFile.referencedFiles, fileReferenceIsEqualTo)) { // tripleslash references has changed - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.SafeModules; } // check imports and module augmentations collectExternalModuleReferences(newSourceFile); if (!arrayIsEqualTo(oldSourceFile.imports, newSourceFile.imports, moduleNameIsEqualTo)) { // imports has changed - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.SafeModules; } if (!arrayIsEqualTo(oldSourceFile.moduleAugmentations, newSourceFile.moduleAugmentations, moduleNameIsEqualTo)) { // moduleAugmentations has changed - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.SafeModules; } if (!arrayIsEqualTo(oldSourceFile.typeReferenceDirectives, newSourceFile.typeReferenceDirectives, fileReferenceIsEqualTo)) { // 'types' references has changed - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; + oldProgram.structureIsReused = StructureIsReused.SafeModules; } // tentatively approve the file @@ -695,7 +713,7 @@ namespace ts { modifiedFilePaths = modifiedSourceFiles.map(f => f.newFile.path); // try to verify results of module resolution - modifiedSourceFilesLoop: for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { + for (const { oldFile: oldSourceFile, newFile: newSourceFile } of modifiedSourceFiles) { const newSourceFilePath = getNormalizedAbsolutePath(newSourceFile.fileName, currentDirectory); if (resolveModuleNamesWorker) { const moduleNames = map(concatenate(newSourceFile.imports, newSourceFile.moduleAugmentations), getTextOfLiteral); @@ -704,8 +722,11 @@ namespace ts { // ensure that module resolution results are still correct const resolutionsChanged = hasChangesInResolutions(moduleNames, resolutions, oldSourceFile.resolvedModules, moduleResolutionIsEqualTo); if (resolutionsChanged) { - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; - continue modifiedSourceFilesLoop; + oldProgram.structureIsReused = StructureIsReused.SafeModules; + newSourceFile.resolvedModules = zipToMap(moduleNames, resolutions); + } + else { + newSourceFile.resolvedModules = oldSourceFile.resolvedModules; } } if (resolveTypeReferenceDirectiveNamesWorker) { @@ -714,13 +735,13 @@ namespace ts { // ensure that types resolutions are still correct const resolutionsChanged = hasChangesInResolutions(typesReferenceDirectives, resolutions, oldSourceFile.resolvedTypeReferenceDirectiveNames, typeDirectiveIsEqualTo); if (resolutionsChanged) { - oldProgram.structureIsReused = StructureIsReused.ModulesInUneditedFiles; - continue modifiedSourceFilesLoop; + oldProgram.structureIsReused = StructureIsReused.SafeModules; + newSourceFile.resolvedTypeReferenceDirectiveNames = zipToMap(typesReferenceDirectives, resolutions); + } + else { + newSourceFile.resolvedTypeReferenceDirectiveNames = oldSourceFile.resolvedTypeReferenceDirectiveNames; } } - // pass the cache of module/types resolutions from the old source file - newSourceFile.resolvedModules = oldSourceFile.resolvedModules; - newSourceFile.resolvedTypeReferenceDirectiveNames = oldSourceFile.resolvedTypeReferenceDirectiveNames; } if (oldProgram.structureIsReused !== StructureIsReused.Completely) { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 4c850d673a9..55b7222796d 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2412,7 +2412,7 @@ namespace ts { /* @internal */ export const enum StructureIsReused { Completely, - ModulesInUneditedFiles, + SafeModules, Not } diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index d7140cfc8b2..41d7045064f 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -261,7 +261,7 @@ namespace ts { `; files[0].text = files[0].text.updateReferences(newReferences); }); - assert.isTrue(program_1.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_1.structureIsReused === StructureIsReused.SafeModules); }); it("fails if change affects type references", () => { @@ -281,7 +281,7 @@ namespace ts { updateProgram(program_1, ["a.ts"], { target }, files => { files[2].text = files[2].text.updateImportsAndExports("import x from 'b'"); }); - assert.isTrue(program_1.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_1.structureIsReused === StructureIsReused.SafeModules); }); it("fails if change affects type directives", () => { @@ -293,7 +293,7 @@ namespace ts { /// `; files[0].text = files[0].text.updateReferences(newReferences); }); - assert.isTrue(program_1.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_1.structureIsReused === StructureIsReused.SafeModules); }); it("fails if module kind changes", () => { @@ -340,7 +340,7 @@ namespace ts { const program_3 = updateProgram(program_2, ["a.ts"], options, files => { files[0].text = files[0].text.updateImportsAndExports(""); }); - assert.isTrue(program_2.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_2.structureIsReused === StructureIsReused.SafeModules); checkResolvedModulesCache(program_3, "a.ts", /*expectedContent*/ undefined); const program_4 = updateProgram(program_3, ["a.ts"], options, files => { @@ -349,7 +349,7 @@ namespace ts { `; files[0].text = files[0].text.updateImportsAndExports(newImports); }); - assert.isTrue(program_3.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_3.structureIsReused === StructureIsReused.SafeModules); checkResolvedModulesCache(program_4, "a.ts", createMapFromTemplate({ "b": createResolvedModule("b.ts"), "c": undefined })); }); @@ -378,7 +378,7 @@ namespace ts { files[0].text = files[0].text.updateReferences(""); }); - assert.isTrue(program_2.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_2.structureIsReused === StructureIsReused.SafeModules); checkResolvedTypeDirectivesCache(program_3, "/a.ts", /*expectedContent*/ undefined); updateProgram(program_3, ["/a.ts"], options, files => { @@ -387,7 +387,7 @@ namespace ts { `; files[0].text = files[0].text.updateReferences(newReferences); }); - assert.isTrue(program_3.structureIsReused === StructureIsReused.ModulesInUneditedFiles); + assert.isTrue(program_3.structureIsReused === StructureIsReused.SafeModules); checkResolvedTypeDirectivesCache(program_1, "/a.ts", createMapFromTemplate({ "typedefs": { resolvedFileName: "/types/typedefs/index.d.ts", primary: true } })); }); From 61f494ec5d669e8805f569bdecae39df7004d982 Mon Sep 17 00:00:00 2001 From: Kagami Sascha Rosylight Date: Mon, 1 May 2017 23:11:59 +0900 Subject: [PATCH 10/28] no space after new on constructor signatures --- src/services/formatting/rules.ts | 9 ++++++++- .../cases/fourslash/formattingOnConstructorSignature.ts | 9 +++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/formattingOnConstructorSignature.ts diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index 097b3494293..c34b5c6aa94 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -84,6 +84,7 @@ namespace ts.formatting { public NoSpaceBeforeComma: Rule; public SpaceAfterCertainKeywords: Rule; + public NoSpaceAfterNewKeywordOnConstructorSignature: Rule; public SpaceAfterLetConstInVariableDeclaration: Rule; public NoSpaceBeforeOpenParenInFuncCall: Rule; public SpaceAfterFunctionInFuncDecl: Rule; @@ -334,6 +335,7 @@ namespace ts.formatting { this.NoSpaceBeforeComma = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.CommaToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Delete)); this.SpaceAfterCertainKeywords = new Rule(RuleDescriptor.create4(Shared.TokenRange.FromTokens([SyntaxKind.VarKeyword, SyntaxKind.ThrowKeyword, SyntaxKind.NewKeyword, SyntaxKind.DeleteKeyword, SyntaxKind.ReturnKeyword, SyntaxKind.TypeOfKeyword, SyntaxKind.AwaitKeyword]), Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Space)); + this.NoSpaceAfterNewKeywordOnConstructorSignature = new Rule(RuleDescriptor.create1(SyntaxKind.NewKeyword, SyntaxKind.OpenParenToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsConstructorSignatureContext), RuleAction.Delete)); this.SpaceAfterLetConstInVariableDeclaration = new Rule(RuleDescriptor.create4(Shared.TokenRange.FromTokens([SyntaxKind.LetKeyword, SyntaxKind.ConstKeyword]), Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsStartOfVariableDeclarationList), RuleAction.Space)); this.NoSpaceBeforeOpenParenInFuncCall = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.OpenParenToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsFunctionCallOrNewContext, Rules.IsPreviousTokenNotComma), RuleAction.Delete)); this.SpaceAfterFunctionInFuncDecl = new Rule(RuleDescriptor.create3(SyntaxKind.FunctionKeyword, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext), RuleAction.Space)); @@ -530,7 +532,8 @@ namespace ts.formatting { this.SpaceBeforeAt, this.NoSpaceAfterAt, this.SpaceAfterDecorator, - this.NoSpaceBeforeNonNullAssertionOperator + this.NoSpaceBeforeNonNullAssertionOperator, + this.NoSpaceAfterNewKeywordOnConstructorSignature ]; // These rules are applied after high priority rules. @@ -881,6 +884,10 @@ namespace ts.formatting { return context.contextNode.kind === SyntaxKind.TypeLiteral; // && context.contextNode.parent.kind !== SyntaxKind.InterfaceDeclaration; } + static IsConstructorSignatureContext(context: FormattingContext): boolean { + return context.contextNode.kind === SyntaxKind.ConstructSignature; + } + static IsTypeArgumentOrParameterOrAssertion(token: TextRangeWithKind, parent: Node): boolean { if (token.kind !== SyntaxKind.LessThanToken && token.kind !== SyntaxKind.GreaterThanToken) { return false; diff --git a/tests/cases/fourslash/formattingOnConstructorSignature.ts b/tests/cases/fourslash/formattingOnConstructorSignature.ts new file mode 100644 index 00000000000..6b91396e532 --- /dev/null +++ b/tests/cases/fourslash/formattingOnConstructorSignature.ts @@ -0,0 +1,9 @@ +/// + +/////*1*/interface Gourai { new () {} } +/////*2*/type Stylet = { new () {} } +format.document(); +goTo.marker("1"); +verify.currentLineContentIs("interface Gourai { new() {} }"); +goTo.marker("2"); +verify.currentLineContentIs("type Stylet = { new() {} }"); \ No newline at end of file From a70b79072b8fae40f6033f9f2958004c09ec98c6 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 1 May 2017 07:54:41 -0700 Subject: [PATCH 11/28] importTracker: Fix error for undefined importClause --- src/services/importTracker.ts | 4 ++++ tests/cases/fourslash/findAllRefsNoImportClause.ts | 11 +++++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/cases/fourslash/findAllRefsNoImportClause.ts diff --git a/src/services/importTracker.ts b/src/services/importTracker.ts index 0108d830dd1..c94d5cc3143 100644 --- a/src/services/importTracker.ts +++ b/src/services/importTracker.ts @@ -212,6 +212,10 @@ namespace ts.FindAllReferences { return; } + if (!decl.importClause) { + return; + } + const { importClause } = decl; const { namedBindings } = importClause; diff --git a/tests/cases/fourslash/findAllRefsNoImportClause.ts b/tests/cases/fourslash/findAllRefsNoImportClause.ts new file mode 100644 index 00000000000..ee9fb7a7313 --- /dev/null +++ b/tests/cases/fourslash/findAllRefsNoImportClause.ts @@ -0,0 +1,11 @@ +/// + +// https://github.com/Microsoft/TypeScript/issues/15452 + +// @Filename: /a.ts +////export const [|x|] = 0; + +// @Filename: /b.ts +////import "./a"; + +verify.singleReferenceGroup("const x: 0"); From 969da26d455cb254171c2b756afc1d707a7df28a Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 1 May 2017 09:03:12 -0700 Subject: [PATCH 12/28] Add isWriteAccess --- tests/cases/fourslash/findAllRefsNoImportClause.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cases/fourslash/findAllRefsNoImportClause.ts b/tests/cases/fourslash/findAllRefsNoImportClause.ts index ee9fb7a7313..b98f5d02d01 100644 --- a/tests/cases/fourslash/findAllRefsNoImportClause.ts +++ b/tests/cases/fourslash/findAllRefsNoImportClause.ts @@ -3,7 +3,7 @@ // https://github.com/Microsoft/TypeScript/issues/15452 // @Filename: /a.ts -////export const [|x|] = 0; +////export const [|{| "isWriteAccess": true, "isDefinition": true |}x|] = 0; // @Filename: /b.ts ////import "./a"; From 3590048d9b3e9880ead57bfd21bfd7a6f9ae7c18 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 1 May 2017 08:52:34 -0700 Subject: [PATCH 13/28] unusedIdentifierFixes: Factor out nested switch statements and fix implicit fallthrough --- .../codefixes/unusedIdentifierFixes.ts | 266 +++++++++--------- 1 file changed, 137 insertions(+), 129 deletions(-) diff --git a/src/services/codefixes/unusedIdentifierFixes.ts b/src/services/codefixes/unusedIdentifierFixes.ts index e1debfa4ffb..422a28098a6 100644 --- a/src/services/codefixes/unusedIdentifierFixes.ts +++ b/src/services/codefixes/unusedIdentifierFixes.ts @@ -18,142 +18,150 @@ namespace ts.codefix { switch (token.kind) { case ts.SyntaxKind.Identifier: - switch (token.parent.kind) { - case ts.SyntaxKind.VariableDeclaration: - switch (token.parent.parent.parent.kind) { - case SyntaxKind.ForStatement: - const forStatement = token.parent.parent.parent; - const forInitializer = forStatement.initializer; - if (forInitializer.declarations.length === 1) { - return deleteNode(forInitializer); - } - else { - return deleteNodeInList(token.parent); - } - - case SyntaxKind.ForOfStatement: - const forOfStatement = token.parent.parent.parent; - if (forOfStatement.initializer.kind === SyntaxKind.VariableDeclarationList) { - const forOfInitializer = forOfStatement.initializer; - return replaceNode(forOfInitializer.declarations[0], createObjectLiteral()); - } - break; - - case SyntaxKind.ForInStatement: - // There is no valid fix in the case of: - // for .. in - return undefined; - - case SyntaxKind.CatchClause: - const catchClause = token.parent.parent; - const parameter = catchClause.variableDeclaration.getChildren()[0]; - return deleteNode(parameter); - - default: - const variableStatement = token.parent.parent.parent; - if (variableStatement.declarationList.declarations.length === 1) { - return deleteNode(variableStatement); - } - else { - return deleteNodeInList(token.parent); - } - } - // TODO: #14885 - // falls through - - case SyntaxKind.TypeParameter: - const typeParameters = (token.parent.parent).typeParameters; - if (typeParameters.length === 1) { - const previousToken = getTokenAtPosition(sourceFile, typeParameters.pos - 1); - if (!previousToken || previousToken.kind !== SyntaxKind.LessThanToken) { - return deleteRange(typeParameters); - } - const nextToken = getTokenAtPosition(sourceFile, typeParameters.end); - if (!nextToken || nextToken.kind !== SyntaxKind.GreaterThanToken) { - return deleteRange(typeParameters); - } - return deleteNodeRange(previousToken, nextToken); - } - else { - return deleteNodeInList(token.parent); - } - - case ts.SyntaxKind.Parameter: - const functionDeclaration = token.parent.parent; - if (functionDeclaration.parameters.length === 1) { - return deleteNode(token.parent); - } - else { - return deleteNodeInList(token.parent); - } - - // handle case where 'import a = A;' - case SyntaxKind.ImportEqualsDeclaration: - const importEquals = getAncestor(token, SyntaxKind.ImportEqualsDeclaration); - return deleteNode(importEquals); - - case SyntaxKind.ImportSpecifier: - const namedImports = token.parent.parent; - if (namedImports.elements.length === 1) { - // Only 1 import and it is unused. So the entire declaration should be removed. - const importSpec = getAncestor(token, SyntaxKind.ImportDeclaration); - return deleteNode(importSpec); - } - else { - // delete import specifier - return deleteNodeInList(token.parent); - } - - // handle case where "import d, * as ns from './file'" - // or "'import {a, b as ns} from './file'" - case SyntaxKind.ImportClause: // this covers both 'import |d|' and 'import |d,| *' - const importClause = token.parent; - if (!importClause.namedBindings) { // |import d from './file'| or |import * as ns from './file'| - const importDecl = getAncestor(importClause, SyntaxKind.ImportDeclaration); - return deleteNode(importDecl); - } - else { - // import |d,| * as ns from './file' - const start = importClause.name.getStart(sourceFile); - const nextToken = getTokenAtPosition(sourceFile, importClause.name.end); - if (nextToken && nextToken.kind === SyntaxKind.CommaToken) { - // shift first non-whitespace position after comma to the start position of the node - return deleteRange({ pos: start, end: skipTrivia(sourceFile.text, nextToken.end, /*stopAfterLineBreaks*/ false, /*stopAtComments*/ true) }); - } - else { - return deleteNode(importClause.name); - } - } - - case SyntaxKind.NamespaceImport: - const namespaceImport = token.parent; - if (namespaceImport.name === token && !(namespaceImport.parent).name) { - const importDecl = getAncestor(namespaceImport, SyntaxKind.ImportDeclaration); - return deleteNode(importDecl); - } - else { - const previousToken = getTokenAtPosition(sourceFile, namespaceImport.pos - 1); - if (previousToken && previousToken.kind === SyntaxKind.CommaToken) { - const startPosition = textChanges.getAdjustedStartPosition(sourceFile, previousToken, {}, textChanges.Position.FullStart); - return deleteRange({ pos: startPosition, end: namespaceImport.end }); - } - return deleteRange(namespaceImport); - } - } - break; + return deleteIdentifier(); case SyntaxKind.PropertyDeclaration: case SyntaxKind.NamespaceImport: return deleteNode(token.parent); + + default: + return deleteDefault(); } - if (isDeclarationName(token)) { - return deleteNode(token.parent); + + function deleteDefault() { + if (isDeclarationName(token)) { + return deleteNode(token.parent); + } + else if (isLiteralComputedPropertyDeclarationName(token)) { + return deleteNode(token.parent.parent); + } + else { + return undefined; + } } - else if (isLiteralComputedPropertyDeclarationName(token)) { - return deleteNode(token.parent.parent); + + function deleteIdentifier(): CodeAction[] | undefined { + switch (token.parent.kind) { + case ts.SyntaxKind.VariableDeclaration: + return deleteVariableDeclaration(token.parent); + + case SyntaxKind.TypeParameter: + const typeParameters = (token.parent.parent).typeParameters; + if (typeParameters.length === 1) { + const previousToken = getTokenAtPosition(sourceFile, typeParameters.pos - 1); + if (!previousToken || previousToken.kind !== SyntaxKind.LessThanToken) { + return deleteRange(typeParameters); + } + const nextToken = getTokenAtPosition(sourceFile, typeParameters.end); + if (!nextToken || nextToken.kind !== SyntaxKind.GreaterThanToken) { + return deleteRange(typeParameters); + } + return deleteNodeRange(previousToken, nextToken); + } + else { + return deleteNodeInList(token.parent); + } + + case ts.SyntaxKind.Parameter: + const functionDeclaration = token.parent.parent; + if (functionDeclaration.parameters.length === 1) { + return deleteNode(token.parent); + } + else { + return deleteNodeInList(token.parent); + } + + // handle case where 'import a = A;' + case SyntaxKind.ImportEqualsDeclaration: + const importEquals = getAncestor(token, SyntaxKind.ImportEqualsDeclaration); + return deleteNode(importEquals); + + case SyntaxKind.ImportSpecifier: + const namedImports = token.parent.parent; + if (namedImports.elements.length === 1) { + // Only 1 import and it is unused. So the entire declaration should be removed. + const importSpec = getAncestor(token, SyntaxKind.ImportDeclaration); + return deleteNode(importSpec); + } + else { + // delete import specifier + return deleteNodeInList(token.parent); + } + + // handle case where "import d, * as ns from './file'" + // or "'import {a, b as ns} from './file'" + case SyntaxKind.ImportClause: // this covers both 'import |d|' and 'import |d,| *' + const importClause = token.parent; + if (!importClause.namedBindings) { // |import d from './file'| or |import * as ns from './file'| + const importDecl = getAncestor(importClause, SyntaxKind.ImportDeclaration); + return deleteNode(importDecl); + } + else { + // import |d,| * as ns from './file' + const start = importClause.name.getStart(sourceFile); + const nextToken = getTokenAtPosition(sourceFile, importClause.name.end); + if (nextToken && nextToken.kind === SyntaxKind.CommaToken) { + // shift first non-whitespace position after comma to the start position of the node + return deleteRange({ pos: start, end: skipTrivia(sourceFile.text, nextToken.end, /*stopAfterLineBreaks*/ false, /*stopAtComments*/ true) }); + } + else { + return deleteNode(importClause.name); + } + } + + case SyntaxKind.NamespaceImport: + const namespaceImport = token.parent; + if (namespaceImport.name === token && !(namespaceImport.parent).name) { + const importDecl = getAncestor(namespaceImport, SyntaxKind.ImportDeclaration); + return deleteNode(importDecl); + } + else { + const previousToken = getTokenAtPosition(sourceFile, namespaceImport.pos - 1); + if (previousToken && previousToken.kind === SyntaxKind.CommaToken) { + const startPosition = textChanges.getAdjustedStartPosition(sourceFile, previousToken, {}, textChanges.Position.FullStart); + return deleteRange({ pos: startPosition, end: namespaceImport.end }); + } + return deleteRange(namespaceImport); + } + + default: + return deleteDefault(); + } } - else { - return undefined; + + // token.parent is a variableDeclaration + function deleteVariableDeclaration(varDecl: ts.VariableDeclaration): CodeAction[] | undefined { + switch (varDecl.parent.parent.kind) { + case SyntaxKind.ForStatement: + const forStatement = varDecl.parent.parent; + const forInitializer = forStatement.initializer; + if (forInitializer.declarations.length === 1) { + return deleteNode(forInitializer); + } + else { + return deleteNodeInList(varDecl); + } + + case SyntaxKind.ForOfStatement: + const forOfStatement = varDecl.parent.parent; + Debug.assert(forOfStatement.initializer.kind === SyntaxKind.VariableDeclarationList); + const forOfInitializer = forOfStatement.initializer; + return replaceNode(forOfInitializer.declarations[0], createObjectLiteral()); + + case SyntaxKind.ForInStatement: + // There is no valid fix in the case of: + // for .. in + return undefined; + + default: + const variableStatement = varDecl.parent.parent; + if (variableStatement.declarationList.declarations.length === 1) { + return deleteNode(variableStatement); + } + else { + return deleteNodeInList(varDecl); + } + } } function deleteNode(n: Node) { From b39a2a7377b59d86b49c833293f4545d55231c2d Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Mon, 1 May 2017 12:19:28 -0500 Subject: [PATCH 14/28] Add {create|update}InterfaceDeclaration methods (closes #15497) --- src/compiler/factory.ts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index dbaaffdc629..2eb472ebdf0 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -1440,6 +1440,28 @@ namespace ts { : node; } + export function createInterfaceDeclaration(decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: string | Identifier | undefined, typeParameters: TypeParameterDeclaration[] | undefined, heritageClauses: HeritageClause[], members: TypeElement[]) { + const node = createSynthesizedNode(SyntaxKind.InterfaceDeclaration); + node.decorators = asNodeArray(decorators); + node.modifiers = asNodeArray(modifiers); + node.name = asName(name); + node.typeParameters = asNodeArray(typeParameters); + node.heritageClauses = asNodeArray(heritageClauses); + node.members = createNodeArray(members); + return node; + } + + export function updateInterfaceDeclaration(node: InterfaceDeclaration, decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: Identifier | undefined, typeParameters: TypeParameterDeclaration[] | undefined, heritageClauses: HeritageClause[], members: TypeElement[]) { + return node.decorators !== decorators + || node.modifiers !== modifiers + || node.name !== name + || node.typeParameters !== typeParameters + || node.heritageClauses !== heritageClauses + || node.members !== members + ? updateNode(createInterfaceDeclaration(decorators, modifiers, name, typeParameters, heritageClauses, members), node) + : node; + } + export function createEnumDeclaration(decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: string | Identifier, members: EnumMember[]) { const node = createSynthesizedNode(SyntaxKind.EnumDeclaration); node.decorators = asNodeArray(decorators); From a6f7f5e70d25d7c77455bf815b02c94c7e0533ff Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Mon, 1 May 2017 13:46:08 -0500 Subject: [PATCH 15/28] Update type signatures for {create|update}InterfaceDeclaration --- src/compiler/factory.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 2eb472ebdf0..95878917b3d 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -1440,7 +1440,7 @@ namespace ts { : node; } - export function createInterfaceDeclaration(decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: string | Identifier | undefined, typeParameters: TypeParameterDeclaration[] | undefined, heritageClauses: HeritageClause[], members: TypeElement[]) { + export function createInterfaceDeclaration(decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: string | Identifier, typeParameters: TypeParameterDeclaration[] | undefined, heritageClauses: HeritageClause[] | undefined, members: TypeElement[]) { const node = createSynthesizedNode(SyntaxKind.InterfaceDeclaration); node.decorators = asNodeArray(decorators); node.modifiers = asNodeArray(modifiers); @@ -1451,7 +1451,7 @@ namespace ts { return node; } - export function updateInterfaceDeclaration(node: InterfaceDeclaration, decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: Identifier | undefined, typeParameters: TypeParameterDeclaration[] | undefined, heritageClauses: HeritageClause[], members: TypeElement[]) { + export function updateInterfaceDeclaration(node: InterfaceDeclaration, decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: Identifier, typeParameters: TypeParameterDeclaration[] | undefined, heritageClauses: HeritageClause[] | undefined, members: TypeElement[]) { return node.decorators !== decorators || node.modifiers !== modifiers || node.name !== name From 293a04caa73056adeb7d179c447c7296d8da65cf Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 11:47:03 -0700 Subject: [PATCH 16/28] More thorough module reuse test --- .../unittests/reuseProgramStructure.ts | 182 +++++++++++++++--- 1 file changed, 151 insertions(+), 31 deletions(-) diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index 41d7045064f..5f193394422 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -499,8 +499,49 @@ namespace ts { const options: CompilerOptions = { target: ScriptTarget.ES2015, traceResolution: true, moduleResolution: ModuleResolutionKind.Classic }; const program_1 = newProgram(files, files.map(f => f.name), options); - assert.deepEqual(program_1.host.getTrace(), - [ + let expectedErrors = 0; + { + assert.deepEqual(program_1.host.getTrace(), + [ + "======== Resolving type reference directive 'typerefs1', containing file 'f1.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs1/package.json' does not exist.", + "File 'node_modules/@types/typerefs1/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs1' was successfully resolved to 'node_modules/@types/typerefs1/index.d.ts', primary: true. ========", + "======== Resolving module './b1' from 'f1.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "======== Resolving module './b2' from 'f2.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b2.ts' exist - use it as a name resolution result.", + "======== Module name './b2' was successfully resolved to 'b2.ts'. ========", + "======== Resolving module './f1' from 'f2.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'f1.ts' exist - use it as a name resolution result.", + "======== Module name './f1' was successfully resolved to 'f1.ts'. ========" + ], + "program_1: execute module reoslution normally."); + + const program_1Diagnostics = program_1.getSemanticDiagnostics(program_1.getSourceFile("f2.ts")); + assert(program_1Diagnostics.length === expectedErrors, `initial program should be well-formed`); + } + const indexOfF1 = 6; + const program_2 = updateProgram(program_1, program_1.getRootFileNames(), options, f => { + const newSourceText = f[indexOfF1].text.updateReferences(`/// ${newLine}/// `); + f[indexOfF1] = { name: "f1.ts", text: newSourceText }; + }); + + { + const program_2Diagnostics = program_2.getSemanticDiagnostics(program_2.getSourceFile("f2.ts")); + assert(program_2Diagnostics.length === expectedErrors, `removing no-default-lib shouldn't affect any types used.`); + + assert.deepEqual(program_2.host.getTrace(), [ "======== Resolving type reference directive 'typerefs1', containing file 'f1.ts', root directory 'node_modules/@types'. ========", "Resolving with primary search path 'node_modules/@types'.", "File 'node_modules/@types/typerefs1/package.json' does not exist.", @@ -515,41 +556,120 @@ namespace ts { "File 'node_modules/@types/typerefs2/package.json' does not exist.", "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", - "======== Resolving module './b2' from 'f2.ts'. ========", - "Explicitly specified module resolution kind: 'Classic'.", - "File 'b2.ts' exist - use it as a name resolution result.", - "======== Module name './b2' was successfully resolved to 'b2.ts'. ========", - "======== Resolving module './f1' from 'f2.ts'. ========", - "Explicitly specified module resolution kind: 'Classic'.", - "File 'f1.ts' exist - use it as a name resolution result.", - "======== Module name './f1' was successfully resolved to 'f1.ts'. ========" - ], - "First program should execute module reoslution normally."); + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." + ], "program_2: reuse module resolutions in f2 since it is unchanged"); + } - const program_1Diagnostics = program_1.getSemanticDiagnostics(program_1.getSourceFile("f2.ts")); - assert(program_1Diagnostics.length === 0, `initial program should be well-formed`); - - const indexOfF1 = 6; - const program_2 = updateProgram(program_1, program_1.getRootFileNames(), options, f => { - let newSourceText = f[indexOfF1].text.updateProgram(""); - newSourceText = newSourceText.updateImportsAndExports(""); - newSourceText = newSourceText.updateReferences(""); + const program_3 = updateProgram(program_2, program_2.getRootFileNames(), options, f => { + const newSourceText = f[indexOfF1].text.updateReferences(`/// `); f[indexOfF1] = { name: "f1.ts", text: newSourceText }; }); + { + const program_3Diagnostics = program_3.getSemanticDiagnostics(program_3.getSourceFile("f2.ts")); + assert(program_3Diagnostics.length === expectedErrors, `typerefs2 was unused, so diagnostics should be unaffected.`); - const program_2Diagnostics = program_2.getSemanticDiagnostics(program_2.getSourceFile("f2.ts")); - assert(program_2Diagnostics.length === 1, `f1 no longer augments BB, should get one error.`); + assert.deepEqual(program_3.host.getTrace(), [ + "======== Resolving module './b1' from 'f1.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." + ], "program_3: reuse module resolutions in f2 since it is unchanged"); + } - assert.deepEqual(program_2.host.getTrace(), [ - "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", - "Resolving with primary search path 'node_modules/@types'.", - "File 'node_modules/@types/typerefs2/package.json' does not exist.", - "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", - "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", - "Reusing resolution of module './b2' to file 'f2.ts' from old program.", - "Reusing resolution of module './f1' to file 'f2.ts' from old program." - ], "should reuse module resolutions in f2 since it is unchanged"); + + const program_4 = updateProgram(program_3, program_3.getRootFileNames(), options, f => { + const newSourceText = f[indexOfF1].text.updateReferences(""); + f[indexOfF1] = { name: "f1.ts", text: newSourceText }; + }); + + { + const program_4Diagnostics = program_4.getSemanticDiagnostics(program_4.getSourceFile("f2.ts")); + assert(program_4Diagnostics.length === expectedErrors, `a1.ts was unused, so diagnostics should be unaffected.`); + + assert.deepEqual(program_4.host.getTrace(), [ + "======== Resolving module './b1' from 'f1.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." + ], "program_4: reuse module resolutions in f2 since it is unchanged"); + } + + const program_5 = updateProgram(program_4, program_4.getRootFileNames(), options, f => { + const newSourceText = f[indexOfF1].text.updateImportsAndExports(`import { B } from './b1';`); + f[indexOfF1] = { name: "f1.ts", text: newSourceText }; + }); + + { + const program_5Diagnostics = program_5.getSemanticDiagnostics(program_5.getSourceFile("f2.ts")); + assert(program_5Diagnostics.length === ++expectedErrors, `import of BB in f1 fails. BB is of type any. Add one error`); + + assert.deepEqual(program_5.host.getTrace(), [ + "======== Resolving module './b1' from 'f1.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========" + ], "program_5: exports do not affect program structure, so f2's reoslutions are silently reused."); + } + + const program_6 = updateProgram(program_5, program_5.getRootFileNames(), options, f => { + const newSourceText = f[indexOfF1].text.updateProgram(""); + f[indexOfF1] = { name: "f1.ts", text: newSourceText }; + }); + + { + const program_6Diagnostics = program_6.getSemanticDiagnostics(program_6.getSourceFile("f2.ts")); + assert(program_6Diagnostics.length === expectedErrors, `import of BB in f1 fails.`); + + assert.deepEqual(program_6.host.getTrace(), [ + "======== Resolving module './b1' from 'f1.ts'. ========", + "Explicitly specified module resolution kind: 'Classic'.", + "File 'b1.ts' exist - use it as a name resolution result.", + "======== Module name './b1' was successfully resolved to 'b1.ts'. ========", + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." + ], "program_6: reuse module resolutions in f2 since it is unchanged"); + } + + const program_7 = updateProgram(program_6, program_6.getRootFileNames(), options, f => { + const newSourceText = f[indexOfF1].text.updateImportsAndExports(""); + f[indexOfF1] = { name: "f1.ts", text: newSourceText }; + }); + + { + const program_7Diagnostics = program_7.getSemanticDiagnostics(program_7.getSourceFile("f2.ts")); + assert(program_7Diagnostics.length === expectedErrors, `removing import is noop with respect to program, so no change in diangostics.`); + + assert.deepEqual(program_7.host.getTrace(), [ + "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", + "Resolving with primary search path 'node_modules/@types'.", + "File 'node_modules/@types/typerefs2/package.json' does not exist.", + "File 'node_modules/@types/typerefs2/index.d.ts' exist - use it as a name resolution result.", + "======== Type reference directive 'typerefs2' was successfully resolved to 'node_modules/@types/typerefs2/index.d.ts', primary: true. ========", + "Reusing resolution of module './b2' to file 'f2.ts' from old program.", + "Reusing resolution of module './f1' to file 'f2.ts' from old program." + ], "program_7 should reuse module resolutions in f2 since it is unchanged"); + } }); }); From ef1cd50dfc71c99fff469c12ac58bac8fe663734 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 11:47:12 -0700 Subject: [PATCH 17/28] clarifying comments --- src/compiler/program.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index c44fdce391e..4ef1d7939c6 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -499,7 +499,7 @@ namespace ts { // so it is defined iff we already called the current function on `file`. // That call happened no later than the creation of the `file` object, // which per above occured during the current program creation. - // Since we model program creation as an atomic operation w/r/t IO, + // Since we assume the filesystem does not change during program creation, // it is safe to reuse resolutions from the earlier call. const result: ResolvedModuleFull[] = []; for (const moduleName of moduleNames) { @@ -581,6 +581,8 @@ namespace ts { let j = 0; for (let i = 0; i < result.length; i++) { if (result[i]) { + // `result[i]` is either a `ResolvedModuleFull` or a marker. + // If it is the former, we can leave it as is. if (result[i] === predictedToResolveToAmbientModuleMarker) { result[i] = undefined; } From 872fdee4a51a15be999ae42df1f2891dc23f641b Mon Sep 17 00:00:00 2001 From: Mohsen Azimi Date: Mon, 1 May 2017 12:55:40 -0700 Subject: [PATCH 18/28] Add createTypeAliasDeclaration and updateTypeAliasDeclaration factories --- src/compiler/factory.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 95878917b3d..72f8e255cb4 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -1462,6 +1462,22 @@ namespace ts { : node; } + export function createTypeAliasDeclaration(name: string | Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: ts.TypeNode) { + const node = createSynthesizedNode(SyntaxKind.TypeAliasDeclaration); + node.name = asName(name); + node.typeParameters = asNodeArray(typeParameters); + node.type = type; + return node; + } + + export function updateTypeAliasDeclaration(node: TypeAliasDeclaration, name: string | Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: ts.TypeNode) { + return node.name !== name + || node.typeParameters !== typeParameters + || node.type !== type + ? updateNode(createTypeAliasDeclaration(name, typeParameters, type), node) + : node; + } + export function createEnumDeclaration(decorators: Decorator[] | undefined, modifiers: Modifier[] | undefined, name: string | Identifier, members: EnumMember[]) { const node = createSynthesizedNode(SyntaxKind.EnumDeclaration); node.decorators = asNodeArray(decorators); From f9c46b5f820d098ab64054055579ca7819259ce0 Mon Sep 17 00:00:00 2001 From: Mohsen Azimi Date: Mon, 1 May 2017 14:00:16 -0700 Subject: [PATCH 19/28] Fix issues raised in PR review --- src/compiler/factory.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 72f8e255cb4..011cedcad81 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -1462,7 +1462,7 @@ namespace ts { : node; } - export function createTypeAliasDeclaration(name: string | Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: ts.TypeNode) { + export function createTypeAliasDeclaration(name: Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: TypeNode) { const node = createSynthesizedNode(SyntaxKind.TypeAliasDeclaration); node.name = asName(name); node.typeParameters = asNodeArray(typeParameters); @@ -1470,7 +1470,7 @@ namespace ts { return node; } - export function updateTypeAliasDeclaration(node: TypeAliasDeclaration, name: string | Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: ts.TypeNode) { + export function updateTypeAliasDeclaration(node: TypeAliasDeclaration, name: Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: TypeNode) { return node.name !== name || node.typeParameters !== typeParameters || node.type !== type From b27aec2247f5c104b52fec588f0274f4d47e5230 Mon Sep 17 00:00:00 2001 From: Mohsen Azimi Date: Mon, 1 May 2017 14:58:36 -0700 Subject: [PATCH 20/28] Allow string for name when creating a TypeAliasDeclaration --- src/compiler/factory.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 011cedcad81..cce95c23c22 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -1462,7 +1462,7 @@ namespace ts { : node; } - export function createTypeAliasDeclaration(name: Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: TypeNode) { + export function createTypeAliasDeclaration(name: string | Identifier, typeParameters: TypeParameterDeclaration[] | undefined, type: TypeNode) { const node = createSynthesizedNode(SyntaxKind.TypeAliasDeclaration); node.name = asName(name); node.typeParameters = asNodeArray(typeParameters); From 2118de09e7195874c25af3f93aade124f3a5aa8a Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 15:16:42 -0700 Subject: [PATCH 21/28] Simplify returns --- src/compiler/program.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 4ef1d7939c6..97f09c083f5 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -643,13 +643,11 @@ namespace ts { // there is an old program, check if we can reuse its structure const oldRootNames = oldProgram.getRootFileNames(); if (!arrayIsEqualTo(oldRootNames, rootNames)) { - oldProgram.structureIsReused = StructureIsReused.Not; - return StructureIsReused.Not; + return oldProgram.structureIsReused = StructureIsReused.Not; } if (!arrayIsEqualTo(options.types, oldOptions.types)) { - oldProgram.structureIsReused = StructureIsReused.Not; - return StructureIsReused.Not; + return oldProgram.structureIsReused = StructureIsReused.Not; } // check if program source files has changed in the way that can affect structure of the program From 28843c41978ea40ea5fa4ed6a4b7252bef713761 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 15:16:58 -0700 Subject: [PATCH 22/28] npm install test --- .../unittests/reuseProgramStructure.ts | 80 +++++++++++++++++-- 1 file changed, 73 insertions(+), 7 deletions(-) diff --git a/src/harness/unittests/reuseProgramStructure.ts b/src/harness/unittests/reuseProgramStructure.ts index 5f193394422..2eb3d1f0890 100644 --- a/src/harness/unittests/reuseProgramStructure.ts +++ b/src/harness/unittests/reuseProgramStructure.ts @@ -159,12 +159,14 @@ namespace ts { return program; } - function updateProgram(oldProgram: ProgramWithSourceTexts, rootNames: string[], options: CompilerOptions, updater: (files: NamedSourceText[]) => void) { - const texts: NamedSourceText[] = (oldProgram).sourceTexts.slice(0); - updater(texts); - const host = createTestCompilerHost(texts, options.target, oldProgram); + function updateProgram(oldProgram: ProgramWithSourceTexts, rootNames: string[], options: CompilerOptions, updater: (files: NamedSourceText[]) => void, newTexts?: NamedSourceText[]) { + if (!newTexts) { + newTexts = (oldProgram).sourceTexts.slice(0); + } + updater(newTexts); + const host = createTestCompilerHost(newTexts, options.target, oldProgram); const program = createProgram(rootNames, options, host, oldProgram); - program.sourceTexts = texts; + program.sourceTexts = newTexts; program.host = host; return program; } @@ -391,6 +393,70 @@ namespace ts { checkResolvedTypeDirectivesCache(program_1, "/a.ts", createMapFromTemplate({ "typedefs": { resolvedFileName: "/types/typedefs/index.d.ts", primary: true } })); }); + it("fetches imports after npm install", () => { + const file1Ts = { name: "file1.ts", text: SourceText.New("", `import * as a from "a";`, "const myX: number = a.x;") }; + const file2Ts = { name: "file2.ts", text: SourceText.New("", "", "") }; + const indexDTS = { name: "node_modules/a/index.d.ts", text: SourceText.New("", "export declare let x: number;", "") }; + const options: CompilerOptions = { target: ScriptTarget.ES2015, traceResolution: true, moduleResolution: ModuleResolutionKind.NodeJs }; + const rootFiles = [file1Ts, file2Ts]; + const filesAfterNpmInstall = [file1Ts, file2Ts, indexDTS]; + + const initialProgram = newProgram(rootFiles, rootFiles.map(f => f.name), options); + { + assert.deepEqual(initialProgram.host.getTrace(), + [ + "======== Resolving module 'a' from 'file1.ts'. ========", + "Explicitly specified module resolution kind: 'NodeJs'.", + "Loading module 'a' from 'node_modules' folder, target file type 'TypeScript'.", + "File 'node_modules/a.ts' does not exist.", + "File 'node_modules/a.tsx' does not exist.", + "File 'node_modules/a.d.ts' does not exist.", + "File 'node_modules/a/package.json' does not exist.", + "File 'node_modules/a/index.ts' does not exist.", + "File 'node_modules/a/index.tsx' does not exist.", + "File 'node_modules/a/index.d.ts' does not exist.", + "File 'node_modules/@types/a.d.ts' does not exist.", + "File 'node_modules/@types/a/package.json' does not exist.", + "File 'node_modules/@types/a/index.d.ts' does not exist.", + "Loading module 'a' from 'node_modules' folder, target file type 'JavaScript'.", + "File 'node_modules/a.js' does not exist.", + "File 'node_modules/a.jsx' does not exist.", + "File 'node_modules/a/package.json' does not exist.", + "File 'node_modules/a/index.js' does not exist.", + "File 'node_modules/a/index.jsx' does not exist.", + "======== Module name 'a' was not resolved. ========" + ], + "initialProgram: execute module resolution normally."); + + const initialProgramDiagnostics = initialProgram.getSemanticDiagnostics(initialProgram.getSourceFile("file1.ts")); + assert(initialProgramDiagnostics.length === 1, `initialProgram: import should fail.`); + } + + const afterNpmInstallProgram = updateProgram(initialProgram, rootFiles.map(f => f.name), options, f => { + f[1].text = f[1].text.updateReferences(`/// `); + }, filesAfterNpmInstall); + { + assert.deepEqual(afterNpmInstallProgram.host.getTrace(), + [ + "======== Resolving module 'a' from 'file1.ts'. ========", + "Explicitly specified module resolution kind: 'NodeJs'.", + "Loading module 'a' from 'node_modules' folder, target file type 'TypeScript'.", + "File 'node_modules/a.ts' does not exist.", + "File 'node_modules/a.tsx' does not exist.", + "File 'node_modules/a.d.ts' does not exist.", + "File 'node_modules/a/package.json' does not exist.", + "File 'node_modules/a/index.ts' does not exist.", + "File 'node_modules/a/index.tsx' does not exist.", + "File 'node_modules/a/index.d.ts' exist - use it as a name resolution result.", + "======== Module name 'a' was successfully resolved to 'node_modules/a/index.d.ts'. ========" + ], + "afterNpmInstallProgram: execute module resolution normally."); + + const afterNpmInstallProgramDiagnostics = afterNpmInstallProgram.getSemanticDiagnostics(afterNpmInstallProgram.getSourceFile("file1.ts")); + assert(afterNpmInstallProgramDiagnostics.length === 0, `afterNpmInstallProgram: program is well-formed with import.`); + } + }); + it("can reuse ambient module declarations from non-modified files", () => { const files = [ { name: "/a/b/app.ts", text: SourceText.New("", "import * as fs from 'fs'", "") }, @@ -624,7 +690,7 @@ namespace ts { "Explicitly specified module resolution kind: 'Classic'.", "File 'b1.ts' exist - use it as a name resolution result.", "======== Module name './b1' was successfully resolved to 'b1.ts'. ========" - ], "program_5: exports do not affect program structure, so f2's reoslutions are silently reused."); + ], "program_5: exports do not affect program structure, so f2's resolutions are silently reused."); } const program_6 = updateProgram(program_5, program_5.getRootFileNames(), options, f => { @@ -658,7 +724,7 @@ namespace ts { { const program_7Diagnostics = program_7.getSemanticDiagnostics(program_7.getSourceFile("f2.ts")); - assert(program_7Diagnostics.length === expectedErrors, `removing import is noop with respect to program, so no change in diangostics.`); + assert(program_7Diagnostics.length === expectedErrors, `removing import is noop with respect to program, so no change in diagnostics.`); assert.deepEqual(program_7.host.getTrace(), [ "======== Resolving type reference directive 'typerefs2', containing file 'f2.ts', root directory 'node_modules/@types'. ========", From d45d289f36e3dc0f9121c3355ae4e9457ed65657 Mon Sep 17 00:00:00 2001 From: Vladimir Kurchatkin Date: Tue, 2 May 2017 01:39:02 +0300 Subject: [PATCH 23/28] Allow indexed access to empty property Fixes: https://github.com/Microsoft/TypeScript/issues/15209 --- src/compiler/checker.ts | 2 +- .../reference/noImplicitAnyStringIndexerOnObject.errors.txt | 3 ++- .../baselines/reference/noImplicitAnyStringIndexerOnObject.js | 4 +++- tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 18e57303c12..332e4a7fdc1 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7257,7 +7257,7 @@ namespace ts { accessExpression && checkThatExpressionIsProperSymbolReference(accessExpression.argumentExpression, indexType, /*reportError*/ false) ? getPropertyNameForKnownSymbolName(((accessExpression.argumentExpression).name).text) : undefined; - if (propName) { + if (propName !== undefined) { const prop = getPropertyOfType(objectType, propName); if (prop) { if (accessExpression) { diff --git a/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.errors.txt b/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.errors.txt index 8a068048f3b..b3f3b4bc6e0 100644 --- a/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.errors.txt +++ b/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.errors.txt @@ -4,4 +4,5 @@ tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts(1,9): error TS7017: E ==== tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts (1 errors) ==== var x = {}["hello"]; ~~~~~~~~~~~ -!!! error TS7017: Element implicitly has an 'any' type because type '{}' has no index signature. \ No newline at end of file +!!! error TS7017: Element implicitly has an 'any' type because type '{}' has no index signature. + var y: string = { '': 'foo' }['']; \ No newline at end of file diff --git a/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.js b/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.js index cc2309192e1..2ea3291e5db 100644 --- a/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.js +++ b/tests/baselines/reference/noImplicitAnyStringIndexerOnObject.js @@ -1,5 +1,7 @@ //// [noImplicitAnyStringIndexerOnObject.ts] -var x = {}["hello"]; +var x = {}["hello"]; +var y: string = { '': 'foo' }['']; //// [noImplicitAnyStringIndexerOnObject.js] var x = {}["hello"]; +var y = { '': 'foo' }['']; diff --git a/tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts b/tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts index 814c6d2fa6d..9a84cc328b2 100644 --- a/tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts +++ b/tests/cases/compiler/noImplicitAnyStringIndexerOnObject.ts @@ -1,3 +1,4 @@ // @noimplicitany: true -var x = {}["hello"]; \ No newline at end of file +var x = {}["hello"]; +var y: string = { '': 'foo' }['']; \ No newline at end of file From 6cd85dbb25a59a2cbcc5dfa4181e0b3a840095ad Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 1 May 2017 16:05:41 -0700 Subject: [PATCH 24/28] Fix failing transpileModule test --- src/harness/unittests/transform.ts | 3 +++ .../transformApi/transformsCorrectly.fromTranspileModule.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/harness/unittests/transform.ts b/src/harness/unittests/transform.ts index 71f50ed94a9..deee352ebf1 100644 --- a/src/harness/unittests/transform.ts +++ b/src/harness/unittests/transform.ts @@ -68,6 +68,9 @@ namespace ts { transformers: { before: [replaceUndefinedWithVoid0], after: [replaceIdentifiersNamedOldNameWithNewName] + }, + compilerOptions: { + newLine: NewLineKind.CarriageReturnLineFeed } }).outputText; }); diff --git a/tests/baselines/reference/transformApi/transformsCorrectly.fromTranspileModule.js b/tests/baselines/reference/transformApi/transformsCorrectly.fromTranspileModule.js index ddf3fd0e8d6..714feaea305 100644 --- a/tests/baselines/reference/transformApi/transformsCorrectly.fromTranspileModule.js +++ b/tests/baselines/reference/transformApi/transformsCorrectly.fromTranspileModule.js @@ -1 +1 @@ -var newName = void 0 /*undefined*/; +var newName = void 0 /*undefined*/; From df3630b92c7975c59a358db279e6d0dc6a73bc86 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 16:37:18 -0700 Subject: [PATCH 25/28] reorder enum --- src/compiler/types.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 55b7222796d..729fc7ab57c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2411,9 +2411,9 @@ namespace ts { /* @internal */ export const enum StructureIsReused { - Completely, - SafeModules, - Not + Not = 0, + SafeModules = 1 << 0, + Completely = 1 << 1, } export interface CustomTransformers { From 7d1d22ce4be65ce8bdc2e9f8a9cf862436a6f43d Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 16:37:27 -0700 Subject: [PATCH 26/28] assert length --- src/compiler/utilities.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 4381682c9f4..f952f372181 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -122,9 +122,8 @@ namespace ts { /* @internal */ export function hasChangesInResolutions(names: string[], newResolutions: T[], oldResolutions: Map, comparer: (oldResolution: T, newResolution: T) => boolean): boolean { - if (names.length !== newResolutions.length) { - return true; - } + Debug.assert(names.length === newResolutions.length); + for (let i = 0; i < names.length; i++) { const newResolution = newResolutions[i]; const oldResolution = oldResolutions && oldResolutions.get(names[i]); From 3f95b86e65ea09319111d76bdfcaeac1828393a5 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 1 May 2017 16:45:06 -0700 Subject: [PATCH 27/28] Optimize signature relationship checks in instantiations of same type --- src/compiler/checker.ts | 44 +++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 332e4a7fdc1..34a176ac23c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9228,25 +9228,39 @@ namespace ts { let result = Ternary.True; const saveErrorInfo = errorInfo; - outer: for (const t of targetSignatures) { - // Only elaborate errors from the first failure - let shouldElaborateErrors = reportErrors; - for (const s of sourceSignatures) { - const related = signatureRelatedTo(s, t, shouldElaborateErrors); - if (related) { - result &= related; - errorInfo = saveErrorInfo; - continue outer; + if (getObjectFlags(source) & ObjectFlags.Instantiated && getObjectFlags(target) & ObjectFlags.Instantiated && source.symbol === target.symbol) { + // We instantiations of the same anonymous type (which typically will be the type of a method). + // Simply do a pairwise comparison of the signatures in the two signature lists instead of the + // much more expensive N * M comparison matrix we explore below. + for (let i = 0; i < targetSignatures.length; i++) { + const related = signatureRelatedTo(sourceSignatures[i], targetSignatures[i], reportErrors); + if (!related) { + return Ternary.False; } - shouldElaborateErrors = false; + result &= related; } + } + else { + outer: for (const t of targetSignatures) { + // Only elaborate errors from the first failure + let shouldElaborateErrors = reportErrors; + for (const s of sourceSignatures) { + const related = signatureRelatedTo(s, t, shouldElaborateErrors); + if (related) { + result &= related; + errorInfo = saveErrorInfo; + continue outer; + } + shouldElaborateErrors = false; + } - if (shouldElaborateErrors) { - reportError(Diagnostics.Type_0_provides_no_match_for_the_signature_1, - typeToString(source), - signatureToString(t, /*enclosingDeclaration*/ undefined, /*flags*/ undefined, kind)); + if (shouldElaborateErrors) { + reportError(Diagnostics.Type_0_provides_no_match_for_the_signature_1, + typeToString(source), + signatureToString(t, /*enclosingDeclaration*/ undefined, /*flags*/ undefined, kind)); + } + return Ternary.False; } - return Ternary.False; } return result; } From 60825143a4efe70263456d210ededabf8a10acdc Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Mon, 1 May 2017 17:49:19 -0700 Subject: [PATCH 28/28] respond to comments --- src/compiler/diagnosticMessages.json | 2 +- src/compiler/program.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index b045def34e7..f819c60371e 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3203,7 +3203,7 @@ }, "Reusing resolution of module '{0}' to file '{1}' from old program.": { "category": "Message", - "code": 618 + "code": 6183 }, "Reusing module resolutions originating in '{0}' since resolutions are unchanged from old program.": { "category": "Message", diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 97f09c083f5..61561726670 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -531,7 +531,7 @@ namespace ts { for (let i = 0; i < moduleNames.length; i++) { const moduleName = moduleNames[i]; - // TODO: if we want to reuse resolutions more aggressively, refine this to check for whether the + // If we want to reuse resolutions more aggressively, we can refine this to check for whether the // text of the corresponding modulenames has changed. if (file === oldSourceFile) { const oldResolvedModule = oldSourceFile && oldSourceFile.resolvedModules.get(moduleName); @@ -596,7 +596,7 @@ namespace ts { return result; - // TODO: if we change our policy of rechecking failed lookups on each program create, + // If we change our policy of rechecking failed lookups on each program create, // we should adjust the value returned here. function moduleNameResolvesToAmbientModuleInNonModifiedFile(moduleName: string, oldProgramState: OldProgramState): boolean { const resolutionToFile = getResolvedModule(oldProgramState.file, moduleName);