From d5f246fd9919072338e0d31331e1fd7276eebd7c Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 18 Apr 2017 15:48:01 -0700 Subject: [PATCH 01/30] 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 f2d44adb1f..8081775b07 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/30] 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 8081775b07..d2e257867f 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/30] 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 d2e257867f..ed9c08d6d9 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/30] 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 a1200b6a07..ee37978776 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 373ccb08d4..2a9891e9d6 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 43014f25af..e53aa0b452 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/30] 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 2a9891e9d6..199788d47b 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 8874fd1a79..ec722a9309 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 0725bf36a3..1b87fc848b 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 14e13b6045..a4220f5032 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 e53aa0b452..8810d8a640 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 09c8d74c8c..56c394379a 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/30] 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 199788d47b..8bd7bbbc76 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/30] 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 ee37978776..b045def34e 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 8bd7bbbc76..15dd8a097d 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 ec722a9309..4c850d673a 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 f34c15ee34..4381682c9f 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 8810d8a640..d7140cfc8b 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/30] 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 15dd8a097d..63d257c359 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/30] 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 3c86702448..a024a574a9 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 63d257c359..c44fdce391 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 4c850d673a..55b7222796 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 d7140cfc8b..41d7045064 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 b39a2a7377b59d86b49c833293f4545d55231c2d Mon Sep 17 00:00:00 2001 From: Blaine Bublitz Date: Mon, 1 May 2017 12:19:28 -0500 Subject: [PATCH 10/30] 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 dbaaffdc62..2eb472ebdf 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 11/30] 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 2eb472ebdf..95878917b3 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 12/30] 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 41d7045064..5f19339442 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 13/30] 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 c44fdce391..4ef1d7939c 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 14/30] 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 95878917b3..72f8e255cb 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 15/30] 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 72f8e255cb..011cedcad8 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 16/30] 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 011cedcad8..cce95c23c2 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 17/30] 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 4ef1d7939c..97f09c083f 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 18/30] 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 5f19339442..2eb3d1f089 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 19/30] 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 18e57303c1..332e4a7fdc 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 8a068048f3..b3f3b4bc6e 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 cc2309192e..2ea3291e5d 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 814c6d2fa6..9a84cc328b 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 20/30] 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 71f50ed94a..deee352ebf 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 ddf3fd0e8d..714feaea30 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 21/30] 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 55b7222796..729fc7ab57 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 22/30] 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 4381682c9f..f952f37218 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 23/30] 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 332e4a7fdc..34a176ac23 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 24/30] 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 b045def34e..f819c60371 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 97f09c083f..6156172667 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); From b6bd396983d50a59349f2f3ad6aa02af6f0822b6 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 2 May 2017 08:39:22 -0700 Subject: [PATCH 25/30] Fix formatting for async computed method: Allow space between 'async' and '[' --- src/services/formatting/rules.ts | 4 +- src/services/formatting/rulesMap.ts | 3 +- src/services/formatting/tokenRange.ts | 46 +++++++++++++++---- .../fourslash/formatAsyncComputedMethod.ts | 9 ++++ 4 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/formatAsyncComputedMethod.ts diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index c34b5c6aa9..2c1becb8dd 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -283,7 +283,9 @@ namespace ts.formatting { this.NoSpaceAfterDot = new Rule(RuleDescriptor.create3(SyntaxKind.DotToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Delete)); // No space before and after indexer - this.NoSpaceBeforeOpenBracket = new Rule(RuleDescriptor.create2(Shared.TokenRange.Any, SyntaxKind.OpenBracketToken), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Delete)); + this.NoSpaceBeforeOpenBracket = new Rule( + RuleDescriptor.create2(Shared.TokenRange.AnyExcept(SyntaxKind.AsyncKeyword), SyntaxKind.OpenBracketToken), + RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext), RuleAction.Delete)); this.NoSpaceAfterCloseBracket = new Rule(RuleDescriptor.create3(SyntaxKind.CloseBracketToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsNonJsxSameLineTokenContext, Rules.IsNotBeforeBlockInFunctionDeclarationContext), RuleAction.Delete)); // Place a space before open brace in a function declaration diff --git a/src/services/formatting/rulesMap.ts b/src/services/formatting/rulesMap.ts index cd6f2e539d..5b4eacd2c2 100644 --- a/src/services/formatting/rulesMap.ts +++ b/src/services/formatting/rulesMap.ts @@ -41,8 +41,7 @@ namespace ts.formatting { } private FillRule(rule: Rule, rulesBucketConstructionStateList: RulesBucketConstructionState[]): void { - const specificRule = rule.Descriptor.LeftTokenRange !== Shared.TokenRange.Any && - rule.Descriptor.RightTokenRange !== Shared.TokenRange.Any; + const specificRule = rule.Descriptor.LeftTokenRange.isSpecific() && rule.Descriptor.RightTokenRange.isSpecific(); rule.Descriptor.LeftTokenRange.GetTokens().forEach((left) => { rule.Descriptor.RightTokenRange.GetTokens().forEach((right) => { diff --git a/src/services/formatting/tokenRange.ts b/src/services/formatting/tokenRange.ts index 05d7369e77..28f22cec47 100644 --- a/src/services/formatting/tokenRange.ts +++ b/src/services/formatting/tokenRange.ts @@ -6,6 +6,7 @@ namespace ts.formatting { export interface ITokenAccess { GetTokens(): SyntaxKind[]; Contains(token: SyntaxKind): boolean; + isSpecific(): boolean; } export class TokenRangeAccess implements ITokenAccess { @@ -27,6 +28,8 @@ namespace ts.formatting { public Contains(token: SyntaxKind): boolean { return this.tokens.indexOf(token) >= 0; } + + public isSpecific() { return true; } } export class TokenValuesAccess implements ITokenAccess { @@ -43,6 +46,8 @@ namespace ts.formatting { public Contains(token: SyntaxKind): boolean { return this.tokens.indexOf(token) >= 0; } + + public isSpecific() { return true; } } export class TokenSingleValueAccess implements ITokenAccess { @@ -56,15 +61,18 @@ namespace ts.formatting { public Contains(tokenValue: SyntaxKind): boolean { return tokenValue === this.token; } + + public isSpecific() { return true; } + } + + const allTokens: SyntaxKind[] = []; + for (let token = SyntaxKind.FirstToken; token <= SyntaxKind.LastToken; token++) { + allTokens.push(token); } export class TokenAllAccess implements ITokenAccess { public GetTokens(): SyntaxKind[] { - const result: SyntaxKind[] = []; - for (let token = SyntaxKind.FirstToken; token <= SyntaxKind.LastToken; token++) { - result.push(token); - } - return result; + return allTokens; } public Contains(): boolean { @@ -74,6 +82,22 @@ namespace ts.formatting { public toString(): string { return "[allTokens]"; } + + public isSpecific() { return false; } + } + + export class TokenAllExceptAccess implements ITokenAccess { + constructor(readonly except: SyntaxKind) {} + + public GetTokens(): SyntaxKind[] { + return allTokens.filter(t => t !== this.except); + } + + public Contains(token: SyntaxKind): boolean { + return token !== this.except; + } + + public isSpecific() { return false; } } export class TokenRange { @@ -92,8 +116,8 @@ namespace ts.formatting { return new TokenRange(new TokenRangeAccess(f, to, except)); } - static AllTokens(): TokenRange { - return new TokenRange(new TokenAllAccess()); + static AnyExcept(token: SyntaxKind): TokenRange { + return new TokenRange(new TokenAllExceptAccess(token)); } public GetTokens(): SyntaxKind[] { @@ -108,8 +132,12 @@ namespace ts.formatting { return this.tokenAccess.toString(); } - static Any: TokenRange = TokenRange.AllTokens(); - static AnyIncludingMultilineComments = TokenRange.FromTokens(TokenRange.Any.GetTokens().concat([SyntaxKind.MultiLineCommentTrivia])); + public isSpecific() { + return this.tokenAccess.isSpecific(); + } + + static Any: TokenRange = new TokenRange(new TokenAllAccess()); + static AnyIncludingMultilineComments = TokenRange.FromTokens([...allTokens, SyntaxKind.MultiLineCommentTrivia]); static Keywords = TokenRange.FromRange(SyntaxKind.FirstKeyword, SyntaxKind.LastKeyword); static BinaryOperators = TokenRange.FromRange(SyntaxKind.FirstBinaryOperator, SyntaxKind.LastBinaryOperator); static BinaryKeywordOperators = TokenRange.FromTokens([SyntaxKind.InKeyword, SyntaxKind.InstanceOfKeyword, SyntaxKind.OfKeyword, SyntaxKind.AsKeyword, SyntaxKind.IsKeyword]); diff --git a/tests/cases/fourslash/formatAsyncComputedMethod.ts b/tests/cases/fourslash/formatAsyncComputedMethod.ts new file mode 100644 index 0000000000..b0365fd35c --- /dev/null +++ b/tests/cases/fourslash/formatAsyncComputedMethod.ts @@ -0,0 +1,9 @@ +/// + +////class C { +//// /*method*/async [0]() { } +////} + +format.document(); +goTo.marker("method"); +verify.currentLineContentIs(" async [0]() { }"); From 04c894888d90417949262a703c8c12b359b88bbe Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 2 May 2017 13:21:39 -0700 Subject: [PATCH 26/30] Re-use code from 'getTargetOfExportSpecifier' --- src/compiler/checker.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 34a176ac23..ca5d65068a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1400,10 +1400,10 @@ namespace ts { return resolveExternalModuleSymbol(node.parent.symbol, dontResolveAlias); } - function getTargetOfExportSpecifier(node: ExportSpecifier, dontResolveAlias?: boolean): Symbol { - return (node.parent.parent).moduleSpecifier ? - getExternalModuleMember(node.parent.parent, node, dontResolveAlias) : - resolveEntityName(node.propertyName || node.name, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace, /*ignoreErrors*/ false, dontResolveAlias); + function getTargetOfExportSpecifier(node: ExportSpecifier, meaning: SymbolFlags, dontResolveAlias?: boolean) { + return node.parent.parent.moduleSpecifier ? + getExternalModuleMember(node.parent.parent, node, dontResolveAlias) : + resolveEntityName(node.propertyName || node.name, meaning, /*ignoreErrors*/ false, dontResolveAlias); } function getTargetOfExportAssignment(node: ExportAssignment, dontResolveAlias: boolean): Symbol { @@ -1421,7 +1421,7 @@ namespace ts { case SyntaxKind.ImportSpecifier: return getTargetOfImportSpecifier(node, dontRecursivelyResolve); case SyntaxKind.ExportSpecifier: - return getTargetOfExportSpecifier(node, dontRecursivelyResolve); + return getTargetOfExportSpecifier(node, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace, dontRecursivelyResolve); case SyntaxKind.ExportAssignment: return getTargetOfExportAssignment(node, dontRecursivelyResolve); case SyntaxKind.NamespaceExportDeclaration: @@ -3721,10 +3721,7 @@ namespace ts { exportSymbol = resolveName(node.parent, node.text, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias, Diagnostics.Cannot_find_name_0, node); } else if (node.parent.kind === SyntaxKind.ExportSpecifier) { - const exportSpecifier = node.parent; - exportSymbol = (exportSpecifier.parent.parent).moduleSpecifier ? - getExternalModuleMember(exportSpecifier.parent.parent, exportSpecifier) : - resolveEntityName(exportSpecifier.propertyName || exportSpecifier.name, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias); + exportSymbol = getTargetOfExportSpecifier(node.parent, SymbolFlags.Value | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias); } const result: Node[] = []; if (exportSymbol) { From 4c0735218eff9ed294162534de86080b1d427f46 Mon Sep 17 00:00:00 2001 From: Arthur Ozga Date: Tue, 2 May 2017 15:36:14 -0700 Subject: [PATCH 27/30] move import getCodeActions into named method --- src/services/codefixes/importFixes.ts | 777 +++++++++++++------------- 1 file changed, 389 insertions(+), 388 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 78c27a1276..9322f09cb8 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1,5 +1,13 @@ /* @internal */ namespace ts.codefix { + registerCodeFix({ + errorCodes: [ + Diagnostics.Cannot_find_name_0.code, + Diagnostics.Cannot_find_namespace_0.code, + Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code + ], + getCodeActions: getImportCodeActions + }); type ImportCodeActionKind = "CodeChange" | "InsertingIntoExistingImport" | "NewImport"; interface ImportCodeAction extends CodeAction { @@ -113,465 +121,458 @@ namespace ts.codefix { } } - registerCodeFix({ - errorCodes: [ - Diagnostics.Cannot_find_name_0.code, - Diagnostics.Cannot_find_namespace_0.code, - Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code - ], - getCodeActions: (context: CodeFixContext) => { - const sourceFile = context.sourceFile; - const checker = context.program.getTypeChecker(); - const allSourceFiles = context.program.getSourceFiles(); - const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; + function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { + const sourceFile = context.sourceFile; + const checker = context.program.getTypeChecker(); + const allSourceFiles = context.program.getSourceFiles(); + const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; - const token = getTokenAtPosition(sourceFile, context.span.start); - const name = token.getText(); - const symbolIdActionMap = new ImportCodeActionMap(); + const token = getTokenAtPosition(sourceFile, context.span.start); + const name = token.getText(); + const symbolIdActionMap = new ImportCodeActionMap(); - // this is a module id -> module import declaration map - const cachedImportDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[][] = []; - let lastImportDeclaration: Node; + // this is a module id -> module import declaration map + const cachedImportDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[][] = []; + let lastImportDeclaration: Node; - const currentTokenMeaning = getMeaningFromLocation(token); - if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { - const symbol = checker.getAliasedSymbol(checker.getSymbolAtLocation(token)); - return getCodeActionForImport(symbol, /*isDefault*/ false, /*isNamespaceImport*/ true); + const currentTokenMeaning = getMeaningFromLocation(token); + if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { + const symbol = checker.getAliasedSymbol(checker.getSymbolAtLocation(token)); + return getCodeActionForImport(symbol, /*isDefault*/ false, /*isNamespaceImport*/ true); + } + + const candidateModules = checker.getAmbientModules(); + for (const otherSourceFile of allSourceFiles) { + if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { + candidateModules.push(otherSourceFile.symbol); } + } - const candidateModules = checker.getAmbientModules(); - for (const otherSourceFile of allSourceFiles) { - if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { - candidateModules.push(otherSourceFile.symbol); + for (const moduleSymbol of candidateModules) { + context.cancellationToken.throwIfCancellationRequested(); + + // check the default export + const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); + if (defaultExport) { + const localSymbol = getLocalSymbolForExportDefault(defaultExport); + if (localSymbol && localSymbol.name === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { + // check if this symbol is already used + const symbolId = getUniqueSymbolId(localSymbol); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, /*isDefault*/ true)); } } - for (const moduleSymbol of candidateModules) { - context.cancellationToken.throwIfCancellationRequested(); + // check exports with the same name + const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExports(name, moduleSymbol); + if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { + const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol)); + } + } - // check the default export - const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); - if (defaultExport) { - const localSymbol = getLocalSymbolForExportDefault(defaultExport); - if (localSymbol && localSymbol.name === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { - // check if this symbol is already used - const symbolId = getUniqueSymbolId(localSymbol); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, /*isDefault*/ true)); + return symbolIdActionMap.getAllActions(); + + function getImportDeclarations(moduleSymbol: Symbol) { + const moduleSymbolId = getUniqueSymbolId(moduleSymbol); + + const cached = cachedImportDeclarations[moduleSymbolId]; + if (cached) { + return cached; + } + + const existingDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[] = []; + for (const importModuleSpecifier of sourceFile.imports) { + const importSymbol = checker.getSymbolAtLocation(importModuleSpecifier); + if (importSymbol === moduleSymbol) { + existingDeclarations.push(getImportDeclaration(importModuleSpecifier)); + } + } + cachedImportDeclarations[moduleSymbolId] = existingDeclarations; + return existingDeclarations; + + function getImportDeclaration(moduleSpecifier: LiteralExpression) { + let node: Node = moduleSpecifier; + while (node) { + if (node.kind === SyntaxKind.ImportDeclaration) { + return node; } + if (node.kind === SyntaxKind.ImportEqualsDeclaration) { + return node; + } + node = node.parent; } + return undefined; + } + } - // check exports with the same name - const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExports(name, moduleSymbol); - if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { - const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol)); - } + function getUniqueSymbolId(symbol: Symbol) { + if (symbol.flags & SymbolFlags.Alias) { + return getSymbolId(checker.getAliasedSymbol(symbol)); + } + return getSymbolId(symbol); + } + + function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { + const declarations = symbol.getDeclarations(); + return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; + } + + function getCodeActionForImport(moduleSymbol: Symbol, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { + const existingDeclarations = getImportDeclarations(moduleSymbol); + if (existingDeclarations.length > 0) { + // With an existing import statement, there are more than one actions the user can do. + return getCodeActionsForExistingImport(existingDeclarations); + } + else { + return [getCodeActionForNewImport()]; } - return symbolIdActionMap.getAllActions(); + function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { + const actions: ImportCodeAction[] = []; - function getImportDeclarations(moduleSymbol: Symbol) { - const moduleSymbolId = getUniqueSymbolId(moduleSymbol); - - const cached = cachedImportDeclarations[moduleSymbolId]; - if (cached) { - return cached; - } - - const existingDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[] = []; - for (const importModuleSpecifier of sourceFile.imports) { - const importSymbol = checker.getSymbolAtLocation(importModuleSpecifier); - if (importSymbol === moduleSymbol) { - existingDeclarations.push(getImportDeclaration(importModuleSpecifier)); - } - } - cachedImportDeclarations[moduleSymbolId] = existingDeclarations; - return existingDeclarations; - - function getImportDeclaration(moduleSpecifier: LiteralExpression) { - let node: Node = moduleSpecifier; - while (node) { - if (node.kind === SyntaxKind.ImportDeclaration) { - return node; + // It is possible that multiple import statements with the same specifier exist in the file. + // e.g. + // + // import * as ns from "foo"; + // import { member1, member2 } from "foo"; + // + // member3/**/ <-- cusor here + // + // in this case we should provie 2 actions: + // 1. change "member3" to "ns.member3" + // 2. add "member3" to the second import statement's import list + // and it is up to the user to decide which one fits best. + let namespaceImportDeclaration: ImportDeclaration | ImportEqualsDeclaration; + let namedImportDeclaration: ImportDeclaration; + let existingModuleSpecifier: string; + for (const declaration of declarations) { + if (declaration.kind === SyntaxKind.ImportDeclaration) { + const namedBindings = declaration.importClause && declaration.importClause.namedBindings; + if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { + // case: + // import * as ns from "foo" + namespaceImportDeclaration = declaration; } - if (node.kind === SyntaxKind.ImportEqualsDeclaration) { - return node; + else { + // cases: + // import default from "foo" + // import { bar } from "foo" or combination with the first one + // import "foo" + namedImportDeclaration = declaration; } - node = node.parent; + existingModuleSpecifier = declaration.moduleSpecifier.getText(); + } + else { + // case: + // import foo = require("foo") + namespaceImportDeclaration = declaration; + existingModuleSpecifier = getModuleSpecifierFromImportEqualsDeclaration(declaration); } - return undefined; } - } - function getUniqueSymbolId(symbol: Symbol) { - if (symbol.flags & SymbolFlags.Alias) { - return getSymbolId(checker.getAliasedSymbol(symbol)); + if (namespaceImportDeclaration) { + actions.push(getCodeActionForNamespaceImport(namespaceImportDeclaration)); } - return getSymbolId(symbol); - } - function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { - const declarations = symbol.getDeclarations(); - return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; - } - - function getCodeActionForImport(moduleSymbol: Symbol, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { - const existingDeclarations = getImportDeclarations(moduleSymbol); - if (existingDeclarations.length > 0) { - // With an existing import statement, there are more than one actions the user can do. - return getCodeActionsForExistingImport(existingDeclarations); + if (!isNamespaceImport && namedImportDeclaration && namedImportDeclaration.importClause && + (namedImportDeclaration.importClause.name || namedImportDeclaration.importClause.namedBindings)) { + /** + * If the existing import declaration already has a named import list, just + * insert the identifier into that list. + */ + const fileTextChanges = getTextChangeForImportClause(namedImportDeclaration.importClause); + const moduleSpecifierWithoutQuotes = stripQuotes(namedImportDeclaration.moduleSpecifier.getText()); + actions.push(createCodeAction( + Diagnostics.Add_0_to_existing_import_declaration_from_1, + [name, moduleSpecifierWithoutQuotes], + fileTextChanges, + "InsertingIntoExistingImport", + moduleSpecifierWithoutQuotes + )); } else { - return [getCodeActionForNewImport()]; + // we need to create a new import statement, but the existing module specifier can be reused. + actions.push(getCodeActionForNewImport(existingModuleSpecifier)); + } + return actions; + + function getModuleSpecifierFromImportEqualsDeclaration(declaration: ImportEqualsDeclaration) { + if (declaration.moduleReference && declaration.moduleReference.kind === SyntaxKind.ExternalModuleReference) { + return declaration.moduleReference.expression.getText(); + } + return declaration.moduleReference.getText(); } - function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { - const actions: ImportCodeAction[] = []; - - // It is possible that multiple import statements with the same specifier exist in the file. - // e.g. - // - // import * as ns from "foo"; - // import { member1, member2 } from "foo"; - // - // member3/**/ <-- cusor here - // - // in this case we should provie 2 actions: - // 1. change "member3" to "ns.member3" - // 2. add "member3" to the second import statement's import list - // and it is up to the user to decide which one fits best. - let namespaceImportDeclaration: ImportDeclaration | ImportEqualsDeclaration; - let namedImportDeclaration: ImportDeclaration; - let existingModuleSpecifier: string; - for (const declaration of declarations) { - if (declaration.kind === SyntaxKind.ImportDeclaration) { - const namedBindings = declaration.importClause && declaration.importClause.namedBindings; - if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { - // case: - // import * as ns from "foo" - namespaceImportDeclaration = declaration; - } - else { - // cases: - // import default from "foo" - // import { bar } from "foo" or combination with the first one - // import "foo" - namedImportDeclaration = declaration; - } - existingModuleSpecifier = declaration.moduleSpecifier.getText(); - } - else { - // case: - // import foo = require("foo") - namespaceImportDeclaration = declaration; - existingModuleSpecifier = getModuleSpecifierFromImportEqualsDeclaration(declaration); - } + function getTextChangeForImportClause(importClause: ImportClause): FileTextChanges[] { + const importList = importClause.namedBindings; + const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name)); + // case 1: + // original text: import default from "module" + // change to: import default, { name } from "module" + // case 2: + // original text: import {} from "module" + // change to: import { name } from "module" + if (!importList || importList.elements.length === 0) { + const newImportClause = createImportClause(importClause.name, createNamedImports([newImportSpecifier])); + return createChangeTracker().replaceNode(sourceFile, importClause, newImportClause).getChanges(); } - if (namespaceImportDeclaration) { - actions.push(getCodeActionForNamespaceImport(namespaceImportDeclaration)); - } - - if (!isNamespaceImport && namedImportDeclaration && namedImportDeclaration.importClause && - (namedImportDeclaration.importClause.name || namedImportDeclaration.importClause.namedBindings)) { - /** - * If the existing import declaration already has a named import list, just - * insert the identifier into that list. - */ - const fileTextChanges = getTextChangeForImportClause(namedImportDeclaration.importClause); - const moduleSpecifierWithoutQuotes = stripQuotes(namedImportDeclaration.moduleSpecifier.getText()); - actions.push(createCodeAction( - Diagnostics.Add_0_to_existing_import_declaration_from_1, - [name, moduleSpecifierWithoutQuotes], - fileTextChanges, - "InsertingIntoExistingImport", - moduleSpecifierWithoutQuotes - )); - } - else { - // we need to create a new import statement, but the existing module specifier can be reused. - actions.push(getCodeActionForNewImport(existingModuleSpecifier)); - } - return actions; - - function getModuleSpecifierFromImportEqualsDeclaration(declaration: ImportEqualsDeclaration) { - if (declaration.moduleReference && declaration.moduleReference.kind === SyntaxKind.ExternalModuleReference) { - return declaration.moduleReference.expression.getText(); - } - return declaration.moduleReference.getText(); - } - - function getTextChangeForImportClause(importClause: ImportClause): FileTextChanges[] { - const importList = importClause.namedBindings; - const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name)); - // case 1: - // original text: import default from "module" - // change to: import default, { name } from "module" - // case 2: - // original text: import {} from "module" - // change to: import { name } from "module" - if (!importList || importList.elements.length === 0) { - const newImportClause = createImportClause(importClause.name, createNamedImports([newImportSpecifier])); - return createChangeTracker().replaceNode(sourceFile, importClause, newImportClause).getChanges(); - } - - /** - * If the import list has one import per line, preserve that. Otherwise, insert on same line as last element - * import { - * foo - * } from "./module"; - */ - return createChangeTracker().insertNodeInListAfter( - sourceFile, - importList.elements[importList.elements.length - 1], - newImportSpecifier).getChanges(); - } - - function getCodeActionForNamespaceImport(declaration: ImportDeclaration | ImportEqualsDeclaration): ImportCodeAction { - let namespacePrefix: string; - if (declaration.kind === SyntaxKind.ImportDeclaration) { - namespacePrefix = (declaration.importClause.namedBindings).name.getText(); - } - else { - namespacePrefix = declaration.name.getText(); - } - namespacePrefix = stripQuotes(namespacePrefix); - - /** - * Cases: - * import * as ns from "mod" - * import default, * as ns from "mod" - * import ns = require("mod") - * - * Because there is no import list, we alter the reference to include the - * namespace instead of altering the import declaration. For example, "foo" would - * become "ns.foo" - */ - return createCodeAction( - Diagnostics.Change_0_to_1, - [name, `${namespacePrefix}.${name}`], - createChangeTracker().replaceNode(sourceFile, token, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), - "CodeChange" - ); - } + /** + * If the import list has one import per line, preserve that. Otherwise, insert on same line as last element + * import { + * foo + * } from "./module"; + */ + return createChangeTracker().insertNodeInListAfter( + sourceFile, + importList.elements[importList.elements.length - 1], + newImportSpecifier).getChanges(); } - function getCodeActionForNewImport(moduleSpecifier?: string): ImportCodeAction { - if (!lastImportDeclaration) { - // insert after any existing imports - for (let i = sourceFile.statements.length - 1; i >= 0; i--) { - const statement = sourceFile.statements[i]; - if (statement.kind === SyntaxKind.ImportEqualsDeclaration || statement.kind === SyntaxKind.ImportDeclaration) { - lastImportDeclaration = statement; - break; - } - } - } - - const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); - const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport()); - const changeTracker = createChangeTracker(); - const importClause = isDefault - ? createImportClause(createIdentifier(name), /*namedBindings*/ undefined) - : isNamespaceImport - ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(name))) - : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name))])); - const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes)); - if (!lastImportDeclaration) { - changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); + function getCodeActionForNamespaceImport(declaration: ImportDeclaration | ImportEqualsDeclaration): ImportCodeAction { + let namespacePrefix: string; + if (declaration.kind === SyntaxKind.ImportDeclaration) { + namespacePrefix = (declaration.importClause.namedBindings).name.getText(); } else { - changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: context.newLineCharacter }); + namespacePrefix = declaration.name.getText(); } + namespacePrefix = stripQuotes(namespacePrefix); - // if this file doesn't have any import statements, insert an import statement and then insert a new line - // between the only import statement and user code. Otherwise just insert the statement because chances - // are there are already a new line seperating code and import statements. + /** + * Cases: + * import * as ns from "mod" + * import default, * as ns from "mod" + * import ns = require("mod") + * + * Because there is no import list, we alter the reference to include the + * namespace instead of altering the import declaration. For example, "foo" would + * become "ns.foo" + */ return createCodeAction( - Diagnostics.Import_0_from_1, - [name, `"${moduleSpecifierWithoutQuotes}"`], - changeTracker.getChanges(), - "NewImport", - moduleSpecifierWithoutQuotes + Diagnostics.Change_0_to_1, + [name, `${namespacePrefix}.${name}`], + createChangeTracker().replaceNode(sourceFile, token, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), + "CodeChange" ); + } + } - function getModuleSpecifierForNewImport() { - const fileName = sourceFile.fileName; - const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; - const sourceDirectory = getDirectoryPath(fileName); - const options = context.program.getCompilerOptions(); - - return tryGetModuleNameFromAmbientModule() || - tryGetModuleNameFromTypeRoots() || - tryGetModuleNameAsNodeModule() || - tryGetModuleNameFromBaseUrl() || - tryGetModuleNameFromRootDirs() || - removeFileExtension(getRelativePath(moduleFileName, sourceDirectory)); - - function tryGetModuleNameFromAmbientModule(): string { - if (moduleSymbol.valueDeclaration.kind !== SyntaxKind.SourceFile) { - return moduleSymbol.name; - } + function getCodeActionForNewImport(moduleSpecifier?: string): ImportCodeAction { + if (!lastImportDeclaration) { + // insert after any existing imports + for (let i = sourceFile.statements.length - 1; i >= 0; i--) { + const statement = sourceFile.statements[i]; + if (statement.kind === SyntaxKind.ImportEqualsDeclaration || statement.kind === SyntaxKind.ImportDeclaration) { + lastImportDeclaration = statement; + break; } + } + } - function tryGetModuleNameFromBaseUrl() { - if (!options.baseUrl) { - return undefined; - } + const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); + const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport()); + const changeTracker = createChangeTracker(); + const importClause = isDefault + ? createImportClause(createIdentifier(name), /*namedBindings*/ undefined) + : isNamespaceImport + ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(name))) + : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name))])); + const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes)); + if (!lastImportDeclaration) { + changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); + } + else { + changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: context.newLineCharacter }); + } - let relativeName = getRelativePathIfInDirectory(moduleFileName, options.baseUrl); - if (!relativeName) { - return undefined; - } + // if this file doesn't have any import statements, insert an import statement and then insert a new line + // between the only import statement and user code. Otherwise just insert the statement because chances + // are there are already a new line seperating code and import statements. + return createCodeAction( + Diagnostics.Import_0_from_1, + [name, `"${moduleSpecifierWithoutQuotes}"`], + changeTracker.getChanges(), + "NewImport", + moduleSpecifierWithoutQuotes + ); - const relativeNameWithIndex = removeFileExtension(relativeName); - relativeName = removeExtensionAndIndexPostFix(relativeName); + function getModuleSpecifierForNewImport() { + const fileName = sourceFile.fileName; + const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; + const sourceDirectory = getDirectoryPath(fileName); + const options = context.program.getCompilerOptions(); - if (options.paths) { - for (const key in options.paths) { - for (const pattern of options.paths[key]) { - const indexOfStar = pattern.indexOf("*"); - if (indexOfStar === 0 && pattern.length === 1) { - continue; - } - else if (indexOfStar !== -1) { - const prefix = pattern.substr(0, indexOfStar); - const suffix = pattern.substr(indexOfStar + 1); - if (relativeName.length >= prefix.length + suffix.length && - startsWith(relativeName, prefix) && - endsWith(relativeName, suffix)) { - const matchedStar = relativeName.substr(prefix.length, relativeName.length - suffix.length); - return key.replace("\*", matchedStar); - } - } - else if (pattern === relativeName || pattern === relativeNameWithIndex) { - return key; - } - } - } - } + return tryGetModuleNameFromAmbientModule() || + tryGetModuleNameFromTypeRoots() || + tryGetModuleNameAsNodeModule() || + tryGetModuleNameFromBaseUrl() || + tryGetModuleNameFromRootDirs() || + removeFileExtension(getRelativePath(moduleFileName, sourceDirectory)); - return relativeName; + function tryGetModuleNameFromAmbientModule(): string { + if (moduleSymbol.valueDeclaration.kind !== SyntaxKind.SourceFile) { + return moduleSymbol.name; } + } - function tryGetModuleNameFromRootDirs() { - if (options.rootDirs) { - const normalizedTargetPath = getPathRelativeToRootDirs(moduleFileName, options.rootDirs); - const normalizedSourcePath = getPathRelativeToRootDirs(sourceDirectory, options.rootDirs); - if (normalizedTargetPath !== undefined) { - const relativePath = normalizedSourcePath !== undefined ? getRelativePath(normalizedTargetPath, normalizedSourcePath) : normalizedTargetPath; - return removeFileExtension(relativePath); - } - } + function tryGetModuleNameFromBaseUrl() { + if (!options.baseUrl) { return undefined; } - function tryGetModuleNameFromTypeRoots() { - const typeRoots = getEffectiveTypeRoots(options, context.host); - if (typeRoots) { - const normalizedTypeRoots = map(typeRoots, typeRoot => toPath(typeRoot, /*basePath*/ undefined, getCanonicalFileName)); - for (const typeRoot of normalizedTypeRoots) { - if (startsWith(moduleFileName, typeRoot)) { - const relativeFileName = moduleFileName.substring(typeRoot.length + 1); - return removeExtensionAndIndexPostFix(relativeFileName); - } - } - } + let relativeName = getRelativePathIfInDirectory(moduleFileName, options.baseUrl); + if (!relativeName) { + return undefined; } - function tryGetModuleNameAsNodeModule() { - if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { - // nothing to do here - return undefined; - } + const relativeNameWithIndex = removeFileExtension(relativeName); + relativeName = removeExtensionAndIndexPostFix(relativeName); - const indexOfNodeModules = moduleFileName.indexOf("node_modules"); - if (indexOfNodeModules < 0) { - return undefined; - } - - let relativeFileName: string; - if (sourceDirectory.indexOf(moduleFileName.substring(0, indexOfNodeModules - 1)) === 0) { - // if node_modules folder is in this folder or any of its parent folder, no need to keep it. - relativeFileName = moduleFileName.substring(indexOfNodeModules + 13 /* "node_modules\".length */); - } - else { - relativeFileName = getRelativePath(moduleFileName, sourceDirectory); - } - - relativeFileName = removeFileExtension(relativeFileName); - if (endsWith(relativeFileName, "/index")) { - relativeFileName = getDirectoryPath(relativeFileName); - } - else { - try { - const moduleDirectory = getDirectoryPath(moduleFileName); - const packageJsonContent = JSON.parse(context.host.readFile(combinePaths(moduleDirectory, "package.json"))); - if (packageJsonContent) { - const mainFile = packageJsonContent.main || packageJsonContent.typings; - if (mainFile) { - const mainExportFile = toPath(mainFile, moduleDirectory, getCanonicalFileName); - if (removeFileExtension(mainExportFile) === removeFileExtension(moduleFileName)) { - relativeFileName = getDirectoryPath(relativeFileName); - } + if (options.paths) { + for (const key in options.paths) { + for (const pattern of options.paths[key]) { + const indexOfStar = pattern.indexOf("*"); + if (indexOfStar === 0 && pattern.length === 1) { + continue; + } + else if (indexOfStar !== -1) { + const prefix = pattern.substr(0, indexOfStar); + const suffix = pattern.substr(indexOfStar + 1); + if (relativeName.length >= prefix.length + suffix.length && + startsWith(relativeName, prefix) && + endsWith(relativeName, suffix)) { + const matchedStar = relativeName.substr(prefix.length, relativeName.length - suffix.length); + return key.replace("\*", matchedStar); } } + else if (pattern === relativeName || pattern === relativeNameWithIndex) { + return key; + } } - catch (e) { } } - - return relativeFileName; } + + return relativeName; } - function getPathRelativeToRootDirs(path: string, rootDirs: string[]) { - for (const rootDir of rootDirs) { - const relativeName = getRelativePathIfInDirectory(path, rootDir); - if (relativeName !== undefined) { - return relativeName; + function tryGetModuleNameFromRootDirs() { + if (options.rootDirs) { + const normalizedTargetPath = getPathRelativeToRootDirs(moduleFileName, options.rootDirs); + const normalizedSourcePath = getPathRelativeToRootDirs(sourceDirectory, options.rootDirs); + if (normalizedTargetPath !== undefined) { + const relativePath = normalizedSourcePath !== undefined ? getRelativePath(normalizedTargetPath, normalizedSourcePath) : normalizedTargetPath; + return removeFileExtension(relativePath); } } return undefined; } - function removeExtensionAndIndexPostFix(fileName: string) { - fileName = removeFileExtension(fileName); - if (endsWith(fileName, "/index")) { - fileName = fileName.substr(0, fileName.length - 6/* "/index".length */); + function tryGetModuleNameFromTypeRoots() { + const typeRoots = getEffectiveTypeRoots(options, context.host); + if (typeRoots) { + const normalizedTypeRoots = map(typeRoots, typeRoot => toPath(typeRoot, /*basePath*/ undefined, getCanonicalFileName)); + for (const typeRoot of normalizedTypeRoots) { + if (startsWith(moduleFileName, typeRoot)) { + const relativeFileName = moduleFileName.substring(typeRoot.length + 1); + return removeExtensionAndIndexPostFix(relativeFileName); + } + } } - return fileName; } - function getRelativePathIfInDirectory(path: string, directoryPath: string) { - const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); - return isRootedDiskPath(relativePath) || startsWith(relativePath, "..") ? undefined : relativePath; - } + function tryGetModuleNameAsNodeModule() { + if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { + // nothing to do here + return undefined; + } - function getRelativePath(path: string, directoryPath: string) { - const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); - return moduleHasNonRelativeName(relativePath) ? "./" + relativePath : relativePath; + const indexOfNodeModules = moduleFileName.indexOf("node_modules"); + if (indexOfNodeModules < 0) { + return undefined; + } + + let relativeFileName: string; + if (sourceDirectory.indexOf(moduleFileName.substring(0, indexOfNodeModules - 1)) === 0) { + // if node_modules folder is in this folder or any of its parent folder, no need to keep it. + relativeFileName = moduleFileName.substring(indexOfNodeModules + 13 /* "node_modules\".length */); + } + else { + relativeFileName = getRelativePath(moduleFileName, sourceDirectory); + } + + relativeFileName = removeFileExtension(relativeFileName); + if (endsWith(relativeFileName, "/index")) { + relativeFileName = getDirectoryPath(relativeFileName); + } + else { + try { + const moduleDirectory = getDirectoryPath(moduleFileName); + const packageJsonContent = JSON.parse(context.host.readFile(combinePaths(moduleDirectory, "package.json"))); + if (packageJsonContent) { + const mainFile = packageJsonContent.main || packageJsonContent.typings; + if (mainFile) { + const mainExportFile = toPath(mainFile, moduleDirectory, getCanonicalFileName); + if (removeFileExtension(mainExportFile) === removeFileExtension(moduleFileName)) { + relativeFileName = getDirectoryPath(relativeFileName); + } + } + } + } + catch (e) { } + } + + return relativeFileName; } } + function getPathRelativeToRootDirs(path: string, rootDirs: string[]) { + for (const rootDir of rootDirs) { + const relativeName = getRelativePathIfInDirectory(path, rootDir); + if (relativeName !== undefined) { + return relativeName; + } + } + return undefined; + } + + function removeExtensionAndIndexPostFix(fileName: string) { + fileName = removeFileExtension(fileName); + if (endsWith(fileName, "/index")) { + fileName = fileName.substr(0, fileName.length - 6/* "/index".length */); + } + return fileName; + } + + function getRelativePathIfInDirectory(path: string, directoryPath: string) { + const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + return isRootedDiskPath(relativePath) || startsWith(relativePath, "..") ? undefined : relativePath; + } + + function getRelativePath(path: string, directoryPath: string) { + const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + return moduleHasNonRelativeName(relativePath) ? "./" + relativePath : relativePath; + } } - function createChangeTracker() { - return textChanges.ChangeTracker.fromCodeFixContext(context); - } - - function createCodeAction( - description: DiagnosticMessage, - diagnosticArgs: string[], - changes: FileTextChanges[], - kind: ImportCodeActionKind, - moduleSpecifier?: string): ImportCodeAction { - return { - description: formatMessage.apply(undefined, [undefined, description].concat(diagnosticArgs)), - changes, - kind, - moduleSpecifier - }; - } } - }); + + function createChangeTracker() { + return textChanges.ChangeTracker.fromCodeFixContext(context); + } + + function createCodeAction( + description: DiagnosticMessage, + diagnosticArgs: string[], + changes: FileTextChanges[], + kind: ImportCodeActionKind, + moduleSpecifier?: string): ImportCodeAction { + return { + description: formatMessage.apply(undefined, [undefined, description].concat(diagnosticArgs)), + changes, + kind, + moduleSpecifier + }; + } + } } From 1125657210ae94f1990683425208b88b41d32557 Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 2 May 2017 20:54:27 -0700 Subject: [PATCH 28/30] Don't stop checking other attributes even though we see spread type. This is so that things are correctly marked as reference and type-checked --- src/compiler/checker.ts | 42 ++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index ca5d65068a..1e483eb80c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13248,6 +13248,8 @@ namespace ts { let attributesTable = createMap(); let spread: Type = emptyObjectType; let attributesArray: Symbol[] = []; + let hasSpreadAnyType = false; + for (const attributeDecl of attributes.properties) { const member = attributeDecl.symbol; if (isJsxAttribute(attributeDecl)) { @@ -13276,31 +13278,33 @@ namespace ts { const exprType = checkExpression(attributeDecl.expression); if (!isValidSpreadType(exprType)) { error(attributeDecl, Diagnostics.Spread_types_may_only_be_created_from_object_types); - return anyType; + hasSpreadAnyType = true; } if (isTypeAny(exprType)) { - return anyType; + hasSpreadAnyType = true; } spread = getSpreadType(spread, exprType); } } - if (spread !== emptyObjectType) { - if (attributesArray.length > 0) { - spread = getSpreadType(spread, createJsxAttributesType(attributes.symbol, attributesTable)); - attributesArray = []; - attributesTable = createMap(); - } - attributesArray = getPropertiesOfType(spread); - } - - attributesTable = createMap(); - if (attributesArray) { - forEach(attributesArray, (attr) => { - if (!filter || filter(attr)) { - attributesTable.set(attr.name, attr); + if (!hasSpreadAnyType) { + if (spread !== emptyObjectType) { + if (attributesArray.length > 0) { + spread = getSpreadType(spread, createJsxAttributesType(attributes.symbol, attributesTable)); + attributesArray = []; + attributesTable = createMap(); } - }); + attributesArray = getPropertiesOfType(spread); + } + + attributesTable = createMap(); + if (attributesArray) { + forEach(attributesArray, (attr) => { + if (!filter || filter(attr)) { + attributesTable.set(attr.name, attr); + } + }); + } } // Handle children attribute @@ -13324,7 +13328,7 @@ namespace ts { // Error if there is a attribute named "children" and children element. // This is because children element will overwrite the value from attributes const jsxChildrenPropertyName = getJsxElementChildrenPropertyname(); - if (jsxChildrenPropertyName && jsxChildrenPropertyName !== "") { + if (!hasSpreadAnyType && jsxChildrenPropertyName && jsxChildrenPropertyName !== "") { if (attributesTable.has(jsxChildrenPropertyName)) { error(attributes, Diagnostics._0_are_specified_twice_The_attribute_named_0_will_be_overwritten, jsxChildrenPropertyName); } @@ -13338,7 +13342,7 @@ namespace ts { } } - return createJsxAttributesType(attributes.symbol, attributesTable); + return hasSpreadAnyType ? anyType : createJsxAttributesType(attributes.symbol, attributesTable); /** * Create anonymous type from given attributes symbol table. From 6d4f83f1b2ed247d0564a887b2fa8a16a1371ecb Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 2 May 2017 20:54:42 -0700 Subject: [PATCH 29/30] Update baseline --- ...atelessFunctionComponentsWithTypeArguments3.errors.txt | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/baselines/reference/tsxStatelessFunctionComponentsWithTypeArguments3.errors.txt b/tests/baselines/reference/tsxStatelessFunctionComponentsWithTypeArguments3.errors.txt index 1d03542738..fdf65ca174 100644 --- a/tests/baselines/reference/tsxStatelessFunctionComponentsWithTypeArguments3.errors.txt +++ b/tests/baselines/reference/tsxStatelessFunctionComponentsWithTypeArguments3.errors.txt @@ -3,10 +3,12 @@ tests/cases/conformance/jsx/file.tsx(10,33): error TS2698: Spread types may only tests/cases/conformance/jsx/file.tsx(11,33): error TS2698: Spread types may only be created from object types. tests/cases/conformance/jsx/file.tsx(12,33): error TS2698: Spread types may only be created from object types. tests/cases/conformance/jsx/file.tsx(14,33): error TS2698: Spread types may only be created from object types. +tests/cases/conformance/jsx/file.tsx(14,63): error TS2698: Spread types may only be created from object types. tests/cases/conformance/jsx/file.tsx(15,33): error TS2698: Spread types may only be created from object types. +tests/cases/conformance/jsx/file.tsx(15,55): error TS2698: Spread types may only be created from object types. -==== tests/cases/conformance/jsx/file.tsx (6 errors) ==== +==== tests/cases/conformance/jsx/file.tsx (8 errors) ==== import React = require('react') declare function OverloadComponent(): JSX.Element; @@ -30,9 +32,13 @@ tests/cases/conformance/jsx/file.tsx(15,33): error TS2698: Spread types may only let a4 = ; let a5 = ; ~~~~~~~~~ +!!! error TS2698: Spread types may only be created from object types. + ~~~~~~~~~ !!! error TS2698: Spread types may only be created from object types. let a6 = ; ~~~~~~~~~ +!!! error TS2698: Spread types may only be created from object types. + ~~~~~~~~~ !!! error TS2698: Spread types may only be created from object types. } From f3a3198bec48c21a1ad623f172ac2b449c86f42b Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 3 May 2017 13:30:30 -0700 Subject: [PATCH 30/30] Add tests and update baselines --- .../correctlyMarkAliasAsReferences1.js | 23 ++++++++++++ .../correctlyMarkAliasAsReferences1.symbols | 29 +++++++++++++++ .../correctlyMarkAliasAsReferences1.types | 35 ++++++++++++++++++ .../correctlyMarkAliasAsReferences2.js | 23 ++++++++++++ .../correctlyMarkAliasAsReferences2.symbols | 30 ++++++++++++++++ .../correctlyMarkAliasAsReferences2.types | 36 +++++++++++++++++++ .../correctlyMarkAliasAsReferences3.js | 23 ++++++++++++ .../correctlyMarkAliasAsReferences3.symbols | 29 +++++++++++++++ .../correctlyMarkAliasAsReferences3.types | 35 ++++++++++++++++++ .../correctlyMarkAliasAsReferences4.js | 19 ++++++++++ .../correctlyMarkAliasAsReferences4.symbols | 24 +++++++++++++ .../correctlyMarkAliasAsReferences4.types | 29 +++++++++++++++ .../jsx/correctlyMarkAliasAsReferences1.tsx | 17 +++++++++ .../jsx/correctlyMarkAliasAsReferences2.tsx | 17 +++++++++ .../jsx/correctlyMarkAliasAsReferences3.tsx | 18 ++++++++++ .../jsx/correctlyMarkAliasAsReferences4.tsx | 15 ++++++++ tests/lib/react.d.ts | 4 +-- 17 files changed, 404 insertions(+), 2 deletions(-) create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences1.js create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences1.symbols create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences1.types create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences2.js create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences2.symbols create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences2.types create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences3.js create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences3.symbols create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences3.types create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences4.js create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences4.symbols create mode 100644 tests/baselines/reference/correctlyMarkAliasAsReferences4.types create mode 100644 tests/cases/conformance/jsx/correctlyMarkAliasAsReferences1.tsx create mode 100644 tests/cases/conformance/jsx/correctlyMarkAliasAsReferences2.tsx create mode 100644 tests/cases/conformance/jsx/correctlyMarkAliasAsReferences3.tsx create mode 100644 tests/cases/conformance/jsx/correctlyMarkAliasAsReferences4.tsx diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences1.js b/tests/baselines/reference/correctlyMarkAliasAsReferences1.js new file mode 100644 index 0000000000..7816881f4a --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences1.js @@ -0,0 +1,23 @@ +//// [tests/cases/conformance/jsx/correctlyMarkAliasAsReferences1.tsx] //// + +//// [declaration.d.ts] +declare module "classnames"; + +//// [0.tsx] +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps; // any +let k = ; + + +//// [0.js] +/// +import * as cx from 'classnames'; +import * as React from "react"; +let buttonProps; // any +let k = React.createElement("button", Object.assign({}, buttonProps), + React.createElement("span", { className: cx('class1', { class2: true }) })); diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences1.symbols b/tests/baselines/reference/correctlyMarkAliasAsReferences1.symbols new file mode 100644 index 0000000000..3267a29050 --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences1.symbols @@ -0,0 +1,29 @@ +=== tests/cases/conformance/jsx/0.tsx === +/// +import * as cx from 'classnames'; +>cx : Symbol(cx, Decl(0.tsx, 1, 6)) + +import * as React from "react"; +>React : Symbol(React, Decl(0.tsx, 2, 6)) + +let buttonProps; // any +>buttonProps : Symbol(buttonProps, Decl(0.tsx, 4, 3)) + +let k = ; +>button : Symbol(JSX.IntrinsicElements.button, Decl(react.d.ts, 2385, 43)) + +=== tests/cases/conformance/jsx/declaration.d.ts === +declare module "classnames"; +No type information for this code. +No type information for this code. \ No newline at end of file diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences1.types b/tests/baselines/reference/correctlyMarkAliasAsReferences1.types new file mode 100644 index 0000000000..dbcad2fdc3 --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences1.types @@ -0,0 +1,35 @@ +=== tests/cases/conformance/jsx/0.tsx === +/// +import * as cx from 'classnames'; +>cx : any + +import * as React from "react"; +>React : typeof React + +let buttonProps; // any +>buttonProps : any + +let k = : JSX.Element +>button : any +>buttonProps : any + + +> : JSX.Element +>span : any +>className : any +>cx('class1', { class2: true }) : any +>cx : any +>'class1' : "class1" +>{ class2: true } : { class2: boolean; } +>class2 : boolean +>true : true + + ; +>button : any + +=== tests/cases/conformance/jsx/declaration.d.ts === +declare module "classnames"; +No type information for this code. +No type information for this code. \ No newline at end of file diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences2.js b/tests/baselines/reference/correctlyMarkAliasAsReferences2.js new file mode 100644 index 0000000000..7af59b017a --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences2.js @@ -0,0 +1,23 @@ +//// [tests/cases/conformance/jsx/correctlyMarkAliasAsReferences2.tsx] //// + +//// [declaration.d.ts] +declare module "classnames"; + +//// [0.tsx] +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps : {[attributeName: string]: ''} +let k = ; + + +//// [0.js] +/// +import * as cx from 'classnames'; +import * as React from "react"; +let buttonProps; +let k = React.createElement("button", Object.assign({}, buttonProps), + React.createElement("span", { className: cx('class1', { class2: true }) })); diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences2.symbols b/tests/baselines/reference/correctlyMarkAliasAsReferences2.symbols new file mode 100644 index 0000000000..76d9b8cdfe --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences2.symbols @@ -0,0 +1,30 @@ +=== tests/cases/conformance/jsx/0.tsx === +/// +import * as cx from 'classnames'; +>cx : Symbol(cx, Decl(0.tsx, 1, 6)) + +import * as React from "react"; +>React : Symbol(React, Decl(0.tsx, 2, 6)) + +let buttonProps : {[attributeName: string]: ''} +>buttonProps : Symbol(buttonProps, Decl(0.tsx, 4, 3)) +>attributeName : Symbol(attributeName, Decl(0.tsx, 4, 20)) + +let k = ; +>button : Symbol(JSX.IntrinsicElements.button, Decl(react.d.ts, 2385, 43)) + +=== tests/cases/conformance/jsx/declaration.d.ts === +declare module "classnames"; +No type information for this code. +No type information for this code. \ No newline at end of file diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences2.types b/tests/baselines/reference/correctlyMarkAliasAsReferences2.types new file mode 100644 index 0000000000..cfaba960da --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences2.types @@ -0,0 +1,36 @@ +=== tests/cases/conformance/jsx/0.tsx === +/// +import * as cx from 'classnames'; +>cx : any + +import * as React from "react"; +>React : typeof React + +let buttonProps : {[attributeName: string]: ''} +>buttonProps : { [attributeName: string]: ""; } +>attributeName : string + +let k = : JSX.Element +>button : any +>buttonProps : { [attributeName: string]: ""; } + + +> : JSX.Element +>span : any +>className : any +>cx('class1', { class2: true }) : any +>cx : any +>'class1' : "class1" +>{ class2: true } : { class2: boolean; } +>class2 : boolean +>true : true + + ; +>button : any + +=== tests/cases/conformance/jsx/declaration.d.ts === +declare module "classnames"; +No type information for this code. +No type information for this code. \ No newline at end of file diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences3.js b/tests/baselines/reference/correctlyMarkAliasAsReferences3.js new file mode 100644 index 0000000000..166e29d56d --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences3.js @@ -0,0 +1,23 @@ +//// [tests/cases/conformance/jsx/correctlyMarkAliasAsReferences3.tsx] //// + +//// [declaration.d.ts] +declare module "classnames"; + +//// [0.tsx] +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps; +let k = ; + + +//// [0.js] +/// +import * as cx from 'classnames'; +import * as React from "react"; +let buttonProps; +let k = React.createElement("button", Object.assign({}, buttonProps), + React.createElement("span", { className: cx('class1', { class2: true }) })); diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences3.symbols b/tests/baselines/reference/correctlyMarkAliasAsReferences3.symbols new file mode 100644 index 0000000000..088f553baa --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences3.symbols @@ -0,0 +1,29 @@ +=== tests/cases/conformance/jsx/0.tsx === +/// +import * as cx from 'classnames'; +>cx : Symbol(cx, Decl(0.tsx, 1, 6)) + +import * as React from "react"; +>React : Symbol(React, Decl(0.tsx, 2, 6)) + +let buttonProps; +>buttonProps : Symbol(buttonProps, Decl(0.tsx, 4, 3)) + +let k = ; +>button : Symbol(JSX.IntrinsicElements.button, Decl(react.d.ts, 2385, 43)) + +=== tests/cases/conformance/jsx/declaration.d.ts === +declare module "classnames"; +No type information for this code. +No type information for this code. \ No newline at end of file diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences3.types b/tests/baselines/reference/correctlyMarkAliasAsReferences3.types new file mode 100644 index 0000000000..cca59ae2f5 --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences3.types @@ -0,0 +1,35 @@ +=== tests/cases/conformance/jsx/0.tsx === +/// +import * as cx from 'classnames'; +>cx : any + +import * as React from "react"; +>React : typeof React + +let buttonProps; +>buttonProps : any + +let k = : JSX.Element +>button : any +>buttonProps : undefined + + +> : JSX.Element +>span : any +>className : any +>cx('class1', { class2: true }) : any +>cx : any +>'class1' : "class1" +>{ class2: true } : { class2: boolean; } +>class2 : boolean +>true : true + + ; +>button : any + +=== tests/cases/conformance/jsx/declaration.d.ts === +declare module "classnames"; +No type information for this code. +No type information for this code. \ No newline at end of file diff --git a/tests/baselines/reference/correctlyMarkAliasAsReferences4.js b/tests/baselines/reference/correctlyMarkAliasAsReferences4.js new file mode 100644 index 0000000000..631d06cc5b --- /dev/null +++ b/tests/baselines/reference/correctlyMarkAliasAsReferences4.js @@ -0,0 +1,19 @@ +//// [tests/cases/conformance/jsx/correctlyMarkAliasAsReferences4.tsx] //// + +//// [declaration.d.ts] +declare module "classnames"; + +//// [0.tsx] +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps : {[attributeName: string]: ''} +let k = ; diff --git a/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences2.tsx b/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences2.tsx new file mode 100644 index 0000000000..9755c624fc --- /dev/null +++ b/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences2.tsx @@ -0,0 +1,17 @@ +// @target: es2017 +// @jsx: react +// @moduleResolution: node +// @libFiles: react.d.ts,lib.d.ts + +// @filename: declaration.d.ts +declare module "classnames"; + +// @filename: 0.tsx +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps : {[attributeName: string]: ''} +let k = ; diff --git a/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences3.tsx b/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences3.tsx new file mode 100644 index 0000000000..c3bfb0c8a4 --- /dev/null +++ b/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences3.tsx @@ -0,0 +1,18 @@ +// @target: es2017 +// @jsx: react +// @moduleResolution: node +// @noImplicitAny: true +// @libFiles: react.d.ts,lib.d.ts + +// @filename: declaration.d.ts +declare module "classnames"; + +// @filename: 0.tsx +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps; +let k = ; diff --git a/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences4.tsx b/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences4.tsx new file mode 100644 index 0000000000..edb9f75f87 --- /dev/null +++ b/tests/cases/conformance/jsx/correctlyMarkAliasAsReferences4.tsx @@ -0,0 +1,15 @@ +// @target: es2017 +// @jsx: react +// @moduleResolution: node +// @libFiles: react.d.ts,lib.d.ts + +// @filename: declaration.d.ts +declare module "classnames"; + +// @filename: 0.tsx +/// +import * as cx from 'classnames'; +import * as React from "react"; + +let buttonProps : {[attributeName: string]: ''} +let k =