From 17ee9231b7cd3cf27ee2d9d923bb4482cb798665 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 17 Aug 2018 14:05:05 -0700 Subject: [PATCH 01/21] Write first test with --build and --watch --- src/compiler/program.ts | 50 +++++++------- src/harness/virtualFileSystemWithWatch.ts | 14 +++- src/testRunner/tsconfig.json | 1 + src/testRunner/unittests/tsbuildWatchMode.ts | 69 +++++++++++++++++++ src/testRunner/unittests/tscWatchMode.ts | 25 ++++--- .../unittests/tsserverProjectSystem.ts | 6 +- .../reference/api/tsserverlibrary.d.ts | 2 +- tests/baselines/reference/api/typescript.d.ts | 2 +- 8 files changed, 123 insertions(+), 46 deletions(-) create mode 100644 src/testRunner/unittests/tsbuildWatchMode.ts diff --git a/src/compiler/program.ts b/src/compiler/program.ts index abbc935cf6..1c7430e330 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -66,20 +66,20 @@ namespace ts { mtime: Date; } - export function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean): CompilerHost { + export function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean, system = sys): CompilerHost { const existingDirectories = createMap(); function getCanonicalFileName(fileName: string): string { // if underlying system can distinguish between two files whose names differs only in cases then file name already in canonical form. // otherwise use toLowerCase as a canonical form. - return sys.useCaseSensitiveFileNames ? fileName : fileName.toLowerCase(); + return system.useCaseSensitiveFileNames ? fileName : fileName.toLowerCase(); } function getSourceFile(fileName: string, languageVersion: ScriptTarget, onError?: (message: string) => void): SourceFile | undefined { let text: string | undefined; try { performance.mark("beforeIORead"); - text = sys.readFile(fileName, options.charset); + text = system.readFile(fileName, options.charset); performance.mark("afterIORead"); performance.measure("I/O Read", "beforeIORead", "afterIORead"); } @@ -97,7 +97,7 @@ namespace ts { if (existingDirectories.has(directoryPath)) { return true; } - if (sys.directoryExists(directoryPath)) { + if (system.directoryExists(directoryPath)) { existingDirectories.set(directoryPath, true); return true; } @@ -108,7 +108,7 @@ namespace ts { if (directoryPath.length > getRootLength(directoryPath) && !directoryExists(directoryPath)) { const parentDirectory = getDirectoryPath(directoryPath); ensureDirectoriesExist(parentDirectory); - sys.createDirectory(directoryPath); + system.createDirectory(directoryPath); } } @@ -119,8 +119,8 @@ namespace ts { outputFingerprints = createMap(); } - const hash = sys.createHash!(data); // TODO: GH#18217 - const mtimeBefore = sys.getModifiedTime!(fileName); // TODO: GH#18217 + const hash = system.createHash!(data); // TODO: GH#18217 + const mtimeBefore = system.getModifiedTime!(fileName); // TODO: GH#18217 if (mtimeBefore) { const fingerprint = outputFingerprints.get(fileName); @@ -133,9 +133,9 @@ namespace ts { } } - sys.writeFile(fileName, data, writeByteOrderMark); + system.writeFile(fileName, data, writeByteOrderMark); - const mtimeAfter = sys.getModifiedTime!(fileName) || missingFileModifiedTime; // TODO: GH#18217 + const mtimeAfter = system.getModifiedTime!(fileName) || missingFileModifiedTime; // TODO: GH#18217 outputFingerprints.set(fileName, { hash, @@ -149,11 +149,11 @@ namespace ts { performance.mark("beforeIOWrite"); ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); - if (isWatchSet(options) && sys.createHash && sys.getModifiedTime) { + if (isWatchSet(options) && system.createHash && system.getModifiedTime) { writeFileIfUpdated(fileName, data, writeByteOrderMark); } else { - sys.writeFile(fileName, data, writeByteOrderMark); + system.writeFile(fileName, data, writeByteOrderMark); } performance.mark("afterIOWrite"); @@ -167,32 +167,32 @@ namespace ts { } function getDefaultLibLocation(): string { - return getDirectoryPath(normalizePath(sys.getExecutingFilePath())); + return getDirectoryPath(normalizePath(system.getExecutingFilePath())); } const newLine = getNewLineCharacter(options); - const realpath = sys.realpath && ((path: string) => sys.realpath!(path)); + const realpath = system.realpath && ((path: string) => system.realpath!(path)); return { getSourceFile, getDefaultLibLocation, getDefaultLibFileName: options => combinePaths(getDefaultLibLocation(), getDefaultLibFileName(options)), writeFile, - getCurrentDirectory: memoize(() => sys.getCurrentDirectory()), - useCaseSensitiveFileNames: () => sys.useCaseSensitiveFileNames, + getCurrentDirectory: memoize(() => system.getCurrentDirectory()), + useCaseSensitiveFileNames: () => system.useCaseSensitiveFileNames, getCanonicalFileName, getNewLine: () => newLine, - fileExists: fileName => sys.fileExists(fileName), - readFile: fileName => sys.readFile(fileName), - trace: (s: string) => sys.write(s + newLine), - directoryExists: directoryName => sys.directoryExists(directoryName), - getEnvironmentVariable: name => sys.getEnvironmentVariable ? sys.getEnvironmentVariable(name) : "", - getDirectories: (path: string) => sys.getDirectories(path), + fileExists: fileName => system.fileExists(fileName), + readFile: fileName => system.readFile(fileName), + trace: (s: string) => system.write(s + newLine), + directoryExists: directoryName => system.directoryExists(directoryName), + getEnvironmentVariable: name => system.getEnvironmentVariable ? system.getEnvironmentVariable(name) : "", + getDirectories: (path: string) => system.getDirectories(path), realpath, - readDirectory: (path, extensions, include, exclude, depth) => sys.readDirectory(path, extensions, include, exclude, depth), - getModifiedTime: sys.getModifiedTime && (path => sys.getModifiedTime!(path)), - setModifiedTime: sys.setModifiedTime && ((path, date) => sys.setModifiedTime!(path, date)), - deleteFile: sys.deleteFile && (path => sys.deleteFile!(path)) + readDirectory: (path, extensions, include, exclude, depth) => system.readDirectory(path, extensions, include, exclude, depth), + getModifiedTime: system.getModifiedTime && (path => system.getModifiedTime!(path)), + setModifiedTime: system.setModifiedTime && ((path, date) => system.setModifiedTime!(path, date)), + deleteFile: system.deleteFile && (path => system.deleteFile!(path)) }; } diff --git a/src/harness/virtualFileSystemWithWatch.ts b/src/harness/virtualFileSystemWithWatch.ts index 2a2ed4ef7d..57fa33a84c 100644 --- a/src/harness/virtualFileSystemWithWatch.ts +++ b/src/harness/virtualFileSystemWithWatch.ts @@ -620,14 +620,14 @@ interface Array {}` } } - removeFile(filePath: string) { + deleteFile(filePath: string) { const path = this.toFullPath(filePath); const currentEntry = this.fs.get(path) as FsFile; Debug.assert(isFsFile(currentEntry)); this.removeFileOrFolder(currentEntry, returnFalse); } - removeFolder(folderPath: string, recursive?: boolean) { + deleteFolder(folderPath: string, recursive?: boolean) { const path = this.toFullPath(folderPath); const currentEntry = this.fs.get(path) as FsFolder; Debug.assert(isFsFolder(currentEntry)); @@ -635,7 +635,7 @@ interface Array {}` const subEntries = currentEntry.entries.slice(); subEntries.forEach(fsEntry => { if (isFsFolder(fsEntry)) { - this.removeFolder(fsEntry.fullPath, recursive); + this.deleteFolder(fsEntry.fullPath, recursive); } else { this.removeFileOrFolder(fsEntry, returnFalse); @@ -766,6 +766,14 @@ interface Array {}` return (fsEntry && fsEntry.modifiedTime)!; // TODO: GH#18217 } + setModifiedTime(s: string, date: Date) { + const path = this.toFullPath(s); + const fsEntry = this.fs.get(path); + if (fsEntry) { + fsEntry.modifiedTime = date; + } + } + readFile(s: string): string | undefined { const fsEntry = this.getRealFile(this.toFullPath(s)); return fsEntry ? fsEntry.content : undefined; diff --git a/src/testRunner/tsconfig.json b/src/testRunner/tsconfig.json index 12c671f9cf..fa7900f1ca 100644 --- a/src/testRunner/tsconfig.json +++ b/src/testRunner/tsconfig.json @@ -80,6 +80,7 @@ "unittests/transform.ts", "unittests/transpile.ts", "unittests/tsbuild.ts", + "unittests/tsbuildWatchMode.ts", "unittests/tsconfigParsing.ts", "unittests/tscWatchMode.ts", "unittests/versionCache.ts", diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts new file mode 100644 index 0000000000..9f608180d0 --- /dev/null +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -0,0 +1,69 @@ +namespace ts.tscWatch { + export import libFile = TestFSWithWatch.libFile; + function createSolutionBuilder(host: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { + const compilerHost = createCompilerHost({}, /*setParentNodes*/ undefined, host); + const reportDiag = createDiagnosticReporter(host); + const report = (message: DiagnosticMessage, ...args: string[]) => reportDiag(createCompilerDiagnostic(message, ...args)); + const buildHost: BuildHost = { + error: report, + verbose: report, + message: report, + errorDiagnostic: d => reportDiag(d) + }; + return ts.createSolutionBuilder(compilerHost, buildHost, rootNames, defaultOptions || { dry: false, force: false, verbose: false }, host); + } + + function createSolutionBuilderWithWatch(host: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { + const solutionBuilder = createSolutionBuilder(host, rootNames, defaultOptions); + solutionBuilder.buildAllProjects(); + solutionBuilder.startWatching(); + return solutionBuilder; + } + + describe("tsbuild-watch program updates", () => { + const projectsLocation = "/user/username/projects"; + const project = "sample1"; + const enum SubProject { + core = "core", + logic = "logic", + tests = "tests", + ui = "ui" + } + type ReadonlyFile = Readonly; + /** [tsconfig, index] | [tsconfig, index, anotherModule, someDecl] */ + type SubProjectFiles = [ReadonlyFile, ReadonlyFile] | [ReadonlyFile, ReadonlyFile, ReadonlyFile, ReadonlyFile]; + const root = Harness.IO.getWorkspaceRoot(); + function projectFile(subProject: SubProject, baseFileName: string): File { + return { + path: `${projectsLocation}/${project}/${subProject}/${baseFileName.toLowerCase()}`, + content: Harness.IO.readFile(`${root}/tests/projects/${project}/${subProject}/${baseFileName}`)! + }; + } + function subProjectFiles(subProject: SubProject, anotherModuleAndSomeDecl?: true): SubProjectFiles { + const tsconfig = projectFile(subProject, "tsconfig.json"); + const index = projectFile(subProject, "index.ts"); + if (!anotherModuleAndSomeDecl) { + return [tsconfig, index]; + } + const anotherModule = projectFile(SubProject.core, "anotherModule.ts"); + const someDecl = projectFile(SubProject.core, "some_decl.ts"); + return [tsconfig, index, anotherModule, someDecl]; + } + + const core = subProjectFiles(SubProject.core, /*anotherModuleAndSomeDecl*/ true); + const logic = subProjectFiles(SubProject.logic); + const tests = subProjectFiles(SubProject.tests); + const ui = subProjectFiles(SubProject.ui); + const allFiles: ReadonlyArray = [libFile, ...core, ...logic, ...tests, ...ui]; + const testProjectExpectedWatchedFiles = [core[0], core[1], core[2], ...logic, ...tests].map(f => f.path); + it("creates solution in watch mode", () => { + const host = createWatchedSystem(allFiles, { currentDirectory: projectsLocation }); + const originalWrite = host.write; + host.write = s => { console.log(s); originalWrite.call(host, s); }; + createSolutionBuilderWithWatch(host, [`${project}/${SubProject.tests}`]); + checkWatchedFiles(host, testProjectExpectedWatchedFiles); + checkWatchedDirectories(host, emptyArray, /*recursive*/ false); + checkWatchedDirectories(host, emptyArray, /*recursive*/ true); // TODO: #26524 + }); + }); +} diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index d91559c72f..51c0d888f0 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -1,17 +1,16 @@ namespace ts.tscWatch { - import WatchedSystem = TestFSWithWatch.TestServerHost; - type File = TestFSWithWatch.File; - type SymLink = TestFSWithWatch.SymLink; - import createWatchedSystem = TestFSWithWatch.createWatchedSystem; - import checkArray = TestFSWithWatch.checkArray; - import libFile = TestFSWithWatch.libFile; - import checkWatchedFiles = TestFSWithWatch.checkWatchedFiles; - import checkWatchedFilesDetailed = TestFSWithWatch.checkWatchedFilesDetailed; - import checkWatchedDirectories = TestFSWithWatch.checkWatchedDirectories; - import checkWatchedDirectoriesDetailed = TestFSWithWatch.checkWatchedDirectoriesDetailed; - import checkOutputContains = TestFSWithWatch.checkOutputContains; - import checkOutputDoesNotContain = TestFSWithWatch.checkOutputDoesNotContain; - import Tsc_WatchDirectory = TestFSWithWatch.Tsc_WatchDirectory; + export import WatchedSystem = TestFSWithWatch.TestServerHost; + export type File = TestFSWithWatch.File; + export type SymLink = TestFSWithWatch.SymLink; + export import createWatchedSystem = TestFSWithWatch.createWatchedSystem; + export import checkArray = TestFSWithWatch.checkArray; + export import checkWatchedFiles = TestFSWithWatch.checkWatchedFiles; + export import checkWatchedFilesDetailed = TestFSWithWatch.checkWatchedFilesDetailed; + export import checkWatchedDirectories = TestFSWithWatch.checkWatchedDirectories; + export import checkWatchedDirectoriesDetailed = TestFSWithWatch.checkWatchedDirectoriesDetailed; + export import checkOutputContains = TestFSWithWatch.checkOutputContains; + export import checkOutputDoesNotContain = TestFSWithWatch.checkOutputDoesNotContain; + export import Tsc_WatchDirectory = TestFSWithWatch.Tsc_WatchDirectory; export function checkProgramActualFiles(program: Program, expectedFiles: string[]) { checkArray(`Program actual files`, program.getSourceFiles().map(file => file.fileName), expectedFiles); diff --git a/src/testRunner/unittests/tsserverProjectSystem.ts b/src/testRunner/unittests/tsserverProjectSystem.ts index b99a2d9fe9..5491d83965 100644 --- a/src/testRunner/unittests/tsserverProjectSystem.ts +++ b/src/testRunner/unittests/tsserverProjectSystem.ts @@ -8257,7 +8257,7 @@ new C();` verifyProjectWithResolvedModule(session); - host.removeFolder(recognizersTextDist, /*recursive*/ true); + host.deleteFolder(recognizersTextDist, /*recursive*/ true); host.runQueuedTimeoutCallbacks(); verifyProjectWithUnresolvedModule(session); @@ -9173,7 +9173,7 @@ describe("Test Suite 1", () => { checkProjectActualFiles(project, expectedFilesWithUnitTest1); const navBarResultUnitTest1 = navBarFull(session, unitTest1); - host.removeFile(unitTest1.path); + host.deleteFile(unitTest1.path); host.checkTimeoutQueueLengthAndRun(2); checkProjectActualFiles(project, expectedFilesWithoutUnitTest1); @@ -9297,7 +9297,7 @@ export function Test2() { checkDeclarationFiles(bTs, session, [bDtsMap, bDts]); // Testing what happens if we delete the original sources. - host.removeFile(bTs.path); + host.deleteFile(bTs.path); openFilesForSession([userTs], session); const service = session.getProjectService(); diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 5e992f3847..e41db162ae 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4125,7 +4125,7 @@ declare namespace ts { declare namespace ts { function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName?: string): string | undefined; function resolveTripleslashReference(moduleName: string, containingFile: string): string; - function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean): CompilerHost; + function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean, system?: System): CompilerHost; function getPreEmitDiagnostics(program: Program, sourceFile?: SourceFile, cancellationToken?: CancellationToken): Diagnostic[]; interface FormatDiagnosticsHost { getCurrentDirectory(): string; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 3276115655..fca9d26c30 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4125,7 +4125,7 @@ declare namespace ts { declare namespace ts { function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName?: string): string | undefined; function resolveTripleslashReference(moduleName: string, containingFile: string): string; - function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean): CompilerHost; + function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean, system?: System): CompilerHost; function getPreEmitDiagnostics(program: Program, sourceFile?: SourceFile, cancellationToken?: CancellationToken): Diagnostic[]; interface FormatDiagnosticsHost { getCurrentDirectory(): string; From d2240a40e19ecdb5b2547a2a6a58edb1852b0757 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 17 Aug 2018 15:24:37 -0700 Subject: [PATCH 02/21] Ger rid of unnecessary upto date host and functions pulled out --- src/compiler/tsbuild.ts | 373 +++++++++++++++++++--------------------- src/compiler/types.ts | 10 -- 2 files changed, 181 insertions(+), 202 deletions(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 966d795ed8..60f1d843e7 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -520,16 +520,6 @@ namespace ts { let context = createBuildContext(defaultOptions); const existingWatchersForWildcards = createMap(); - - const upToDateHost: UpToDateHost = { - fileExists: fileName => compilerHost.fileExists(fileName), - getModifiedTime: fileName => compilerHost.getModifiedTime!(fileName), - getUnchangedTime: fileName => context.unchangedOutputs.getValueOrUndefined(fileName), - getLastStatus: fileName => context.projectStatus.getValueOrUndefined(fileName), - setLastStatus: (fileName, status) => context.projectStatus.setValue(fileName, status), - parseConfigFile: configFilePath => configFileCache.parseConfigFile(configFilePath) - }; - return { buildAllProjects, getUpToDateStatus, @@ -611,7 +601,180 @@ namespace ts { } function getUpToDateStatus(project: ParsedCommandLine | undefined): UpToDateStatus { - return ts.getUpToDateStatus(upToDateHost, project); + if (project === undefined) { + return { type: UpToDateStatusType.Unbuildable, reason: "File deleted mid-build" }; + } + + const prior = context.projectStatus.getValueOrUndefined(project.options.configFilePath!); + if (prior !== undefined) { + return prior; + } + + const actual = getUpToDateStatusWorker(project); + context.projectStatus.setValue(project.options.configFilePath!, actual); + return actual; + } + + function getUpToDateStatusWorker(project: ParsedCommandLine): UpToDateStatus { + let newestInputFileName: string = undefined!; + let newestInputFileTime = minimumDate; + // Get timestamps of input files + for (const inputFile of project.fileNames) { + if (!compilerHost.fileExists(inputFile)) { + return { + type: UpToDateStatusType.Unbuildable, + reason: `${inputFile} does not exist` + }; + } + + const inputTime = compilerHost.getModifiedTime!(inputFile) || missingFileModifiedTime; + if (inputTime > newestInputFileTime) { + newestInputFileName = inputFile; + newestInputFileTime = inputTime; + } + } + + // Collect the expected outputs of this project + const outputs = getAllProjectOutputs(project); + + if (outputs.length === 0) { + return { + type: UpToDateStatusType.ContainerOnly + }; + } + + // Now see if all outputs are newer than the newest input + let oldestOutputFileName = "(none)"; + let oldestOutputFileTime = maximumDate; + let newestOutputFileName = "(none)"; + let newestOutputFileTime = minimumDate; + let missingOutputFileName: string | undefined; + let newestDeclarationFileContentChangedTime = minimumDate; + let isOutOfDateWithInputs = false; + for (const output of outputs) { + // Output is missing; can stop checking + // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status + if (!compilerHost.fileExists(output)) { + missingOutputFileName = output; + break; + } + + const outputTime = compilerHost.getModifiedTime!(output) || missingFileModifiedTime; + if (outputTime < oldestOutputFileTime) { + oldestOutputFileTime = outputTime; + oldestOutputFileName = output; + } + + // If an output is older than the newest input, we can stop checking + // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status + if (outputTime < newestInputFileTime) { + isOutOfDateWithInputs = true; + break; + } + + if (outputTime > newestOutputFileTime) { + newestOutputFileTime = outputTime; + newestOutputFileName = output; + } + + // Keep track of when the most recent time a .d.ts file was changed. + // In addition to file timestamps, we also keep track of when a .d.ts file + // had its file touched but not had its contents changed - this allows us + // to skip a downstream typecheck + if (isDeclarationFile(output)) { + const unchangedTime = context.unchangedOutputs.getValueOrUndefined(output); + if (unchangedTime !== undefined) { + newestDeclarationFileContentChangedTime = newer(unchangedTime, newestDeclarationFileContentChangedTime); + } + else { + const outputModifiedTime = compilerHost.getModifiedTime!(output) || missingFileModifiedTime; + newestDeclarationFileContentChangedTime = newer(newestDeclarationFileContentChangedTime, outputModifiedTime); + } + } + } + + let pseudoUpToDate = false; + let usesPrepend = false; + let upstreamChangedProject: string | undefined; + if (project.projectReferences) { + for (const ref of project.projectReferences) { + usesPrepend = usesPrepend || !!(ref.prepend); + const resolvedRef = resolveProjectReferencePath(compilerHost, ref); + const refStatus = getUpToDateStatus(configFileCache.parseConfigFile(resolvedRef)); + + // An upstream project is blocked + if (refStatus.type === UpToDateStatusType.Unbuildable) { + return { + type: UpToDateStatusType.UpstreamBlocked, + upstreamProjectName: ref.path + }; + } + + // If the upstream project is out of date, then so are we (someone shouldn't have asked, though?) + if (refStatus.type !== UpToDateStatusType.UpToDate) { + return { + type: UpToDateStatusType.UpstreamOutOfDate, + upstreamProjectName: ref.path + }; + } + + // If the upstream project's newest file is older than our oldest output, we + // can't be out of date because of it + if (refStatus.newestInputFileTime && refStatus.newestInputFileTime <= oldestOutputFileTime) { + continue; + } + + // If the upstream project has only change .d.ts files, and we've built + // *after* those files, then we're "psuedo up to date" and eligible for a fast rebuild + if (refStatus.newestDeclarationFileContentChangedTime && refStatus.newestDeclarationFileContentChangedTime <= oldestOutputFileTime) { + pseudoUpToDate = true; + upstreamChangedProject = ref.path; + continue; + } + + // We have an output older than an upstream output - we are out of date + Debug.assert(oldestOutputFileName !== undefined, "Should have an oldest output filename here"); + return { + type: UpToDateStatusType.OutOfDateWithUpstream, + outOfDateOutputFileName: oldestOutputFileName, + newerProjectName: ref.path + }; + } + } + + if (missingOutputFileName !== undefined) { + return { + type: UpToDateStatusType.OutputMissing, + missingOutputFileName + }; + } + + if (isOutOfDateWithInputs) { + return { + type: UpToDateStatusType.OutOfDateWithSelf, + outOfDateOutputFileName: oldestOutputFileName, + newerInputFileName: newestInputFileName + }; + } + + if (usesPrepend && pseudoUpToDate) { + return { + type: UpToDateStatusType.OutOfDateWithUpstream, + outOfDateOutputFileName: oldestOutputFileName, + newerProjectName: upstreamChangedProject! + }; + } + + // Up to date + return { + type: pseudoUpToDate ? UpToDateStatusType.UpToDateWithUpstreamTypes : UpToDateStatusType.UpToDate, + newestDeclarationFileContentChangedTime, + newestInputFileTime, + newestOutputFileTime, + newestInputFileName, + newestOutputFileName, + oldestOutputFileName + }; } function invalidateProject(configFileName: string) { @@ -1030,187 +1193,13 @@ namespace ts { } } - /** - * Gets the UpToDateStatus for a project - */ - export function getUpToDateStatus(host: UpToDateHost, project: ParsedCommandLine | undefined): UpToDateStatus { - if (project === undefined) { - return { type: UpToDateStatusType.Unbuildable, reason: "File deleted mid-build" }; - } - - const prior = host.getLastStatus ? host.getLastStatus(project.options.configFilePath!) : undefined; - if (prior !== undefined) { - return prior; - } - - const actual = getUpToDateStatusWorker(host, project); - if (host.setLastStatus) { - host.setLastStatus(project.options.configFilePath!, actual); - } - - return actual; - } - - function getUpToDateStatusWorker(host: UpToDateHost, project: ParsedCommandLine): UpToDateStatus { - let newestInputFileName: string = undefined!; - let newestInputFileTime = minimumDate; - // Get timestamps of input files - for (const inputFile of project.fileNames) { - if (!host.fileExists(inputFile)) { - return { - type: UpToDateStatusType.Unbuildable, - reason: `${inputFile} does not exist` - }; - } - - const inputTime = host.getModifiedTime(inputFile) || missingFileModifiedTime; - if (inputTime > newestInputFileTime) { - newestInputFileName = inputFile; - newestInputFileTime = inputTime; - } - } - - // Collect the expected outputs of this project - const outputs = getAllProjectOutputs(project); - - if (outputs.length === 0) { - return { - type: UpToDateStatusType.ContainerOnly - }; - } - - // Now see if all outputs are newer than the newest input - let oldestOutputFileName = "(none)"; - let oldestOutputFileTime = maximumDate; - let newestOutputFileName = "(none)"; - let newestOutputFileTime = minimumDate; - let missingOutputFileName: string | undefined; - let newestDeclarationFileContentChangedTime = minimumDate; - let isOutOfDateWithInputs = false; - for (const output of outputs) { - // Output is missing; can stop checking - // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status - if (!host.fileExists(output)) { - missingOutputFileName = output; - break; - } - - const outputTime = host.getModifiedTime(output) || missingFileModifiedTime; - if (outputTime < oldestOutputFileTime) { - oldestOutputFileTime = outputTime; - oldestOutputFileName = output; - } - - // If an output is older than the newest input, we can stop checking - // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status - if (outputTime < newestInputFileTime) { - isOutOfDateWithInputs = true; - break; - } - - if (outputTime > newestOutputFileTime) { - newestOutputFileTime = outputTime; - newestOutputFileName = output; - } - - // Keep track of when the most recent time a .d.ts file was changed. - // In addition to file timestamps, we also keep track of when a .d.ts file - // had its file touched but not had its contents changed - this allows us - // to skip a downstream typecheck - if (isDeclarationFile(output)) { - const unchangedTime = host.getUnchangedTime ? host.getUnchangedTime(output) : undefined; - if (unchangedTime !== undefined) { - newestDeclarationFileContentChangedTime = newer(unchangedTime, newestDeclarationFileContentChangedTime); - } - else { - const outputModifiedTime = host.getModifiedTime(output) || missingFileModifiedTime; - newestDeclarationFileContentChangedTime = newer(newestDeclarationFileContentChangedTime, outputModifiedTime); - } - } - } - - let pseudoUpToDate = false; - let usesPrepend = false; - let upstreamChangedProject: string | undefined; - if (project.projectReferences && host.parseConfigFile) { - for (const ref of project.projectReferences) { - usesPrepend = usesPrepend || !!(ref.prepend); - const resolvedRef = resolveProjectReferencePath(host, ref); - const refStatus = getUpToDateStatus(host, host.parseConfigFile(resolvedRef)); - - // An upstream project is blocked - if (refStatus.type === UpToDateStatusType.Unbuildable) { - return { - type: UpToDateStatusType.UpstreamBlocked, - upstreamProjectName: ref.path - }; - } - - // If the upstream project is out of date, then so are we (someone shouldn't have asked, though?) - if (refStatus.type !== UpToDateStatusType.UpToDate) { - return { - type: UpToDateStatusType.UpstreamOutOfDate, - upstreamProjectName: ref.path - }; - } - - // If the upstream project's newest file is older than our oldest output, we - // can't be out of date because of it - if (refStatus.newestInputFileTime && refStatus.newestInputFileTime <= oldestOutputFileTime) { - continue; - } - - // If the upstream project has only change .d.ts files, and we've built - // *after* those files, then we're "psuedo up to date" and eligible for a fast rebuild - if (refStatus.newestDeclarationFileContentChangedTime && refStatus.newestDeclarationFileContentChangedTime <= oldestOutputFileTime) { - pseudoUpToDate = true; - upstreamChangedProject = ref.path; - continue; - } - - // We have an output older than an upstream output - we are out of date - Debug.assert(oldestOutputFileName !== undefined, "Should have an oldest output filename here"); - return { - type: UpToDateStatusType.OutOfDateWithUpstream, - outOfDateOutputFileName: oldestOutputFileName, - newerProjectName: ref.path - }; - } - } - - if (missingOutputFileName !== undefined) { - return { - type: UpToDateStatusType.OutputMissing, - missingOutputFileName - }; - } - - if (isOutOfDateWithInputs) { - return { - type: UpToDateStatusType.OutOfDateWithSelf, - outOfDateOutputFileName: oldestOutputFileName, - newerInputFileName: newestInputFileName - }; - } - - if (usesPrepend && pseudoUpToDate) { - return { - type: UpToDateStatusType.OutOfDateWithUpstream, - outOfDateOutputFileName: oldestOutputFileName, - newerProjectName: upstreamChangedProject! - }; - } - - // Up to date - return { - type: pseudoUpToDate ? UpToDateStatusType.UpToDateWithUpstreamTypes : UpToDateStatusType.UpToDate, - newestDeclarationFileContentChangedTime, - newestInputFileTime, - newestOutputFileTime, - newestInputFileName, - newestOutputFileName, - oldestOutputFileName - }; + export interface UpToDateHost { + fileExists(fileName: string): boolean; + getModifiedTime(fileName: string): Date | undefined; + getUnchangedTime?(fileName: string): Date | undefined; + getLastStatus?(fileName: string): UpToDateStatus | undefined; + setLastStatus?(fileName: string, status: UpToDateStatus): void; + parseConfigFile?(configFilePath: ResolvedConfigFileName): ParsedCommandLine | undefined; } export function getAllProjectOutputs(project: ParsedCommandLine): ReadonlyArray { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 4330a776d0..96129411db 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4715,16 +4715,6 @@ namespace ts { verticalTab = 0x0B, // \v } - /*@internal*/ - export interface UpToDateHost { - fileExists(fileName: string): boolean; - getModifiedTime(fileName: string): Date | undefined; - getUnchangedTime?(fileName: string): Date | undefined; - getLastStatus?(fileName: string): UpToDateStatus | undefined; - setLastStatus?(fileName: string, status: UpToDateStatus): void; - parseConfigFile?(configFilePath: ResolvedConfigFileName): ParsedCommandLine | undefined; - } - export interface ModuleResolutionHost { // TODO: GH#18217 Optional methods frequently used as non-optional From 8e49fec80fda70e64a719d99fda3388161686f11 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 17 Aug 2018 16:42:59 -0700 Subject: [PATCH 03/21] Move perform build to tsc instead of tsbuild --- src/compiler/tsbuild.ts | 122 ---------------------------------------- src/tsc/tsc.ts | 122 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 122 deletions(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 60f1d843e7..87dcb0e2b5 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -385,128 +385,6 @@ namespace ts { }; } - const buildOpts: CommandLineOption[] = [ - { - name: "verbose", - shortName: "v", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Enable_verbose_logging, - type: "boolean" - }, - { - name: "dry", - shortName: "d", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Show_what_would_be_built_or_deleted_if_specified_with_clean, - type: "boolean" - }, - { - name: "force", - shortName: "f", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Build_all_projects_including_those_that_appear_to_be_up_to_date, - type: "boolean" - }, - { - name: "clean", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Delete_the_outputs_of_all_projects, - type: "boolean" - }, - { - name: "watch", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Watch_input_files, - type: "boolean" - } - ]; - - export function performBuild(args: string[], compilerHost: CompilerHost, buildHost: BuildHost, system?: System): number | undefined { - let verbose = false; - let dry = false; - let force = false; - let clean = false; - let watch = false; - - const projects: string[] = []; - for (const arg of args) { - switch (arg.toLowerCase()) { - case "-v": - case "--verbose": - verbose = true; - continue; - case "-d": - case "--dry": - dry = true; - continue; - case "-f": - case "--force": - force = true; - continue; - case "--clean": - clean = true; - continue; - case "--watch": - case "-w": - watch = true; - continue; - - case "--?": - case "-?": - case "--help": - printHelp(buildOpts, "--build "); - return ExitStatus.Success; - } - // Not a flag, parse as filename - addProject(arg); - } - - // Nonsensical combinations - if (clean && force) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force"); - return ExitStatus.DiagnosticsPresent_OutputsSkipped; - } - if (clean && verbose) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "verbose"); - return ExitStatus.DiagnosticsPresent_OutputsSkipped; - } - if (clean && watch) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "watch"); - return ExitStatus.DiagnosticsPresent_OutputsSkipped; - } - if (watch && dry) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "watch", "dry"); - return ExitStatus.DiagnosticsPresent_OutputsSkipped; - } - - if (projects.length === 0) { - // tsc -b invoked with no extra arguments; act as if invoked with "tsc -b ." - addProject("."); - } - - const builder = createSolutionBuilder(compilerHost, buildHost, projects, { dry, force, verbose }, system); - if (clean) { - return builder.cleanAllProjects(); - } - - if (watch) { - builder.buildAllProjects(); - builder.startWatching(); - return undefined; - } - - return builder.buildAllProjects(); - - function addProject(projectSpecification: string) { - const fileName = resolvePath(compilerHost.getCurrentDirectory(), projectSpecification); - const refPath = resolveProjectReferencePath(compilerHost, { path: fileName }); - if (!compilerHost.fileExists(refPath)) { - return buildHost.error(Diagnostics.File_0_does_not_exist, fileName); - } - projects.push(refPath); - } - } - /** * A SolutionBuilder has an immutable set of rootNames that are the "entry point" projects, but * can dynamically add/remove other projects based on changes on the rootNames' references diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index 97e239c8bc..f6f0db6e9a 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -172,6 +172,128 @@ namespace ts { } } + const buildOpts: CommandLineOption[] = [ + { + name: "verbose", + shortName: "v", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Enable_verbose_logging, + type: "boolean" + }, + { + name: "dry", + shortName: "d", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Show_what_would_be_built_or_deleted_if_specified_with_clean, + type: "boolean" + }, + { + name: "force", + shortName: "f", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Build_all_projects_including_those_that_appear_to_be_up_to_date, + type: "boolean" + }, + { + name: "clean", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Delete_the_outputs_of_all_projects, + type: "boolean" + }, + { + name: "watch", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Watch_input_files, + type: "boolean" + } + ]; + + function performBuild(args: string[], compilerHost: CompilerHost, buildHost: BuildHost, system?: System): number | undefined { + let verbose = false; + let dry = false; + let force = false; + let clean = false; + let watch = false; + + const projects: string[] = []; + for (const arg of args) { + switch (arg.toLowerCase()) { + case "-v": + case "--verbose": + verbose = true; + continue; + case "-d": + case "--dry": + dry = true; + continue; + case "-f": + case "--force": + force = true; + continue; + case "--clean": + clean = true; + continue; + case "--watch": + case "-w": + watch = true; + continue; + + case "--?": + case "-?": + case "--help": + printHelp(buildOpts, "--build "); + return ExitStatus.Success; + } + // Not a flag, parse as filename + addProject(arg); + } + + // Nonsensical combinations + if (clean && force) { + buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force"); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } + if (clean && verbose) { + buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "verbose"); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } + if (clean && watch) { + buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "watch"); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } + if (watch && dry) { + buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "watch", "dry"); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } + + if (projects.length === 0) { + // tsc -b invoked with no extra arguments; act as if invoked with "tsc -b ." + addProject("."); + } + + const builder = createSolutionBuilder(compilerHost, buildHost, projects, { dry, force, verbose }, system); + if (clean) { + return builder.cleanAllProjects(); + } + + if (watch) { + builder.buildAllProjects(); + builder.startWatching(); + return undefined; + } + + return builder.buildAllProjects(); + + function addProject(projectSpecification: string) { + const fileName = resolvePath(compilerHost.getCurrentDirectory(), projectSpecification); + const refPath = resolveProjectReferencePath(compilerHost, { path: fileName }); + if (!compilerHost.fileExists(refPath)) { + return buildHost.error(Diagnostics.File_0_does_not_exist, fileName); + } + projects.push(refPath); + } + } + function performCompilation(rootNames: string[], projectReferences: ReadonlyArray | undefined, options: CompilerOptions, configFileParsingDiagnostics?: ReadonlyArray) { const host = createCompilerHost(options); enableStatistics(options); From 071d790dec9a679d089fd7b196b900b8e678fc33 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 17 Aug 2018 17:22:08 -0700 Subject: [PATCH 04/21] Unify tsbuild option parsing with command line options parsing --- src/compiler/commandLineParser.ts | 17 +-- src/compiler/diagnosticMessages.json | 5 + src/compiler/tsbuild.ts | 11 +- src/tsc/tsc.ts | 154 +++++++++++++-------------- 4 files changed, 99 insertions(+), 88 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 0092032572..f1dd409395 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -815,10 +815,11 @@ namespace ts { } function getOptionNameMap(): OptionNameMap { - if (optionNameMapCache) { - return optionNameMapCache; - } + return optionNameMapCache || (optionNameMapCache = createOptionNameMap(optionDeclarations)); + } + /*@internal*/ + export function createOptionNameMap(optionDeclarations: ReadonlyArray): OptionNameMap { const optionNameMap = createMap(); const shortOptionNames = createMap(); forEach(optionDeclarations, option => { @@ -828,8 +829,7 @@ namespace ts { } }); - optionNameMapCache = { optionNameMap, shortOptionNames }; - return optionNameMapCache; + return { optionNameMap, shortOptionNames }; } /* @internal */ @@ -979,7 +979,12 @@ namespace ts { } /** @internal */ - export function getOptionFromName(optionName: string, allowShort = false): CommandLineOption | undefined { + export function getOptionFromName(optionName: string, allowShort?: boolean): CommandLineOption | undefined { + return getOptionDeclarationFromName(getOptionNameMap, optionName, allowShort); + } + + /*@internal*/ + export function getOptionDeclarationFromName(getOptionNameMap: () => OptionNameMap, optionName: string, allowShort = false): CommandLineOption | undefined { optionName = optionName.toLowerCase(); const { optionNameMap, shortOptionNames } = getOptionNameMap(); // Try to translate short option names to their full equivalents. diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 6000c48de4..6c517709f4 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -2900,6 +2900,11 @@ "category": "Error", "code": 5071 }, + "Unknown build option '{0}'.": { + "category": "Error", + "code": 5072 + }, + "Generates a sourcemap for each corresponding '.d.ts' file.": { "category": "Message", diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 87dcb0e2b5..54737d470e 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -47,10 +47,13 @@ namespace ts { dependencyMap: Mapper; } - interface BuildOptions { - dry: boolean; - force: boolean; - verbose: boolean; + export interface BuildOptions { + dry?: boolean; + force?: boolean; + verbose?: boolean; + /*@internal*/ clean?: boolean; + /*@internal*/ watch?: boolean; + /*@internal*/ help?: boolean; } enum BuildResultFlags { diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index f6f0db6e9a..a863c68493 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -62,7 +62,7 @@ namespace ts { message: report, errorDiagnostic: d => reportDiag(d) }; - const result = performBuild(args.slice(1), createCompilerHost({}), buildHost, sys); + const result = performBuild(args.slice(1), createCompilerHost({}), buildHost); // undefined = in watch mode, do not exit if (result !== undefined) { return sys.exit(result); @@ -172,96 +172,94 @@ namespace ts { } } - const buildOpts: CommandLineOption[] = [ - { - name: "verbose", - shortName: "v", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Enable_verbose_logging, - type: "boolean" - }, - { - name: "dry", - shortName: "d", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Show_what_would_be_built_or_deleted_if_specified_with_clean, - type: "boolean" - }, - { - name: "force", - shortName: "f", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Build_all_projects_including_those_that_appear_to_be_up_to_date, - type: "boolean" - }, - { - name: "clean", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Delete_the_outputs_of_all_projects, - type: "boolean" - }, - { - name: "watch", - category: Diagnostics.Command_line_Options, - description: Diagnostics.Watch_input_files, - type: "boolean" - } - ]; - - function performBuild(args: string[], compilerHost: CompilerHost, buildHost: BuildHost, system?: System): number | undefined { - let verbose = false; - let dry = false; - let force = false; - let clean = false; - let watch = false; + function performBuild(args: string[], compilerHost: CompilerHost, buildHost: BuildHost): number | undefined { + const buildOpts: CommandLineOption[] = [ + { + name: "help", + shortName: "h", + type: "boolean", + showInSimplifiedHelpView: true, + category: Diagnostics.Command_line_Options, + description: Diagnostics.Print_this_message, + }, + { + name: "help", + shortName: "?", + type: "boolean" + }, + { + name: "verbose", + shortName: "v", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Enable_verbose_logging, + type: "boolean" + }, + { + name: "dry", + shortName: "d", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Show_what_would_be_built_or_deleted_if_specified_with_clean, + type: "boolean" + }, + { + name: "force", + shortName: "f", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Build_all_projects_including_those_that_appear_to_be_up_to_date, + type: "boolean" + }, + { + name: "clean", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Delete_the_outputs_of_all_projects, + type: "boolean" + }, + { + name: "watch", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Watch_input_files, + type: "boolean" + } + ]; + let buildOptionNameMap: OptionNameMap | undefined; + const returnBuildOptionNameMap = () => (buildOptionNameMap || (buildOptionNameMap = createOptionNameMap(buildOpts))); + const buildOptions: BuildOptions = {}; const projects: string[] = []; for (const arg of args) { - switch (arg.toLowerCase()) { - case "-v": - case "--verbose": - verbose = true; - continue; - case "-d": - case "--dry": - dry = true; - continue; - case "-f": - case "--force": - force = true; - continue; - case "--clean": - clean = true; - continue; - case "--watch": - case "-w": - watch = true; - continue; - - case "--?": - case "-?": - case "--help": - printHelp(buildOpts, "--build "); - return ExitStatus.Success; + if (arg.charCodeAt(0) === CharacterCodes.minus) { + const opt = getOptionDeclarationFromName(returnBuildOptionNameMap, arg.slice(arg.charCodeAt(1) === CharacterCodes.minus ? 2 : 1), /*allowShort*/ true); + if (opt) { + buildOptions[opt.name as keyof BuildOptions] = true; + } + else { + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Unknown_build_option_0, arg)); + } } - // Not a flag, parse as filename - addProject(arg); + else { + // Not a flag, parse as filename + addProject(arg); + } + } + + if (buildOptions.help) { + printHelp(buildOpts, "--build "); return ExitStatus.Success; } // Nonsensical combinations - if (clean && force) { + if (buildOptions.clean && buildOptions.force) { buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force"); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } - if (clean && verbose) { + if (buildOptions.clean && buildOptions.verbose) { buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "verbose"); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } - if (clean && watch) { + if (buildOptions.clean && buildOptions.watch) { buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "watch"); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } - if (watch && dry) { + if (buildOptions.watch && buildOptions.dry) { buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "watch", "dry"); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } @@ -271,12 +269,12 @@ namespace ts { addProject("."); } - const builder = createSolutionBuilder(compilerHost, buildHost, projects, { dry, force, verbose }, system); - if (clean) { + const builder = createSolutionBuilder(compilerHost, buildHost, projects, buildOptions); + if (buildOptions.clean) { return builder.cleanAllProjects(); } - if (watch) { + if (buildOptions.watch) { builder.buildAllProjects(); builder.startWatching(); return undefined; From e20a7d851fe2e0843cc3648cf31daa2d28153a9f Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Fri, 17 Aug 2018 17:27:19 -0700 Subject: [PATCH 05/21] Remove unnecessary usage of system and compilerHost --- src/tsc/tsc.ts | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index a863c68493..ff16a92f0f 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -54,15 +54,7 @@ namespace ts { export function executeCommandLine(args: string[]): void { if (args.length > 0 && ((args[0].toLowerCase() === "--build") || (args[0].toLowerCase() === "-b"))) { - const reportDiag = createDiagnosticReporter(sys, defaultIsPretty()); - const report = (message: DiagnosticMessage, ...args: string[]) => reportDiag(createCompilerDiagnostic(message, ...args)); - const buildHost: BuildHost = { - error: report, - verbose: report, - message: report, - errorDiagnostic: d => reportDiag(d) - }; - const result = performBuild(args.slice(1), createCompilerHost({}), buildHost); + const result = performBuild(args.slice(1)); // undefined = in watch mode, do not exit if (result !== undefined) { return sys.exit(result); @@ -172,7 +164,7 @@ namespace ts { } } - function performBuild(args: string[], compilerHost: CompilerHost, buildHost: BuildHost): number | undefined { + function performBuild(args: string[]): number | undefined { const buildOpts: CommandLineOption[] = [ { name: "help", @@ -243,24 +235,28 @@ namespace ts { } if (buildOptions.help) { - printHelp(buildOpts, "--build "); return ExitStatus.Success; + printHelp(buildOpts, "--build "); + return ExitStatus.Success; } + // Update to pretty if host supports it + updateReportDiagnostic({}); + // Nonsensical combinations if (buildOptions.clean && buildOptions.force) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force"); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force")); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } if (buildOptions.clean && buildOptions.verbose) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "verbose"); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "verbose")); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } if (buildOptions.clean && buildOptions.watch) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "watch"); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "watch")); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } if (buildOptions.watch && buildOptions.dry) { - buildHost.error(Diagnostics.Options_0_and_1_cannot_be_combined, "watch", "dry"); + reportDiagnostic(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "watch", "dry")); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } @@ -269,7 +265,15 @@ namespace ts { addProject("."); } - const builder = createSolutionBuilder(compilerHost, buildHost, projects, buildOptions); + const report = (message: DiagnosticMessage, ...args: string[]) => reportDiagnostic(createCompilerDiagnostic(message, ...args)); + const buildHost: BuildHost = { + error: report, + verbose: report, + message: report, + errorDiagnostic: d => reportDiagnostic(d) + }; + + const builder = createSolutionBuilder(createCompilerHost({}), buildHost, projects, buildOptions); if (buildOptions.clean) { return builder.cleanAllProjects(); } @@ -283,9 +287,9 @@ namespace ts { return builder.buildAllProjects(); function addProject(projectSpecification: string) { - const fileName = resolvePath(compilerHost.getCurrentDirectory(), projectSpecification); - const refPath = resolveProjectReferencePath(compilerHost, { path: fileName }); - if (!compilerHost.fileExists(refPath)) { + const fileName = resolvePath(sys.getCurrentDirectory(), projectSpecification); + const refPath = resolveProjectReferencePath(sys, { path: fileName }); + if (!sys.fileExists(refPath)) { return buildHost.error(Diagnostics.File_0_does_not_exist, fileName); } projects.push(refPath); From dade3365d6c2f121d18dda25d68b83a67723ae92 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 20 Aug 2018 11:13:24 -0700 Subject: [PATCH 06/21] Print version along with help when doing --build --- src/tsc/tsc.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index ff16a92f0f..a2413d132b 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -235,6 +235,7 @@ namespace ts { } if (buildOptions.help) { + printVersion(); printHelp(buildOpts, "--build "); return ExitStatus.Success; } From 0c4003e7351aa4d6f8e9285fc30905715485050d Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 20 Aug 2018 11:47:39 -0700 Subject: [PATCH 07/21] Use SolutionBuilderHost instead of using compilerhost for solution builder --- src/compiler/program.ts | 5 +- src/compiler/tsbuild.ts | 73 +++++++++---------- src/compiler/types.ts | 4 - src/harness/fakes.ts | 2 +- src/testRunner/unittests/tsbuildWatchMode.ts | 8 +- src/tsc/tsc.ts | 7 +- .../reference/api/tsserverlibrary.d.ts | 3 - tests/baselines/reference/api/typescript.d.ts | 3 - 8 files changed, 47 insertions(+), 58 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 1c7430e330..6ca29707cb 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -189,10 +189,7 @@ namespace ts { getEnvironmentVariable: name => system.getEnvironmentVariable ? system.getEnvironmentVariable(name) : "", getDirectories: (path: string) => system.getDirectories(path), realpath, - readDirectory: (path, extensions, include, exclude, depth) => system.readDirectory(path, extensions, include, exclude, depth), - getModifiedTime: system.getModifiedTime && (path => system.getModifiedTime!(path)), - setModifiedTime: system.setModifiedTime && ((path, date) => system.setModifiedTime!(path, date)), - deleteFile: system.deleteFile && (path => system.deleteFile!(path)) + readDirectory: (path, extensions, include, exclude, depth) => system.readDirectory(path, extensions, include, exclude, depth) }; } diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 54737d470e..f0a81fe8b4 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -388,16 +388,27 @@ namespace ts { }; } + export interface SolutionBuilderHost extends CompilerHost { + getModifiedTime(fileName: string): Date | undefined; + setModifiedTime(fileName: string, date: Date): void; + deleteFile(fileName: string): void; + } + + export function createSolutionBuilderHost(system = sys) { + const host = createCompilerHost({}, /*setParentNodes*/ undefined, system) as SolutionBuilderHost; + host.getModifiedTime = system.getModifiedTime ? path => system.getModifiedTime!(path) : () => undefined; + host.setModifiedTime = system.setModifiedTime ? (path, date) => system.setModifiedTime!(path, date) : noop; + host.deleteFile = system.deleteFile ? path => system.deleteFile!(path) : noop; + return host; + } + /** * A SolutionBuilder has an immutable set of rootNames that are the "entry point" projects, but * can dynamically add/remove other projects based on changes on the rootNames' references */ - export function createSolutionBuilder(compilerHost: CompilerHost, buildHost: BuildHost, rootNames: ReadonlyArray, defaultOptions: BuildOptions, system?: System) { - if (!compilerHost.getModifiedTime || !compilerHost.setModifiedTime) { - throw new Error("Host must support timestamp APIs"); - } + export function createSolutionBuilder(host: SolutionBuilderHost, buildHost: BuildHost, rootNames: ReadonlyArray, defaultOptions: BuildOptions, system?: System) { - const configFileCache = createConfigFileCache(compilerHost); + const configFileCache = createConfigFileCache(host); let context = createBuildContext(defaultOptions); const existingWatchersForWildcards = createMap(); @@ -501,14 +512,14 @@ namespace ts { let newestInputFileTime = minimumDate; // Get timestamps of input files for (const inputFile of project.fileNames) { - if (!compilerHost.fileExists(inputFile)) { + if (!host.fileExists(inputFile)) { return { type: UpToDateStatusType.Unbuildable, reason: `${inputFile} does not exist` }; } - const inputTime = compilerHost.getModifiedTime!(inputFile) || missingFileModifiedTime; + const inputTime = host.getModifiedTime(inputFile) || missingFileModifiedTime; if (inputTime > newestInputFileTime) { newestInputFileName = inputFile; newestInputFileTime = inputTime; @@ -535,12 +546,12 @@ namespace ts { for (const output of outputs) { // Output is missing; can stop checking // Don't immediately return because we can still be upstream-blocked, which is a higher-priority status - if (!compilerHost.fileExists(output)) { + if (!host.fileExists(output)) { missingOutputFileName = output; break; } - const outputTime = compilerHost.getModifiedTime!(output) || missingFileModifiedTime; + const outputTime = host.getModifiedTime(output) || missingFileModifiedTime; if (outputTime < oldestOutputFileTime) { oldestOutputFileTime = outputTime; oldestOutputFileName = output; @@ -568,7 +579,7 @@ namespace ts { newestDeclarationFileContentChangedTime = newer(unchangedTime, newestDeclarationFileContentChangedTime); } else { - const outputModifiedTime = compilerHost.getModifiedTime!(output) || missingFileModifiedTime; + const outputModifiedTime = host.getModifiedTime(output) || missingFileModifiedTime; newestDeclarationFileContentChangedTime = newer(newestDeclarationFileContentChangedTime, outputModifiedTime); } } @@ -580,7 +591,7 @@ namespace ts { if (project.projectReferences) { for (const ref of project.projectReferences) { usesPrepend = usesPrepend || !!(ref.prepend); - const resolvedRef = resolveProjectReferencePath(compilerHost, ref); + const resolvedRef = resolveProjectReferencePath(host, ref); const refStatus = getUpToDateStatus(configFileCache.parseConfigFile(resolvedRef)); // An upstream project is blocked @@ -809,7 +820,7 @@ namespace ts { const programOptions: CreateProgramOptions = { projectReferences: configFile.projectReferences, - host: compilerHost, + host, rootNames: configFile.fileNames, options: configFile.options }; @@ -858,18 +869,18 @@ namespace ts { program.emit(/*targetSourceFile*/ undefined, (fileName, content, writeBom, onError) => { let priorChangeTime: Date | undefined; - if (!anyDtsChanged && isDeclarationFile(fileName) && compilerHost.fileExists(fileName)) { - if (compilerHost.readFile(fileName) === content) { + if (!anyDtsChanged && isDeclarationFile(fileName) && host.fileExists(fileName)) { + if (host.readFile(fileName) === content) { // Check for unchanged .d.ts files resultFlags &= ~BuildResultFlags.DeclarationOutputUnchanged; - priorChangeTime = compilerHost.getModifiedTime && compilerHost.getModifiedTime(fileName); + priorChangeTime = host.getModifiedTime(fileName); } else { anyDtsChanged = true; } } - compilerHost.writeFile(fileName, content, writeBom, onError, emptyArray); + host.writeFile(fileName, content, writeBom, onError, emptyArray); if (priorChangeTime !== undefined) { newestDeclarationFileContentChangedTime = newer(priorChangeTime, newestDeclarationFileContentChangedTime); context.unchangedOutputs.setValue(fileName, priorChangeTime); @@ -898,10 +909,10 @@ namespace ts { let priorNewestUpdateTime = minimumDate; for (const file of outputs) { if (isDeclarationFile(file)) { - priorNewestUpdateTime = newer(priorNewestUpdateTime, compilerHost.getModifiedTime!(file) || missingFileModifiedTime); + priorNewestUpdateTime = newer(priorNewestUpdateTime, host.getModifiedTime(file) || missingFileModifiedTime); } - compilerHost.setModifiedTime!(file, now); + host.setModifiedTime(file, now); } context.projectStatus.setValue(proj.options.configFilePath!, { type: UpToDateStatusType.UpToDate, newestDeclarationFileContentChangedTime: priorNewestUpdateTime } as UpToDateStatus); @@ -924,7 +935,7 @@ namespace ts { } const outputs = getAllProjectOutputs(parsed); for (const output of outputs) { - if (compilerHost.fileExists(output)) { + if (host.fileExists(output)) { filesToDelete.push(output); } } @@ -958,25 +969,20 @@ namespace ts { return ExitStatus.Success; } - // Do this check later to allow --clean --dry to function even if the host can't delete files - if (!compilerHost.deleteFile) { - throw new Error("Host does not support deleting files"); - } - for (const output of filesToDelete) { - compilerHost.deleteFile(output); + host.deleteFile(output); } return ExitStatus.Success; } function resolveProjectName(name: string): ResolvedConfigFileName | undefined { - const fullPath = resolvePath(compilerHost.getCurrentDirectory(), name); - if (compilerHost.fileExists(fullPath)) { + const fullPath = resolvePath(host.getCurrentDirectory(), name); + if (host.fileExists(fullPath)) { return fullPath as ResolvedConfigFileName; } const fullPathWithTsconfig = combinePaths(fullPath, "tsconfig.json"); - if (compilerHost.fileExists(fullPathWithTsconfig)) { + if (host.fileExists(fullPathWithTsconfig)) { return fullPathWithTsconfig as ResolvedConfigFileName; } buildHost.error(Diagnostics.File_0_not_found, relName(fullPath)); @@ -1058,7 +1064,7 @@ namespace ts { } function relName(path: string): string { - return convertToRelativePath(path, compilerHost.getCurrentDirectory(), f => compilerHost.getCanonicalFileName(f)); + return convertToRelativePath(path, host.getCurrentDirectory(), f => host.getCanonicalFileName(f)); } function reportVerbose(message: DiagnosticMessage, ...args: string[]) { @@ -1074,15 +1080,6 @@ namespace ts { } } - export interface UpToDateHost { - fileExists(fileName: string): boolean; - getModifiedTime(fileName: string): Date | undefined; - getUnchangedTime?(fileName: string): Date | undefined; - getLastStatus?(fileName: string): UpToDateStatus | undefined; - setLastStatus?(fileName: string, status: UpToDateStatus): void; - parseConfigFile?(configFilePath: ResolvedConfigFileName): ParsedCommandLine | undefined; - } - export function getAllProjectOutputs(project: ParsedCommandLine): ReadonlyArray { if (project.options.outFile) { return getOutFileOutputs(project); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 96129411db..2733348e56 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -4845,10 +4845,6 @@ namespace ts { /* @internal */ hasInvalidatedResolution?: HasInvalidatedResolution; /* @internal */ hasChangedAutomaticTypeDirectiveNames?: boolean; createHash?(data: string): string; - - getModifiedTime?(fileName: string): Date | undefined; - setModifiedTime?(fileName: string, date: Date): void; - deleteFile?(fileName: string): void; } /* @internal */ diff --git a/src/harness/fakes.ts b/src/harness/fakes.ts index 1bb358698a..b4dd97878b 100644 --- a/src/harness/fakes.ts +++ b/src/harness/fakes.ts @@ -205,7 +205,7 @@ namespace fakes { /** * A fake `ts.CompilerHost` that leverages a virtual file system. */ - export class CompilerHost implements ts.CompilerHost { + export class CompilerHost implements ts.CompilerHost, ts.SolutionBuilderHost { public readonly sys: System; public readonly defaultLibLocation: string; public readonly outputs: documents.TextDocument[] = []; diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts index 9f608180d0..dd6a95f0f1 100644 --- a/src/testRunner/unittests/tsbuildWatchMode.ts +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -1,8 +1,8 @@ namespace ts.tscWatch { export import libFile = TestFSWithWatch.libFile; - function createSolutionBuilder(host: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { - const compilerHost = createCompilerHost({}, /*setParentNodes*/ undefined, host); - const reportDiag = createDiagnosticReporter(host); + function createSolutionBuilder(system: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { + const host = createSolutionBuilderHost(system); + const reportDiag = createDiagnosticReporter(system); const report = (message: DiagnosticMessage, ...args: string[]) => reportDiag(createCompilerDiagnostic(message, ...args)); const buildHost: BuildHost = { error: report, @@ -10,7 +10,7 @@ namespace ts.tscWatch { message: report, errorDiagnostic: d => reportDiag(d) }; - return ts.createSolutionBuilder(compilerHost, buildHost, rootNames, defaultOptions || { dry: false, force: false, verbose: false }, host); + return ts.createSolutionBuilder(host, buildHost, rootNames, defaultOptions || { dry: false, force: false, verbose: false }, system); } function createSolutionBuilderWithWatch(host: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index a2413d132b..d53aa1f56f 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -243,6 +243,11 @@ namespace ts { // Update to pretty if host supports it updateReportDiagnostic({}); + if (!sys.getModifiedTime || !sys.setModifiedTime || (buildOptions.clean && !sys.deleteFile)) { + reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--build")); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } + // Nonsensical combinations if (buildOptions.clean && buildOptions.force) { reportDiagnostic(createCompilerDiagnostic(Diagnostics.Options_0_and_1_cannot_be_combined, "clean", "force")); @@ -274,7 +279,7 @@ namespace ts { errorDiagnostic: d => reportDiagnostic(d) }; - const builder = createSolutionBuilder(createCompilerHost({}), buildHost, projects, buildOptions); + const builder = createSolutionBuilder(createSolutionBuilderHost(), buildHost, projects, buildOptions); if (buildOptions.clean) { return builder.cleanAllProjects(); } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index e41db162ae..5c50a7cf45 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2690,9 +2690,6 @@ declare namespace ts { resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; getEnvironmentVariable?(name: string): string | undefined; createHash?(data: string): string; - getModifiedTime?(fileName: string): Date | undefined; - setModifiedTime?(fileName: string, date: Date): void; - deleteFile?(fileName: string): void; } interface SourceMapRange extends TextRange { source?: SourceMapSource; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index fca9d26c30..29e485415f 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -2690,9 +2690,6 @@ declare namespace ts { resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; getEnvironmentVariable?(name: string): string | undefined; createHash?(data: string): string; - getModifiedTime?(fileName: string): Date | undefined; - setModifiedTime?(fileName: string, date: Date): void; - deleteFile?(fileName: string): void; } interface SourceMapRange extends TextRange { source?: SourceMapSource; From 26b4b6c9ad87e616c38aeb7e6c1121daadc63f9c Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 20 Aug 2018 12:52:09 -0700 Subject: [PATCH 08/21] Create api with watchHost to include in solution builder host --- src/compiler/tsbuild.ts | 36 +++++++++---- src/compiler/watch.ts | 52 ++++++++++++------- src/testRunner/unittests/tsbuildWatchMode.ts | 4 +- src/tsc/tsc.ts | 6 ++- .../reference/api/tsserverlibrary.d.ts | 25 +++++---- tests/baselines/reference/api/typescript.d.ts | 25 +++++---- 6 files changed, 94 insertions(+), 54 deletions(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index f0a81fe8b4..e6bc8d252d 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -394,6 +394,9 @@ namespace ts { deleteFile(fileName: string): void; } + export interface SolutionBuilderWithWatchHost extends SolutionBuilderHost, WatchHost { + } + export function createSolutionBuilderHost(system = sys) { const host = createCompilerHost({}, /*setParentNodes*/ undefined, system) as SolutionBuilderHost; host.getModifiedTime = system.getModifiedTime ? path => system.getModifiedTime!(path) : () => undefined; @@ -402,12 +405,25 @@ namespace ts { return host; } + export function createSolutionBuilderWithWatchHost(system = sys, reportWatchStatus?: WatchStatusReporter) { + const host = createSolutionBuilderHost(system) as SolutionBuilderWithWatchHost; + const watchHost = createWatchHost(system, reportWatchStatus); + host.onWatchStatusChange = watchHost.onWatchStatusChange; + host.watchFile = watchHost.watchFile; + host.watchDirectory = watchHost.watchDirectory; + host.setTimeout = watchHost.setTimeout; + host.clearTimeout = watchHost.clearTimeout; + return host; + } + /** * A SolutionBuilder has an immutable set of rootNames that are the "entry point" projects, but * can dynamically add/remove other projects based on changes on the rootNames' references + * TODO: use SolutionBuilderWithWatchHost => watchedSolution + * use SolutionBuilderHost => Solution */ - export function createSolutionBuilder(host: SolutionBuilderHost, buildHost: BuildHost, rootNames: ReadonlyArray, defaultOptions: BuildOptions, system?: System) { - + export function createSolutionBuilder(host: SolutionBuilderHost, buildHost: BuildHost, rootNames: ReadonlyArray, defaultOptions: BuildOptions) { + const hostWithWatch = host as SolutionBuilderWithWatchHost; const configFileCache = createConfigFileCache(host); let context = createBuildContext(defaultOptions); @@ -430,9 +446,6 @@ namespace ts { }; function startWatching() { - if (!system) throw new Error("System host must be provided if using --watch"); - if (!system.watchFile || !system.watchDirectory || !system.setTimeout) throw new Error("System host must support watchFile / watchDirectory / setTimeout if using --watch"); - const graph = getGlobalDependencyGraph()!; if (!graph.buildQueue) { // Everything is broken - we don't even know what to watch. Give up. @@ -443,7 +456,7 @@ namespace ts { const cfg = configFileCache.parseConfigFile(resolved); if (cfg) { // Watch this file - system.watchFile(resolved, () => { + hostWithWatch.watchFile(resolved, () => { configFileCache.removeKey(resolved); invalidateProjectAndScheduleBuilds(resolved); }); @@ -451,7 +464,7 @@ namespace ts { // Update watchers for wildcard directories if (cfg.configFileSpecs) { updateWatchingWildcardDirectories(existingWatchersForWildcards, createMapFromTemplate(cfg.configFileSpecs.wildcardDirectories), (dir, flags) => { - return system.watchDirectory!(dir, () => { + return hostWithWatch.watchDirectory(dir, () => { invalidateProjectAndScheduleBuilds(resolved); }, !!(flags & WatchDirectoryFlags.Recursive)); }); @@ -459,7 +472,7 @@ namespace ts { // Watch input files for (const input of cfg.fileNames) { - system.watchFile(input, () => { + hostWithWatch.watchFile(input, () => { invalidateProjectAndScheduleBuilds(resolved); }); } @@ -468,8 +481,11 @@ namespace ts { function invalidateProjectAndScheduleBuilds(resolved: ResolvedConfigFileName) { invalidateProject(resolved); - system!.setTimeout!(buildInvalidatedProjects, 100); - system!.setTimeout!(buildDependentInvalidatedProjects, 3000); + if (!hostWithWatch.setTimeout) { + return; + } + hostWithWatch.setTimeout(buildInvalidatedProjects, 100); + hostWithWatch.setTimeout(buildDependentInvalidatedProjects, 3000); } } diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 9c759158f6..63904ffa43 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -174,6 +174,17 @@ namespace ts { const noopFileWatcher: FileWatcher = { close: noop }; + export function createWatchHost(system = sys, reportWatchStatus?: WatchStatusReporter): WatchHost { + const onWatchStatusChange = reportWatchStatus || createWatchStatusReporter(system); + return { + onWatchStatusChange, + watchFile: system.watchFile ? ((path, callback, pollingInterval) => system.watchFile!(path, callback, pollingInterval)) : () => noopFileWatcher, + watchDirectory: system.watchDirectory ? ((path, callback, recursive) => system.watchDirectory!(path, callback, recursive)) : () => noopFileWatcher, + setTimeout: system.setTimeout ? ((callback, ms, ...args: any[]) => system.setTimeout!.call(system, callback, ms, ...args)) : noop, + clearTimeout: system.clearTimeout ? (timeoutId => system.clearTimeout!(timeoutId)) : noop + }; + } + /** * Creates the watch compiler host that can be extended with config file or root file names and options host */ @@ -186,7 +197,7 @@ namespace ts { host; // tslint:disable-line no-unused-expression (TODO: `host` is unused!) const useCaseSensitiveFileNames = () => system.useCaseSensitiveFileNames; const writeFileName = (s: string) => system.write(s + system.newLine); - const onWatchStatusChange = reportWatchStatus || createWatchStatusReporter(system); + const { onWatchStatusChange, watchFile, watchDirectory, setTimeout, clearTimeout } = createWatchHost(system, reportWatchStatus); return { useCaseSensitiveFileNames, getNewLine: () => system.newLine, @@ -200,10 +211,10 @@ namespace ts { readDirectory: (path, extensions, exclude, include, depth) => system.readDirectory(path, extensions, exclude, include, depth), realpath: system.realpath && (path => system.realpath!(path)), getEnvironmentVariable: system.getEnvironmentVariable && (name => system.getEnvironmentVariable(name)), - watchFile: system.watchFile ? ((path, callback, pollingInterval) => system.watchFile!(path, callback, pollingInterval)) : () => noopFileWatcher, - watchDirectory: system.watchDirectory ? ((path, callback, recursive) => system.watchDirectory!(path, callback, recursive)) : () => noopFileWatcher, - setTimeout: system.setTimeout ? ((callback, ms, ...args: any[]) => system.setTimeout!.call(system, callback, ms, ...args)) : noop, - clearTimeout: system.clearTimeout ? (timeoutId => system.clearTimeout!(timeoutId)) : noop, + watchFile, + watchDirectory, + setTimeout, + clearTimeout, trace: s => system.write(s), onWatchStatusChange, createDirectory: path => system.createDirectory(path), @@ -224,10 +235,10 @@ namespace ts { const reportSummary = (errorCount: number) => { if (errorCount === 1) { - onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_1_error_Watching_for_file_changes, errorCount), newLine, compilerOptions); + onWatchStatusChange!(createCompilerDiagnostic(Diagnostics.Found_1_error_Watching_for_file_changes, errorCount), newLine, compilerOptions); } else { - onWatchStatusChange(createCompilerDiagnostic(Diagnostics.Found_0_errors_Watching_for_file_changes, errorCount, errorCount), newLine, compilerOptions); + onWatchStatusChange!(createCompilerDiagnostic(Diagnostics.Found_0_errors_Watching_for_file_changes, errorCount, errorCount), newLine, compilerOptions); } }; @@ -270,7 +281,21 @@ namespace ts { export type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string, options: CompilerOptions) => void; /** Create the program with rootNames and options, if they are undefined, oldProgram and new configFile diagnostics create new program */ export type CreateProgram = (rootNames: ReadonlyArray | undefined, options: CompilerOptions | undefined, host?: CompilerHost, oldProgram?: T, configFileParsingDiagnostics?: ReadonlyArray) => T; - export interface WatchCompilerHost { + /** Host that has watch functionality used in --watch mode */ + export interface WatchHost { + /** If provided, called with Diagnostic message that informs about change in watch status */ + onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; + + /** Used to watch changes in source files, missing files needed to update the program or config file */ + watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; + /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */ + watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; + /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */ + setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; + /** If provided, will be used to reset existing delayed compilation */ + clearTimeout?(timeoutId: any): void; + } + export interface WatchCompilerHost extends WatchHost { // TODO: GH#18217 Optional methods are frequently asserted /** @@ -279,8 +304,6 @@ namespace ts { createProgram: CreateProgram; /** If provided, callback to invoke after every new program creation */ afterProgramCreate?(program: T): void; - /** If provided, called with Diagnostic message that informs about change in watch status */ - onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; // Only for testing /*@internal*/ @@ -323,15 +346,6 @@ namespace ts { resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; /** If provided, used to resolve type reference directives, otherwise typescript's default resolution */ resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; - - /** Used to watch changes in source files, missing files needed to update the program or config file */ - watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; - /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */ - watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; - /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */ - setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; - /** If provided, will be used to reset existing delayed compilation */ - clearTimeout?(timeoutId: any): void; } /** Internal interface used to wire emit through same host */ diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts index dd6a95f0f1..87224c0933 100644 --- a/src/testRunner/unittests/tsbuildWatchMode.ts +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -1,7 +1,7 @@ namespace ts.tscWatch { export import libFile = TestFSWithWatch.libFile; function createSolutionBuilder(system: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { - const host = createSolutionBuilderHost(system); + const host = createSolutionBuilderWithWatchHost(system); const reportDiag = createDiagnosticReporter(system); const report = (message: DiagnosticMessage, ...args: string[]) => reportDiag(createCompilerDiagnostic(message, ...args)); const buildHost: BuildHost = { @@ -10,7 +10,7 @@ namespace ts.tscWatch { message: report, errorDiagnostic: d => reportDiag(d) }; - return ts.createSolutionBuilder(host, buildHost, rootNames, defaultOptions || { dry: false, force: false, verbose: false }, system); + return ts.createSolutionBuilder(host, buildHost, rootNames, defaultOptions || { dry: false, force: false, verbose: false, watch: true }); } function createSolutionBuilderWithWatch(host: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index d53aa1f56f..5c7af1a4b9 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -247,6 +247,9 @@ namespace ts { reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--build")); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } + if (buildOptions.watch) { + reportWatchModeWithoutSysSupport(); + } // Nonsensical combinations if (buildOptions.clean && buildOptions.force) { @@ -279,7 +282,8 @@ namespace ts { errorDiagnostic: d => reportDiagnostic(d) }; - const builder = createSolutionBuilder(createSolutionBuilderHost(), buildHost, projects, buildOptions); + // TODO: change this to host if watch => watchHost otherwiue without wathc + const builder = createSolutionBuilder(createSolutionBuilderWithWatchHost(), buildHost, projects, buildOptions); if (buildOptions.clean) { return builder.cleanAllProjects(); } diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 5c50a7cf45..dd5f3c7c65 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4309,15 +4309,26 @@ declare namespace ts { type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string, options: CompilerOptions) => void; /** Create the program with rootNames and options, if they are undefined, oldProgram and new configFile diagnostics create new program */ type CreateProgram = (rootNames: ReadonlyArray | undefined, options: CompilerOptions | undefined, host?: CompilerHost, oldProgram?: T, configFileParsingDiagnostics?: ReadonlyArray) => T; - interface WatchCompilerHost { + /** Host that has watch functionality used in --watch mode */ + interface WatchHost { + /** If provided, called with Diagnostic message that informs about change in watch status */ + onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; + /** Used to watch changes in source files, missing files needed to update the program or config file */ + watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; + /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */ + watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; + /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */ + setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; + /** If provided, will be used to reset existing delayed compilation */ + clearTimeout?(timeoutId: any): void; + } + interface WatchCompilerHost extends WatchHost { /** * Used to create the program when need for program creation or recreation detected */ createProgram: CreateProgram; /** If provided, callback to invoke after every new program creation */ afterProgramCreate?(program: T): void; - /** If provided, called with Diagnostic message that informs about change in watch status */ - onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; useCaseSensitiveFileNames(): boolean; getNewLine(): string; getCurrentDirectory(): string; @@ -4350,14 +4361,6 @@ declare namespace ts { resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; /** If provided, used to resolve type reference directives, otherwise typescript's default resolution */ resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; - /** Used to watch changes in source files, missing files needed to update the program or config file */ - watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; - /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */ - watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; - /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */ - setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; - /** If provided, will be used to reset existing delayed compilation */ - clearTimeout?(timeoutId: any): void; } /** * Host to create watch with root files and options diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 29e485415f..2938fc9bcc 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4309,15 +4309,26 @@ declare namespace ts { type WatchStatusReporter = (diagnostic: Diagnostic, newLine: string, options: CompilerOptions) => void; /** Create the program with rootNames and options, if they are undefined, oldProgram and new configFile diagnostics create new program */ type CreateProgram = (rootNames: ReadonlyArray | undefined, options: CompilerOptions | undefined, host?: CompilerHost, oldProgram?: T, configFileParsingDiagnostics?: ReadonlyArray) => T; - interface WatchCompilerHost { + /** Host that has watch functionality used in --watch mode */ + interface WatchHost { + /** If provided, called with Diagnostic message that informs about change in watch status */ + onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; + /** Used to watch changes in source files, missing files needed to update the program or config file */ + watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; + /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */ + watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; + /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */ + setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; + /** If provided, will be used to reset existing delayed compilation */ + clearTimeout?(timeoutId: any): void; + } + interface WatchCompilerHost extends WatchHost { /** * Used to create the program when need for program creation or recreation detected */ createProgram: CreateProgram; /** If provided, callback to invoke after every new program creation */ afterProgramCreate?(program: T): void; - /** If provided, called with Diagnostic message that informs about change in watch status */ - onWatchStatusChange?(diagnostic: Diagnostic, newLine: string, options: CompilerOptions): void; useCaseSensitiveFileNames(): boolean; getNewLine(): string; getCurrentDirectory(): string; @@ -4350,14 +4361,6 @@ declare namespace ts { resolveModuleNames?(moduleNames: string[], containingFile: string, reusedNames?: string[]): ResolvedModule[]; /** If provided, used to resolve type reference directives, otherwise typescript's default resolution */ resolveTypeReferenceDirectives?(typeReferenceDirectiveNames: string[], containingFile: string): ResolvedTypeReferenceDirective[]; - /** Used to watch changes in source files, missing files needed to update the program or config file */ - watchFile(path: string, callback: FileWatcherCallback, pollingInterval?: number): FileWatcher; - /** Used to watch resolved module's failed lookup locations, config file specs, type roots where auto type reference directives are added */ - watchDirectory(path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher; - /** If provided, will be used to set delayed compilation, so that multiple changes in short span are compiled together */ - setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; - /** If provided, will be used to reset existing delayed compilation */ - clearTimeout?(timeoutId: any): void; } /** * Host to create watch with root files and options From dedb2aefc0b0b2b17b03b671ee89f28ab3403417 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 20 Aug 2018 13:33:54 -0700 Subject: [PATCH 09/21] Combine buildHost methods into SolutionBuilderHost's reportDiagnostic and reportStatus --- src/compiler/tsbuild.ts | 66 +++++---- src/harness/fakes.ts | 38 +++++- src/testRunner/unittests/tsbuild.ts | 134 +++++++------------ src/testRunner/unittests/tsbuildWatchMode.ts | 10 +- src/tsc/tsc.ts | 22 +-- 5 files changed, 132 insertions(+), 138 deletions(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index e6bc8d252d..7efecab608 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -392,21 +392,37 @@ namespace ts { getModifiedTime(fileName: string): Date | undefined; setModifiedTime(fileName: string, date: Date): void; deleteFile(fileName: string): void; + + reportDiagnostic: DiagnosticReporter; // Technically we want to move it out and allow steps of actions on Solution, but for now just merge stuff in build host here + reportSolutionBuilderStatus: DiagnosticReporter; } export interface SolutionBuilderWithWatchHost extends SolutionBuilderHost, WatchHost { } - export function createSolutionBuilderHost(system = sys) { + /** + * Create a function that reports watch status by writing to the system and handles the formating of the diagnostic + */ + export function createBuilderStatusReporter(system: System, pretty?: boolean): DiagnosticReporter { + return diagnostic => { + let output = pretty ? `[${formatColorAndReset(new Date().toLocaleTimeString(), ForegroundColorEscapeSequences.Grey)}] ` : `${new Date().toLocaleTimeString()} - `; + output += `${flattenDiagnosticMessageText(diagnostic.messageText, system.newLine)}${system.newLine + system.newLine}`; + system.write(output); + }; + } + + export function createSolutionBuilderHost(system = sys, reportDiagnostic?: DiagnosticReporter, reportSolutionBuilderStatus?: DiagnosticReporter) { const host = createCompilerHost({}, /*setParentNodes*/ undefined, system) as SolutionBuilderHost; host.getModifiedTime = system.getModifiedTime ? path => system.getModifiedTime!(path) : () => undefined; host.setModifiedTime = system.setModifiedTime ? (path, date) => system.setModifiedTime!(path, date) : noop; host.deleteFile = system.deleteFile ? path => system.deleteFile!(path) : noop; + host.reportDiagnostic = reportDiagnostic || createDiagnosticReporter(system); + host.reportSolutionBuilderStatus = reportSolutionBuilderStatus || createBuilderStatusReporter(system); return host; } - export function createSolutionBuilderWithWatchHost(system = sys, reportWatchStatus?: WatchStatusReporter) { - const host = createSolutionBuilderHost(system) as SolutionBuilderWithWatchHost; + export function createSolutionBuilderWithWatchHost(system = sys, reportDiagnostic?: DiagnosticReporter, reportSolutionBuilderStatus?: DiagnosticReporter, reportWatchStatus?: WatchStatusReporter) { + const host = createSolutionBuilderHost(system, reportDiagnostic, reportSolutionBuilderStatus) as SolutionBuilderWithWatchHost; const watchHost = createWatchHost(system, reportWatchStatus); host.onWatchStatusChange = watchHost.onWatchStatusChange; host.watchFile = watchHost.watchFile; @@ -422,7 +438,7 @@ namespace ts { * TODO: use SolutionBuilderWithWatchHost => watchedSolution * use SolutionBuilderHost => Solution */ - export function createSolutionBuilder(host: SolutionBuilderHost, buildHost: BuildHost, rootNames: ReadonlyArray, defaultOptions: BuildOptions) { + export function createSolutionBuilder(host: SolutionBuilderHost, rootNames: ReadonlyArray, defaultOptions: BuildOptions) { const hostWithWatch = host as SolutionBuilderWithWatchHost; const configFileCache = createConfigFileCache(host); let context = createBuildContext(defaultOptions); @@ -445,6 +461,10 @@ namespace ts { startWatching }; + function reportStatus(message: DiagnosticMessage, ...args: string[]) { + host.reportSolutionBuilderStatus(createCompilerDiagnostic(message, ...args)); + } + function startWatching() { const graph = getGlobalDependencyGraph()!; if (!graph.buildQueue) { @@ -743,7 +763,7 @@ namespace ts { verboseReportProjectStatus(next, status); if (status.type === UpToDateStatusType.UpstreamBlocked) { - if (context.options.verbose) buildHost.verbose(Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors, resolved, status.upstreamProjectName); + if (context.options.verbose) reportStatus(Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors, resolved, status.upstreamProjectName); continue; } @@ -780,7 +800,7 @@ namespace ts { if (temporaryMarks[projPath]) { if (!inCircularContext) { hadError = true; - buildHost.error(Diagnostics.Project_references_may_not_form_a_circular_graph_Cycle_detected_Colon_0, circularityReportStack.join("\r\n")); + reportStatus(Diagnostics.Project_references_may_not_form_a_circular_graph_Cycle_detected_Colon_0, circularityReportStack.join("\r\n")); return; } } @@ -812,11 +832,11 @@ namespace ts { function buildSingleProject(proj: ResolvedConfigFileName): BuildResultFlags { if (context.options.dry) { - buildHost.message(Diagnostics.A_non_dry_build_would_build_project_0, proj); + reportStatus(Diagnostics.A_non_dry_build_would_build_project_0, proj); return BuildResultFlags.Success; } - if (context.options.verbose) buildHost.verbose(Diagnostics.Building_project_0, proj); + if (context.options.verbose) reportStatus(Diagnostics.Building_project_0, proj); let resultFlags = BuildResultFlags.None; resultFlags |= BuildResultFlags.DeclarationOutputUnchanged; @@ -850,7 +870,7 @@ namespace ts { if (syntaxDiagnostics.length) { resultFlags |= BuildResultFlags.SyntaxErrors; for (const diag of syntaxDiagnostics) { - buildHost.errorDiagnostic(diag); + host.reportDiagnostic(diag); } context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Syntactic errors" }); return resultFlags; @@ -862,7 +882,7 @@ namespace ts { if (declDiagnostics.length) { resultFlags |= BuildResultFlags.DeclarationEmitErrors; for (const diag of declDiagnostics) { - buildHost.errorDiagnostic(diag); + host.reportDiagnostic(diag); } context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Declaration file errors" }); return resultFlags; @@ -874,7 +894,7 @@ namespace ts { if (semanticDiagnostics.length) { resultFlags |= BuildResultFlags.TypeErrors; for (const diag of semanticDiagnostics) { - buildHost.errorDiagnostic(diag); + host.reportDiagnostic(diag); } context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Semantic errors" }); return resultFlags; @@ -913,11 +933,11 @@ namespace ts { function updateOutputTimestamps(proj: ParsedCommandLine) { if (context.options.dry) { - return buildHost.message(Diagnostics.A_non_dry_build_would_build_project_0, proj.options.configFilePath!); + return reportStatus(Diagnostics.A_non_dry_build_would_build_project_0, proj.options.configFilePath!); } if (context.options.verbose) { - buildHost.verbose(Diagnostics.Updating_output_timestamps_of_project_0, proj.options.configFilePath!); + reportStatus(Diagnostics.Updating_output_timestamps_of_project_0, proj.options.configFilePath!); } const now = new Date(); @@ -970,18 +990,18 @@ namespace ts { function cleanAllProjects() { const resolvedNames: ReadonlyArray | undefined = getAllProjectsInScope(); if (resolvedNames === undefined) { - buildHost.message(Diagnostics.Skipping_clean_because_not_all_projects_could_be_located); + reportStatus(Diagnostics.Skipping_clean_because_not_all_projects_could_be_located); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } const filesToDelete = getFilesToClean(resolvedNames); if (filesToDelete === undefined) { - buildHost.message(Diagnostics.Skipping_clean_because_not_all_projects_could_be_located); + reportStatus(Diagnostics.Skipping_clean_because_not_all_projects_could_be_located); return ExitStatus.DiagnosticsPresent_OutputsSkipped; } if (context.options.dry) { - buildHost.message(Diagnostics.A_non_dry_build_would_delete_the_following_files_Colon_0, filesToDelete.map(f => `\r\n * ${f}`).join("")); + reportStatus(Diagnostics.A_non_dry_build_would_delete_the_following_files_Colon_0, filesToDelete.map(f => `\r\n * ${f}`).join("")); return ExitStatus.Success; } @@ -1001,7 +1021,7 @@ namespace ts { if (host.fileExists(fullPathWithTsconfig)) { return fullPathWithTsconfig as ResolvedConfigFileName; } - buildHost.error(Diagnostics.File_0_not_found, relName(fullPath)); + host.reportDiagnostic(createCompilerDiagnostic(Diagnostics.File_0_not_found, relName(fullPath))); return undefined; } @@ -1039,7 +1059,7 @@ namespace ts { // Up to date, skip if (defaultOptions.dry) { // In a dry build, inform the user of this fact - buildHost.message(Diagnostics.Project_0_is_up_to_date, projName); + reportStatus(Diagnostics.Project_0_is_up_to_date, projName); } continue; } @@ -1051,7 +1071,7 @@ namespace ts { } if (status.type === UpToDateStatusType.UpstreamBlocked) { - if (context.options.verbose) buildHost.verbose(Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors, projName, status.upstreamProjectName); + if (context.options.verbose) reportStatus(Diagnostics.Skipping_build_of_project_0_because_its_dependency_1_has_errors, projName, status.upstreamProjectName); continue; } @@ -1076,23 +1096,19 @@ namespace ts { for (const name of graph.buildQueue) { names.push(name); } - if (context.options.verbose) buildHost.verbose(Diagnostics.Projects_in_this_build_Colon_0, names.map(s => "\r\n * " + relName(s)).join("")); + if (context.options.verbose) reportStatus(Diagnostics.Projects_in_this_build_Colon_0, names.map(s => "\r\n * " + relName(s)).join("")); } function relName(path: string): string { return convertToRelativePath(path, host.getCurrentDirectory(), f => host.getCanonicalFileName(f)); } - function reportVerbose(message: DiagnosticMessage, ...args: string[]) { - buildHost.verbose(message, ...args); - } - /** * Report the up-to-date status of a project if we're in verbose mode */ function verboseReportProjectStatus(configFileName: string, status: UpToDateStatus) { if (!context.options.verbose) return; - return formatUpToDateStatus(configFileName, status, relName, reportVerbose); + return formatUpToDateStatus(configFileName, status, relName, reportStatus); } } diff --git a/src/harness/fakes.ts b/src/harness/fakes.ts index b4dd97878b..bd9d62f90e 100644 --- a/src/harness/fakes.ts +++ b/src/harness/fakes.ts @@ -205,7 +205,7 @@ namespace fakes { /** * A fake `ts.CompilerHost` that leverages a virtual file system. */ - export class CompilerHost implements ts.CompilerHost, ts.SolutionBuilderHost { + export class CompilerHost implements ts.CompilerHost { public readonly sys: System; public readonly defaultLibLocation: string; public readonly outputs: documents.TextDocument[] = []; @@ -374,5 +374,41 @@ namespace fakes { return parsed; } } + + export class SolutionBuilderHost extends CompilerHost implements ts.SolutionBuilderHost { + diagnostics: ts.Diagnostic[] = []; + + reportDiagnostic(diagnostic: ts.Diagnostic) { + this.diagnostics.push(diagnostic); + } + + reportSolutionBuilderStatus(diagnostic: ts.Diagnostic) { + this.diagnostics.push(diagnostic); + } + + clearDiagnostics() { + this.diagnostics.length = 0; + } + + assertDiagnosticMessages(...expected: ts.DiagnosticMessage[]) { + const actual = this.diagnostics.slice(); + if (actual.length !== expected.length) { + assert.fail(actual, expected, `Diagnostic arrays did not match - got\r\n${actual.map(a => " " + a.messageText).join("\r\n")}\r\nexpected\r\n${expected.map(e => " " + e.message).join("\r\n")}`); + } + for (let i = 0; i < actual.length; i++) { + if (actual[i].code !== expected[i].code) { + assert.fail(actual[i].messageText, expected[i].message, `Mismatched error code - expected diagnostic ${i} "${actual[i].messageText}" to match ${expected[i].message}`); + } + } + } + + printDiagnostics(header = "== Diagnostics ==") { + const out = ts.createDiagnosticReporter(ts.sys); + ts.sys.write(header + "\r\n"); + for (const d of this.diagnostics) { + out(d); + } + } + } } diff --git a/src/testRunner/unittests/tsbuild.ts b/src/testRunner/unittests/tsbuild.ts index b65220588c..f8813ecd1a 100644 --- a/src/testRunner/unittests/tsbuild.ts +++ b/src/testRunner/unittests/tsbuild.ts @@ -1,15 +1,5 @@ namespace ts { let currentTime = 100; - let lastDiagnostics: Diagnostic[] = []; - const reportDiagnostic: DiagnosticReporter = diagnostic => lastDiagnostics.push(diagnostic); - const report = (message: DiagnosticMessage, ...args: string[]) => reportDiagnostic(createCompilerDiagnostic(message, ...args)); - const buildHost: BuildHost = { - error: report, - verbose: report, - message: report, - errorDiagnostic: d => reportDiagnostic(d) - }; - export namespace Sample1 { tick(); const projFs = loadProjectFromDisk("tests/projects/sample1"); @@ -21,12 +11,12 @@ namespace ts { describe("tsbuild - sanity check of clean build of 'sample1' project", () => { it("can build the sample project 'sample1' without error", () => { const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: false, verbose: false }); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: false, verbose: false }); - clearDiagnostics(); + host.clearDiagnostics(); builder.buildAllProjects(); - assertDiagnosticMessages(/*empty*/); + host.assertDiagnosticMessages(/*empty*/); // Check for outputs. Not an exhaustive list for (const output of allExpectedOutputs) { @@ -37,12 +27,11 @@ namespace ts { describe("tsbuild - dry builds", () => { it("doesn't write any files in a dry build", () => { - clearDiagnostics(); const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: true, force: false, verbose: false }); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: true, force: false, verbose: false }); builder.buildAllProjects(); - assertDiagnosticMessages(Diagnostics.A_non_dry_build_would_build_project_0, Diagnostics.A_non_dry_build_would_build_project_0, Diagnostics.A_non_dry_build_would_build_project_0); + host.assertDiagnosticMessages(Diagnostics.A_non_dry_build_would_build_project_0, Diagnostics.A_non_dry_build_would_build_project_0, Diagnostics.A_non_dry_build_would_build_project_0); // Check for outputs to not be written. Not an exhaustive list for (const output of allExpectedOutputs) { @@ -51,28 +40,26 @@ namespace ts { }); it("indicates that it would skip builds during a dry build", () => { - clearDiagnostics(); const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); + const host = new fakes.SolutionBuilderHost(fs); - let builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: false, verbose: false }); + let builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: false, verbose: false }); builder.buildAllProjects(); tick(); - clearDiagnostics(); - builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: true, force: false, verbose: false }); + host.clearDiagnostics(); + builder = createSolutionBuilder(host, ["/src/tests"], { dry: true, force: false, verbose: false }); builder.buildAllProjects(); - assertDiagnosticMessages(Diagnostics.Project_0_is_up_to_date, Diagnostics.Project_0_is_up_to_date, Diagnostics.Project_0_is_up_to_date); + host.assertDiagnosticMessages(Diagnostics.Project_0_is_up_to_date, Diagnostics.Project_0_is_up_to_date, Diagnostics.Project_0_is_up_to_date); }); }); describe("tsbuild - clean builds", () => { it("removes all files it built", () => { - clearDiagnostics(); const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); + const host = new fakes.SolutionBuilderHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: false, verbose: false }); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: false, verbose: false }); builder.buildAllProjects(); // Verify they exist for (const output of allExpectedOutputs) { @@ -91,9 +78,9 @@ namespace ts { describe("tsbuild - force builds", () => { it("always builds under --force", () => { const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); + const host = new fakes.SolutionBuilderHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: true, verbose: false }); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: true, verbose: false }); builder.buildAllProjects(); let currentTime = time(); checkOutputTimestamps(currentTime); @@ -116,14 +103,14 @@ namespace ts { describe("tsbuild - can detect when and what to rebuild", () => { const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: false, verbose: true }); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: false, verbose: true }); it("Builds the project", () => { - clearDiagnostics(); + host.clearDiagnostics(); builder.resetBuildContext(); builder.buildAllProjects(); - assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, + host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, Diagnostics.Building_project_0, Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, @@ -135,10 +122,10 @@ namespace ts { // All three projects are up to date it("Detects that all projects are up to date", () => { - clearDiagnostics(); + host.clearDiagnostics(); builder.resetBuildContext(); builder.buildAllProjects(); - assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, + host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2); @@ -147,12 +134,12 @@ namespace ts { // Update a file in the leaf node (tests), only it should rebuild the last one it("Only builds the leaf node project", () => { - clearDiagnostics(); + host.clearDiagnostics(); fs.writeFileSync("/src/tests/index.ts", "const m = 10;"); builder.resetBuildContext(); builder.buildAllProjects(); - assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, + host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, Diagnostics.Project_0_is_up_to_date_because_newest_input_1_is_older_than_oldest_output_2, Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, @@ -162,12 +149,12 @@ namespace ts { // Update a file in the parent (without affecting types), should get fast downstream builds it("Detects type-only changes in upstream projects", () => { - clearDiagnostics(); + host.clearDiagnostics(); replaceText(fs, "/src/core/index.ts", "HELLO WORLD", "WELCOME PLANET"); builder.resetBuildContext(); builder.buildAllProjects(); - assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, + host.assertDiagnosticMessages(Diagnostics.Projects_in_this_build_Colon_0, Diagnostics.Project_0_is_out_of_date_because_oldest_output_1_is_older_than_newest_input_2, Diagnostics.Building_project_0, Diagnostics.Project_0_is_up_to_date_with_d_ts_files_from_its_dependencies, @@ -180,15 +167,13 @@ namespace ts { describe("tsbuild - downstream-blocked compilations", () => { it("won't build downstream projects if upstream projects have errors", () => { const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: false, verbose: true }); - - clearDiagnostics(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: false, verbose: true }); // Induce an error in the middle project replaceText(fs, "/src/logic/index.ts", "c.multiply(10, 15)", `c.muitply()`); builder.buildAllProjects(); - assertDiagnosticMessages( + host.assertDiagnosticMessages( Diagnostics.Projects_in_this_build_Colon_0, Diagnostics.Project_0_is_out_of_date_because_output_file_1_does_not_exist, Diagnostics.Building_project_0, @@ -204,12 +189,11 @@ namespace ts { describe("tsbuild - project invalidation", () => { it("invalidates projects correctly", () => { const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/tests"], { dry: false, force: false, verbose: false }); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/tests"], { dry: false, force: false, verbose: false }); - clearDiagnostics(); builder.buildAllProjects(); - assertDiagnosticMessages(/*empty*/); + host.assertDiagnosticMessages(/*empty*/); // Update a timestamp in the middle project tick(); @@ -240,11 +224,10 @@ namespace ts { function verifyProjectWithResolveJsonModule(configFile: string, ...expectedDiagnosticMessages: DiagnosticMessage[]) { const fs = projFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, [configFile], { dry: false, force: false, verbose: false }); - clearDiagnostics(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, [configFile], { dry: false, force: false, verbose: false }); builder.buildAllProjects(); - assertDiagnosticMessages(...expectedDiagnosticMessages); + host.assertDiagnosticMessages(...expectedDiagnosticMessages); if (!expectedDiagnosticMessages.length) { // Check for outputs. Not an exhaustive list for (const output of allExpectedOutputs) { @@ -274,11 +257,11 @@ namespace ts { let fs: vfs.FileSystem | undefined; before(() => { fs = outFileFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/third"], { dry: false, force: false, verbose: false }); - clearDiagnostics(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/third"], { dry: false, force: false, verbose: false }); + host.clearDiagnostics(); builder.buildAllProjects(); - assertDiagnosticMessages(/*none*/); + host.assertDiagnosticMessages(/*none*/); }); after(() => { fs = undefined; @@ -293,25 +276,24 @@ namespace ts { describe("tsbuild - downstream prepend projects always get rebuilt", () => { it("", () => { const fs = outFileFs.shadow(); - const host = new fakes.CompilerHost(fs); - const builder = createSolutionBuilder(host, buildHost, ["/src/third"], { dry: false, force: false, verbose: false }); - clearDiagnostics(); + const host = new fakes.SolutionBuilderHost(fs); + const builder = createSolutionBuilder(host, ["/src/third"], { dry: false, force: false, verbose: false }); builder.buildAllProjects(); - assertDiagnosticMessages(/*none*/); + host.assertDiagnosticMessages(/*none*/); assert.equal(fs.statSync("src/third/thirdjs/output/third-output.js").mtimeMs, time(), "First build timestamp is correct"); tick(); replaceText(fs, "src/first/first_PART1.ts", "Hello", "Hola"); tick(); builder.resetBuildContext(); builder.buildAllProjects(); - assertDiagnosticMessages(/*none*/); + host.assertDiagnosticMessages(/*none*/); assert.equal(fs.statSync("src/third/thirdjs/output/third-output.js").mtimeMs, time(), "Second build timestamp is correct"); }); }); } describe("tsbuild - graph-ordering", () => { - let host: fakes.CompilerHost | undefined; + let host: fakes.SolutionBuilderHost | undefined; const deps: [string, string][] = [ ["A", "B"], ["B", "C"], @@ -324,7 +306,7 @@ namespace ts { before(() => { const fs = new vfs.FileSystem(false); - host = new fakes.CompilerHost(fs); + host = new fakes.SolutionBuilderHost(fs); writeProjects(fs, ["A", "B", "C", "D", "E", "F", "G"], deps); }); @@ -349,7 +331,7 @@ namespace ts { }); function checkGraphOrdering(rootNames: string[], expectedBuildSet: string[]) { - const builder = createSolutionBuilder(host!, buildHost, rootNames, { dry: true, force: false, verbose: false }); + const builder = createSolutionBuilder(host!, rootNames, { dry: true, force: false, verbose: false }); const projFileNames = rootNames.map(getProjectFileName); const graph = builder.getBuildGraph(projFileNames); @@ -404,30 +386,6 @@ namespace ts { fs.writeFileSync(path, newContent, "utf-8"); } - function assertDiagnosticMessages(...expected: DiagnosticMessage[]) { - const actual = lastDiagnostics.slice(); - if (actual.length !== expected.length) { - assert.fail(actual, expected, `Diagnostic arrays did not match - got\r\n${actual.map(a => " " + a.messageText).join("\r\n")}\r\nexpected\r\n${expected.map(e => " " + e.message).join("\r\n")}`); - } - for (let i = 0; i < actual.length; i++) { - if (actual[i].code !== expected[i].code) { - assert.fail(actual[i].messageText, expected[i].message, `Mismatched error code - expected diagnostic ${i} "${actual[i].messageText}" to match ${expected[i].message}`); - } - } - } - - function clearDiagnostics() { - lastDiagnostics = []; - } - - export function printDiagnostics(header = "== Diagnostics ==") { - const out = createDiagnosticReporter(sys); - sys.write(header + "\r\n"); - for (const d of lastDiagnostics) { - out(d); - } - } - function tick() { currentTime += 60_000; } diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts index 87224c0933..d43a24a0b9 100644 --- a/src/testRunner/unittests/tsbuildWatchMode.ts +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -2,15 +2,7 @@ namespace ts.tscWatch { export import libFile = TestFSWithWatch.libFile; function createSolutionBuilder(system: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { const host = createSolutionBuilderWithWatchHost(system); - const reportDiag = createDiagnosticReporter(system); - const report = (message: DiagnosticMessage, ...args: string[]) => reportDiag(createCompilerDiagnostic(message, ...args)); - const buildHost: BuildHost = { - error: report, - verbose: report, - message: report, - errorDiagnostic: d => reportDiag(d) - }; - return ts.createSolutionBuilder(host, buildHost, rootNames, defaultOptions || { dry: false, force: false, verbose: false, watch: true }); + return ts.createSolutionBuilder(host, rootNames, defaultOptions || { dry: false, force: false, verbose: false, watch: true }); } function createSolutionBuilderWithWatch(host: WatchedSystem, rootNames: ReadonlyArray, defaultOptions?: BuildOptions) { diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index 5c7af1a4b9..5be8660187 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -13,7 +13,7 @@ namespace ts { } let reportDiagnostic = createDiagnosticReporter(sys); - function updateReportDiagnostic(options: CompilerOptions) { + function updateReportDiagnostic(options?: CompilerOptions) { if (shouldBePretty(options)) { reportDiagnostic = createDiagnosticReporter(sys, /*pretty*/ true); } @@ -23,8 +23,8 @@ namespace ts { return !!sys.writeOutputIsTTY && sys.writeOutputIsTTY(); } - function shouldBePretty(options: CompilerOptions) { - if (typeof options.pretty === "undefined") { + function shouldBePretty(options?: CompilerOptions) { + if (!options || typeof options.pretty === "undefined") { return defaultIsPretty(); } return options.pretty; @@ -241,7 +241,7 @@ namespace ts { } // Update to pretty if host supports it - updateReportDiagnostic({}); + updateReportDiagnostic(); if (!sys.getModifiedTime || !sys.setModifiedTime || (buildOptions.clean && !sys.deleteFile)) { reportDiagnostic(createCompilerDiagnostic(Diagnostics.The_current_host_does_not_support_the_0_option, "--build")); @@ -274,16 +274,8 @@ namespace ts { addProject("."); } - const report = (message: DiagnosticMessage, ...args: string[]) => reportDiagnostic(createCompilerDiagnostic(message, ...args)); - const buildHost: BuildHost = { - error: report, - verbose: report, - message: report, - errorDiagnostic: d => reportDiagnostic(d) - }; - // TODO: change this to host if watch => watchHost otherwiue without wathc - const builder = createSolutionBuilder(createSolutionBuilderWithWatchHost(), buildHost, projects, buildOptions); + const builder = createSolutionBuilder(createSolutionBuilderWithWatchHost(sys, reportDiagnostic, createBuilderStatusReporter(sys, shouldBePretty()), createWatchStatusReporter()), projects, buildOptions); if (buildOptions.clean) { return builder.cleanAllProjects(); } @@ -300,7 +292,7 @@ namespace ts { const fileName = resolvePath(sys.getCurrentDirectory(), projectSpecification); const refPath = resolveProjectReferencePath(sys, { path: fileName }); if (!sys.fileExists(refPath)) { - return buildHost.error(Diagnostics.File_0_does_not_exist, fileName); + return reportDiagnostic(createCompilerDiagnostic(Diagnostics.File_0_does_not_exist, fileName)); } projects.push(refPath); } @@ -339,7 +331,7 @@ namespace ts { }; } - function createWatchStatusReporter(options: CompilerOptions) { + function createWatchStatusReporter(options?: CompilerOptions) { return ts.createWatchStatusReporter(sys, shouldBePretty(options)); } From 7960090bb68bda21857c8759cec637dd3fbedd36 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 20 Aug 2018 14:22:58 -0700 Subject: [PATCH 10/21] Add preserveWatchOutput option to build option and report starting compilation and file changes detected status --- src/compiler/tsbuild.ts | 9 +++++++++ src/tsc/tsc.ts | 8 +++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 7efecab608..2d66ddfe75 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -54,6 +54,7 @@ namespace ts { /*@internal*/ clean?: boolean; /*@internal*/ watch?: boolean; /*@internal*/ help?: boolean; + preserveWatchOutput?: boolean; } enum BuildResultFlags { @@ -465,6 +466,12 @@ namespace ts { host.reportSolutionBuilderStatus(createCompilerDiagnostic(message, ...args)); } + function reportWatchStatus(message: DiagnosticMessage, ...args: string[]) { + if (hostWithWatch.onWatchStatusChange) { + hostWithWatch.onWatchStatusChange(createCompilerDiagnostic(message, ...args), host.getNewLine(), { preserveWatchOutput: context.options.preserveWatchOutput }); + } + } + function startWatching() { const graph = getGlobalDependencyGraph()!; if (!graph.buildQueue) { @@ -500,6 +507,7 @@ namespace ts { } function invalidateProjectAndScheduleBuilds(resolved: ResolvedConfigFileName) { + reportWatchStatus(Diagnostics.File_change_detected_Starting_incremental_compilation); invalidateProject(resolved); if (!hostWithWatch.setTimeout) { return; @@ -1038,6 +1046,7 @@ namespace ts { } function buildAllProjects(): ExitStatus { + if (context.options.watch) { reportWatchStatus(Diagnostics.Starting_compilation_in_watch_mode); } const graph = getGlobalDependencyGraph(); if (graph === undefined) return ExitStatus.DiagnosticsPresent_OutputsSkipped; diff --git a/src/tsc/tsc.ts b/src/tsc/tsc.ts index 5be8660187..1685dfc344 100644 --- a/src/tsc/tsc.ts +++ b/src/tsc/tsc.ts @@ -211,7 +211,13 @@ namespace ts { category: Diagnostics.Command_line_Options, description: Diagnostics.Watch_input_files, type: "boolean" - } + }, + { + name: "preserveWatchOutput", + type: "boolean", + category: Diagnostics.Command_line_Options, + description: Diagnostics.Whether_to_keep_outdated_console_output_in_watch_mode_instead_of_clearing_the_screen, + }, ]; let buildOptionNameMap: OptionNameMap | undefined; const returnBuildOptionNameMap = () => (buildOptionNameMap || (buildOptionNameMap = createOptionNameMap(buildOpts))); From 0831edab3648dc0f8e2ecb9a7dd30d868d8de27b Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Mon, 20 Aug 2018 16:06:13 -0700 Subject: [PATCH 11/21] Keep only errors starting on new screen --- src/compiler/watch.ts | 8 +------- src/testRunner/unittests/tscWatchMode.ts | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 63904ffa43..c091ad5c30 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -27,12 +27,6 @@ namespace ts { }; } - /** @internal */ - export const nonClearingMessageCodes: number[] = [ - Diagnostics.Found_1_error_Watching_for_file_changes.code, - Diagnostics.Found_0_errors_Watching_for_file_changes.code - ]; - /** * @returns Whether the screen was cleared. */ @@ -41,7 +35,7 @@ namespace ts { !options.preserveWatchOutput && !options.extendedDiagnostics && !options.diagnostics && - !contains(nonClearingMessageCodes, diagnostic.code)) { + contains(screenStartingMessageCodes, diagnostic.code)) { system.clearScreen(); return true; } diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index 51c0d888f0..c881e9c14d 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -110,7 +110,7 @@ namespace ts.tscWatch { function assertWatchDiagnostic(diagnostic: Diagnostic) { const expected = getWatchDiagnosticWithoutDate(diagnostic); - if (!disableConsoleClears && !contains(nonClearingMessageCodes, diagnostic.code)) { + if (!disableConsoleClears && contains(screenStartingMessageCodes, diagnostic.code)) { assert.equal(host.screenClears[screenClears], index, `Expected screen clear at this diagnostic: ${expected}`); screenClears++; } From 4193846108b7bb7e7caadf7a9e62b487714c60e9 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 21 Aug 2018 13:52:04 -0700 Subject: [PATCH 12/21] Do not expose change in createCompilerHost --- src/compiler/program.ts | 7 ++++++- src/compiler/tsbuild.ts | 2 +- tests/baselines/reference/api/tsserverlibrary.d.ts | 2 +- tests/baselines/reference/api/typescript.d.ts | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 6ca29707cb..08bb472778 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -66,7 +66,12 @@ namespace ts { mtime: Date; } - export function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean, system = sys): CompilerHost { + export function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean): CompilerHost { + return createCompilerHostWorker(options, setParentNodes); + } + /*@internal*/ + // TODO(shkamat): update this after reworking ts build API + export function createCompilerHostWorker(options: CompilerOptions, setParentNodes?: boolean, system = sys): CompilerHost { const existingDirectories = createMap(); function getCanonicalFileName(fileName: string): string { diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 2d66ddfe75..22b2237d1a 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -413,7 +413,7 @@ namespace ts { } export function createSolutionBuilderHost(system = sys, reportDiagnostic?: DiagnosticReporter, reportSolutionBuilderStatus?: DiagnosticReporter) { - const host = createCompilerHost({}, /*setParentNodes*/ undefined, system) as SolutionBuilderHost; + const host = createCompilerHostWorker({}, /*setParentNodes*/ undefined, system) as SolutionBuilderHost; host.getModifiedTime = system.getModifiedTime ? path => system.getModifiedTime!(path) : () => undefined; host.setModifiedTime = system.setModifiedTime ? (path, date) => system.setModifiedTime!(path, date) : noop; host.deleteFile = system.deleteFile ? path => system.deleteFile!(path) : noop; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index dd5f3c7c65..3b7764cfd7 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -4122,7 +4122,7 @@ declare namespace ts { declare namespace ts { function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName?: string): string | undefined; function resolveTripleslashReference(moduleName: string, containingFile: string): string; - function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean, system?: System): CompilerHost; + function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean): CompilerHost; function getPreEmitDiagnostics(program: Program, sourceFile?: SourceFile, cancellationToken?: CancellationToken): Diagnostic[]; interface FormatDiagnosticsHost { getCurrentDirectory(): string; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 2938fc9bcc..e92e3d6e1d 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -4122,7 +4122,7 @@ declare namespace ts { declare namespace ts { function findConfigFile(searchPath: string, fileExists: (fileName: string) => boolean, configName?: string): string | undefined; function resolveTripleslashReference(moduleName: string, containingFile: string): string; - function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean, system?: System): CompilerHost; + function createCompilerHost(options: CompilerOptions, setParentNodes?: boolean): CompilerHost; function getPreEmitDiagnostics(program: Program, sourceFile?: SourceFile, cancellationToken?: CancellationToken): Diagnostic[]; interface FormatDiagnosticsHost { getCurrentDirectory(): string; From cec1b0a7179983eb880e806120ae25ac207029a9 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 21 Aug 2018 16:25:57 -0700 Subject: [PATCH 13/21] Report error summary from the queue. --- src/compiler/program.ts | 2 +- src/compiler/tsbuild.ts | 166 ++++++++++++++----- src/testRunner/unittests/tsbuild.ts | 4 +- src/testRunner/unittests/tsbuildWatchMode.ts | 80 ++++++++- src/testRunner/unittests/tscWatchMode.ts | 4 +- 5 files changed, 206 insertions(+), 50 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 08bb472778..d1168046b8 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -175,7 +175,7 @@ namespace ts { return getDirectoryPath(normalizePath(system.getExecutingFilePath())); } - const newLine = getNewLineCharacter(options); + const newLine = getNewLineCharacter(options, () => system.newLine); const realpath = system.realpath && ((path: string) => system.realpath!(path)); return { diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 22b2237d1a..5fdab16895 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -35,9 +35,11 @@ namespace ts { * Map from config file name to up-to-date status */ projectStatus: FileMap; + diagnostics?: FileMap; // TODO(shkamat): this should be really be diagnostics but thats for later time - invalidatedProjects: FileMap; - queuedProjects: FileMap; + invalidateProject(project: ResolvedConfigFileName, dependencyGraph: DependencyGraph | undefined): void; + getNextInvalidatedProject(): ResolvedConfigFileName | undefined; + pendingInvalidatedProjects(): boolean; missingRoots: Map; } @@ -194,6 +196,7 @@ namespace ts { hasKey(fileName: string): boolean; removeKey(fileName: string): void; getKeys(): string[]; + getSize(): number; } /** @@ -209,7 +212,8 @@ namespace ts { getValueOrUndefined, removeKey, getKeys, - hasKey + hasKey, + getSize }; function getKeys(): string[] { @@ -242,6 +246,10 @@ namespace ts { const f = normalizePath(fileName); return lookup.get(f); } + + function getSize() { + return lookup.size; + } } function createDependencyMapper() { @@ -375,18 +383,64 @@ namespace ts { } export function createBuildContext(options: BuildOptions): BuildContext { - const invalidatedProjects = createFileMap(); - const queuedProjects = createFileMap(); + const invalidatedProjectQueue = [] as ResolvedConfigFileName[]; + let nextIndex = 0; + const projectPendingBuild = createFileMap(); const missingRoots = createMap(); + const diagnostics = options.watch ? createFileMap() : undefined; return { options, projectStatus: createFileMap(), + diagnostics, unchangedOutputs: createFileMap(), - invalidatedProjects, - missingRoots, - queuedProjects + invalidateProject, + getNextInvalidatedProject, + pendingInvalidatedProjects, + missingRoots }; + + function invalidateProject(proj: ResolvedConfigFileName, dependancyGraph: DependencyGraph | undefined) { + if (!projectPendingBuild.hasKey(proj)) { + addProjToQueue(proj); + if (dependancyGraph) { + queueBuildForDownstreamReferences(proj, dependancyGraph); + } + } + } + + function addProjToQueue(proj: ResolvedConfigFileName) { + projectPendingBuild.setValue(proj, true); + invalidatedProjectQueue.push(proj); + } + + function getNextInvalidatedProject() { + if (nextIndex < invalidatedProjectQueue.length) { + const proj = invalidatedProjectQueue[nextIndex]; + nextIndex++; + projectPendingBuild.removeKey(proj); + if (!projectPendingBuild.getSize()) { + invalidatedProjectQueue.length = 0; + } + return proj; + } + } + + function pendingInvalidatedProjects() { + return !!projectPendingBuild.getSize(); + } + + // Mark all downstream projects of this one needing to be built "later" + function queueBuildForDownstreamReferences(root: ResolvedConfigFileName, dependancyGraph: DependencyGraph) { + const deps = dependancyGraph.dependencyMap.getReferencesTo(root); + for (const ref of deps) { + // Can skip circular references + if (!projectPendingBuild.hasKey(ref)) { + addProjToQueue(ref); + queueBuildForDownstreamReferences(ref, dependancyGraph); + } + } + } } export interface SolutionBuilderHost extends CompilerHost { @@ -443,6 +497,7 @@ namespace ts { const hostWithWatch = host as SolutionBuilderWithWatchHost; const configFileCache = createConfigFileCache(host); let context = createBuildContext(defaultOptions); + let timerToBuildInvalidatedProject: any; const existingWatchersForWildcards = createMap(); return { @@ -454,8 +509,7 @@ namespace ts { getBuildGraph, invalidateProject, - buildInvalidatedProjects, - buildDependentInvalidatedProjects, + buildInvalidatedProject, resolveProjectName, @@ -466,7 +520,19 @@ namespace ts { host.reportSolutionBuilderStatus(createCompilerDiagnostic(message, ...args)); } - function reportWatchStatus(message: DiagnosticMessage, ...args: string[]) { + function storeErrors(proj: ResolvedConfigFileName, diagnostics: ReadonlyArray) { + if (context.options.watch) { + storeErrorSummary(proj, diagnostics.filter(diagnostic => diagnostic.category === DiagnosticCategory.Error).length); + } + } + + function storeErrorSummary(proj: ResolvedConfigFileName, errorCount: number) { + if (context.options.watch) { + context.diagnostics!.setValue(proj, errorCount); + } + } + + function reportWatchStatus(message: DiagnosticMessage, ...args: (string | number | undefined)[]) { if (hostWithWatch.onWatchStatusChange) { hostWithWatch.onWatchStatusChange(createCompilerDiagnostic(message, ...args), host.getNewLine(), { preserveWatchOutput: context.options.preserveWatchOutput }); } @@ -506,15 +572,12 @@ namespace ts { } } - function invalidateProjectAndScheduleBuilds(resolved: ResolvedConfigFileName) { - reportWatchStatus(Diagnostics.File_change_detected_Starting_incremental_compilation); - invalidateProject(resolved); - if (!hostWithWatch.setTimeout) { - return; - } - hostWithWatch.setTimeout(buildInvalidatedProjects, 100); - hostWithWatch.setTimeout(buildDependentInvalidatedProjects, 3000); - } + } + + function invalidateProjectAndScheduleBuilds(resolved: ResolvedConfigFileName) { + reportWatchStatus(Diagnostics.File_change_detected_Starting_incremental_compilation); + invalidateProject(resolved); + scheduleBuildInvalidatedProject(); } function resetBuildContext(opts = defaultOptions) { @@ -725,33 +788,44 @@ namespace ts { } configFileCache.removeKey(resolved); - context.invalidatedProjects.setValue(resolved, true); context.projectStatus.removeKey(resolved); - - const graph = getGlobalDependencyGraph()!; - if (graph) { - queueBuildForDownstreamReferences(resolved); + if (context.options.watch) { + context.diagnostics!.removeKey(resolved); } - // Mark all downstream projects of this one needing to be built "later" - function queueBuildForDownstreamReferences(root: ResolvedConfigFileName) { - const deps = graph.dependencyMap.getReferencesTo(root); - for (const ref of deps) { - // Can skip circular references - if (!context.queuedProjects.hasKey(ref)) { - context.queuedProjects.setValue(ref, true); - queueBuildForDownstreamReferences(ref); - } + context.invalidateProject(resolved, getGlobalDependencyGraph()); + } + + function scheduleBuildInvalidatedProject() { + if (!hostWithWatch.setTimeout || !hostWithWatch.clearTimeout) { + return; + } + if (timerToBuildInvalidatedProject) { + hostWithWatch.clearTimeout(timerToBuildInvalidatedProject); + } + timerToBuildInvalidatedProject = hostWithWatch.setTimeout(buildInvalidatedProject, 250); + } + + function buildInvalidatedProject() { + timerToBuildInvalidatedProject = undefined; + const buildProject = context.getNextInvalidatedProject(); + buildSomeProjects(p => p === buildProject); + if (context.pendingInvalidatedProjects()) { + if (!timerToBuildInvalidatedProject) { + scheduleBuildInvalidatedProject(); } } + else { + reportErrorSummary(); + } } - function buildInvalidatedProjects() { - buildSomeProjects(p => context.invalidatedProjects.hasKey(p)); - } - - function buildDependentInvalidatedProjects() { - buildSomeProjects(p => context.queuedProjects.hasKey(p)); + function reportErrorSummary() { + if (context.options.watch) { + let errorCount = 0; + context.diagnostics!.getKeys().forEach(resolved => errorCount += context.diagnostics!.getValue(resolved)); + reportWatchStatus(errorCount === 1 ? Diagnostics.Found_1_error_Watching_for_file_changes : Diagnostics.Found_0_errors_Watching_for_file_changes, errorCount); + } } function buildSomeProjects(predicate: (projName: ResolvedConfigFileName) => boolean) { @@ -808,6 +882,7 @@ namespace ts { if (temporaryMarks[projPath]) { if (!inCircularContext) { hadError = true; + // TODO(shkamat): Account for this error reportStatus(Diagnostics.Project_references_may_not_form_a_circular_graph_Cycle_detected_Colon_0, circularityReportStack.join("\r\n")); return; } @@ -853,6 +928,7 @@ namespace ts { if (!configFile) { // Failed to read the config file resultFlags |= BuildResultFlags.ConfigFileErrors; + storeErrorSummary(proj, 1); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Config file errors" }); return resultFlags; } @@ -880,6 +956,7 @@ namespace ts { for (const diag of syntaxDiagnostics) { host.reportDiagnostic(diag); } + storeErrors(proj, syntaxDiagnostics); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Syntactic errors" }); return resultFlags; } @@ -892,6 +969,7 @@ namespace ts { for (const diag of declDiagnostics) { host.reportDiagnostic(diag); } + storeErrors(proj, declDiagnostics); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Declaration file errors" }); return resultFlags; } @@ -904,6 +982,7 @@ namespace ts { for (const diag of semanticDiagnostics) { host.reportDiagnostic(diag); } + storeErrors(proj, semanticDiagnostics); context.projectStatus.setValue(proj, { type: UpToDateStatusType.Unbuildable, reason: "Semantic errors" }); return resultFlags; } @@ -1029,6 +1108,7 @@ namespace ts { if (host.fileExists(fullPathWithTsconfig)) { return fullPathWithTsconfig as ResolvedConfigFileName; } + // TODO(shkamat): right now this is accounted as 1 error in config file, but we need to do better host.reportDiagnostic(createCompilerDiagnostic(Diagnostics.File_0_not_found, relName(fullPath))); return undefined; } @@ -1048,7 +1128,10 @@ namespace ts { function buildAllProjects(): ExitStatus { if (context.options.watch) { reportWatchStatus(Diagnostics.Starting_compilation_in_watch_mode); } const graph = getGlobalDependencyGraph(); - if (graph === undefined) return ExitStatus.DiagnosticsPresent_OutputsSkipped; + if (graph === undefined) { + reportErrorSummary(); + return ExitStatus.DiagnosticsPresent_OutputsSkipped; + } const queue = graph.buildQueue; reportBuildQueue(graph); @@ -1092,6 +1175,7 @@ namespace ts { const buildResult = buildSingleProject(next); anyFailed = anyFailed || !!(buildResult & BuildResultFlags.AnyErrors); } + reportErrorSummary(); return anyFailed ? ExitStatus.DiagnosticsPresent_OutputsSkipped : ExitStatus.Success; } diff --git a/src/testRunner/unittests/tsbuild.ts b/src/testRunner/unittests/tsbuild.ts index f8813ecd1a..b7d708446c 100644 --- a/src/testRunner/unittests/tsbuild.ts +++ b/src/testRunner/unittests/tsbuild.ts @@ -205,14 +205,14 @@ namespace ts { // Rebuild this project tick(); builder.invalidateProject("/src/logic"); - builder.buildInvalidatedProjects(); + builder.buildInvalidatedProject(); // The file should be updated assert.equal(fs.statSync("/src/logic/index.js").mtimeMs, time(), "JS file should have been rebuilt"); assert.isBelow(fs.statSync("/src/tests/index.js").mtimeMs, time(), "Downstream JS file should *not* have been rebuilt"); // Build downstream projects should update 'tests', but not 'core' tick(); - builder.buildDependentInvalidatedProjects(); + builder.buildInvalidatedProject(); assert.equal(fs.statSync("/src/tests/index.js").mtimeMs, time(), "Downstream JS file should have been rebuilt"); assert.isBelow(fs.statSync("/src/core/index.js").mtimeMs, time(), "Upstream JS file should not have been rebuilt"); }); diff --git a/src/testRunner/unittests/tsbuildWatchMode.ts b/src/testRunner/unittests/tsbuildWatchMode.ts index d43a24a0b9..d1405fa9e2 100644 --- a/src/testRunner/unittests/tsbuildWatchMode.ts +++ b/src/testRunner/unittests/tsbuildWatchMode.ts @@ -25,12 +25,18 @@ namespace ts.tscWatch { /** [tsconfig, index] | [tsconfig, index, anotherModule, someDecl] */ type SubProjectFiles = [ReadonlyFile, ReadonlyFile] | [ReadonlyFile, ReadonlyFile, ReadonlyFile, ReadonlyFile]; const root = Harness.IO.getWorkspaceRoot(); + + function projectFilePath(subProject: SubProject, baseFileName: string) { + return `${projectsLocation}/${project}/${subProject}/${baseFileName.toLowerCase()}`; + } + function projectFile(subProject: SubProject, baseFileName: string): File { return { - path: `${projectsLocation}/${project}/${subProject}/${baseFileName.toLowerCase()}`, + path: projectFilePath(subProject, baseFileName), content: Harness.IO.readFile(`${root}/tests/projects/${project}/${subProject}/${baseFileName}`)! }; } + function subProjectFiles(subProject: SubProject, anotherModuleAndSomeDecl?: true): SubProjectFiles { const tsconfig = projectFile(subProject, "tsconfig.json"); const index = projectFile(subProject, "index.ts"); @@ -42,20 +48,86 @@ namespace ts.tscWatch { return [tsconfig, index, anotherModule, someDecl]; } + function getOutputFileNames(subProject: SubProject, baseFileNameWithoutExtension: string) { + const file = projectFilePath(subProject, baseFileNameWithoutExtension); + return [`${file}.js`, `${file}.d.ts`]; + } + + type OutputFileStamp = [string, Date | undefined]; + function getOutputStamps(host: WatchedSystem, subProject: SubProject, baseFileNameWithoutExtension: string): OutputFileStamp[] { + return getOutputFileNames(subProject, baseFileNameWithoutExtension).map(f => [f, host.getModifiedTime(f)] as OutputFileStamp); + } + + function getOutputFileStamps(host: WatchedSystem): OutputFileStamp[] { + return [ + ...getOutputStamps(host, SubProject.core, "anotherModule"), + ...getOutputStamps(host, SubProject.core, "index"), + ...getOutputStamps(host, SubProject.logic, "index"), + ...getOutputStamps(host, SubProject.tests, "index"), + ]; + } + + function verifyChangedFiles(actualStamps: OutputFileStamp[], oldTimeStamps: OutputFileStamp[], changedFiles: string[]) { + for (let i = 0; i < oldTimeStamps.length; i++) { + const actual = actualStamps[i]; + const old = oldTimeStamps[i]; + if (contains(changedFiles, actual[0])) { + assert.isTrue((actual[1] || 0) > (old[1] || 0), `${actual[0]} expected to written`); + } + else { + assert.equal(actual[1], old[1], `${actual[0]} expected to not change`); + } + } + } + const core = subProjectFiles(SubProject.core, /*anotherModuleAndSomeDecl*/ true); const logic = subProjectFiles(SubProject.logic); const tests = subProjectFiles(SubProject.tests); const ui = subProjectFiles(SubProject.ui); const allFiles: ReadonlyArray = [libFile, ...core, ...logic, ...tests, ...ui]; const testProjectExpectedWatchedFiles = [core[0], core[1], core[2], ...logic, ...tests].map(f => f.path); - it("creates solution in watch mode", () => { + + function createSolutionInWatchMode() { const host = createWatchedSystem(allFiles, { currentDirectory: projectsLocation }); - const originalWrite = host.write; - host.write = s => { console.log(s); originalWrite.call(host, s); }; createSolutionBuilderWithWatch(host, [`${project}/${SubProject.tests}`]); checkWatchedFiles(host, testProjectExpectedWatchedFiles); checkWatchedDirectories(host, emptyArray, /*recursive*/ false); checkWatchedDirectories(host, emptyArray, /*recursive*/ true); // TODO: #26524 + checkOutputErrorsInitial(host, emptyArray); + const outputFileStamps = getOutputFileStamps(host); + for (const stamp of outputFileStamps) { + assert.isDefined(stamp[1], `${stamp[0]} expected to be present`); + } + return { host, outputFileStamps }; + } + it("creates solution in watch mode", () => { + createSolutionInWatchMode(); }); + + it("change builds changes and reports found errors message", () => { + const { host, outputFileStamps } = createSolutionInWatchMode(); + host.writeFile(core[1].path, `${core[1].content} +export class someClass { }`); + host.checkTimeoutQueueLengthAndRun(1); // Builds core + const changedCore = getOutputFileStamps(host); + verifyChangedFiles(changedCore, outputFileStamps, [ + ...getOutputFileNames(SubProject.core, "anotherModule"), // This should not be written really + ...getOutputFileNames(SubProject.core, "index") + ]); + host.checkTimeoutQueueLengthAndRun(1); // Builds tests + const changedTests = getOutputFileStamps(host); + verifyChangedFiles(changedTests, changedCore, [ + ...getOutputFileNames(SubProject.tests, "index") // Again these need not be written + ]); + host.checkTimeoutQueueLengthAndRun(1); // Builds logic + const changedLogic = getOutputFileStamps(host); + verifyChangedFiles(changedLogic, changedTests, [ + ...getOutputFileNames(SubProject.logic, "index") // Again these need not be written + ]); + host.checkTimeoutQueueLength(0); + checkOutputErrorsIncremental(host, emptyArray); + }); + + // TODO: write tests reporting errors but that will have more involved work since file }); } diff --git a/src/testRunner/unittests/tscWatchMode.ts b/src/testRunner/unittests/tscWatchMode.ts index c881e9c14d..da1c4fd0d7 100644 --- a/src/testRunner/unittests/tscWatchMode.ts +++ b/src/testRunner/unittests/tscWatchMode.ts @@ -136,7 +136,7 @@ namespace ts.tscWatch { : createCompilerDiagnostic(Diagnostics.Found_0_errors_Watching_for_file_changes, errors.length); } - function checkOutputErrorsInitial(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeErrors?: string[]) { + export function checkOutputErrorsInitial(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeErrors?: string[]) { checkOutputErrors( host, /*logsBeforeWatchDiagnostic*/ undefined, @@ -147,7 +147,7 @@ namespace ts.tscWatch { createErrorsFoundCompilerDiagnostic(errors)); } - function checkOutputErrorsIncremental(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeWatchDiagnostic?: string[], logsBeforeErrors?: string[]) { + export function checkOutputErrorsIncremental(host: WatchedSystem, errors: ReadonlyArray, disableConsoleClears?: boolean, logsBeforeWatchDiagnostic?: string[], logsBeforeErrors?: string[]) { checkOutputErrors( host, logsBeforeWatchDiagnostic, From d7580755974df4fe3d64af54462b855d3f9f8f43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Thu, 9 Aug 2018 16:40:11 +0800 Subject: [PATCH 14/21] add special check for parameter initializer lookup if targeting es2015+ --- src/compiler/checker.ts | 16 ++-- ...InitializersForwardReferencing1.errors.txt | 45 ++++++++++++ ...arameterInitializersForwardReferencing1.js | 61 ++++++++++++++++ ...terInitializersForwardReferencing1.symbols | 67 +++++++++++++++++ ...meterInitializersForwardReferencing1.types | 73 +++++++++++++++++++ ...ializersForwardReferencing1_es6.errors.txt | 36 +++++++++ ...eterInitializersForwardReferencing1_es6.js | 51 +++++++++++++ ...nitializersForwardReferencing1_es6.symbols | 67 +++++++++++++++++ ...rInitializersForwardReferencing1_es6.types | 73 +++++++++++++++++++ ...arameterInitializersForwardReferencing1.ts | 27 +++++++ ...eterInitializersForwardReferencing1_es6.ts | 29 ++++++++ 11 files changed, 540 insertions(+), 5 deletions(-) create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1.js create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1.symbols create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1.types create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols create mode 100644 tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types create mode 100644 tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts create mode 100644 tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 51eeb0fdb4..36c6d71f67 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1197,16 +1197,22 @@ namespace ts { : false; } if (meaning & SymbolFlags.Value && result.flags & SymbolFlags.FunctionScopedVariable) { - // parameters are visible only inside function body, parameter list and return type - // technically for parameter list case here we might mix parameters and variables declared in function, - // however it is detected separately when checking initializers of parameters - // to make sure that they reference no variables declared after them. - useResult = + // parameter initializer will lookup as normal variable scope when targeting es2015+ + if (compilerOptions.target && compilerOptions.target >= ScriptTarget.ES2015 && isParameter(lastLocation) && lastLocation.initializer === originalLocation && result.valueDeclaration !== lastLocation) { + useResult = false; + } + else { + // parameters are visible only inside function body, parameter list and return type + // technically for parameter list case here we might mix parameters and variables declared in function, + // however it is detected separately when checking initializers of parameters + // to make sure that they reference no variables declared after them. + useResult = lastLocation.kind === SyntaxKind.Parameter || ( lastLocation === (location).type && !!findAncestor(result.valueDeclaration, isParameter) ); + } } } else if (location.kind === SyntaxKind.ConditionalType) { diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt b/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt new file mode 100644 index 0000000000..9879ec320c --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt @@ -0,0 +1,45 @@ +tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(3,20): error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. +tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(8,27): error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. +tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(13,20): error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. +tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(21,18): error TS2372: Parameter 'a' cannot be referenced in its initializer. +tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(25,22): error TS2372: Parameter 'async' cannot be referenced in its initializer. + + +==== tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts (5 errors) ==== + let foo: string = ""; + + function f1 (bar = foo) { // unexpected compiler error; works at runtime + ~~~ +!!! error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. + var foo: number = 2; + return bar; // returns 1 + } + + function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime + ~~~ +!!! error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. + var foo: number = 2; + return bar(); // returns 1 + } + + function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime + ~~~ +!!! error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. + return bar; + } + + function f4 (foo, bar = foo) { + return bar + } + + function f5 (a = a) { + ~ +!!! error TS2372: Parameter 'a' cannot be referenced in its initializer. + return a + } + + function f6 (async = async) { + ~~~~~ +!!! error TS2372: Parameter 'async' cannot be referenced in its initializer. + return async + } \ No newline at end of file diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.js b/tests/baselines/reference/parameterInitializersForwardReferencing1.js new file mode 100644 index 0000000000..43f303a94c --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.js @@ -0,0 +1,61 @@ +//// [parameterInitializersForwardReferencing1.ts] +let foo: string = ""; + +function f1 (bar = foo) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar; // returns 1 +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar(); // returns 1 +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime + return bar; +} + +function f4 (foo, bar = foo) { + return bar +} + +function f5 (a = a) { + return a +} + +function f6 (async = async) { + return async +} + +//// [parameterInitializersForwardReferencing1.js] +var foo = ""; +function f1(bar) { + if (bar === void 0) { bar = foo; } + var foo = 2; + return bar; // returns 1 +} +function f2(bar) { + if (bar === void 0) { bar = function (baz) { + if (baz === void 0) { baz = foo; } + return baz; + }; } + var foo = 2; + return bar(); // returns 1 +} +function f3(bar, foo) { + if (bar === void 0) { bar = foo; } + if (foo === void 0) { foo = 2; } + return bar; +} +function f4(foo, bar) { + if (bar === void 0) { bar = foo; } + return bar; +} +function f5(a) { + if (a === void 0) { a = a; } + return a; +} +function f6(async) { + if (async === void 0) { async = async; } + return async; +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols b/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols new file mode 100644 index 0000000000..6860e39fd8 --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols @@ -0,0 +1,67 @@ +=== tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts === +let foo: string = ""; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 0, 3)) + +function f1 (bar = foo) { // unexpected compiler error; works at runtime +>f1 : Symbol(f1, Decl(parameterInitializersForwardReferencing1.ts, 0, 21)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 2, 13)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 3, 7)) + + var foo: number = 2; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 3, 7)) + + return bar; // returns 1 +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 2, 13)) +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime +>f2 : Symbol(f2, Decl(parameterInitializersForwardReferencing1.ts, 5, 1)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 7, 13)) +>baz : Symbol(baz, Decl(parameterInitializersForwardReferencing1.ts, 7, 20)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 8, 7)) +>baz : Symbol(baz, Decl(parameterInitializersForwardReferencing1.ts, 7, 20)) + + var foo: number = 2; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 8, 7)) + + return bar(); // returns 1 +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 7, 13)) +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime +>f3 : Symbol(f3, Decl(parameterInitializersForwardReferencing1.ts, 10, 1)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 12, 13)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 12, 23)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 12, 23)) + + return bar; +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 12, 13)) +} + +function f4 (foo, bar = foo) { +>f4 : Symbol(f4, Decl(parameterInitializersForwardReferencing1.ts, 14, 1)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 16, 13)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 16, 17)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 16, 13)) + + return bar +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 16, 17)) +} + +function f5 (a = a) { +>f5 : Symbol(f5, Decl(parameterInitializersForwardReferencing1.ts, 18, 1)) +>a : Symbol(a, Decl(parameterInitializersForwardReferencing1.ts, 20, 13)) +>a : Symbol(a, Decl(parameterInitializersForwardReferencing1.ts, 20, 13)) + + return a +>a : Symbol(a, Decl(parameterInitializersForwardReferencing1.ts, 20, 13)) +} + +function f6 (async = async) { +>f6 : Symbol(f6, Decl(parameterInitializersForwardReferencing1.ts, 22, 1)) +>async : Symbol(async, Decl(parameterInitializersForwardReferencing1.ts, 24, 13)) +>async : Symbol(async, Decl(parameterInitializersForwardReferencing1.ts, 24, 13)) + + return async +>async : Symbol(async, Decl(parameterInitializersForwardReferencing1.ts, 24, 13)) +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.types b/tests/baselines/reference/parameterInitializersForwardReferencing1.types new file mode 100644 index 0000000000..c33c227a37 --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.types @@ -0,0 +1,73 @@ +=== tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts === +let foo: string = ""; +>foo : string +>"" : "" + +function f1 (bar = foo) { // unexpected compiler error; works at runtime +>f1 : (bar?: number) => number +>bar : number +>foo : number + + var foo: number = 2; +>foo : number +>2 : 2 + + return bar; // returns 1 +>bar : number +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime +>f2 : (bar?: (baz?: number) => number) => number +>bar : (baz?: number) => number +>(baz = foo) => baz : (baz?: number) => number +>baz : number +>foo : number +>baz : number + + var foo: number = 2; +>foo : number +>2 : 2 + + return bar(); // returns 1 +>bar() : number +>bar : (baz?: number) => number +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime +>f3 : (bar?: number, foo?: number) => number +>bar : number +>foo : number +>foo : number +>2 : 2 + + return bar; +>bar : number +} + +function f4 (foo, bar = foo) { +>f4 : (foo: any, bar?: any) => any +>foo : any +>bar : any +>foo : any + + return bar +>bar : any +} + +function f5 (a = a) { +>f5 : (a?: any) => any +>a : any +>a : any + + return a +>a : any +} + +function f6 (async = async) { +>f6 : (async?: any) => any +>async : any +>async : any + + return async +>async : any +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt new file mode 100644 index 0000000000..cffa83707e --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt @@ -0,0 +1,36 @@ +tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts(21,18): error TS2372: Parameter 'a' cannot be referenced in its initializer. +tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts(25,22): error TS2372: Parameter 'async' cannot be referenced in its initializer. + + +==== tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts (2 errors) ==== + let foo: string = ""; + + function f1 (bar = foo) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar; // returns 1 + } + + function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime + var fooo: number = 2; + return bar(); // returns 1 + } + + function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime + return bar; + } + + function f4 (foo, bar = foo) { + return bar + } + + function f5 (a = a) { + ~ +!!! error TS2372: Parameter 'a' cannot be referenced in its initializer. + return a + } + + function f6 (async = async) { + ~~~~~ +!!! error TS2372: Parameter 'async' cannot be referenced in its initializer. + return async + } \ No newline at end of file diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js new file mode 100644 index 0000000000..add0837e31 --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js @@ -0,0 +1,51 @@ +//// [parameterInitializersForwardReferencing1_es6.ts] +let foo: string = ""; + +function f1 (bar = foo) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar; // returns 1 +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime + var fooo: number = 2; + return bar(); // returns 1 +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime + return bar; +} + +function f4 (foo, bar = foo) { + return bar +} + +function f5 (a = a) { + return a +} + +function f6 (async = async) { + return async +} + +//// [parameterInitializersForwardReferencing1_es6.js] +let foo = ""; +function f1(bar = foo) { + var foo = 2; + return bar; // returns 1 +} +function f2(bar = (baz = foo) => baz) { + var fooo = 2; + return bar(); // returns 1 +} +function f3(bar = foo, foo = 2) { + return bar; +} +function f4(foo, bar = foo) { + return bar; +} +function f5(a = a) { + return a; +} +function f6(async = async) { + return async; +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols new file mode 100644 index 0000000000..c38a392c4d --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols @@ -0,0 +1,67 @@ +=== tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts === +let foo: string = ""; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) + +function f1 (bar = foo) { // unexpected compiler error; works at runtime +>f1 : Symbol(f1, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 21)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 2, 13)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) + + var foo: number = 2; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 3, 7)) + + return bar; // returns 1 +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 2, 13)) +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime +>f2 : Symbol(f2, Decl(parameterInitializersForwardReferencing1_es6.ts, 5, 1)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 7, 13)) +>baz : Symbol(baz, Decl(parameterInitializersForwardReferencing1_es6.ts, 7, 20)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) +>baz : Symbol(baz, Decl(parameterInitializersForwardReferencing1_es6.ts, 7, 20)) + + var fooo: number = 2; +>fooo : Symbol(fooo, Decl(parameterInitializersForwardReferencing1_es6.ts, 8, 7)) + + return bar(); // returns 1 +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 7, 13)) +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime +>f3 : Symbol(f3, Decl(parameterInitializersForwardReferencing1_es6.ts, 10, 1)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 12, 13)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 12, 23)) + + return bar; +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 12, 13)) +} + +function f4 (foo, bar = foo) { +>f4 : Symbol(f4, Decl(parameterInitializersForwardReferencing1_es6.ts, 14, 1)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 16, 13)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 16, 17)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) + + return bar +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 16, 17)) +} + +function f5 (a = a) { +>f5 : Symbol(f5, Decl(parameterInitializersForwardReferencing1_es6.ts, 18, 1)) +>a : Symbol(a, Decl(parameterInitializersForwardReferencing1_es6.ts, 20, 13)) +>a : Symbol(a, Decl(parameterInitializersForwardReferencing1_es6.ts, 20, 13)) + + return a +>a : Symbol(a, Decl(parameterInitializersForwardReferencing1_es6.ts, 20, 13)) +} + +function f6 (async = async) { +>f6 : Symbol(f6, Decl(parameterInitializersForwardReferencing1_es6.ts, 22, 1)) +>async : Symbol(async, Decl(parameterInitializersForwardReferencing1_es6.ts, 24, 13)) +>async : Symbol(async, Decl(parameterInitializersForwardReferencing1_es6.ts, 24, 13)) + + return async +>async : Symbol(async, Decl(parameterInitializersForwardReferencing1_es6.ts, 24, 13)) +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types new file mode 100644 index 0000000000..bd17f48233 --- /dev/null +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types @@ -0,0 +1,73 @@ +=== tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts === +let foo: string = ""; +>foo : string +>"" : "" + +function f1 (bar = foo) { // unexpected compiler error; works at runtime +>f1 : (bar?: string) => string +>bar : string +>foo : string + + var foo: number = 2; +>foo : number +>2 : 2 + + return bar; // returns 1 +>bar : string +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime +>f2 : (bar?: (baz?: string) => string) => string +>bar : (baz?: string) => string +>(baz = foo) => baz : (baz?: string) => string +>baz : string +>foo : string +>baz : string + + var fooo: number = 2; +>fooo : number +>2 : 2 + + return bar(); // returns 1 +>bar() : string +>bar : (baz?: string) => string +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime +>f3 : (bar?: string, foo?: number) => string +>bar : string +>foo : string +>foo : number +>2 : 2 + + return bar; +>bar : string +} + +function f4 (foo, bar = foo) { +>f4 : (foo: any, bar?: string) => string +>foo : any +>bar : string +>foo : string + + return bar +>bar : string +} + +function f5 (a = a) { +>f5 : (a?: any) => any +>a : any +>a : any + + return a +>a : any +} + +function f6 (async = async) { +>f6 : (async?: any) => any +>async : any +>async : any + + return async +>async : any +} diff --git a/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts new file mode 100644 index 0000000000..8492a05385 --- /dev/null +++ b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts @@ -0,0 +1,27 @@ +let foo: string = ""; + +function f1 (bar = foo) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar; // returns 1 +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar(); // returns 1 +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime + return bar; +} + +function f4 (foo, bar = foo) { + return bar +} + +function f5 (a = a) { + return a +} + +function f6 (async = async) { + return async +} \ No newline at end of file diff --git a/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts new file mode 100644 index 0000000000..dfd6b8d4e1 --- /dev/null +++ b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts @@ -0,0 +1,29 @@ +// @target: es2015 + +let foo: string = ""; + +function f1 (bar = foo) { // unexpected compiler error; works at runtime + var foo: number = 2; + return bar; // returns 1 +} + +function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime + var fooo: number = 2; + return bar(); // returns 1 +} + +function f3 (bar = foo, foo = 2) { // correct compiler error, error at runtime + return bar; +} + +function f4 (foo, bar = foo) { + return bar +} + +function f5 (a = a) { + return a +} + +function f6 (async = async) { + return async +} \ No newline at end of file From 8869f39c25f8e9e7660ada38f0d33fcea714e49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Tue, 28 Aug 2018 16:41:26 +0800 Subject: [PATCH 15/21] accept more case --- src/compiler/checker.ts | 6 +++--- ...eterInitializersForwardReferencing1.errors.txt | 13 +++++++++++-- .../parameterInitializersForwardReferencing1.js | 11 ++++++++++- ...rameterInitializersForwardReferencing1.symbols | 10 ++++++++++ ...parameterInitializersForwardReferencing1.types | 11 +++++++++++ ...InitializersForwardReferencing1_es6.errors.txt | 9 +++++++-- ...arameterInitializersForwardReferencing1_es6.js | 14 +++++++++++--- ...terInitializersForwardReferencing1_es6.symbols | 14 ++++++++++++-- ...meterInitializersForwardReferencing1_es6.types | 15 +++++++++++++-- .../parameterInitializersForwardReferencing1.ts | 6 +++++- ...arameterInitializersForwardReferencing1_es6.ts | 8 ++++++-- 11 files changed, 99 insertions(+), 18 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 36c6d71f67..063044eda6 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1196,12 +1196,12 @@ namespace ts { // local types not visible outside the function body : false; } - if (meaning & SymbolFlags.Value && result.flags & SymbolFlags.FunctionScopedVariable) { + if (meaning & SymbolFlags.Value && result.flags & SymbolFlags.Variable) { // parameter initializer will lookup as normal variable scope when targeting es2015+ - if (compilerOptions.target && compilerOptions.target >= ScriptTarget.ES2015 && isParameter(lastLocation) && lastLocation.initializer === originalLocation && result.valueDeclaration !== lastLocation) { + if (compilerOptions.target && compilerOptions.target >= ScriptTarget.ES2015 && isParameter(lastLocation) && result.valueDeclaration !== lastLocation) { useResult = false; } - else { + else if (result.flags & SymbolFlags.FunctionScopedVariable) { // parameters are visible only inside function body, parameter list and return type // technically for parameter list case here we might mix parameters and variables declared in function, // however it is detected separately when checking initializers of parameters diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt b/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt index 9879ec320c..055a37838f 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.errors.txt @@ -3,9 +3,10 @@ tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(8, tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(13,20): error TS2373: Initializer of parameter 'bar' cannot reference identifier 'foo' declared after it. tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(21,18): error TS2372: Parameter 'a' cannot be referenced in its initializer. tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(25,22): error TS2372: Parameter 'async' cannot be referenced in its initializer. +tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(29,15): error TS2448: Block-scoped variable 'foo' used before its declaration. -==== tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts (5 errors) ==== +==== tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts (6 errors) ==== let foo: string = ""; function f1 (bar = foo) { // unexpected compiler error; works at runtime @@ -42,4 +43,12 @@ tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts(25 ~~~~~ !!! error TS2372: Parameter 'async' cannot be referenced in its initializer. return async - } \ No newline at end of file + } + + function f7({[foo]: bar}: any[]) { + ~~~ +!!! error TS2448: Block-scoped variable 'foo' used before its declaration. +!!! related TS2728 tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts:30:9: 'foo' is declared here. + let foo: number = 2; + } + \ No newline at end of file diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.js b/tests/baselines/reference/parameterInitializersForwardReferencing1.js index 43f303a94c..ad68f64ca2 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1.js +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.js @@ -25,7 +25,12 @@ function f5 (a = a) { function f6 (async = async) { return async -} +} + +function f7({[foo]: bar}: any[]) { + let foo: number = 2; +} + //// [parameterInitializersForwardReferencing1.js] var foo = ""; @@ -59,3 +64,7 @@ function f6(async) { if (async === void 0) { async = async; } return async; } +function f7(_a) { + var _b = foo, bar = _a[_b]; + var foo = 2; +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols b/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols index 6860e39fd8..f3ea0f1e7e 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.symbols @@ -65,3 +65,13 @@ function f6 (async = async) { return async >async : Symbol(async, Decl(parameterInitializersForwardReferencing1.ts, 24, 13)) } + +function f7({[foo]: bar}: any[]) { +>f7 : Symbol(f7, Decl(parameterInitializersForwardReferencing1.ts, 26, 1)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 29, 7)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1.ts, 28, 13)) + + let foo: number = 2; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1.ts, 29, 7)) +} + diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1.types b/tests/baselines/reference/parameterInitializersForwardReferencing1.types index c33c227a37..3a1d8d5864 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1.types +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1.types @@ -71,3 +71,14 @@ function f6 (async = async) { return async >async : any } + +function f7({[foo]: bar}: any[]) { +>f7 : ({ [foo]: bar }: any[]) => void +>foo : number +>bar : any + + let foo: number = 2; +>foo : number +>2 : 2 +} + diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt index cffa83707e..e7708894b6 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.errors.txt @@ -11,7 +11,7 @@ tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.t } function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime - var fooo: number = 2; + var foo: number = 2; return bar(); // returns 1 } @@ -33,4 +33,9 @@ tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.t ~~~~~ !!! error TS2372: Parameter 'async' cannot be referenced in its initializer. return async - } \ No newline at end of file + } + + function f7({[foo]: bar}: any[]) { + let foo: number = 2; + } + \ No newline at end of file diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js index add0837e31..0c89b79b76 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.js @@ -7,7 +7,7 @@ function f1 (bar = foo) { // unexpected compiler error; works at runtime } function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime - var fooo: number = 2; + var foo: number = 2; return bar(); // returns 1 } @@ -25,7 +25,12 @@ function f5 (a = a) { function f6 (async = async) { return async -} +} + +function f7({[foo]: bar}: any[]) { + let foo: number = 2; +} + //// [parameterInitializersForwardReferencing1_es6.js] let foo = ""; @@ -34,7 +39,7 @@ function f1(bar = foo) { return bar; // returns 1 } function f2(bar = (baz = foo) => baz) { - var fooo = 2; + var foo = 2; return bar(); // returns 1 } function f3(bar = foo, foo = 2) { @@ -49,3 +54,6 @@ function f5(a = a) { function f6(async = async) { return async; } +function f7({ [foo]: bar }) { + let foo = 2; +} diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols index c38a392c4d..f9366034aa 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.symbols @@ -21,8 +21,8 @@ function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at >foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) >baz : Symbol(baz, Decl(parameterInitializersForwardReferencing1_es6.ts, 7, 20)) - var fooo: number = 2; ->fooo : Symbol(fooo, Decl(parameterInitializersForwardReferencing1_es6.ts, 8, 7)) + var foo: number = 2; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 8, 7)) return bar(); // returns 1 >bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 7, 13)) @@ -65,3 +65,13 @@ function f6 (async = async) { return async >async : Symbol(async, Decl(parameterInitializersForwardReferencing1_es6.ts, 24, 13)) } + +function f7({[foo]: bar}: any[]) { +>f7 : Symbol(f7, Decl(parameterInitializersForwardReferencing1_es6.ts, 26, 1)) +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 0, 3)) +>bar : Symbol(bar, Decl(parameterInitializersForwardReferencing1_es6.ts, 28, 13)) + + let foo: number = 2; +>foo : Symbol(foo, Decl(parameterInitializersForwardReferencing1_es6.ts, 29, 7)) +} + diff --git a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types index bd17f48233..16bd49f1cf 100644 --- a/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types +++ b/tests/baselines/reference/parameterInitializersForwardReferencing1_es6.types @@ -24,8 +24,8 @@ function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at >foo : string >baz : string - var fooo: number = 2; ->fooo : number + var foo: number = 2; +>foo : number >2 : 2 return bar(); // returns 1 @@ -71,3 +71,14 @@ function f6 (async = async) { return async >async : any } + +function f7({[foo]: bar}: any[]) { +>f7 : ({ [foo]: bar }: any[]) => void +>foo : string +>bar : any + + let foo: number = 2; +>foo : number +>2 : 2 +} + diff --git a/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts index 8492a05385..640900253b 100644 --- a/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts +++ b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1.ts @@ -24,4 +24,8 @@ function f5 (a = a) { function f6 (async = async) { return async -} \ No newline at end of file +} + +function f7({[foo]: bar}: any[]) { + let foo: number = 2; +} diff --git a/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts index dfd6b8d4e1..55afbbf7c2 100644 --- a/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts +++ b/tests/cases/conformance/functions/parameterInitializersForwardReferencing1_es6.ts @@ -8,7 +8,7 @@ function f1 (bar = foo) { // unexpected compiler error; works at runtime } function f2 (bar = (baz = foo) => baz) { // unexpected compiler error; works at runtime - var fooo: number = 2; + var foo: number = 2; return bar(); // returns 1 } @@ -26,4 +26,8 @@ function f5 (a = a) { function f6 (async = async) { return async -} \ No newline at end of file +} + +function f7({[foo]: bar}: any[]) { + let foo: number = 2; +} From b183418124626b401ea4f7721cee455d6fd92d2f Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 28 Aug 2018 11:06:14 -0700 Subject: [PATCH 16/21] Fix bug: Don't go to *any* constructor signature for jsx element (#26715) --- src/services/goToDefinition.ts | 13 ++++++++++++- .../fourslash/goToDefinitionSignatureAlias.ts | 19 ++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/services/goToDefinition.ts b/src/services/goToDefinition.ts index 973c040a76..02d590bb8b 100644 --- a/src/services/goToDefinition.ts +++ b/src/services/goToDefinition.ts @@ -29,7 +29,7 @@ namespace ts.GoToDefinition { const calledDeclaration = tryGetSignatureDeclaration(typeChecker, node); // Don't go to the component constructor definition for a JSX element, just go to the component definition. - if (calledDeclaration && !(isJsxOpeningLikeElement(node.parent) && isConstructorDeclaration(calledDeclaration))) { + if (calledDeclaration && !(isJsxOpeningLikeElement(node.parent) && isConstructorLike(calledDeclaration))) { const sigInfo = createDefinitionFromSignatureDeclaration(typeChecker, calledDeclaration); // For a function, if this is the original function definition, return just sigInfo. // If this is the original constructor definition, parent is the class. @@ -319,4 +319,15 @@ namespace ts.GoToDefinition { // Don't go to a function type, go to the value having that type. return tryCast(signature && signature.declaration, (d): d is SignatureDeclaration => isFunctionLike(d) && !isFunctionTypeNode(d)); } + + function isConstructorLike(node: Node): boolean { + switch (node.kind) { + case SyntaxKind.Constructor: + case SyntaxKind.ConstructorType: + case SyntaxKind.ConstructSignature: + return true; + default: + return false; + } + } } diff --git a/tests/cases/fourslash/goToDefinitionSignatureAlias.ts b/tests/cases/fourslash/goToDefinitionSignatureAlias.ts index 909322582e..4ace3635cc 100644 --- a/tests/cases/fourslash/goToDefinitionSignatureAlias.ts +++ b/tests/cases/fourslash/goToDefinitionSignatureAlias.ts @@ -21,9 +21,20 @@ ////o.[|/*useM*/m|](); ////class Component { /*componentCtr*/constructor(props: {}) {} } +////type ComponentClass = /*ComponentClass*/new () => Component; +////interface ComponentClass2 { /*ComponentClass2*/new(): Component; } +//// ////class /*MyComponent*/MyComponent extends Component {} -////<[|/*jsxMyComponent*/MyComponent|] /> +////<[|/*jsxMyComponent*/MyComponent|] />; ////new [|/*newMyComponent*/MyComponent|]({}); +//// +////declare const /*MyComponent2*/MyComponent2: ComponentClass; +////<[|/*jsxMyComponent2*/MyComponent2|] />; +////new [|/*newMyComponent2*/MyComponent2|](); +//// +////declare const /*MyComponent3*/MyComponent3: ComponentClass2; +////<[|/*jsxMyComponent3*/MyComponent3|] />; +////new [|/*newMyComponent3*/MyComponent3|](); verify.noErrors(); @@ -38,4 +49,10 @@ verify.goToDefinition({ jsxMyComponent: "MyComponent", newMyComponent: ["MyComponent", "componentCtr"], + + jsxMyComponent2: "MyComponent2", + newMyComponent2: ["MyComponent2", "ComponentClass"], + + jsxMyComponent3: "MyComponent3", + newMyComponent3: ["MyComponent3", "ComponentClass2"], }); From 868cf3e6de627aae3e811bc0bcc40839a9b7af5f Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Tue, 28 Aug 2018 11:21:35 -0700 Subject: [PATCH 17/21] renames per PR feedback --- src/compiler/tsbuild.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/compiler/tsbuild.ts b/src/compiler/tsbuild.ts index 5fdab16895..f038136e6a 100644 --- a/src/compiler/tsbuild.ts +++ b/src/compiler/tsbuild.ts @@ -39,7 +39,7 @@ namespace ts { invalidateProject(project: ResolvedConfigFileName, dependencyGraph: DependencyGraph | undefined): void; getNextInvalidatedProject(): ResolvedConfigFileName | undefined; - pendingInvalidatedProjects(): boolean; + hasPendingInvalidatedProjects(): boolean; missingRoots: Map; } @@ -396,20 +396,21 @@ namespace ts { unchangedOutputs: createFileMap(), invalidateProject, getNextInvalidatedProject, - pendingInvalidatedProjects, + hasPendingInvalidatedProjects, missingRoots }; - function invalidateProject(proj: ResolvedConfigFileName, dependancyGraph: DependencyGraph | undefined) { + function invalidateProject(proj: ResolvedConfigFileName, dependencyGraph: DependencyGraph | undefined) { if (!projectPendingBuild.hasKey(proj)) { addProjToQueue(proj); - if (dependancyGraph) { - queueBuildForDownstreamReferences(proj, dependancyGraph); + if (dependencyGraph) { + queueBuildForDownstreamReferences(proj, dependencyGraph); } } } function addProjToQueue(proj: ResolvedConfigFileName) { + Debug.assert(!projectPendingBuild.hasKey(proj)); projectPendingBuild.setValue(proj, true); invalidatedProjectQueue.push(proj); } @@ -426,18 +427,18 @@ namespace ts { } } - function pendingInvalidatedProjects() { + function hasPendingInvalidatedProjects() { return !!projectPendingBuild.getSize(); } // Mark all downstream projects of this one needing to be built "later" - function queueBuildForDownstreamReferences(root: ResolvedConfigFileName, dependancyGraph: DependencyGraph) { - const deps = dependancyGraph.dependencyMap.getReferencesTo(root); + function queueBuildForDownstreamReferences(root: ResolvedConfigFileName, dependencyGraph: DependencyGraph) { + const deps = dependencyGraph.dependencyMap.getReferencesTo(root); for (const ref of deps) { // Can skip circular references if (!projectPendingBuild.hasKey(ref)) { addProjToQueue(ref); - queueBuildForDownstreamReferences(ref, dependancyGraph); + queueBuildForDownstreamReferences(ref, dependencyGraph); } } } @@ -810,7 +811,7 @@ namespace ts { timerToBuildInvalidatedProject = undefined; const buildProject = context.getNextInvalidatedProject(); buildSomeProjects(p => p === buildProject); - if (context.pendingInvalidatedProjects()) { + if (context.hasPendingInvalidatedProjects()) { if (!timerToBuildInvalidatedProject) { scheduleBuildInvalidatedProject(); } From 3931b72118e82884f882b7941cb275f212f3afdd Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 28 Aug 2018 11:43:45 -0700 Subject: [PATCH 18/21] noUnusedLocals: Destructuring assignment is a write (#26365) * noUnusedLocals: Destructuring assignment is a write * Code review * Clarify test --- src/compiler/utilities.ts | 33 ++++++- .../noUnusedLocals_writeOnly.errors.txt | 13 ++- .../reference/noUnusedLocals_writeOnly.js | 25 +++++- .../noUnusedLocals_writeOnly.symbols | 51 +++++++++-- .../reference/noUnusedLocals_writeOnly.types | 87 ++++++++++++++++++- .../compiler/noUnusedLocals_writeOnly.ts | 12 ++- 6 files changed, 206 insertions(+), 15 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 39a298329c..b808a1f2fd 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2331,6 +2331,13 @@ namespace ts { return node; } + function skipParenthesesUp(node: Node): Node { + while (node.kind === SyntaxKind.ParenthesizedExpression) { + node = node.parent; + } + return node; + } + // a node is delete target iff. it is PropertyAccessExpression/ElementAccessExpression with parentheses skipped export function isDeleteTarget(node: Node): boolean { if (node.kind !== SyntaxKind.PropertyAccessExpression && node.kind !== SyntaxKind.ElementAccessExpression) { @@ -4206,6 +4213,8 @@ namespace ts { if (!parent) return AccessKind.Read; switch (parent.kind) { + case SyntaxKind.ParenthesizedExpression: + return accessKind(parent); case SyntaxKind.PostfixUnaryExpression: case SyntaxKind.PrefixUnaryExpression: const { operator } = parent as PrefixUnaryExpression | PostfixUnaryExpression; @@ -4217,13 +4226,35 @@ namespace ts { : AccessKind.Read; case SyntaxKind.PropertyAccessExpression: return (parent as PropertyAccessExpression).name !== node ? AccessKind.Read : accessKind(parent); + case SyntaxKind.PropertyAssignment: { + const parentAccess = accessKind(parent.parent); + // In `({ x: varname }) = { x: 1 }`, the left `x` is a read, the right `x` is a write. + return node === (parent as PropertyAssignment).name ? reverseAccessKind(parentAccess) : parentAccess; + } + case SyntaxKind.ShorthandPropertyAssignment: + // Assume it's the local variable being accessed, since we don't check public properties for --noUnusedLocals. + return node === (parent as ShorthandPropertyAssignment).objectAssignmentInitializer ? AccessKind.Read : accessKind(parent.parent); + case SyntaxKind.ArrayLiteralExpression: + return accessKind(parent); default: return AccessKind.Read; } function writeOrReadWrite(): AccessKind { // If grandparent is not an ExpressionStatement, this is used as an expression in addition to having a side effect. - return parent.parent && parent.parent.kind === SyntaxKind.ExpressionStatement ? AccessKind.Write : AccessKind.ReadWrite; + return parent.parent && skipParenthesesUp(parent.parent).kind === SyntaxKind.ExpressionStatement ? AccessKind.Write : AccessKind.ReadWrite; + } + } + function reverseAccessKind(a: AccessKind): AccessKind { + switch (a) { + case AccessKind.Read: + return AccessKind.Write; + case AccessKind.Write: + return AccessKind.Read; + case AccessKind.ReadWrite: + return AccessKind.ReadWrite; + default: + return Debug.assertNever(a); } } diff --git a/tests/baselines/reference/noUnusedLocals_writeOnly.errors.txt b/tests/baselines/reference/noUnusedLocals_writeOnly.errors.txt index 09ebea0f65..347e181d94 100644 --- a/tests/baselines/reference/noUnusedLocals_writeOnly.errors.txt +++ b/tests/baselines/reference/noUnusedLocals_writeOnly.errors.txt @@ -1,14 +1,22 @@ tests/cases/compiler/noUnusedLocals_writeOnly.ts(1,12): error TS6133: 'x' is declared but its value is never read. -tests/cases/compiler/noUnusedLocals_writeOnly.ts(10,9): error TS6133: 'z' is declared but its value is never read. +tests/cases/compiler/noUnusedLocals_writeOnly.ts(18,9): error TS6133: 'z' is declared but its value is never read. ==== tests/cases/compiler/noUnusedLocals_writeOnly.ts (2 errors) ==== - function f(x = 0) { + function f(x = 0, b = false) { ~ !!! error TS6133: 'x' is declared but its value is never read. + // None of these statements read from 'x', so it will be marked unused. x = 1; x++; x /= 2; + ([x] = [1]); + ({ x } = { x: 1 }); + ({ x: x } = { x: 1 }); + ({ a: [{ b: x }] } = { a: [{ b: 1 }] }); + ({ x = 2 } = { x: b ? 1 : undefined }); + let used = 1; + ({ x = used } = { x: b ? 1 : undefined }); let y = 0; // This is a write access to y, but not a write-*only* access. @@ -19,4 +27,5 @@ tests/cases/compiler/noUnusedLocals_writeOnly.ts(10,9): error TS6133: 'z' is dec !!! error TS6133: 'z' is declared but its value is never read. f(z = 1); // This effectively doesn't use `z`, values just pass through it. } + function f2(_: ReadonlyArray): void {} \ No newline at end of file diff --git a/tests/baselines/reference/noUnusedLocals_writeOnly.js b/tests/baselines/reference/noUnusedLocals_writeOnly.js index bc00779304..55a927bfc2 100644 --- a/tests/baselines/reference/noUnusedLocals_writeOnly.js +++ b/tests/baselines/reference/noUnusedLocals_writeOnly.js @@ -1,8 +1,16 @@ //// [noUnusedLocals_writeOnly.ts] -function f(x = 0) { +function f(x = 0, b = false) { + // None of these statements read from 'x', so it will be marked unused. x = 1; x++; x /= 2; + ([x] = [1]); + ({ x } = { x: 1 }); + ({ x: x } = { x: 1 }); + ({ a: [{ b: x }] } = { a: [{ b: 1 }] }); + ({ x = 2 } = { x: b ? 1 : undefined }); + let used = 1; + ({ x = used } = { x: b ? 1 : undefined }); let y = 0; // This is a write access to y, but not a write-*only* access. @@ -11,17 +19,30 @@ function f(x = 0) { let z = 0; f(z = 1); // This effectively doesn't use `z`, values just pass through it. } +function f2(_: ReadonlyArray): void {} //// [noUnusedLocals_writeOnly.js] -function f(x) { +"use strict"; +function f(x, b) { if (x === void 0) { x = 0; } + if (b === void 0) { b = false; } + var _a, _b; + // None of these statements read from 'x', so it will be marked unused. x = 1; x++; x /= 2; + (x = [1][0]); + (x = { x: 1 }.x); + (x = { x: 1 }.x); + (x = { a: [{ b: 1 }] }.a[0].b); + (_a = { x: b ? 1 : undefined }.x, x = _a === void 0 ? 2 : _a); + var used = 1; + (_b = { x: b ? 1 : undefined }.x, x = _b === void 0 ? used : _b); var y = 0; // This is a write access to y, but not a write-*only* access. f(y++); var z = 0; f(z = 1); // This effectively doesn't use `z`, values just pass through it. } +function f2(_) { } diff --git a/tests/baselines/reference/noUnusedLocals_writeOnly.symbols b/tests/baselines/reference/noUnusedLocals_writeOnly.symbols index afb13f9aa2..a794291cad 100644 --- a/tests/baselines/reference/noUnusedLocals_writeOnly.symbols +++ b/tests/baselines/reference/noUnusedLocals_writeOnly.symbols @@ -1,8 +1,10 @@ === tests/cases/compiler/noUnusedLocals_writeOnly.ts === -function f(x = 0) { +function f(x = 0, b = false) { >f : Symbol(f, Decl(noUnusedLocals_writeOnly.ts, 0, 0)) >x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 0, 11)) +>b : Symbol(b, Decl(noUnusedLocals_writeOnly.ts, 0, 17)) + // None of these statements read from 'x', so it will be marked unused. x = 1; >x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 0, 11)) @@ -12,19 +14,58 @@ function f(x = 0) { x /= 2; >x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 0, 11)) + ([x] = [1]); +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 0, 11)) + + ({ x } = { x: 1 }); +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 6, 6)) +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 6, 14)) + + ({ x: x } = { x: 1 }); +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 7, 6)) +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 0, 11)) +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 7, 17)) + + ({ a: [{ b: x }] } = { a: [{ b: 1 }] }); +>a : Symbol(a, Decl(noUnusedLocals_writeOnly.ts, 8, 6)) +>b : Symbol(b, Decl(noUnusedLocals_writeOnly.ts, 8, 12)) +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 0, 11)) +>a : Symbol(a, Decl(noUnusedLocals_writeOnly.ts, 8, 26)) +>b : Symbol(b, Decl(noUnusedLocals_writeOnly.ts, 8, 32)) + + ({ x = 2 } = { x: b ? 1 : undefined }); +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 9, 6)) +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 9, 18)) +>b : Symbol(b, Decl(noUnusedLocals_writeOnly.ts, 0, 17)) +>undefined : Symbol(undefined) + + let used = 1; +>used : Symbol(used, Decl(noUnusedLocals_writeOnly.ts, 10, 7)) + + ({ x = used } = { x: b ? 1 : undefined }); +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 11, 6)) +>used : Symbol(used, Decl(noUnusedLocals_writeOnly.ts, 10, 7)) +>x : Symbol(x, Decl(noUnusedLocals_writeOnly.ts, 11, 21)) +>b : Symbol(b, Decl(noUnusedLocals_writeOnly.ts, 0, 17)) +>undefined : Symbol(undefined) + let y = 0; ->y : Symbol(y, Decl(noUnusedLocals_writeOnly.ts, 5, 7)) +>y : Symbol(y, Decl(noUnusedLocals_writeOnly.ts, 13, 7)) // This is a write access to y, but not a write-*only* access. f(y++); >f : Symbol(f, Decl(noUnusedLocals_writeOnly.ts, 0, 0)) ->y : Symbol(y, Decl(noUnusedLocals_writeOnly.ts, 5, 7)) +>y : Symbol(y, Decl(noUnusedLocals_writeOnly.ts, 13, 7)) let z = 0; ->z : Symbol(z, Decl(noUnusedLocals_writeOnly.ts, 9, 7)) +>z : Symbol(z, Decl(noUnusedLocals_writeOnly.ts, 17, 7)) f(z = 1); // This effectively doesn't use `z`, values just pass through it. >f : Symbol(f, Decl(noUnusedLocals_writeOnly.ts, 0, 0)) ->z : Symbol(z, Decl(noUnusedLocals_writeOnly.ts, 9, 7)) +>z : Symbol(z, Decl(noUnusedLocals_writeOnly.ts, 17, 7)) } +function f2(_: ReadonlyArray): void {} +>f2 : Symbol(f2, Decl(noUnusedLocals_writeOnly.ts, 19, 1)) +>_ : Symbol(_, Decl(noUnusedLocals_writeOnly.ts, 20, 12)) +>ReadonlyArray : Symbol(ReadonlyArray, Decl(lib.es5.d.ts, --, --)) diff --git a/tests/baselines/reference/noUnusedLocals_writeOnly.types b/tests/baselines/reference/noUnusedLocals_writeOnly.types index 4b90d7b13e..43a393a1ed 100644 --- a/tests/baselines/reference/noUnusedLocals_writeOnly.types +++ b/tests/baselines/reference/noUnusedLocals_writeOnly.types @@ -1,9 +1,12 @@ === tests/cases/compiler/noUnusedLocals_writeOnly.ts === -function f(x = 0) { ->f : (x?: number) => void +function f(x = 0, b = false) { +>f : (x?: number, b?: boolean) => void >x : number >0 : 0 +>b : boolean +>false : false + // None of these statements read from 'x', so it will be marked unused. x = 1; >x = 1 : 1 >x : number @@ -18,6 +21,79 @@ function f(x = 0) { >x : number >2 : 2 + ([x] = [1]); +>([x] = [1]) : [number] +>[x] = [1] : [number] +>[x] : [number] +>x : number +>[1] : [number] +>1 : 1 + + ({ x } = { x: 1 }); +>({ x } = { x: 1 }) : { x: number; } +>{ x } = { x: 1 } : { x: number; } +>{ x } : { x: number; } +>x : number +>{ x: 1 } : { x: number; } +>x : number +>1 : 1 + + ({ x: x } = { x: 1 }); +>({ x: x } = { x: 1 }) : { x: number; } +>{ x: x } = { x: 1 } : { x: number; } +>{ x: x } : { x: number; } +>x : number +>x : number +>{ x: 1 } : { x: number; } +>x : number +>1 : 1 + + ({ a: [{ b: x }] } = { a: [{ b: 1 }] }); +>({ a: [{ b: x }] } = { a: [{ b: 1 }] }) : { a: [{ b: number; }]; } +>{ a: [{ b: x }] } = { a: [{ b: 1 }] } : { a: [{ b: number; }]; } +>{ a: [{ b: x }] } : { a: [{ b: number; }]; } +>a : [{ b: number; }] +>[{ b: x }] : [{ b: number; }] +>{ b: x } : { b: number; } +>b : number +>x : number +>{ a: [{ b: 1 }] } : { a: [{ b: number; }]; } +>a : [{ b: number; }] +>[{ b: 1 }] : [{ b: number; }] +>{ b: 1 } : { b: number; } +>b : number +>1 : 1 + + ({ x = 2 } = { x: b ? 1 : undefined }); +>({ x = 2 } = { x: b ? 1 : undefined }) : { x?: number | undefined; } +>{ x = 2 } = { x: b ? 1 : undefined } : { x?: number | undefined; } +>{ x = 2 } : { x?: number; } +>x : number +>2 : 2 +>{ x: b ? 1 : undefined } : { x?: number | undefined; } +>x : number | undefined +>b ? 1 : undefined : 1 | undefined +>b : boolean +>1 : 1 +>undefined : undefined + + let used = 1; +>used : number +>1 : 1 + + ({ x = used } = { x: b ? 1 : undefined }); +>({ x = used } = { x: b ? 1 : undefined }) : { x?: number | undefined; } +>{ x = used } = { x: b ? 1 : undefined } : { x?: number | undefined; } +>{ x = used } : { x?: number; } +>x : number +>used : number +>{ x: b ? 1 : undefined } : { x?: number | undefined; } +>x : number | undefined +>b ? 1 : undefined : 1 | undefined +>b : boolean +>1 : 1 +>undefined : undefined + let y = 0; >y : number >0 : 0 @@ -25,7 +101,7 @@ function f(x = 0) { // This is a write access to y, but not a write-*only* access. f(y++); >f(y++) : void ->f : (x?: number) => void +>f : (x?: number, b?: boolean) => void >y++ : number >y : number @@ -35,9 +111,12 @@ function f(x = 0) { f(z = 1); // This effectively doesn't use `z`, values just pass through it. >f(z = 1) : void ->f : (x?: number) => void +>f : (x?: number, b?: boolean) => void >z = 1 : 1 >z : number >1 : 1 } +function f2(_: ReadonlyArray): void {} +>f2 : (_: ReadonlyArray) => void +>_ : ReadonlyArray diff --git a/tests/cases/compiler/noUnusedLocals_writeOnly.ts b/tests/cases/compiler/noUnusedLocals_writeOnly.ts index ca1df86991..8d2de0d517 100644 --- a/tests/cases/compiler/noUnusedLocals_writeOnly.ts +++ b/tests/cases/compiler/noUnusedLocals_writeOnly.ts @@ -1,10 +1,19 @@ +// @strict: true // @noUnusedLocals: true // @noUnusedParameters: true -function f(x = 0) { +function f(x = 0, b = false) { + // None of these statements read from 'x', so it will be marked unused. x = 1; x++; x /= 2; + ([x] = [1]); + ({ x } = { x: 1 }); + ({ x: x } = { x: 1 }); + ({ a: [{ b: x }] } = { a: [{ b: 1 }] }); + ({ x = 2 } = { x: b ? 1 : undefined }); + let used = 1; + ({ x = used } = { x: b ? 1 : undefined }); let y = 0; // This is a write access to y, but not a write-*only* access. @@ -13,3 +22,4 @@ function f(x = 0) { let z = 0; f(z = 1); // This effectively doesn't use `z`, values just pass through it. } +function f2(_: ReadonlyArray): void {} From b94061c5871195642965c2df9fa341758572098f Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 28 Aug 2018 13:03:24 -0700 Subject: [PATCH 19/21] getEditsForFileRename: Avoid changing import specifier ending (#26177) * getEditsForFileRename: Avoid changing import specifier ending * Support .json and .jsx extensions * Restore typeRoots tests * Fix json test * When --jsx preserve is set, import ".tsx" file with ".jsx" extension * Support ending preference in UserPreferences --- src/compiler/checker.ts | 17 +- src/compiler/moduleSpecifiers.ts | 198 +++++++++--------- src/compiler/types.ts | 15 +- src/compiler/utilities.ts | 5 + src/services/getEditsForFileRename.ts | 9 +- src/services/types.ts | 8 - .../reference/api/tsserverlibrary.d.ts | 18 +- tests/baselines/reference/api/typescript.d.ts | 18 +- tests/cases/fourslash/fourslash.ts | 9 +- ...etEditsForFileRename_preservePathEnding.ts | 44 ++++ .../importNameCodeFixNewImportTypeRoots0.ts | 45 ++-- .../importNameCodeFixNewImportTypeRoots1.ts | 50 +++-- .../importNameCodeFix_endingPreference.ts | 26 +++ .../importNameCodeFix_jsExtension.ts | 16 +- .../importNameCodeFix_types_classic.ts | 18 +- 15 files changed, 309 insertions(+), 187 deletions(-) create mode 100644 tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts create mode 100644 tests/cases/fourslash/importNameCodeFix_endingPreference.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6106505df8..26e5a6b83b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -3926,13 +3926,22 @@ namespace ts { const links = getSymbolLinks(symbol); let specifier = links.specifierCache && links.specifierCache.get(contextFile.path); if (!specifier) { - specifier = moduleSpecifiers.getModuleSpecifierForDeclarationFile( + const isBundle = (compilerOptions.out || compilerOptions.outFile); + // For declaration bundles, we need to generate absolute paths relative to the common source dir for imports, + // just like how the declaration emitter does for the ambient module declarations - we can easily accomplish this + // using the `baseUrl` compiler option (which we would otherwise never use in declaration emit) and a non-relative + // specifier preference + const { moduleResolverHost } = context.tracker; + const specifierCompilerOptions = isBundle ? { ...compilerOptions, baseUrl: moduleResolverHost.getCommonSourceDirectory() } : compilerOptions; + specifier = first(first(moduleSpecifiers.getModuleSpecifiers( symbol, - compilerOptions, + specifierCompilerOptions, contextFile, - context.tracker.moduleResolverHost, + moduleResolverHost, + host.getSourceFiles(), + { importModuleSpecifierPreference: isBundle ? "non-relative" : "relative" }, host.redirectTargetsMap, - ); + ))); links.specifierCache = links.specifierCache || createMap(); links.specifierCache.set(contextFile.path, specifier); } diff --git a/src/compiler/moduleSpecifiers.ts b/src/compiler/moduleSpecifiers.ts index 8c695e10be..18b2ea76b0 100644 --- a/src/compiler/moduleSpecifiers.ts +++ b/src/compiler/moduleSpecifiers.ts @@ -1,8 +1,52 @@ // Used by importFixes, getEditsForFileRename, and declaration emit to synthesize import module specifiers. /* @internal */ namespace ts.moduleSpecifiers { - export interface ModuleSpecifierPreferences { - readonly importModuleSpecifierPreference?: "relative" | "non-relative"; + const enum RelativePreference { Relative, NonRelative, Auto } + // See UserPreferences#importPathEnding + const enum Ending { Minimal, Index, JsExtension } + + // Processed preferences + interface Preferences { + readonly relativePreference: RelativePreference; + readonly ending: Ending; + } + + function getPreferences({ importModuleSpecifierPreference, importModuleSpecifierEnding }: UserPreferences, compilerOptions: CompilerOptions, importingSourceFile: SourceFile): Preferences { + return { + relativePreference: importModuleSpecifierPreference === "relative" ? RelativePreference.Relative : importModuleSpecifierPreference === "non-relative" ? RelativePreference.NonRelative : RelativePreference.Auto, + ending: getEnding(), + }; + function getEnding(): Ending { + switch (importModuleSpecifierEnding) { + case "minimal": return Ending.Minimal; + case "index": return Ending.Index; + case "js": return Ending.JsExtension; + default: return usesJsExtensionOnImports(importingSourceFile) ? Ending.JsExtension + : getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeJs ? Ending.Index : Ending.Minimal; + } + } + } + + function getPreferencesForUpdate(compilerOptions: CompilerOptions, oldImportSpecifier: string): Preferences { + return { + relativePreference: isExternalModuleNameRelative(oldImportSpecifier) ? RelativePreference.Relative : RelativePreference.NonRelative, + ending: hasJavaScriptOrJsonFileExtension(oldImportSpecifier) ? Ending.JsExtension + : getEmitModuleResolutionKind(compilerOptions) !== ModuleResolutionKind.NodeJs || endsWith(oldImportSpecifier, "index") ? Ending.Index : Ending.Minimal, + }; + } + + export function updateModuleSpecifier( + compilerOptions: CompilerOptions, + importingSourceFileName: Path, + toFileName: string, + host: ModuleSpecifierResolutionHost, + files: ReadonlyArray, + redirectTargetsMap: RedirectTargetsMap, + oldImportSpecifier: string, + ): string | undefined { + const res = getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferencesForUpdate(compilerOptions, oldImportSpecifier)); + if (res === oldImportSpecifier) return undefined; + return res; } // Note: importingSourceFile is just for usesJsExtensionOnImports @@ -13,35 +57,25 @@ namespace ts.moduleSpecifiers { toFileName: string, host: ModuleSpecifierResolutionHost, files: ReadonlyArray, - preferences: ModuleSpecifierPreferences = {}, + preferences: UserPreferences = {}, redirectTargetsMap: RedirectTargetsMap, ): string { - const info = getInfo(compilerOptions, importingSourceFile, importingSourceFileName, host); - const modulePaths = getAllModulePaths(files, importingSourceFileName, toFileName, info.getCanonicalFileName, host, redirectTargetsMap); - return firstDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)) || - first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences)); + return getModuleSpecifierWorker(compilerOptions, importingSourceFileName, toFileName, host, files, redirectTargetsMap, getPreferences(preferences, compilerOptions, importingSourceFile)); } - export function getModuleSpecifierForDeclarationFile( - moduleSymbol: Symbol, + function getModuleSpecifierWorker( compilerOptions: CompilerOptions, - importingSourceFile: SourceFile, + importingSourceFileName: Path, + toFileName: string, host: ModuleSpecifierResolutionHost, + files: ReadonlyArray, redirectTargetsMap: RedirectTargetsMap, + preferences: Preferences ): string { - const isBundle = (compilerOptions.out || compilerOptions.outFile); - if (isBundle && host.getCommonSourceDirectory) { - // For declaration bundles, we need to generate absolute paths relative to the common source dir for imports, - // just like how the declaration emitter does for the ambient module declarations - we can easily accomplish this - // using the `baseUrl` compiler option (which we would otherwise never use in declaration emit) and a non-relative - // specifier preference - compilerOptions = { - ...compilerOptions, - baseUrl: host.getCommonSourceDirectory(), - }; - } - const preferences: ModuleSpecifierPreferences = { importModuleSpecifierPreference: isBundle ? "non-relative" : "relative" }; - return first(first(getModuleSpecifiers(moduleSymbol, compilerOptions, importingSourceFile, host, host.getSourceFiles ? host.getSourceFiles() : [importingSourceFile], preferences, redirectTargetsMap))); + const info = getInfo(importingSourceFileName, host); + const modulePaths = getAllModulePaths(files, importingSourceFileName, toFileName, info.getCanonicalFileName, host, redirectTargetsMap); + return firstDefined(modulePaths, moduleFileName => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions)) || + first(getLocalModuleSpecifiers(toFileName, info, compilerOptions, preferences)); } // For each symlink/original for a module, returns a list of ways to import that file. @@ -51,60 +85,39 @@ namespace ts.moduleSpecifiers { importingSourceFile: SourceFile, host: ModuleSpecifierResolutionHost, files: ReadonlyArray, - preferences: ModuleSpecifierPreferences, + userPreferences: UserPreferences, redirectTargetsMap: RedirectTargetsMap, ): ReadonlyArray> { const ambient = tryGetModuleNameFromAmbientModule(moduleSymbol); if (ambient) return [[ambient]]; - const info = getInfo(compilerOptions, importingSourceFile, importingSourceFile.path, host); - if (!files) { - return Debug.fail("Files list must be present to resolve symlinks in specifier resolution"); - } + const info = getInfo(importingSourceFile.path, host); const moduleSourceFile = getSourceFileOfNode(moduleSymbol.valueDeclaration || getNonAugmentationDeclaration(moduleSymbol)); const modulePaths = getAllModulePaths(files, importingSourceFile.path, moduleSourceFile.fileName, info.getCanonicalFileName, host, redirectTargetsMap); - const global = mapDefined(modulePaths, moduleFileName => getGlobalModuleSpecifier(moduleFileName, info, host, compilerOptions)); + const preferences = getPreferences(userPreferences, compilerOptions, importingSourceFile); + const global = mapDefined(modulePaths, moduleFileName => tryGetModuleNameAsNodeModule(moduleFileName, info, host, compilerOptions)); return global.length ? global.map(g => [g]) : modulePaths.map(moduleFileName => getLocalModuleSpecifiers(moduleFileName, info, compilerOptions, preferences)); } interface Info { - readonly moduleResolutionKind: ModuleResolutionKind; - readonly addJsExtension: boolean; readonly getCanonicalFileName: GetCanonicalFileName; readonly sourceDirectory: Path; } // importingSourceFileName is separate because getEditsForFileRename may need to specify an updated path - function getInfo(compilerOptions: CompilerOptions, importingSourceFile: SourceFile, importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info { - const moduleResolutionKind = getEmitModuleResolutionKind(compilerOptions); - const addJsExtension = usesJsExtensionOnImports(importingSourceFile); + function getInfo(importingSourceFileName: Path, host: ModuleSpecifierResolutionHost): Info { const getCanonicalFileName = createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : true); const sourceDirectory = getDirectoryPath(importingSourceFileName); - return { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }; + return { getCanonicalFileName, sourceDirectory }; } - function getGlobalModuleSpecifier( - moduleFileName: string, - { addJsExtension, getCanonicalFileName, sourceDirectory }: Info, - host: ModuleSpecifierResolutionHost, - compilerOptions: CompilerOptions, - ) { - return tryGetModuleNameFromTypeRoots(compilerOptions, host, getCanonicalFileName, moduleFileName, addJsExtension) - || tryGetModuleNameAsNodeModule(compilerOptions, moduleFileName, host, getCanonicalFileName, sourceDirectory); - } - - function getLocalModuleSpecifiers( - moduleFileName: string, - { moduleResolutionKind, addJsExtension, getCanonicalFileName, sourceDirectory }: Info, - compilerOptions: CompilerOptions, - preferences: ModuleSpecifierPreferences, - ): ReadonlyArray { + function getLocalModuleSpecifiers(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, compilerOptions: CompilerOptions, { ending, relativePreference }: Preferences): ReadonlyArray { const { baseUrl, paths, rootDirs } = compilerOptions; const relativePath = rootDirs && tryGetModuleNameFromRootDirs(rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) || - removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), moduleResolutionKind, addJsExtension); - if (!baseUrl || preferences.importModuleSpecifierPreference === "relative") { + removeExtensionAndIndexPostFix(ensurePathIsNonModuleName(getRelativePathFromDirectory(sourceDirectory, moduleFileName, getCanonicalFileName)), ending, compilerOptions); + if (!baseUrl || relativePreference === RelativePreference.Relative) { return [relativePath]; } @@ -113,7 +126,7 @@ namespace ts.moduleSpecifiers { return [relativePath]; } - const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, moduleResolutionKind, addJsExtension); + const importRelativeToBaseUrl = removeExtensionAndIndexPostFix(relativeToBaseUrl, ending, compilerOptions); if (paths) { const fromPaths = tryGetModuleNameFromPaths(removeFileExtension(relativeToBaseUrl), importRelativeToBaseUrl, paths); if (fromPaths) { @@ -121,11 +134,11 @@ namespace ts.moduleSpecifiers { } } - if (preferences.importModuleSpecifierPreference === "non-relative") { + if (relativePreference === RelativePreference.NonRelative) { return [importRelativeToBaseUrl]; } - if (preferences.importModuleSpecifierPreference !== undefined) Debug.assertNever(preferences.importModuleSpecifierPreference); + if (relativePreference !== RelativePreference.Auto) Debug.assertNever(relativePreference); if (isPathRelativeToParent(relativeToBaseUrl)) { return [relativePath]; @@ -164,7 +177,7 @@ namespace ts.moduleSpecifiers { } function usesJsExtensionOnImports({ imports }: SourceFile): boolean { - return firstDefined(imports, ({ text }) => pathIsRelative(text) ? fileExtensionIs(text, Extension.Js) : undefined) || false; + return firstDefined(imports, ({ text }) => pathIsRelative(text) ? hasJavaScriptOrJsonFileExtension(text) : undefined) || false; } function stringsEqual(a: string, b: string, getCanonicalFileName: GetCanonicalFileName): boolean { @@ -283,37 +296,8 @@ namespace ts.moduleSpecifiers { return removeFileExtension(relativePath); } - function tryGetModuleNameFromTypeRoots( - options: CompilerOptions, - host: GetEffectiveTypeRootsHost, - getCanonicalFileName: (file: string) => string, - moduleFileName: string, - addJsExtension: boolean, - ): string | undefined { - const roots = getEffectiveTypeRoots(options, host); - return firstDefined(roots, unNormalizedTypeRoot => { - const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName); - if (startsWith(moduleFileName, typeRoot)) { - // For a type definition, we can strip `/index` even with classic resolution. - return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1), ModuleResolutionKind.NodeJs, addJsExtension); - } - }); - } - - function tryGetModuleNameAsNodeModule( - options: CompilerOptions, - moduleFileName: string, - host: ModuleSpecifierResolutionHost, - getCanonicalFileName: (file: string) => string, - sourceDirectory: Path, - ): string | undefined { - if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { - // nothing to do here - return undefined; - } - + function tryGetModuleNameAsNodeModule(moduleFileName: string, { getCanonicalFileName, sourceDirectory }: Info, host: ModuleSpecifierResolutionHost, options: CompilerOptions): string | undefined { const parts: NodeModulePathParts = getNodeModulePathParts(moduleFileName)!; - if (!parts) { return undefined; } @@ -325,8 +309,12 @@ namespace ts.moduleSpecifiers { // Get a path that's relative to node_modules or the importing file's path // if node_modules folder is in this folder or any of its parent folders, no need to keep it. if (!startsWith(sourceDirectory, getCanonicalFileName(moduleSpecifier.substring(0, parts.topLevelNodeModulesIndex)))) return undefined; + // If the module was found in @types, get the actual Node package name - return getPackageNameFromAtTypesDirectory(moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1)); + const nodeModulesDirectoryName = moduleSpecifier.substring(parts.topLevelPackageNameIndex + 1); + const packageName = getPackageNameFromAtTypesDirectory(nodeModulesDirectoryName); + // For classic resolution, only allow importing from node_modules/@types, not other node_modules + return getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs && packageName === nodeModulesDirectoryName ? undefined : packageName; function getDirectoryOrExtensionlessFileName(path: string): string { // If the file is the main module, it can be imported by the package name @@ -440,13 +428,35 @@ namespace ts.moduleSpecifiers { }); } - function removeExtensionAndIndexPostFix(fileName: string, moduleResolutionKind: ModuleResolutionKind, addJsExtension: boolean): string { + function removeExtensionAndIndexPostFix(fileName: string, ending: Ending, options: CompilerOptions): string { const noExtension = removeFileExtension(fileName); - return addJsExtension - ? noExtension + ".js" - : moduleResolutionKind === ModuleResolutionKind.NodeJs - ? removeSuffix(noExtension, "/index") - : noExtension; + switch (ending) { + case Ending.Minimal: + return removeSuffix(noExtension, "/index"); + case Ending.Index: + return noExtension; + case Ending.JsExtension: + return noExtension + getJavaScriptExtensionForFile(fileName, options); + default: + return Debug.assertNever(ending); + } + } + + function getJavaScriptExtensionForFile(fileName: string, options: CompilerOptions): Extension { + const ext = extensionFromPath(fileName); + switch (ext) { + case Extension.Ts: + case Extension.Dts: + return Extension.Js; + case Extension.Tsx: + return options.jsx === JsxEmit.Preserve ? Extension.Jsx : Extension.Js; + case Extension.Js: + case Extension.Jsx: + case Extension.Json: + return ext; + default: + return Debug.assertNever(ext); + } } function getRelativePathIfInDirectory(path: string, directoryPath: string, getCanonicalFileName: GetCanonicalFileName): string | undefined { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index d9d57cfcf1..8977d9be28 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5331,8 +5331,6 @@ namespace ts { useCaseSensitiveFileNames?(): boolean; fileExists?(path: string): boolean; readFile?(path: string): string | undefined; - getSourceFiles?(): ReadonlyArray; // Used for cached resolutions to find symlinks without traversing the fs (again) - getCommonSourceDirectory?(): string; } // Note: this used to be deprecated in our public API, but is still used internally @@ -5345,7 +5343,7 @@ namespace ts { reportInaccessibleThisError?(): void; reportPrivateInBaseOfClassExpression?(propertyName: string): void; reportInaccessibleUniqueSymbolError?(): void; - moduleResolverHost?: ModuleSpecifierResolutionHost; + moduleResolverHost?: EmitHost; trackReferencedAmbientModule?(decl: ModuleDeclaration, symbol: Symbol): void; trackExternalModuleSymbolOfImportTypeNode?(symbol: Symbol): void; } @@ -5595,4 +5593,15 @@ namespace ts { get(key: TKey): PragmaPsuedoMap[TKey] | PragmaPsuedoMap[TKey][]; forEach(action: (value: PragmaPsuedoMap[TKey] | PragmaPsuedoMap[TKey][], key: TKey) => void): void; } + + export interface UserPreferences { + readonly disableSuggestions?: boolean; + readonly quotePreference?: "double" | "single"; + readonly includeCompletionsForModuleExports?: boolean; + readonly includeCompletionsWithInsertText?: boolean; + readonly importModuleSpecifierPreference?: "relative" | "non-relative"; + /** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */ + readonly importModuleSpecifierEnding?: "minimal" | "index" | "js"; + readonly allowTextChangesInNewFiles?: boolean; + } } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index b808a1f2fd..d04f921ec9 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -7936,6 +7936,7 @@ namespace ts { /** Must have ".d.ts" first because if ".ts" goes first, that will be detected as the extension instead of ".d.ts". */ export const supportedTypescriptExtensionsForExtractExtension: ReadonlyArray = [Extension.Dts, Extension.Ts, Extension.Tsx]; export const supportedJavascriptExtensions: ReadonlyArray = [Extension.Js, Extension.Jsx]; + export const supportedJavaScriptAndJsonExtensions: ReadonlyArray = [Extension.Js, Extension.Jsx, Extension.Json]; const allSupportedExtensions: ReadonlyArray = [...supportedTypeScriptExtensions, ...supportedJavascriptExtensions]; export function getSupportedExtensions(options?: CompilerOptions, extraFileExtensions?: ReadonlyArray): ReadonlyArray { @@ -7961,6 +7962,10 @@ namespace ts { return some(supportedJavascriptExtensions, extension => fileExtensionIs(fileName, extension)); } + export function hasJavaScriptOrJsonFileExtension(fileName: string): boolean { + return supportedJavaScriptAndJsonExtensions.some(ext => fileExtensionIs(fileName, ext)); + } + export function hasTypeScriptFileExtension(fileName: string): boolean { return some(supportedTypeScriptExtensions, extension => fileExtensionIs(fileName, extension)); } diff --git a/src/services/getEditsForFileRename.ts b/src/services/getEditsForFileRename.ts index bd00ce5dd9..4fe024787f 100644 --- a/src/services/getEditsForFileRename.ts +++ b/src/services/getEditsForFileRename.ts @@ -6,7 +6,7 @@ namespace ts { newFileOrDirPath: string, host: LanguageServiceHost, formatContext: formatting.FormatContext, - preferences: UserPreferences, + _preferences: UserPreferences, sourceMapper: SourceMapper, ): ReadonlyArray { const useCaseSensitiveFileNames = hostUsesCaseSensitiveFileNames(host); @@ -15,7 +15,7 @@ namespace ts { const newToOld = getPathUpdater(newFileOrDirPath, oldFileOrDirPath, getCanonicalFileName, sourceMapper); return textChanges.ChangeTracker.with({ host, formatContext }, changeTracker => { updateTsconfigFiles(program, changeTracker, oldToNew, newFileOrDirPath, host.getCurrentDirectory(), useCaseSensitiveFileNames); - updateImports(program, changeTracker, oldToNew, newToOld, host, getCanonicalFileName, preferences); + updateImports(program, changeTracker, oldToNew, newToOld, host, getCanonicalFileName); }); } @@ -122,7 +122,6 @@ namespace ts { newToOld: PathUpdater, host: LanguageServiceHost, getCanonicalFileName: GetCanonicalFileName, - preferences: UserPreferences, ): void { const allFiles = program.getSourceFiles(); for (const sourceFile of allFiles) { @@ -156,7 +155,7 @@ namespace ts { // Need an update if the imported file moved, or the importing file moved and was using a relative path. return toImport !== undefined && (toImport.updated || (importingSourceFileMoved && pathIsRelative(importLiteral.text))) - ? moduleSpecifiers.getModuleSpecifier(program.getCompilerOptions(), sourceFile, newImportFromPath, toImport.newFileName, host, allFiles, preferences, program.redirectTargetsMap) + ? moduleSpecifiers.updateModuleSpecifier(program.getCompilerOptions(), newImportFromPath, toImport.newFileName, host, allFiles, program.redirectTargetsMap, importLiteral.text) : undefined; }); } @@ -210,7 +209,7 @@ namespace ts { } function updateImportsWorker(sourceFile: SourceFile, changeTracker: textChanges.ChangeTracker, updateRef: (refText: string) => string | undefined, updateImport: (importLiteral: StringLiteralLike) => string | undefined) { - for (const ref of sourceFile.referencedFiles) { + for (const ref of sourceFile.referencedFiles || emptyArray) { // TODO: GH#26162 const updated = updateRef(ref.fileName); if (updated !== undefined && updated !== sourceFile.text.slice(ref.pos, ref.end)) changeTracker.replaceRangeWithText(sourceFile, ref, updated); } diff --git a/src/services/types.ts b/src/services/types.ts index 51c98724c0..9bf0edf315 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -233,14 +233,6 @@ namespace ts { installPackage?(options: InstallPackageOptions): Promise; } - export interface UserPreferences { - readonly disableSuggestions?: boolean; - readonly quotePreference?: "double" | "single"; - readonly includeCompletionsForModuleExports?: boolean; - readonly includeCompletionsWithInsertText?: boolean; - readonly importModuleSpecifierPreference?: "relative" | "non-relative"; - readonly allowTextChangesInNewFiles?: boolean; - } /* @internal */ export const emptyOptions = {}; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 5c261c55a4..e8cda88590 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -2990,6 +2990,16 @@ declare namespace ts { Parameters = 1296, IndexSignatureParameters = 4432 } + interface UserPreferences { + readonly disableSuggestions?: boolean; + readonly quotePreference?: "double" | "single"; + readonly includeCompletionsForModuleExports?: boolean; + readonly includeCompletionsWithInsertText?: boolean; + readonly importModuleSpecifierPreference?: "relative" | "non-relative"; + /** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */ + readonly importModuleSpecifierEnding?: "minimal" | "index" | "js"; + readonly allowTextChangesInNewFiles?: boolean; + } } declare function setTimeout(handler: (...args: any[]) => void, timeout: number): any; declare function clearTimeout(handle: any): void; @@ -4636,14 +4646,6 @@ declare namespace ts { isKnownTypesPackageName?(name: string): boolean; installPackage?(options: InstallPackageOptions): Promise; } - interface UserPreferences { - readonly disableSuggestions?: boolean; - readonly quotePreference?: "double" | "single"; - readonly includeCompletionsForModuleExports?: boolean; - readonly includeCompletionsWithInsertText?: boolean; - readonly importModuleSpecifierPreference?: "relative" | "non-relative"; - readonly allowTextChangesInNewFiles?: boolean; - } interface LanguageService { cleanupSemanticCache(): void; getSyntacticDiagnostics(fileName: string): DiagnosticWithLocation[]; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index e92e3d6e1d..c12d797bea 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -2990,6 +2990,16 @@ declare namespace ts { Parameters = 1296, IndexSignatureParameters = 4432 } + interface UserPreferences { + readonly disableSuggestions?: boolean; + readonly quotePreference?: "double" | "single"; + readonly includeCompletionsForModuleExports?: boolean; + readonly includeCompletionsWithInsertText?: boolean; + readonly importModuleSpecifierPreference?: "relative" | "non-relative"; + /** Determines whether we import `foo/index.ts` as "foo", "foo/index", or "foo/index.js" */ + readonly importModuleSpecifierEnding?: "minimal" | "index" | "js"; + readonly allowTextChangesInNewFiles?: boolean; + } } declare function setTimeout(handler: (...args: any[]) => void, timeout: number): any; declare function clearTimeout(handle: any): void; @@ -4636,14 +4646,6 @@ declare namespace ts { isKnownTypesPackageName?(name: string): boolean; installPackage?(options: InstallPackageOptions): Promise; } - interface UserPreferences { - readonly disableSuggestions?: boolean; - readonly quotePreference?: "double" | "single"; - readonly includeCompletionsForModuleExports?: boolean; - readonly includeCompletionsWithInsertText?: boolean; - readonly importModuleSpecifierPreference?: "relative" | "non-relative"; - readonly allowTextChangesInNewFiles?: boolean; - } interface LanguageService { cleanupSemanticCache(): void; getSyntacticDiagnostics(fileName: string): DiagnosticWithLocation[]; diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 8a91718dd7..2d73be1a91 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -528,10 +528,11 @@ declare namespace FourSlashInterface { filesToSearch?: ReadonlyArray; } interface UserPreferences { - quotePreference?: "double" | "single"; - includeCompletionsForModuleExports?: boolean; - includeInsertTextCompletions?: boolean; - importModuleSpecifierPreference?: "relative" | "non-relative"; + readonly quotePreference?: "double" | "single"; + readonly includeCompletionsForModuleExports?: boolean; + readonly includeInsertTextCompletions?: boolean; + readonly importModuleSpecifierPreference?: "relative" | "non-relative"; + readonly importModuleSpecifierEnding?: "minimal" | "index" | "js"; } interface CompletionsAtOptions extends UserPreferences { triggerCharacter?: string; diff --git a/tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts b/tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts new file mode 100644 index 0000000000..54b3e0e061 --- /dev/null +++ b/tests/cases/fourslash/getEditsForFileRename_preservePathEnding.ts @@ -0,0 +1,44 @@ +/// + +// @allowJs: true +// @checkJs: true +// @strict: true +// @jsx: preserve +// @resolveJsonModule: true + +// @Filename: /index.js +////export const x = 0; + +// @Filename: /jsx.jsx +////export const y = 0; + +// @Filename: /j.jonah.json +////{ "j": 0 } + +// @Filename: /a.js +////import { x as x0 } from "."; +////import { x as x1 } from "./index"; +////import { x as x2 } from "./index.js"; +////import { y } from "./jsx.jsx"; +////import { j } from "./j.jonah.json"; + +verify.noErrors(); + +verify.getEditsForFileRename({ + oldPath: "/a.js", + newPath: "/b.js", + newFileContents: {}, // No change +}); + +verify.getEditsForFileRename({ + oldPath: "/b.js", + newPath: "/src/b.js", + newFileContents: { + "/b.js": +`import { x as x0 } from ".."; +import { x as x1 } from "../index"; +import { x as x2 } from "../index.js"; +import { y } from "../jsx.jsx"; +import { j } from "../j.jonah.json";`, + }, +}); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts b/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts index cb7ef465af..ddc6d1790c 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots0.ts @@ -1,22 +1,23 @@ -/// - -// @Filename: a/f1.ts -//// [|foo/*0*/();|] - -// @Filename: types/random/index.ts -//// export function foo() {}; - -// @Filename: tsconfig.json -//// { -//// "compilerOptions": { -//// "typeRoots": [ -//// "./types" -//// ] -//// } -//// } - -verify.importFixAtPosition([ -`import { foo } from "random"; - -foo();` -]); +/// + +// @Filename: a/f1.ts +//// [|foo/*0*/();|] + +// @Filename: types/random/index.ts +//// export function foo() {}; + +// @Filename: tsconfig.json +//// { +//// "compilerOptions": { +//// "typeRoots": [ +//// "./types" +//// ] +//// } +//// } + +// "typeRoots" does not affect module resolution. Importing from "random" would be a compile error. +verify.importFixAtPosition([ +`import { foo } from "../types/random"; + +foo();` +]); diff --git a/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots1.ts b/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots1.ts index 1398a5d786..9743d164a9 100644 --- a/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots1.ts +++ b/tests/cases/fourslash/importNameCodeFixNewImportTypeRoots1.ts @@ -1,23 +1,27 @@ -/// - -// @Filename: a/f1.ts -//// [|foo/*0*/();|] - -// @Filename: types/random/index.ts -//// export function foo() {}; - -// @Filename: tsconfig.json -//// { -//// "compilerOptions": { -//// "baseUrl": ".", -//// "typeRoots": [ -//// "./types" -//// ] -//// } -//// } - -verify.importFixAtPosition([ -`import { foo } from "random"; - -foo();` -]); +/// + +// @Filename: a/f1.ts +//// [|foo/*0*/();|] + +// @Filename: types/random/index.ts +//// export function foo() {}; + +// @Filename: tsconfig.json +//// { +//// "compilerOptions": { +//// "baseUrl": ".", +//// "typeRoots": [ +//// "./types" +//// ] +//// } +//// } + +// "typeRoots" does not affect module resolution. Importing from "random" would be a compile error. +verify.importFixAtPosition([ +`import { foo } from "types/random"; + +foo();`, +`import { foo } from "../types/random"; + +foo();` +]); diff --git a/tests/cases/fourslash/importNameCodeFix_endingPreference.ts b/tests/cases/fourslash/importNameCodeFix_endingPreference.ts new file mode 100644 index 0000000000..41a96e9cf0 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_endingPreference.ts @@ -0,0 +1,26 @@ +/// + +// @moduleResolution: node + +// @Filename: /foo/index.ts +////export const foo = 0; + +// @Filename: /a.ts +////foo; + +// @Filename: /b.ts +////foo; + +// @Filename: /c.ts +////foo; + +const tests: ReadonlyArray<[string, FourSlashInterface.UserPreferences["importModuleSpecifierEnding"], string]> = [ + ["/a.ts", "js", "./foo/index.js"], + ["/b.ts", "index", "./foo/index"], + ["/c.ts", "minimal", "./foo"], +]; + +for (const [fileName, importModuleSpecifierEnding, specifier] of tests) { + goTo.file(fileName); + verify.importFixAtPosition([`import { foo } from "${specifier}";\n\nfoo;`,], /*errorCode*/ undefined, { importModuleSpecifierEnding }); +} diff --git a/tests/cases/fourslash/importNameCodeFix_jsExtension.ts b/tests/cases/fourslash/importNameCodeFix_jsExtension.ts index e719f1aeee..b47e67d82b 100644 --- a/tests/cases/fourslash/importNameCodeFix_jsExtension.ts +++ b/tests/cases/fourslash/importNameCodeFix_jsExtension.ts @@ -2,6 +2,7 @@ // @moduleResolution: node // @noLib: true +// @jsx: preserve // @Filename: /a.ts ////export function a() {} @@ -9,17 +10,24 @@ // @Filename: /b.ts ////export function b() {} +// @Filename: /c.tsx +////export function c() {} + // @Filename: /c.ts ////import * as g from "global"; // Global imports skipped ////import { a } from "./a.js"; ////import { a as a2 } from "./a"; // Ignored, only the first relative import is considered -////b; +////b; c; goTo.file("/c.ts"); -verify.importFixAtPosition([ +verify.codeFixAll({ + fixId: "fixMissingImport", + fixAllDescription: "Add all missing imports", + newFileContent: `import * as g from "global"; // Global imports skipped import { a } from "./a.js"; import { a as a2 } from "./a"; // Ignored, only the first relative import is considered import { b } from "./b.js"; -b;`, -]); +import { c } from "./c.jsx"; +b; c;`, +}); diff --git a/tests/cases/fourslash/importNameCodeFix_types_classic.ts b/tests/cases/fourslash/importNameCodeFix_types_classic.ts index b9ba463fad..4d3bdda4d3 100644 --- a/tests/cases/fourslash/importNameCodeFix_types_classic.ts +++ b/tests/cases/fourslash/importNameCodeFix_types_classic.ts @@ -5,12 +5,22 @@ // @Filename: /node_modules/@types/foo/index.d.ts ////export const xyz: number; +// @Filename: /node_modules/bar/index.d.ts +////export const qrs: number; + // @Filename: /a.ts -////[|xyz|] +////xyz; +////qrs; goTo.file("/a.ts"); -verify.importFixAtPosition([ +verify.codeFixAll({ + fixId: "fixMissingImport", + fixAllDescription: "Add all missing imports", + newFileContent: `import { xyz } from "foo"; -xyz` -]); +import { qrs } from "./node_modules/bar/index"; + +xyz; +qrs;`, +}); From 552bd1c8a2cba709c5161e8bd667d60c591eccef Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 28 Aug 2018 13:04:11 -0700 Subject: [PATCH 20/21] Support import fix/completions for `export =` (#25708) --- src/compiler/checker.ts | 2 - src/compiler/types.ts | 4 ++ src/harness/fourslash.ts | 6 ++- src/services/codefixes/importFixes.ts | 49 +++++++++++------ src/services/completions.ts | 8 +++ .../completionsImport_default_anonymous.ts | 2 +- .../completionsImport_exportEquals.ts | 53 +++++++++++++++++++ ...mport_exportEqualsNamespace_noDuplicate.ts | 2 +- .../cases/fourslash/completionsImport_tsx.ts | 2 +- .../completionsUniqueSymbol_import.ts | 2 +- .../cases/fourslash/importNameCodeFix_all.ts | 2 +- .../importNameCodeFix_exportEquals.ts | 25 +++++++++ 12 files changed, 132 insertions(+), 25 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_exportEquals.ts create mode 100644 tests/cases/fourslash/importNameCodeFix_exportEquals.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 26e5a6b83b..41573a7a8f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -2286,8 +2286,6 @@ namespace ts { return getPackagesSet().has(getTypesPackageName(packageName)); } - // An external module with an 'export =' declaration resolves to the target of the 'export =' declaration, - // and an external module with no 'export =' declaration resolves to the module itself. function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol; function resolveExternalModuleSymbol(moduleSymbol: Symbol | undefined, dontResolveAlias?: boolean): Symbol | undefined; function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 8977d9be28..e024885d6e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3097,6 +3097,10 @@ namespace ts { */ /* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] | undefined; /* @internal */ getTypePredicateOfSignature(signature: Signature): TypePredicate; + /** + * An external module with an 'export =' declaration resolves to the target of the 'export =' declaration, + * and an external module with no 'export =' declaration resolves to the module itself. + */ /* @internal */ resolveExternalModuleSymbol(symbol: Symbol): Symbol; /** @param node A location where we might consider accessing `this`. Not necessarily a ThisExpression. */ /* @internal */ tryGetThisTypeAt(node: Node): Type | undefined; diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 5d1ac5c491..214e1f295a 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -908,8 +908,8 @@ namespace FourSlash { } private verifyCompletionEntry(actual: ts.CompletionEntry, expected: FourSlashInterface.ExpectedCompletionEntry) { - const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, sourceDisplay } = typeof expected === "string" - ? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, sourceDisplay: undefined } + const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, source, sourceDisplay } = typeof expected === "string" + ? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, source: undefined, sourceDisplay: undefined } : expected; if (actual.insertText !== insertText) { @@ -927,6 +927,7 @@ namespace FourSlash { assert.equal(actual.hasAction, hasAction); assert.equal(actual.isRecommended, isRecommended); + assert.equal(actual.source, source); if (text) { const actualDetails = this.getCompletionEntryDetails(actual.name, actual.source)!; @@ -4789,6 +4790,7 @@ namespace FourSlashInterface { export type ExpectedCompletionEntry = string | { readonly name: string, + readonly source?: string, readonly insertText?: string, readonly replacementSpan?: FourSlash.Range, readonly hasAction?: boolean, // If not specified, will assert that this is false. diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 3c5b1d06ea..7ff5d30c95 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -163,7 +163,7 @@ namespace ts.codefix { position: number, preferences: UserPreferences, ): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } { - const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getTypeChecker(), program.getSourceFiles()); + const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getCompilerOptions(), program.getTypeChecker(), program.getSourceFiles()); Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol)); // We sort the best codefixes first, so taking `first` is best for completions. const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences)).moduleSpecifier; @@ -175,7 +175,7 @@ namespace ts.codefix { return { description, changes, commands }; } - function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { + function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, compilerOptions: CompilerOptions, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { const result: SymbolExportInfo[] = []; forEachExternalModule(checker, allSourceFiles, (moduleSymbol, moduleFile) => { // Don't import from a re-export when looking "up" like to `./index` or `../index`. @@ -183,10 +183,14 @@ namespace ts.codefix { return; } + const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions); + if (defaultInfo && defaultInfo.name === symbolName && skipAlias(defaultInfo.symbol, checker) === exportedSymbol) { + result.push({ moduleSymbol, importKind: defaultInfo.kind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(defaultInfo.symbol) }); + } + for (const exported of checker.getExportsOfModule(moduleSymbol)) { - if ((exported.escapedName === InternalSymbolName.Default || exported.name === symbolName) && skipAlias(exported, checker) === exportedSymbol) { - const isDefaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol) === exported; - result.push({ moduleSymbol, importKind: isDefaultExport ? ImportKind.Default : ImportKind.Named, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exported) }); + if (exported.name === symbolName && skipAlias(exported, checker) === exportedSymbol) { + result.push({ moduleSymbol, importKind: ImportKind.Named, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exported) }); } } }); @@ -400,13 +404,9 @@ namespace ts.codefix { forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => { cancellationToken.throwIfCancellationRequested(); - // check the default export - const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); - if (defaultExport) { - const info = getDefaultExportInfo(defaultExport, moduleSymbol, program); - if (info && info.name === symbolName && symbolHasMeaning(info.symbolForMeaning, currentTokenMeaning)) { - addSymbol(moduleSymbol, defaultExport, ImportKind.Default); - } + const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, program.getCompilerOptions()); + if (defaultInfo && defaultInfo.name === symbolName && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) { + addSymbol(moduleSymbol, defaultInfo.symbol, defaultInfo.kind); } // check exports with the same name @@ -418,7 +418,24 @@ namespace ts.codefix { return originalSymbolToExportInfos; } - function getDefaultExportInfo(defaultExport: Symbol, moduleSymbol: Symbol, program: Program): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { + function getDefaultLikeExportInfo( + moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions, + ): { readonly symbol: Symbol, readonly symbolForMeaning: Symbol, readonly name: string, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined { + const exported = getDefaultLikeExportWorker(moduleSymbol, checker); + if (!exported) return undefined; + const { symbol, kind } = exported; + const info = getDefaultExportInfoWorker(symbol, moduleSymbol, checker, compilerOptions); + return info && { symbol, symbolForMeaning: info.symbolForMeaning, name: info.name, kind }; + } + + function getDefaultLikeExportWorker(moduleSymbol: Symbol, checker: TypeChecker): { readonly symbol: Symbol, readonly kind: ImportKind.Default | ImportKind.Equals } | undefined { + const defaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol); + if (defaultExport) return { symbol: defaultExport, kind: ImportKind.Default }; + const exportEquals = checker.resolveExternalModuleSymbol(moduleSymbol); + return exportEquals === moduleSymbol ? undefined : { symbol: exportEquals, kind: ImportKind.Equals }; + } + + function getDefaultExportInfoWorker(defaultExport: Symbol, moduleSymbol: Symbol, checker: TypeChecker, compilerOptions: CompilerOptions): { readonly symbolForMeaning: Symbol, readonly name: string } | undefined { const localSymbol = getLocalSymbolForExportDefault(defaultExport); if (localSymbol) return { symbolForMeaning: localSymbol, name: localSymbol.name }; @@ -426,11 +443,11 @@ namespace ts.codefix { if (name !== undefined) return { symbolForMeaning: defaultExport, name }; if (defaultExport.flags & SymbolFlags.Alias) { - const aliased = program.getTypeChecker().getImmediateAliasedSymbol(defaultExport); - return aliased && getDefaultExportInfo(aliased, Debug.assertDefined(aliased.parent), program); + const aliased = checker.getImmediateAliasedSymbol(defaultExport); + return aliased && getDefaultExportInfoWorker(aliased, Debug.assertDefined(aliased.parent), checker, compilerOptions); } else { - return { symbolForMeaning: defaultExport, name: moduleSymbolToValidIdentifier(moduleSymbol, program.getCompilerOptions().target!) }; + return { symbolForMeaning: defaultExport, name: moduleSymbolToValidIdentifier(moduleSymbol, compilerOptions.target!) }; } } diff --git a/src/services/completions.ts b/src/services/completions.ts index 4098654331..7f82c9e748 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1378,6 +1378,14 @@ namespace ts.Completions { return; } + if (resolvedModuleSymbol !== moduleSymbol && + // Don't add another completion for `export =` of a symbol that's already global. + // So in `declare namespace foo {} declare module "foo" { export = foo; }`, there will just be the global completion for `foo`. + resolvedModuleSymbol.declarations.some(d => !!d.getSourceFile().externalModuleIndicator)) { + symbols.push(resolvedModuleSymbol); + symbolToOriginInfoMap[getSymbolId(resolvedModuleSymbol)] = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport: false }; + } + for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) { // Don't add a completion for a re-export, only for the original. // The actual import fix might end up coming from a re-export -- we don't compute that until getting completion details. diff --git a/tests/cases/fourslash/completionsImport_default_anonymous.ts b/tests/cases/fourslash/completionsImport_default_anonymous.ts index c406259dd6..82e5f07b06 100644 --- a/tests/cases/fourslash/completionsImport_default_anonymous.ts +++ b/tests/cases/fourslash/completionsImport_default_anonymous.ts @@ -11,7 +11,7 @@ ////fooB/*1*/ goTo.marker("0"); -const preferences = { includeCompletionsForModuleExports: true }; +const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true }; verify.completions( { marker: "0", excludes: { name: "default", source: "/src/foo-bar" }, preferences }, { marker: "1", includes: { name: "fooBar", source: "/src/foo-bar", sourceDisplay: "./foo-bar", text: "(property) default: 0", kind: "property", hasAction: true }, preferences } diff --git a/tests/cases/fourslash/completionsImport_exportEquals.ts b/tests/cases/fourslash/completionsImport_exportEquals.ts new file mode 100644 index 0000000000..37e78e09f3 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_exportEquals.ts @@ -0,0 +1,53 @@ +/// + +// @module: commonjs + +// @Filename: /a.d.ts +////declare function a(): void; +////declare namespace a { +//// export interface b {} +////} +////export = a; + +// @Filename: /b.ts +////a/*0*/; +////let x: b/*1*/; + +const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true }; +verify.completions( + { + marker: "0", + includes: { name: "a", source: "/a", hasAction: true, }, + preferences, + }, + { + marker: "1", + includes: { name: "b", source: "/a", hasAction: true }, + preferences, + } +); + +// Import { b } first, or it will just add a qualified name from 'a' (which isn't what we're trying to test) +verify.applyCodeActionFromCompletion("1", { + name: "b", + source: "/a", + description: `Import 'b' from module "./a"`, + newFileContent: +`import { b } from "./a"; + +a; +let x: b;`, +}); + +verify.applyCodeActionFromCompletion("0", { + name: "a", + source: "/a", + description: `Import 'a' from module "./a"`, + newFileContent: +`import { b } from "./a"; +import a = require("./a"); + +a; +let x: b;`, +}); + diff --git a/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts b/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts index ede97c2d81..8a687f5a19 100644 --- a/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts +++ b/tests/cases/fourslash/completionsImport_exportEqualsNamespace_noDuplicate.ts @@ -18,7 +18,7 @@ verify.completions({ marker: "", // Tester will assert that it is only included once - includes: [{ name: "foo", hasAction: true }], + includes: [{ name: "foo", source: "a", hasAction: true }], preferences: { includeCompletionsForModuleExports: true, } diff --git a/tests/cases/fourslash/completionsImport_tsx.ts b/tests/cases/fourslash/completionsImport_tsx.ts index f2f8752082..9fb4d23922 100644 --- a/tests/cases/fourslash/completionsImport_tsx.ts +++ b/tests/cases/fourslash/completionsImport_tsx.ts @@ -12,7 +12,7 @@ verify.completions({ marker: "", - includes: { name: "Foo", source: "/a.tsx", hasAction: true }, + includes: { name: "Foo", source: "/a", hasAction: true }, excludes: "Bar", preferences: { includeCompletionsForModuleExports: true, diff --git a/tests/cases/fourslash/completionsUniqueSymbol_import.ts b/tests/cases/fourslash/completionsUniqueSymbol_import.ts index df3034e56f..7f98f8c1dd 100644 --- a/tests/cases/fourslash/completionsUniqueSymbol_import.ts +++ b/tests/cases/fourslash/completionsUniqueSymbol_import.ts @@ -24,7 +24,7 @@ verify.completions({ marker: "", exact: [ "n", - { name: "publicSym", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true }, + { name: "publicSym", source: "/a", insertText: "[publicSym]", replacementSpan: test.ranges()[0], hasAction: true }, ], preferences: { includeInsertTextCompletions: true, diff --git a/tests/cases/fourslash/importNameCodeFix_all.ts b/tests/cases/fourslash/importNameCodeFix_all.ts index 4aa22413d5..d5bb525900 100644 --- a/tests/cases/fourslash/importNameCodeFix_all.ts +++ b/tests/cases/fourslash/importNameCodeFix_all.ts @@ -37,11 +37,11 @@ verify.codeFixAll({ fixId: "fixMissingImport", fixAllDescription: "Add all missing imports", newFileContent: -// TODO: GH#25135 (should import 'e') `import bd, * as b from "./b"; import cd, { c0 } from "./c"; import dd, { d0, d1 } from "./d"; import ad, { a0 } from "./a"; +import e = require("./e"); ad; ad; a0; a0; bd; bd; b.b0; b.b0; diff --git a/tests/cases/fourslash/importNameCodeFix_exportEquals.ts b/tests/cases/fourslash/importNameCodeFix_exportEquals.ts new file mode 100644 index 0000000000..4ab87df7e7 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_exportEquals.ts @@ -0,0 +1,25 @@ +/// + +// @Filename: /a.d.ts +////declare function a(): void; +////declare namespace a { +//// export interface b {} +////} +////export = a; + +// @Filename: /b.ts +////a; +////let x: b; + +goTo.file("/b.ts"); +verify.codeFixAll({ + fixId: "fixMissingImport", + fixAllDescription: "Add all missing imports", + newFileContent: +`import { b } from "./a"; + +import a = require("./a"); + +a; +let x: b;`, +}); From 9106fdbc47f4b1e7ddb6458f62c2e00c775591d0 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 28 Aug 2018 14:21:09 -0700 Subject: [PATCH 21/21] Support signature help for type parameters of a type (#26702) --- src/compiler/checker.ts | 4 +- src/compiler/types.ts | 2 + src/services/signatureHelp.ts | 76 +++++++++++++++---- .../genericParameterHelpTypeReferences.ts | 50 +++++++----- 4 files changed, 99 insertions(+), 33 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 41573a7a8f..cb3fc5614f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -370,7 +370,9 @@ namespace ts { finally { cancellationToken = undefined; } - } + }, + + getLocalTypeParametersOfClassOrInterfaceOrTypeAlias, }; function getResolvedSignatureWorker(nodeIn: CallLikeExpression, candidatesOutArray: Signature[] | undefined, argumentCount: number | undefined, isForSignatureHelp: boolean): Signature | undefined { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index e024885d6e..10076014d9 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3118,6 +3118,8 @@ namespace ts { * and the operation is cancelled, then it should be discarded, otherwise it is safe to keep. */ runWithCancellationToken(token: CancellationToken, cb: (checker: TypeChecker) => T): T; + + /* @internal */ getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol: Symbol): ReadonlyArray | undefined; } /* @internal */ diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index d1705b5104..5211b244e2 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -2,7 +2,7 @@ namespace ts.SignatureHelp { const enum InvocationKind { Call, TypeArgs, Contextual } interface CallInvocation { readonly kind: InvocationKind.Call; readonly node: CallLikeExpression; } - interface TypeArgsInvocation { readonly kind: InvocationKind.TypeArgs; readonly called: Expression; } + interface TypeArgsInvocation { readonly kind: InvocationKind.TypeArgs; readonly called: Identifier; } interface ContextualInvocation { readonly kind: InvocationKind.Contextual; readonly signature: Signature; @@ -44,7 +44,7 @@ namespace ts.SignatureHelp { cancellationToken.throwIfCancellationRequested(); // Extra syntactic and semantic filtering of signature help - const candidateInfo = getCandidateInfo(argumentInfo, typeChecker, sourceFile, startingToken, onlyUseSyntacticOwners); + const candidateInfo = getCandidateOrTypeInfo(argumentInfo, typeChecker, sourceFile, startingToken, onlyUseSyntacticOwners); cancellationToken.throwIfCancellationRequested(); if (!candidateInfo) { @@ -53,11 +53,24 @@ namespace ts.SignatureHelp { return isSourceFileJavaScript(sourceFile) ? createJavaScriptSignatureHelpItems(argumentInfo, program, cancellationToken) : undefined; } - return typeChecker.runWithCancellationToken(cancellationToken, typeChecker => createSignatureHelpItems(candidateInfo.candidates, candidateInfo.resolvedSignature, argumentInfo, sourceFile, typeChecker)); + return typeChecker.runWithCancellationToken(cancellationToken, typeChecker => + candidateInfo.kind === CandidateOrTypeKind.Candidate + ? createSignatureHelpItems(candidateInfo.candidates, candidateInfo.resolvedSignature, argumentInfo, sourceFile, typeChecker) + : createTypeHelpItems(candidateInfo.symbol, argumentInfo, sourceFile, typeChecker)); } - interface CandidateInfo { readonly candidates: ReadonlyArray; readonly resolvedSignature: Signature; } - function getCandidateInfo({ invocation, argumentCount }: ArgumentListInfo, checker: TypeChecker, sourceFile: SourceFile, startingToken: Node, onlyUseSyntacticOwners: boolean): CandidateInfo | undefined { + const enum CandidateOrTypeKind { Candidate, Type } + interface CandidateInfo { + readonly kind: CandidateOrTypeKind.Candidate; + readonly candidates: ReadonlyArray; + readonly resolvedSignature: Signature; + } + interface TypeInfo { + readonly kind: CandidateOrTypeKind.Type; + readonly symbol: Symbol; + } + + function getCandidateOrTypeInfo({ invocation, argumentCount }: ArgumentListInfo, checker: TypeChecker, sourceFile: SourceFile, startingToken: Node, onlyUseSyntacticOwners: boolean): CandidateInfo | TypeInfo | undefined { switch (invocation.kind) { case InvocationKind.Call: { if (onlyUseSyntacticOwners && !isSyntacticOwner(startingToken, invocation.node, sourceFile)) { @@ -65,17 +78,21 @@ namespace ts.SignatureHelp { } const candidates: Signature[] = []; const resolvedSignature = checker.getResolvedSignatureForSignatureHelp(invocation.node, candidates, argumentCount)!; // TODO: GH#18217 - return candidates.length === 0 ? undefined : { candidates, resolvedSignature }; + return candidates.length === 0 ? undefined : { kind: CandidateOrTypeKind.Candidate, candidates, resolvedSignature }; } case InvocationKind.TypeArgs: { - if (onlyUseSyntacticOwners && !lessThanFollowsCalledExpression(startingToken, sourceFile, invocation.called)) { + const { called } = invocation; + if (onlyUseSyntacticOwners && !containsPrecedingToken(startingToken, sourceFile, isIdentifier(called) ? called.parent : called)) { return undefined; } - const candidates = getPossibleGenericSignatures(invocation.called, argumentCount, checker); - return candidates.length === 0 ? undefined : { candidates, resolvedSignature: first(candidates) }; + const candidates = getPossibleGenericSignatures(called, argumentCount, checker); + if (candidates.length !== 0) return { kind: CandidateOrTypeKind.Candidate, candidates, resolvedSignature: first(candidates) }; + + const symbol = checker.getSymbolAtLocation(called); + return symbol && { kind: CandidateOrTypeKind.Type, symbol }; } case InvocationKind.Contextual: - return { candidates: [invocation.signature], resolvedSignature: invocation.signature }; + return { kind: CandidateOrTypeKind.Candidate, candidates: [invocation.signature], resolvedSignature: invocation.signature }; default: return Debug.assertNever(invocation); } @@ -92,7 +109,7 @@ namespace ts.SignatureHelp { return !!containingList && contains(invocationChildren, containingList); } case SyntaxKind.LessThanToken: - return lessThanFollowsCalledExpression(startingToken, sourceFile, node.expression); + return containsPrecedingToken(startingToken, sourceFile, node.expression); default: return false; } @@ -114,12 +131,12 @@ namespace ts.SignatureHelp { })); } - function lessThanFollowsCalledExpression(startingToken: Node, sourceFile: SourceFile, calledExpression: Expression) { + function containsPrecedingToken(startingToken: Node, sourceFile: SourceFile, container: Node) { const precedingToken = Debug.assertDefined( findPrecedingToken(startingToken.getFullStart(), sourceFile, startingToken.parent, /*excludeJsdoc*/ true) ); - return rangeContainsRange(calledExpression, precedingToken); + return rangeContainsRange(container, precedingToken); } export interface ArgumentInfoForCompletions { @@ -457,6 +474,10 @@ namespace ts.SignatureHelp { return invocation.kind === InvocationKind.Call ? getInvokedExpression(invocation.node) : invocation.called; } + function getEnclosingDeclarationFromInvocation(invocation: Invocation): Node { + return invocation.kind === InvocationKind.Call ? invocation.node : invocation.kind === InvocationKind.TypeArgs ? invocation.called : invocation.node; + } + const signatureHelpNodeBuilderFlags = NodeBuilderFlags.OmitParameterModifiers | NodeBuilderFlags.IgnoreErrors | NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope; function createSignatureHelpItems( candidates: ReadonlyArray, @@ -465,7 +486,7 @@ namespace ts.SignatureHelp { sourceFile: SourceFile, typeChecker: TypeChecker, ): SignatureHelpItems { - const enclosingDeclaration = invocation.kind === InvocationKind.Call ? invocation.node : invocation.kind === InvocationKind.TypeArgs ? invocation.called : invocation.node; + const enclosingDeclaration = getEnclosingDeclarationFromInvocation(invocation); const callTargetSymbol = invocation.kind === InvocationKind.Contextual ? invocation.symbol : typeChecker.getSymbolAtLocation(getExpressionFromInvocation(invocation)); const callTargetDisplayParts = callTargetSymbol ? symbolToDisplayParts(typeChecker, callTargetSymbol, /*enclosingDeclaration*/ undefined, /*meaning*/ undefined) : emptyArray; const items = candidates.map(candidateSignature => getSignatureHelpItem(candidateSignature, callTargetDisplayParts, isTypeParameterList, typeChecker, enclosingDeclaration, sourceFile)); @@ -480,11 +501,36 @@ namespace ts.SignatureHelp { return { items, applicableSpan, selectedItemIndex, argumentIndex, argumentCount }; } + function createTypeHelpItems( + symbol: Symbol, + { argumentCount, argumentsSpan: applicableSpan, invocation, argumentIndex }: ArgumentListInfo, + sourceFile: SourceFile, + checker: TypeChecker + ): SignatureHelpItems | undefined { + const typeParameters = checker.getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol); + if (!typeParameters) return undefined; + const items = [getTypeHelpItem(symbol, typeParameters, checker, getEnclosingDeclarationFromInvocation(invocation), sourceFile)]; + return { items, applicableSpan, selectedItemIndex: 0, argumentIndex, argumentCount }; + } + + function getTypeHelpItem(symbol: Symbol, typeParameters: ReadonlyArray, checker: TypeChecker, enclosingDeclaration: Node, sourceFile: SourceFile): SignatureHelpItem { + const typeSymbolDisplay = symbolToDisplayParts(checker, symbol); + + const printer = createPrinter({ removeComments: true }); + const parameters = typeParameters.map(t => createSignatureHelpParameterForTypeParameter(t, checker, enclosingDeclaration, sourceFile, printer)); + + const documentation = symbol.getDocumentationComment(checker); + const tags = symbol.getJsDocTags(); + const prefixDisplayParts = [...typeSymbolDisplay, punctuationPart(SyntaxKind.LessThanToken)]; + return { isVariadic: false, prefixDisplayParts, suffixDisplayParts: [punctuationPart(SyntaxKind.GreaterThanToken)], separatorDisplayParts, parameters, documentation, tags }; + } + + const separatorDisplayParts: SymbolDisplayPart[] = [punctuationPart(SyntaxKind.CommaToken), spacePart()]; + function getSignatureHelpItem(candidateSignature: Signature, callTargetDisplayParts: ReadonlyArray, isTypeParameterList: boolean, checker: TypeChecker, enclosingDeclaration: Node, sourceFile: SourceFile): SignatureHelpItem { const { isVariadic, parameters, prefix, suffix } = (isTypeParameterList ? itemInfoForTypeParameters : itemInfoForParameters)(candidateSignature, checker, enclosingDeclaration, sourceFile); const prefixDisplayParts = [...callTargetDisplayParts, ...prefix]; const suffixDisplayParts = [...suffix, ...returnTypeToDisplayParts(candidateSignature, enclosingDeclaration, checker)]; - const separatorDisplayParts = [punctuationPart(SyntaxKind.CommaToken), spacePart()]; const documentation = candidateSignature.getDocumentationComment(checker); const tags = candidateSignature.getJsDocTags(); return { isVariadic, prefixDisplayParts, suffixDisplayParts, separatorDisplayParts, parameters, documentation, tags }; diff --git a/tests/cases/fourslash/genericParameterHelpTypeReferences.ts b/tests/cases/fourslash/genericParameterHelpTypeReferences.ts index 5a477d23e5..a5dee83324 100644 --- a/tests/cases/fourslash/genericParameterHelpTypeReferences.ts +++ b/tests/cases/fourslash/genericParameterHelpTypeReferences.ts @@ -11,21 +11,37 @@ ////var x : testClass extends testClass; +//// +////interface I {} +////let i: I; +//// +////type Ty = T; +////let t: Ty; -// TODO: GH#26699 - -if (false) { - verify.signatureHelp( - { - marker: ["type1", "type2", "type3"], - text: "testClass", - parameterName: "T", - parameterSpan: "T extends IFoo", - }, - { - marker: "type4", - parameterName: "M", - parameterSpan: "M extends IFoo", - } - ); -} +verify.signatureHelp( + { + marker: ["type1", "type2", "type3"], + text: "testClass", + parameterName: "T", + parameterSpan: "T extends IFoo", + triggerReason: { kind: "characterTyped", triggerCharacter: "<" }, + }, + { + marker: "type4", + parameterName: "M", + parameterSpan: "M extends IFoo", + triggerReason: { kind: "characterTyped", triggerCharacter: "," }, + }, + { + marker: "interface", + text: "I", + parameterName: "T", + parameterSpan: "T", + }, + { + marker: "typeAlias", + text: "Ty", + parameterName: "T", + parameterSpan: "T", + }, +);